From 448001f532274fb737ae8f6bdb202a3ba58e6135 Mon Sep 17 00:00:00 2001 From: DanielWalnut <45447813+hetaoBackend@users.noreply.github.com> Date: Tue, 15 Jul 2025 18:51:41 +0800 Subject: [PATCH] refactor: human feedback doesn't need to check enough context (#423) --- src/graph/nodes.py | 2 -- tests/integration/test_nodes.py | 10 +++++----- tests/unit/llms/test_llm.py | 20 +++++++++++++++++++- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/graph/nodes.py b/src/graph/nodes.py index 8370e21..b81e7e6 100644 --- a/src/graph/nodes.py +++ b/src/graph/nodes.py @@ -186,8 +186,6 @@ def human_feedback_node( plan_iterations += 1 # parse the plan new_plan = json.loads(current_plan) - if new_plan["has_enough_context"]: - goto = "reporter" except json.JSONDecodeError: logger.warning("Planner response is not a valid JSON") if plan_iterations > 1: # the plan_iterations is increased before this check diff --git a/tests/integration/test_nodes.py b/tests/integration/test_nodes.py index 2eb3b71..73d565e 100644 --- a/tests/integration/test_nodes.py +++ b/tests/integration/test_nodes.py @@ -400,7 +400,7 @@ def mock_state_base(): return { "current_plan": json.dumps( { - "has_enough_context": True, + "has_enough_context": False, "title": "Test Plan", "thought": "Test Thought", "steps": [], @@ -417,9 +417,9 @@ def test_human_feedback_node_auto_accepted(monkeypatch, mock_state_base): state["auto_accepted_plan"] = True result = human_feedback_node(state) assert isinstance(result, Command) - assert result.goto == "reporter" + assert result.goto == "research_team" assert result.update["plan_iterations"] == 1 - assert result.update["current_plan"]["has_enough_context"] is True + assert result.update["current_plan"]["has_enough_context"] is False def test_human_feedback_node_edit_plan(monkeypatch, mock_state_base): @@ -441,9 +441,9 @@ def test_human_feedback_node_accepted(monkeypatch, mock_state_base): with patch("src.graph.nodes.interrupt", return_value="[ACCEPTED] Looks good!"): result = human_feedback_node(state) assert isinstance(result, Command) - assert result.goto == "reporter" + assert result.goto == "research_team" assert result.update["plan_iterations"] == 1 - assert result.update["current_plan"]["has_enough_context"] is True + assert result.update["current_plan"]["has_enough_context"] is False def test_human_feedback_node_invalid_interrupt(monkeypatch, mock_state_base): diff --git a/tests/unit/llms/test_llm.py b/tests/unit/llms/test_llm.py index dcdbc2f..397ca72 100644 --- a/tests/unit/llms/test_llm.py +++ b/tests/unit/llms/test_llm.py @@ -28,6 +28,11 @@ def dummy_conf(): def test_get_env_llm_conf(monkeypatch): + # Clear any existing environment variables that might interfere + monkeypatch.delenv("BASIC_MODEL__API_KEY", raising=False) + monkeypatch.delenv("BASIC_MODEL__BASE_URL", raising=False) + monkeypatch.delenv("BASIC_MODEL__MODEL", raising=False) + monkeypatch.setenv("BASIC_MODEL__API_KEY", "env_key") monkeypatch.setenv("BASIC_MODEL__BASE_URL", "http://env") conf = llm._get_env_llm_conf("basic") @@ -36,6 +41,9 @@ def test_get_env_llm_conf(monkeypatch): def test_create_llm_use_conf_merges_env(monkeypatch, dummy_conf): + # Clear any existing environment variables that might interfere + monkeypatch.delenv("BASIC_MODEL__BASE_URL", raising=False) + monkeypatch.delenv("BASIC_MODEL__MODEL", raising=False) monkeypatch.setenv("BASIC_MODEL__API_KEY", "env_key") result = llm._create_llm_use_conf("basic", dummy_conf) assert isinstance(result, DummyChatOpenAI) @@ -43,12 +51,22 @@ def test_create_llm_use_conf_merges_env(monkeypatch, dummy_conf): assert result.kwargs["base_url"] == "http://test" -def test_create_llm_use_conf_invalid_type(dummy_conf): +def test_create_llm_use_conf_invalid_type(monkeypatch, dummy_conf): + # Clear any existing environment variables that might interfere + monkeypatch.delenv("BASIC_MODEL__API_KEY", raising=False) + monkeypatch.delenv("BASIC_MODEL__BASE_URL", raising=False) + monkeypatch.delenv("BASIC_MODEL__MODEL", raising=False) + with pytest.raises(ValueError): llm._create_llm_use_conf("unknown", dummy_conf) def test_create_llm_use_conf_empty_conf(monkeypatch): + # Clear any existing environment variables that might interfere + monkeypatch.delenv("BASIC_MODEL__API_KEY", raising=False) + monkeypatch.delenv("BASIC_MODEL__BASE_URL", raising=False) + monkeypatch.delenv("BASIC_MODEL__MODEL", raising=False) + with pytest.raises(ValueError): llm._create_llm_use_conf("basic", {})