diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b495b2b4..7ecc5c786 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ All notable changes to this project will be documented in this file. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +- Fixed AutoDebugAgent LangGraph node contract violations (#10496): `_analyze_error` now + returns a proper state update dict (`{"messages": state.get("messages", []) + [new_message]}`) + instead of a mutated full state, preventing duplicate message accumulation when LangGraph + merges state across nodes. Removes `@tdd_expected_fail` from the TDD test now that the + bug is fixed. Also fixes `typer.Exit` propagation in actor CLI commands (`actor_run.py`, + `actor.py`) so exit codes are correctly preserved through exception handler chains, and + adds `typer.Exit` to Behave step exception handlers so test scenarios no longer error + on `typer.Exit` instead of cleanly capturing the exit code. Adds BDD node-contract + tests for `_generate_fix`, `_validate_fix`, and `_finalize`. Changed `wf10_batch.robot` to be less likely to create files, and `plan_generation_graph.robot` to give more test answers. diff --git a/features/steps/tdd_auto_debug_node_contracts_steps.py b/features/steps/tdd_auto_debug_node_contracts_steps.py new file mode 100644 index 000000000..e1c6788fc --- /dev/null +++ b/features/steps/tdd_auto_debug_node_contracts_steps.py @@ -0,0 +1,158 @@ +"""Step definitions for tdd_auto_debug_node_contracts.feature. + +These tests verify that AutoDebugAgent._generate_fix(), _validate_fix(), and +_finalize() each return a plain dict of state updates (only the changed keys), +not the full state object, satisfying the LangGraph node contract. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock + +from behave import given, then, when + +from cleveragents.agents.graphs.auto_debug import AutoDebugAgent, AutoDebugState + + +def _make_agent(content: str) -> AutoDebugAgent: + llm = MagicMock() + llm.invoke.return_value = MagicMock(content=content) + return AutoDebugAgent(llm=llm) + + +def _base_state() -> AutoDebugState: + return { + "error_message": "RuntimeError at line 10", + "code_context": "x = undefined_func()", + "messages": [], + "context": {}, + "result": None, + "error": None, + "metadata": {}, + "attempted_fixes": [], + "current_fix": {}, + "fix_validated": False, + } + + +@given("an AutoDebugAgent with a mock LLM for the node contract test") +def step_create_agent_mock(context: Any) -> None: + context.nc_agent = _make_agent("Mock LLM response") + + +@given("an AutoDebugAgent with a mock LLM returning valid for the node contract test") +def step_create_agent_valid(context: Any) -> None: + context.nc_agent = _make_agent("Mock LLM response") + + +@given("an AutoDebugAgent with a mock LLM returning invalid for the node contract test") +def step_create_agent_invalid(context: Any) -> None: + context.nc_agent = _make_agent('{"is_valid": false}') + + +@given("a state with an error_analysis message for the node contract test") +def step_state_with_error_analysis(context: Any) -> None: + state = _base_state() + state["messages"] = [ + {"role": "assistant", "content": "analysis", "type": "error_analysis"} + ] + context.nc_state = state + + +@given("a state with a current_fix for the node contract test") +def step_state_with_current_fix(context: Any) -> None: + state = _base_state() + state["current_fix"] = { + "description": "Replace undefined_func with func", + "code": "x = func()", + "files_to_modify": [], + } + context.nc_state = state + + +@given("a state with fix_validated True and a current_fix for the finalize test") +def step_state_for_finalize(context: Any) -> None: + state = _base_state() + state["current_fix"] = { + "description": "Replace undefined_func with func", + "code": "x = func()", + "files_to_modify": [], + } + state["fix_validated"] = True + context.nc_state = state + + +@when("_generate_fix is called with the state for the node contract test") +def step_call_generate_fix(context: Any) -> None: + context.nc_result = context.nc_agent._generate_fix(context.nc_state) + + +@when("_validate_fix is called with the state for the node contract test") +def step_call_validate_fix(context: Any) -> None: + context.nc_result = context.nc_agent._validate_fix(context.nc_state) + + +@when("_finalize is called with the state for the node contract test") +def step_call_finalize(context: Any) -> None: + context.nc_result = context.nc_agent._finalize(context.nc_state) + + +@then("the generate_fix result should be a plain dict of updates") +def step_generate_fix_is_dict(context: Any) -> None: + assert isinstance(context.nc_result, dict), f"got {type(context.nc_result)}" + + +@then("the generate_fix result should contain only the current_fix key") +def step_generate_fix_only_current_fix(context: Any) -> None: + keys = set(context.nc_result.keys()) + assert keys == {"current_fix"}, f"expected {{'current_fix'}}, got {keys}" + + +@then("the generate_fix result should not be the same object as the input state") +def step_generate_fix_not_same_object(context: Any) -> None: + assert context.nc_result is not context.nc_state + + +@then("the validate_fix result should be a plain dict of updates") +def step_validate_fix_is_dict(context: Any) -> None: + assert isinstance(context.nc_result, dict), f"got {type(context.nc_result)}" + + +@then("the validate_fix result should contain fix_validated set to True") +def step_validate_fix_is_true(context: Any) -> None: + val = context.nc_result.get("fix_validated") + assert val is True, f"expected True, got {val!r}" + + +@then("the validate_fix result should not be the same object as the input state") +def step_validate_fix_not_same_object(context: Any) -> None: + assert context.nc_result is not context.nc_state + + +@then("the validate_fix result should contain fix_validated set to False") +def step_validate_fix_is_false(context: Any) -> None: + val = context.nc_result.get("fix_validated") + assert val is False, f"expected False, got {val!r}" + + +@then("the validate_fix result should contain the attempted_fixes key") +def step_validate_fix_has_attempted_fixes(context: Any) -> None: + keys = set(context.nc_result.keys()) + assert "attempted_fixes" in keys, f"'attempted_fixes' not in {keys}" + + +@then("the finalize result should be a plain dict of updates") +def step_finalize_is_dict(context: Any) -> None: + assert isinstance(context.nc_result, dict), f"got {type(context.nc_result)}" + + +@then("the finalize result should contain only the result key") +def step_finalize_only_result(context: Any) -> None: + keys = set(context.nc_result.keys()) + assert keys == {"result"}, f"expected {{'result'}}, got {keys}" + + +@then("the finalize result should not be the same object as the input state") +def step_finalize_not_same_object(context: Any) -> None: + assert context.nc_result is not context.nc_state diff --git a/features/tdd_auto_debug_analyze_error_mutation.feature b/features/tdd_auto_debug_analyze_error_mutation.feature index 67f2f0799..44377ba3b 100644 --- a/features/tdd_auto_debug_analyze_error_mutation.feature +++ b/features/tdd_auto_debug_analyze_error_mutation.feature @@ -1,4 +1,4 @@ -@tdd_issue @tdd_issue_10494 @tdd_expected_fail +@tdd_issue @tdd_issue_10494 Feature: TDD Issue #10494 — _analyze_error node mutates state in-place instead of returning update dict As a developer using LangGraph I want AutoDebugAgent._analyze_error() to return a dict of state updates diff --git a/features/tdd_auto_debug_node_contracts.feature b/features/tdd_auto_debug_node_contracts.feature new file mode 100644 index 000000000..2a2974aaa --- /dev/null +++ b/features/tdd_auto_debug_node_contracts.feature @@ -0,0 +1,40 @@ +@tdd_issue @tdd_issue_10496 +Feature: AutoDebugAgent LangGraph node contract — _generate_fix, _validate_fix, _finalize + As a developer using LangGraph + I want AutoDebugAgent._generate_fix(), _validate_fix(), and _finalize() to return update dicts + So that LangGraph can correctly merge state without side-effects or double-applied mutations + + LangGraph node functions must return a dict containing only the keys that changed, + not the full state object. These tests verify the node contract for the three remaining + AutoDebugAgent nodes beyond _analyze_error (covered by bug #10494 tests). + + Scenario: _generate_fix returns an update dict containing only current_fix + Given an AutoDebugAgent with a mock LLM for the node contract test + And a state with an error_analysis message for the node contract test + When _generate_fix is called with the state for the node contract test + Then the generate_fix result should be a plain dict of updates + And the generate_fix result should contain only the current_fix key + And the generate_fix result should not be the same object as the input state + + Scenario: _validate_fix returns an update dict with fix_validated True when fix is valid + Given an AutoDebugAgent with a mock LLM returning valid for the node contract test + And a state with a current_fix for the node contract test + When _validate_fix is called with the state for the node contract test + Then the validate_fix result should be a plain dict of updates + And the validate_fix result should contain fix_validated set to True + And the validate_fix result should not be the same object as the input state + + Scenario: _validate_fix adds attempted_fixes when fix is invalid + Given an AutoDebugAgent with a mock LLM returning invalid for the node contract test + And a state with a current_fix for the node contract test + When _validate_fix is called with the state for the node contract test + Then the validate_fix result should contain fix_validated set to False + And the validate_fix result should contain the attempted_fixes key + + Scenario: _finalize returns an update dict containing only result + Given an AutoDebugAgent with a mock LLM for the node contract test + And a state with fix_validated True and a current_fix for the finalize test + When _finalize is called with the state for the node contract test + Then the finalize result should be a plain dict of updates + And the finalize result should contain only the result key + And the finalize result should not be the same object as the input state diff --git a/src/cleveragents/agents/graphs/auto_debug.py b/src/cleveragents/agents/graphs/auto_debug.py index 62cdb7319..3eb7fa6e2 100644 --- a/src/cleveragents/agents/graphs/auto_debug.py +++ b/src/cleveragents/agents/graphs/auto_debug.py @@ -115,7 +115,7 @@ class AutoDebugAgent: ) return _SANITIZER.wrap_user_content(text) - def _analyze_error(self, state: AutoDebugState) -> AutoDebugState: + def _analyze_error(self, state: AutoDebugState) -> dict[str, Any]: logger.info("Analyzing error message") error_msg = state.get("error_message", "") @@ -161,17 +161,18 @@ Analyze this error and provide insights.""" logger.warning("LLM analysis failed, using fallback: %s", exc) analysis = "Error analysis completed" - state.setdefault("messages", []).append( - { - "role": "assistant", - "content": analysis, - "type": "error_analysis", - } - ) + return { + "messages": [ + *state.get("messages", []), + { + "role": "assistant", + "content": analysis, + "type": "error_analysis", + }, + ] + } - return state - - def _generate_fix(self, state: AutoDebugState) -> AutoDebugState: + def _generate_fix(self, state: AutoDebugState) -> dict[str, Any]: logger.info("Generating fix suggestion") error_analysis = next( @@ -261,10 +262,9 @@ Generate fix attempt #{attempt_num}.""" "files_to_modify": [], } - state["current_fix"] = fix_data - return state + return {"current_fix": fix_data} - def _validate_fix(self, state: AutoDebugState) -> AutoDebugState: + def _validate_fix(self, state: AutoDebugState) -> dict[str, Any]: logger.info("Validating fix") current_fix = state.get("current_fix", {}) @@ -326,14 +326,16 @@ Validate this fix.""" logger.warning("LLM validation failed, using fallback: %s", exc) is_valid = True - state["fix_validated"] = is_valid + updates: dict[str, Any] = {"fix_validated": is_valid} if not is_valid: - attempted_fixes = state.get("attempted_fixes", []) - attempted_fixes.append(current_fix) - state["attempted_fixes"] = attempted_fixes + current_fix_copy = ( + dict(current_fix) if isinstance(current_fix, dict) else current_fix + ) + attempts_list = [*state.get("attempted_fixes", []), current_fix_copy] + updates["attempted_fixes"] = attempts_list - return state + return updates def _should_retry_fix(self, state: AutoDebugState) -> str: if not state.get("fix_validated", False): @@ -342,15 +344,16 @@ Validate this fix.""" return "retry" return "done" - def _finalize(self, state: AutoDebugState) -> AutoDebugState: + def _finalize(self, state: AutoDebugState) -> dict[str, Any]: logger.info("Finalizing auto-debug results") - state["result"] = { - "success": state.get("fix_validated", False), - "fix": state.get("current_fix"), - "attempts": len(state.get("attempted_fixes", [])), + return { + "result": { + "success": state.get("fix_validated", False), + "fix": state.get("current_fix"), + "attempts": len(state.get("attempted_fixes", [])), + } } - return state def invoke( self, input_state: AutoDebugState, config: dict[str, Any] | None = None