From 22076b7cefb00dbd95ad994fc63f64fb82764b42 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Sun, 19 Apr 2026 08:29:45 +0000 Subject: [PATCH] fix(agents/graphs/auto_debug): return update dicts from node functions instead of mutating state in-place Node functions _analyze_error, _generate_fix, _validate_fix, and _finalize previously mutated the input state dict in-place and returned the full state object, violating the LangGraph node contract which requires returning only a dict of state updates (changed keys). This fix updates all four node functions to return new partial state dicts containing only the changed keys, consistent with the correct pattern already used in context_analysis.py and plan_generation.py. Changes: - _analyze_error: returns {"messages": [*existing, new_message]} - _generate_fix: returns {"current_fix": fix_data} - _validate_fix: returns {"fix_validated": is_valid} plus "attempted_fixes" when invalid - _finalize: returns {"result": {...}} - All four functions now have return type dict[str, Any] instead of AutoDebugState - Added TDD BDD tests verifying no in-place mutation occurs ISSUES CLOSED: #10496 --- .../tdd_auto_debug_state_mutation_steps.py | 249 ++++++++++++++++++ .../tdd_auto_debug_state_mutation.feature | 42 +++ src/cleveragents/agents/graphs/auto_debug.py | 75 ++++-- 3 files changed, 341 insertions(+), 25 deletions(-) create mode 100644 features/steps/tdd_auto_debug_state_mutation_steps.py create mode 100644 features/tdd_auto_debug_state_mutation.feature diff --git a/features/steps/tdd_auto_debug_state_mutation_steps.py b/features/steps/tdd_auto_debug_state_mutation_steps.py new file mode 100644 index 000000000..bd91a1ca4 --- /dev/null +++ b/features/steps/tdd_auto_debug_state_mutation_steps.py @@ -0,0 +1,249 @@ +"""Step definitions for tdd_auto_debug_state_mutation.feature. + +These steps verify that AutoDebug node functions return new state dicts +instead of mutating the input state in-place, respecting the LangGraph +node contract. +""" + +from __future__ import annotations + +import json +from copy import deepcopy +from typing import Any + +from behave import given, then, when + +from cleveragents.agents.graphs.auto_debug import AutoDebugAgent, AutoDebugState + + +class _MockResponse: + """Minimal response object with a content attribute.""" + + def __init__(self, content: str) -> None: + self.content = content + + +class _MockLLM: + """Stub LLM that returns a fixed content string.""" + + def __init__(self, content: str = "Mock LLM response") -> None: + self._content = content + + def invoke(self, messages: Any) -> _MockResponse: + return _MockResponse(self._content) + + +class _InvalidValidationLLM: + """Stub LLM that returns an invalid validation response.""" + + def invoke(self, messages: Any) -> _MockResponse: + return _MockResponse( + json.dumps( + { + "is_valid": False, + "reasoning": "Fix failed", + "issues": ["error persists"], + } + ) + ) + + +def _base_state(**overrides: Any) -> AutoDebugState: + """Create a minimal valid AutoDebugState with optional overrides.""" + state: AutoDebugState = { + "messages": [], + "context": {}, + "result": None, + "error": None, + "metadata": {}, + "error_message": "NameError: name 'x' is not defined", + "code_context": "print(x)", + "attempted_fixes": [], + "current_fix": {}, + "fix_validated": False, + } + state.update(overrides) # type: ignore[typeddict-item] + return state + + +@given("the auto debug state mutation module is imported") +def step_module_imported(context: Any) -> None: + """Verify the auto_debug module is importable.""" + assert AutoDebugAgent is not None + assert AutoDebugState is not None + + +@given("an auto debug agent for mutation testing") +def step_create_agent(context: Any) -> None: + """Create an AutoDebugAgent with a mock LLM.""" + context.mutation_agent = AutoDebugAgent(llm=_MockLLM(), max_fix_attempts=3) + + +@given("an initial auto debug state for mutation testing") +def step_initial_state(context: Any) -> None: + """Create an initial state with empty messages list.""" + context.mutation_input_state = _base_state(messages=[]) + context.mutation_original_messages_len = len( + context.mutation_input_state["messages"] + ) + + +@given("a state with an error analysis for mutation testing") +def step_state_with_analysis(context: Any) -> None: + """Create a state with an error analysis message.""" + context.mutation_input_state = _base_state( + messages=[ + { + "role": "assistant", + "content": "Detected a NameError", + "type": "error_analysis", + } + ], + current_fix={}, + ) + context.mutation_original_current_fix = deepcopy( + context.mutation_input_state["current_fix"] + ) + + +@given("a state with a current fix for mutation testing") +def step_state_with_current_fix(context: Any) -> None: + """Create a state with a current fix.""" + context.mutation_agent = AutoDebugAgent(llm=_MockLLM(), max_fix_attempts=3) + context.mutation_input_state = _base_state( + current_fix={ + "description": "Proposed fix", + "code": "x = 42\nprint(x)", + "files_to_modify": ["main.py"], + }, + fix_validated=False, + ) + context.mutation_original_fix_validated = context.mutation_input_state[ + "fix_validated" + ] + + +@given("a state with a current fix and invalid validation for mutation testing") +def step_state_with_invalid_validation(context: Any) -> None: + """Create a state with a current fix that will fail validation.""" + context.mutation_agent = AutoDebugAgent( + llm=_InvalidValidationLLM(), max_fix_attempts=3 + ) + context.mutation_input_state = _base_state( + current_fix={ + "description": "Proposed fix", + "code": "x = 42\nprint(x)", + "files_to_modify": ["main.py"], + }, + attempted_fixes=[], + fix_validated=False, + ) + context.mutation_original_attempted_fixes_len = len( + context.mutation_input_state["attempted_fixes"] + ) + + +@given("a state ready for finalization for mutation testing") +def step_state_for_finalization(context: Any) -> None: + """Create a state ready for finalization.""" + context.mutation_agent = AutoDebugAgent(llm=_MockLLM(), max_fix_attempts=3) + context.mutation_input_state = _base_state( + fix_validated=True, + current_fix={"description": "Final fix", "code": "# fixed"}, + result=None, + ) + context.mutation_original_result = context.mutation_input_state["result"] + + +@when("I call _analyze_error and capture the result") +def step_call_analyze_error(context: Any) -> None: + """Call _analyze_error and capture both input and output.""" + context.mutation_result_state = context.mutation_agent._analyze_error( + context.mutation_input_state + ) + + +@when("I call _generate_fix and capture the result") +def step_call_generate_fix(context: Any) -> None: + """Call _generate_fix and capture both input and output.""" + context.mutation_result_state = context.mutation_agent._generate_fix( + context.mutation_input_state + ) + + +@when("I call _validate_fix and capture the result") +def step_call_validate_fix(context: Any) -> None: + """Call _validate_fix and capture both input and output.""" + context.mutation_result_state = context.mutation_agent._validate_fix( + context.mutation_input_state + ) + + +@when("I call _finalize and capture the result") +def step_call_finalize(context: Any) -> None: + """Call _finalize and capture both input and output.""" + context.mutation_result_state = context.mutation_agent._finalize( + context.mutation_input_state + ) + + +@then("the returned state should be a different object from the input state") +def step_different_object(context: Any) -> None: + """Verify the returned state is a new dict, not the same object.""" + assert context.mutation_result_state is not context.mutation_input_state, ( + "Node function returned the same state object (mutated in-place). " + "LangGraph node functions must return a new dict." + ) + + +@then("the original state messages list should be unchanged") +def step_messages_unchanged(context: Any) -> None: + """Verify the original state's messages list was not mutated.""" + original_len = context.mutation_original_messages_len + actual_len = len(context.mutation_input_state["messages"]) + assert actual_len == original_len, ( + f"Original state messages list was mutated: " + f"expected {original_len} messages, got {actual_len}" + ) + + +@then("the original state current_fix should be unchanged") +def step_current_fix_unchanged(context: Any) -> None: + """Verify the original state's current_fix was not mutated.""" + original = context.mutation_original_current_fix + actual = context.mutation_input_state["current_fix"] + assert actual == original, ( + f"Original state current_fix was mutated: expected {original!r}, got {actual!r}" + ) + + +@then("the original state fix_validated should be unchanged") +def step_fix_validated_unchanged(context: Any) -> None: + """Verify the original state's fix_validated was not mutated.""" + original = context.mutation_original_fix_validated + actual = context.mutation_input_state["fix_validated"] + assert actual == original, ( + f"Original state fix_validated was mutated: " + f"expected {original!r}, got {actual!r}" + ) + + +@then("the original state attempted_fixes list should be unchanged") +def step_attempted_fixes_unchanged(context: Any) -> None: + """Verify the original state's attempted_fixes list was not mutated.""" + original_len = context.mutation_original_attempted_fixes_len + actual_len = len(context.mutation_input_state["attempted_fixes"]) + assert actual_len == original_len, ( + f"Original state attempted_fixes list was mutated: " + f"expected {original_len} items, got {actual_len}" + ) + + +@then("the original state result should be unchanged") +def step_result_unchanged(context: Any) -> None: + """Verify the original state's result was not mutated.""" + original = context.mutation_original_result + actual = context.mutation_input_state["result"] + assert actual == original, ( + f"Original state result was mutated: expected {original!r}, got {actual!r}" + ) diff --git a/features/tdd_auto_debug_state_mutation.feature b/features/tdd_auto_debug_state_mutation.feature new file mode 100644 index 000000000..415fb7b25 --- /dev/null +++ b/features/tdd_auto_debug_state_mutation.feature @@ -0,0 +1,42 @@ +Feature: AutoDebug node functions must not mutate state in-place + As a LangGraph developer + I want AutoDebug node functions to return new state dicts + So that the LangGraph node contract is respected and state isolation is maintained + + Background: + Given the auto debug state mutation module is imported + + Scenario: _analyze_error does not mutate the original state object + Given an auto debug agent for mutation testing + And an initial auto debug state for mutation testing + When I call _analyze_error and capture the result + Then the returned state should be a different object from the input state + And the original state messages list should be unchanged + + Scenario: _generate_fix does not mutate the original state object + Given an auto debug agent for mutation testing + And a state with an error analysis for mutation testing + When I call _generate_fix and capture the result + Then the returned state should be a different object from the input state + And the original state current_fix should be unchanged + + Scenario: _validate_fix does not mutate the original state object + Given an auto debug agent for mutation testing + And a state with a current fix for mutation testing + When I call _validate_fix and capture the result + Then the returned state should be a different object from the input state + And the original state fix_validated should be unchanged + + Scenario: _validate_fix with invalid fix does not mutate attempted_fixes in-place + Given an auto debug agent for mutation testing + And a state with a current fix and invalid validation for mutation testing + When I call _validate_fix and capture the result + Then the returned state should be a different object from the input state + And the original state attempted_fixes list should be unchanged + + Scenario: _finalize does not mutate the original state object + Given an auto debug agent for mutation testing + And a state ready for finalization for mutation testing + When I call _finalize and capture the result + Then the returned state should be a different object from the input state + And the original state result should be unchanged diff --git a/src/cleveragents/agents/graphs/auto_debug.py b/src/cleveragents/agents/graphs/auto_debug.py index 2bbf5b64b..d9ee5d3f9 100644 --- a/src/cleveragents/agents/graphs/auto_debug.py +++ b/src/cleveragents/agents/graphs/auto_debug.py @@ -89,7 +89,12 @@ class AutoDebugAgent: return workflow - def _analyze_error(self, state: AutoDebugState) -> AutoDebugState: + def _analyze_error(self, state: AutoDebugState) -> dict[str, Any]: + """Analyze the error message and return updated messages. + + Returns a new partial state dict with the updated messages list, + without mutating the input state. + """ logger.info("Analyzing error message") error_msg = state.get("error_message", "") @@ -129,17 +134,24 @@ 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", - } - ) + new_message: dict[str, Any] = { + "role": "assistant", + "content": analysis, + "type": "error_analysis", + } + updated_messages: list[dict[str, Any]] = [ + *state.get("messages", []), + new_message, + ] - return state + return {"messages": updated_messages} - def _generate_fix(self, state: AutoDebugState) -> AutoDebugState: + def _generate_fix(self, state: AutoDebugState) -> dict[str, Any]: + """Generate a fix suggestion and return updated current_fix. + + Returns a new partial state dict with the updated current_fix, + without mutating the input state. + """ logger.info("Generating fix suggestion") error_analysis = next( @@ -220,10 +232,14 @@ 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]: + """Validate the current fix and return updated validation state. + + Returns a new partial state dict with the updated fix_validated and + attempted_fixes fields, without mutating the input state. + """ logger.info("Validating fix") current_fix = state.get("current_fix", {}) @@ -278,14 +294,17 @@ Validate this fix.""" logger.warning("LLM validation failed, using fallback: %s", exc) is_valid = True - state["fix_validated"] = is_valid - if not is_valid: - attempted_fixes = state.get("attempted_fixes", []) - attempted_fixes.append(current_fix) - state["attempted_fixes"] = attempted_fixes + updated_attempted_fixes: list[dict[str, Any]] = [ + *state.get("attempted_fixes", []), + current_fix, + ] + return { + "fix_validated": is_valid, + "attempted_fixes": updated_attempted_fixes, + } - return state + return {"fix_validated": is_valid} def _should_retry_fix(self, state: AutoDebugState) -> str: if not state.get("fix_validated", False): @@ -294,15 +313,21 @@ Validate this fix.""" return "retry" return "done" - def _finalize(self, state: AutoDebugState) -> AutoDebugState: + def _finalize(self, state: AutoDebugState) -> dict[str, Any]: + """Finalize the auto-debug results and return updated result. + + Returns a new partial state dict with the result field populated, + without mutating the input state. + """ 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 -- 2.52.0