diff --git a/src/google/adk/planners/plan_re_act_planner.py b/src/google/adk/planners/plan_re_act_planner.py index d3fd4535..f0d00fe9 100644 --- a/src/google/adk/planners/plan_re_act_planner.py +++ b/src/google/adk/planners/plan_re_act_planner.py @@ -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 diff --git a/tests/unittests/planners/test_plan_re_act_planner.py b/tests/unittests/planners/test_plan_re_act_planner.py index ccafdf48..01830433 100644 --- a/tests/unittests/planners/test_plan_re_act_planner.py +++ b/tests/unittests/planners/test_plan_re_act_planner.py @@ -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.