From 0bad313aa308eabdefdbf4f0ae3df720b50c7f95 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Tue, 12 May 2026 10:08:21 +0000 Subject: [PATCH] fix(agents/graphs/auto_debug): return update dicts from node functions instead of mutating state in-place ISSUES CLOSED: #10494 #10496 --- CHANGELOG.md | 2 + .../tdd_auto_debug_state_mutation_steps.py | 249 ++++++++++++++++++ .../tdd_auto_debug_state_mutation.feature | 42 +++ src/cleveragents/agents/graphs/auto_debug.py | 75 ++++-- 4 files changed, 343 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/CHANGELOG.md b/CHANGELOG.md index 672c4835e..c96fdf61c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -232,6 +232,8 @@ Changed `wf10_batch.robot` to be less likely to create files, and `hot_context_hash`, `hot_context_ref`, `actor_state_ref`, and `relevant_resources` are all populated for Strategize-phase decisions. +- **`auto_debug` node functions return update dicts instead of mutating state** (#10494, #10496): The `update_node()`, `set_active_node()`, and related methods in `src/cleveragents/agents/graphs/auto_debug.py` now return dictionaries containing the computed updates rather than modifying internal state objects in-place. Callers receive explicit update dicts for their own mutation decisions, eliminating silent state corruption from concurrent callers and making the data flow testable. (#10494, #10496) + ### Changed - Fixed stale `AUTO-BUG-POOL` tracking prefix references in automation-tracking.md documentation and agent-system-specification.md spec document, replaced with correct `AUTO-BUG-SUP` prefix used by the bug-hunt-pool-supervisor agent (#7875). 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