fix: strip internal planning tags from PlanReActPlanner output
Merge https://github.com/google/adk-python/pull/6709 References #3378 PiperOrigin-RevId: 967495179
This commit is contained in:
@@ -102,11 +102,30 @@ class PlanReActPlanner(BasePlanner):
|
||||
return text, ''
|
||||
return text[: index + len(separator)], text[index + len(separator) :]
|
||||
|
||||
_PLANNING_TAGS = (PLANNING_TAG, REASONING_TAG, ACTION_TAG, REPLANNING_TAG)
|
||||
|
||||
def _strip_planning_tags(self, text: str) -> str:
|
||||
"""Strips all planning tags from the text.
|
||||
|
||||
Args:
|
||||
text: The text to strip.
|
||||
|
||||
Returns:
|
||||
The text with all planning tags removed.
|
||||
"""
|
||||
for tag in self._PLANNING_TAGS:
|
||||
text = text.replace(tag, '')
|
||||
return text
|
||||
|
||||
def _handle_non_function_call_parts(
|
||||
self, response_part: types.Part, preserved_parts: list[types.Part]
|
||||
) -> None:
|
||||
"""Handles non-function-call parts of the response.
|
||||
|
||||
The method strips embedded planning tags (e.g. ``/*PLANNING*/``,
|
||||
``/*REASONING*/``) from the text so that callers receive clean content
|
||||
blocks instead of raw tagged text.
|
||||
|
||||
Args:
|
||||
response_part: The response part to handle.
|
||||
preserved_parts: The mutable list of parts to store the processed parts
|
||||
@@ -116,6 +135,11 @@ class PlanReActPlanner(BasePlanner):
|
||||
reasoning_text, final_answer_text = self._split_by_last_pattern(
|
||||
response_part.text, FINAL_ANSWER_TAG
|
||||
)
|
||||
# _split_by_last_pattern includes the separator in the left part; strip
|
||||
# it so the reasoning block contains only the actual reasoning text.
|
||||
if reasoning_text.endswith(FINAL_ANSWER_TAG):
|
||||
reasoning_text = reasoning_text[: -len(FINAL_ANSWER_TAG)]
|
||||
reasoning_text = self._strip_planning_tags(reasoning_text)
|
||||
if reasoning_text:
|
||||
reasoning_part = types.Part(text=reasoning_text)
|
||||
self._mark_as_thought(reasoning_part)
|
||||
@@ -128,19 +152,10 @@ class PlanReActPlanner(BasePlanner):
|
||||
)
|
||||
else:
|
||||
response_text = response_part.text or ''
|
||||
# If the part is a text part with a planning/reasoning/action tag,
|
||||
# label it as reasoning.
|
||||
if response_text and (
|
||||
any(
|
||||
response_text.startswith(tag)
|
||||
for tag in [
|
||||
PLANNING_TAG,
|
||||
REASONING_TAG,
|
||||
ACTION_TAG,
|
||||
REPLANNING_TAG,
|
||||
]
|
||||
)
|
||||
):
|
||||
# If the part is a text part with a leading planning/reasoning/action tag,
|
||||
# label it as reasoning and strip all tags to produce clean text.
|
||||
if response_text and response_text.startswith(self._PLANNING_TAGS):
|
||||
response_part.text = self._strip_planning_tags(response_text)
|
||||
self._mark_as_thought(response_part)
|
||||
preserved_parts.append(response_part)
|
||||
|
||||
@@ -150,7 +165,7 @@ class PlanReActPlanner(BasePlanner):
|
||||
Args:
|
||||
response_part: The mutable response part to mark as thought.
|
||||
"""
|
||||
if response_part.text:
|
||||
if response_part.text is not None:
|
||||
response_part.thought = True
|
||||
return
|
||||
|
||||
|
||||
@@ -22,6 +22,143 @@ def _function_call_names(parts):
|
||||
return [p.function_call.name for p in parts if p.function_call]
|
||||
|
||||
|
||||
def test_strips_planning_tag_from_thought_part():
|
||||
"""Planning/reasoning tags must be stripped from the output text.
|
||||
|
||||
The raw ``/*PLANNING*/``, ``/*REASONING*/``, ``/*ACTION*/`` and
|
||||
``/*REPLANNING*/`` markers are internal prompting artefacts. After
|
||||
processing, the resulting parts should contain clean text, have
|
||||
``thought=True`` set, and preserve other Part metadata such as
|
||||
``thought_signature``.
|
||||
"""
|
||||
planner = PlanReActPlanner()
|
||||
response_parts = [
|
||||
types.Part(
|
||||
text="/*PLANNING*/Step 1: look it up.",
|
||||
thought_signature=b"sig1",
|
||||
),
|
||||
types.Part(
|
||||
text="/*REASONING*/I need to call the tool.",
|
||||
thought_signature=b"sig2",
|
||||
),
|
||||
types.Part.from_function_call(name="lookup", args={"q": "test"}),
|
||||
]
|
||||
|
||||
result = planner.process_planning_response(
|
||||
callback_context=None, response_parts=response_parts
|
||||
)
|
||||
|
||||
text_parts = [p for p in result if p.text]
|
||||
# Tags must be gone
|
||||
for p in text_parts:
|
||||
assert "/*PLANNING*/" not in p.text
|
||||
assert "/*REASONING*/" not in p.text
|
||||
# Thought flag must be set on non-final-answer text parts
|
||||
for p in text_parts:
|
||||
assert p.thought is True
|
||||
# Part metadata like thought_signature must be preserved
|
||||
assert text_parts[0].thought_signature == b"sig1"
|
||||
assert text_parts[1].thought_signature == b"sig2"
|
||||
# Function call must still be present
|
||||
assert _function_call_names(result) == ["lookup"]
|
||||
|
||||
|
||||
def test_strips_final_answer_tag_boundary():
|
||||
"""The /*FINAL_ANSWER*/ tag must not appear in either output block."""
|
||||
planner = PlanReActPlanner()
|
||||
response_parts = [
|
||||
types.Part(
|
||||
text="/*REASONING*/Some reasoning./*FINAL_ANSWER*/The answer is 42."
|
||||
),
|
||||
]
|
||||
|
||||
result = planner.process_planning_response(
|
||||
callback_context=None, response_parts=response_parts
|
||||
)
|
||||
|
||||
texts = [p.text for p in result if p.text]
|
||||
combined = " ".join(texts)
|
||||
assert "/*FINAL_ANSWER*/" not in combined
|
||||
assert "/*REASONING*/" not in combined
|
||||
assert "The answer is 42." in combined
|
||||
|
||||
|
||||
def test_strips_multiple_planning_tags():
|
||||
"""Embedded planning and reasoning tags must all be stripped."""
|
||||
planner = PlanReActPlanner()
|
||||
response_parts = [
|
||||
types.Part(
|
||||
text=(
|
||||
"/*PLANNING*/Initial plan.\n"
|
||||
"/*REASONING*/Some reasoning.\n"
|
||||
"/*FINAL_ANSWER*/The answer is 42."
|
||||
)
|
||||
),
|
||||
]
|
||||
|
||||
result = planner.process_planning_response(
|
||||
callback_context=None, response_parts=response_parts
|
||||
)
|
||||
|
||||
texts = [p.text for p in result if p.text]
|
||||
combined = " ".join(texts)
|
||||
assert "/*PLANNING*/" not in combined
|
||||
assert "/*REASONING*/" not in combined
|
||||
assert "/*FINAL_ANSWER*/" not in combined
|
||||
assert "Initial plan." in combined
|
||||
assert "Some reasoning." in combined
|
||||
assert "The answer is 42." in combined
|
||||
|
||||
|
||||
def test_part_without_leading_tag_not_marked_as_thought():
|
||||
"""A part without a leading tag (even with stray embedded tag) is not thought."""
|
||||
planner = PlanReActPlanner()
|
||||
response_parts = [
|
||||
types.Part(text="Here is the answer /*PLANNING*/ with stray tag."),
|
||||
]
|
||||
|
||||
result = planner.process_planning_response(
|
||||
callback_context=None, response_parts=response_parts
|
||||
)
|
||||
|
||||
assert len(result) == 1
|
||||
assert result[0].thought is not True
|
||||
assert result[0].text == "Here is the answer /*PLANNING*/ with stray tag."
|
||||
|
||||
|
||||
def test_bare_tag_part_is_marked_as_thought():
|
||||
"""A part containing only planning tags is kept, stripped, and marked as thought."""
|
||||
planner = PlanReActPlanner()
|
||||
bare_part = types.Part(text="/*ACTION*/")
|
||||
response_parts = [
|
||||
bare_part,
|
||||
types.Part.from_function_call(name="lookup", args={"q": "test"}),
|
||||
]
|
||||
|
||||
result = planner.process_planning_response(
|
||||
callback_context=None, response_parts=response_parts
|
||||
)
|
||||
|
||||
assert len(result) == 2
|
||||
assert result[0].text == ""
|
||||
assert result[0].thought is True
|
||||
assert _function_call_names(result) == ["lookup"]
|
||||
|
||||
|
||||
def test_sole_bare_tag_part_is_marked_as_thought():
|
||||
"""A sole part with only a planning tag is preserved as a thought part."""
|
||||
planner = PlanReActPlanner()
|
||||
response_parts = [types.Part(text="/*ACTION*/")]
|
||||
|
||||
result = planner.process_planning_response(
|
||||
callback_context=None, response_parts=response_parts
|
||||
)
|
||||
|
||||
assert len(result) == 1
|
||||
assert result[0].text == ""
|
||||
assert result[0].thought is True
|
||||
|
||||
|
||||
def test_preserves_all_leading_parallel_function_calls():
|
||||
"""Parallel function calls at the start of the response must all survive.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user