From 4559197505cede4c5fee5f8c41129052b44b5589 Mon Sep 17 00:00:00 2001 From: Willem Jiang Date: Thu, 27 Nov 2025 23:59:31 +0800 Subject: [PATCH] fix: revert the part of patch of issue-710 to extract the content from the plan (#718) --- src/graph/nodes.py | 37 ++---- tests/unit/graph/test_plan_validation.py | 149 ----------------------- 2 files changed, 10 insertions(+), 176 deletions(-) diff --git a/src/graph/nodes.py b/src/graph/nodes.py index 406bdb0..9b47b6c 100644 --- a/src/graph/nodes.py +++ b/src/graph/nodes.py @@ -181,29 +181,6 @@ def validate_and_fix_plan(plan: dict, enforce_web_search: bool = False) -> dict: } ] - # ============================================================ - # SECTION 3: Ensure required Plan fields are present (Issue #710 fix) - # ============================================================ - # Set locale from state if not present - if "locale" not in plan or not plan.get("locale"): - plan["locale"] = "en-US" # Default locale - logger.info("Added missing locale field with default value 'en-US'") - - # Ensure has_enough_context is present - if "has_enough_context" not in plan: - plan["has_enough_context"] = False # Default value - logger.info("Added missing has_enough_context field with default value 'False'") - - # Ensure title is present - if "title" not in plan or not plan.get("title"): - # Try to infer title from steps or use a default - if steps and isinstance(steps[0], dict) and "title" in steps[0]: - plan["title"] = steps[0]["title"] - logger.info(f"Inferred missing title from first step: {plan['title']}") - else: - plan["title"] = "Research Plan" # Default title - logger.info("Added missing title field with default value 'Research Plan'") - return plan @@ -351,6 +328,10 @@ def planner_node( try: curr_plan = json.loads(repair_json_output(full_response)) + # Need to extract the plan from the full_response + curr_plan_content = extract_plan_content(curr_plan) + # load the current_plan + curr_plan = json.loads(repair_json_output(curr_plan_content)) except json.JSONDecodeError: logger.warning("Planner response is not a valid JSON") if plan_iterations > 0: @@ -476,15 +457,17 @@ def human_feedback_node( try: # Safely extract plan content from different types (string, AIMessage, dict) original_plan = current_plan - current_plan_content = extract_plan_content(current_plan) - logger.debug(f"Extracted plan content type: {type(current_plan_content).__name__}") # Repair the JSON output - current_plan = repair_json_output(current_plan_content) + current_plan = repair_json_output(current_plan) + # parse the plan to dict + current_plan = json.loads(current_plan) + current_plan_content = extract_plan_content(current_plan) + # increment the plan iterations plan_iterations += 1 # parse the plan - new_plan = json.loads(current_plan) + new_plan = json.loads(current_plan_content) # Validate and fix plan to ensure web search requirements are met configurable = Configuration.from_runnable_config(config) new_plan = validate_and_fix_plan(new_plan, configurable.enforce_web_search) diff --git a/tests/unit/graph/test_plan_validation.py b/tests/unit/graph/test_plan_validation.py index 66d09e2..7959d3b 100644 --- a/tests/unit/graph/test_plan_validation.py +++ b/tests/unit/graph/test_plan_validation.py @@ -362,155 +362,6 @@ class TestValidateAndFixPlanIntegration: assert result["steps"][1]["step_type"] == "processing" assert result["steps"][1]["need_search"] is False - -class TestValidateAndFixPlanIssue710: - """Specific tests for Issue #710 scenarios - missing required Plan fields.""" - - def test_missing_locale_field_added(self): - """Test that missing locale field is added with default value.""" - plan = { - "has_enough_context": True, - "title": "Test Plan", - "steps": [] - } - - result = validate_and_fix_plan(plan) - - assert "locale" in result - assert result["locale"] == "en-US" - - def test_empty_locale_field_replaced(self): - """Test that empty locale field is replaced with default value.""" - plan = { - "locale": "", - "has_enough_context": True, - "title": "Test Plan", - "steps": [] - } - - result = validate_and_fix_plan(plan) - - assert "locale" in result - assert result["locale"] == "en-US" - - def test_missing_has_enough_context_field_added(self): - """Test that missing has_enough_context field is added with default value.""" - plan = { - "locale": "en-US", - "title": "Test Plan", - "steps": [] - } - - result = validate_and_fix_plan(plan) - - assert "has_enough_context" in result - assert result["has_enough_context"] is False - - def test_missing_title_field_added_from_step(self): - """Test that missing title field is inferred from first step.""" - plan = { - "locale": "en-US", - "has_enough_context": True, - "steps": [ - { - "need_search": True, - "title": "Step Title", - "description": "Step description", - "step_type": "research" - } - ] - } - - result = validate_and_fix_plan(plan) - - assert "title" in result - assert result["title"] == "Step Title" - - def test_missing_title_field_added_default(self): - """Test that missing title field is added with default when no steps available.""" - plan = { - "locale": "en-US", - "has_enough_context": True, - "steps": [] - } - - result = validate_and_fix_plan(plan) - - assert "title" in result - assert result["title"] == "Research Plan" - - def test_all_required_fields_missing(self): - """Test that all missing required fields are added.""" - plan = { - "steps": [ - { - "need_search": True, - "title": "Step 1", - "description": "Description", - "step_type": "research" - } - ] - } - - result = validate_and_fix_plan(plan) - - # All required fields should be present - assert "locale" in result - assert "has_enough_context" in result - assert "title" in result - - # With appropriate defaults - assert result["locale"] == "en-US" - assert result["has_enough_context"] is False - assert result["title"] == "Step 1" # Inferred from first step - - def test_issue_710_scenario_passes_pydantic_validation(self): - """Test that fixed plan can be validated by Pydantic schema (reproduces issue #710 fix).""" - from src.prompts.planner_model import Plan as PlanModel - - # Simulate the problematic plan from issue #710 that's missing required fields - plan = { - "content": '{\n "locale": "en-US",\n "has_enough_context": false,\n "title": "Test Plan",\n "steps": [\n {\n "need_search": true,\n "title": "Research Step",\n "description": "Gather data",\n "step_type": "research"\n }\n ]\n}', - "reasoning": 2368 - } - - # Extract just the JSON part (simulating what would happen in the actual flow) - import json - json_content = json.loads(plan["content"]) - - # Remove required fields to simulate the issue - del json_content["locale"] - del json_content["has_enough_context"] - del json_content["title"] - - # First validate and fix - fixed_plan = validate_and_fix_plan(json_content) - - # Then try Pydantic validation (should not raise) - validated = PlanModel.model_validate(fixed_plan) - - # Validation should succeed - assert validated.locale == "en-US" - assert validated.has_enough_context is False - assert validated.title == "Research Step" # Inferred from first step - - def test_existing_fields_preserved(self): - """Test that existing valid fields are preserved.""" - plan = { - "locale": "zh-CN", - "has_enough_context": True, - "title": "Existing Title", - "steps": [] - } - - result = validate_and_fix_plan(plan) - - # Existing values should be preserved - assert result["locale"] == "zh-CN" - assert result["has_enough_context"] is True - assert result["title"] == "Existing Title" - - class TestValidateAndFixPlanIssue650: """Specific tests for Issue #650 scenarios."""