fix: handle [ACCEPTED] feedback gracefully without TypeError in plan review (#657)

* fix: handle [ACCEPTED] feedback gracefully without TypeError in plan review (#607)

- Add explicit None/empty feedback check to prevent processing None values
- Normalize feedback string once using strip().upper() instead of repeated calls
- Replace TypeError exception with graceful fallback to planner node
- Handle invalid feedback formats by logging warning and returning to planner
- Maintain backward compatibility for '[ACCEPTED]' and '[EDIT_PLAN]' formats
- Add test cases for None feedback, empty string feedback, and invalid formats
- Update existing test to verify graceful handling instead of exception raising

* Update src/graph/nodes.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Willem Jiang
2025-10-25 22:06:19 +08:00
committed by GitHub
parent 1d71f8910e
commit fd5a9aeae4
2 changed files with 41 additions and 6 deletions

View File

@@ -454,12 +454,37 @@ def test_human_feedback_node_accepted(monkeypatch, mock_state_base, mock_config)
def test_human_feedback_node_invalid_interrupt(
monkeypatch, mock_state_base, mock_config
):
# interrupt returns something else, should raise TypeError
# interrupt returns something else, should gracefully return to planner (not raise TypeError)
state = dict(mock_state_base)
state["auto_accepted_plan"] = False
with patch("src.graph.nodes.interrupt", return_value="RANDOM_FEEDBACK"):
with pytest.raises(TypeError):
human_feedback_node(state, mock_config)
result = human_feedback_node(state, mock_config)
assert isinstance(result, Command)
assert result.goto == "planner"
def test_human_feedback_node_none_feedback(
monkeypatch, mock_state_base, mock_config
):
# interrupt returns None, should gracefully return to planner
state = dict(mock_state_base)
state["auto_accepted_plan"] = False
with patch("src.graph.nodes.interrupt", return_value=None):
result = human_feedback_node(state, mock_config)
assert isinstance(result, Command)
assert result.goto == "planner"
def test_human_feedback_node_empty_feedback(
monkeypatch, mock_state_base, mock_config
):
# interrupt returns empty string, should gracefully return to planner
state = dict(mock_state_base)
state["auto_accepted_plan"] = False
with patch("src.graph.nodes.interrupt", return_value=""):
result = human_feedback_node(state, mock_config)
assert isinstance(result, Command)
assert result.goto == "planner"
def test_human_feedback_node_json_decode_error_first_iteration(