diff --git a/features/steps/tdd_auto_debug_analyze_error_mutation_steps.py b/features/steps/tdd_auto_debug_analyze_error_mutation_steps.py new file mode 100644 index 000000000..d7dd42012 --- /dev/null +++ b/features/steps/tdd_auto_debug_analyze_error_mutation_steps.py @@ -0,0 +1,137 @@ +"""Step definitions for tdd_auto_debug_analyze_error_mutation.feature. + +This module captures bug #10494: AutoDebugAgent._analyze_error() mutates the +input state dict in-place and returns the full state object, violating the +LangGraph node contract. + +LangGraph node functions must return a dict of state updates (only the changed +keys), not mutate the input state in-place and return the full state object. +Returning the full mutated state can cause LangGraph to double-apply updates +when merging state, leading to duplicate messages in the messages list. + +Expected fix: return only the changed keys: + + def _analyze_error(self, state: AutoDebugState) -> dict[str, Any]: + # ... + new_message = { + "role": "assistant", + "content": analysis, + "type": "error_analysis", + } + return { + "messages": state.get("messages", []) + [new_message], + } + +See CONTRIBUTING.md > Bug Fix Workflow > TDD Issue Test Tags. +""" + +from __future__ import annotations + +from typing import Any + +from behave import given, then, when +from langchain_community.llms import FakeListLLM + +from cleveragents.agents.graphs.auto_debug import AutoDebugAgent, AutoDebugState + + +@given("an AutoDebugAgent with a FakeListLLM for the mutation test") +def step_create_agent_with_fake_llm(context: Any) -> None: + """Create an AutoDebugAgent backed by a FakeListLLM for deterministic testing.""" + context.mutation_test_llm = FakeListLLM( + responses=["Error analysis: null pointer dereference"] + ) + context.mutation_test_agent = AutoDebugAgent(llm=context.mutation_test_llm) + + +@given("a state with an empty messages list for the mutation test") +def step_create_state_with_empty_messages(context: Any) -> None: + """Create a minimal AutoDebugState with an empty messages list.""" + context.mutation_test_state: AutoDebugState = { + "error_message": "NullPointerException at line 42", + "code_context": "x = obj.method()", + "messages": [], + "context": {}, + "result": None, + "error": None, + "metadata": {}, + "attempted_fixes": [], + "current_fix": {}, + "fix_validated": False, + } + # Capture a reference to the original messages list before calling the node + context.mutation_test_original_messages = context.mutation_test_state["messages"] + + +@when("_analyze_error is called with the state for the mutation test") +def step_call_analyze_error(context: Any) -> None: + """Invoke _analyze_error directly and capture the returned value.""" + context.mutation_test_result = context.mutation_test_agent._analyze_error( + context.mutation_test_state + ) + + +@then("the analyze_error result should not be the same object as the input state") +def step_assert_result_is_not_state(context: Any) -> None: + """Assert that _analyze_error returns a new dict, not the same state object. + + LangGraph node functions must return a dict of updates, not the full state. + This assertion FAILS against the current buggy implementation because + _analyze_error returns ``state`` directly (same object identity). + """ + result = context.mutation_test_result + state = context.mutation_test_state + assert result is not state, ( + "_analyze_error must not return the same state object. " + "LangGraph node functions must return a dict of updates, not the full state." + ) + + +@then("the original messages list should not have been mutated by analyze_error") +def step_assert_messages_not_mutated(context: Any) -> None: + """Assert that the original state messages list was not mutated in-place. + + This assertion FAILS against the current buggy implementation because + _analyze_error calls ``state.setdefault("messages", []).append(...)`` + which mutates the list in-place. + """ + original_messages = context.mutation_test_original_messages + assert original_messages == [], ( + f"_analyze_error mutated state['messages'] in-place: {original_messages}. " + "Node functions must return updates, not mutate state." + ) + + +@then("the analyze_error result should be a plain dict of updates") +def step_assert_result_is_plain_dict(context: Any) -> None: + """Assert that the returned value is a plain dict (update dict, not full state). + + A correct LangGraph node returns only the keys that changed, e.g.: + {"messages": [...new_message...]} + The returned dict should NOT be the full AutoDebugState — it should contain + only the keys that were modified by this node. + """ + result = context.mutation_test_result + assert isinstance(result, dict), ( + f"Expected _analyze_error to return a dict, got {type(result)}" + ) + # A proper update dict should only contain the keys that changed. + # The current buggy implementation returns the full state (all 11 keys). + # A correct implementation returns only {"messages": [...]}. + all_state_keys = { + "messages", + "context", + "result", + "error", + "metadata", + "error_message", + "code_context", + "attempted_fixes", + "current_fix", + "fix_validated", + } + result_keys = set(result.keys()) + assert result_keys != all_state_keys, ( + "_analyze_error returned the full state dict instead of an update dict. " + f"Expected only changed keys (e.g. {{'messages'}}), got all keys: {result_keys}" + ) diff --git a/features/tdd_auto_debug_analyze_error_mutation.feature b/features/tdd_auto_debug_analyze_error_mutation.feature new file mode 100644 index 000000000..67f2f0799 --- /dev/null +++ b/features/tdd_auto_debug_analyze_error_mutation.feature @@ -0,0 +1,24 @@ +@tdd_issue @tdd_issue_10494 @tdd_expected_fail +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 + So that LangGraph can correctly merge state without double-applying mutations + + The current implementation mutates the input state dict in-place and returns + the full state object, violating the LangGraph node contract. This test + captures that violation so it can be tracked and fixed. + + LangGraph node functions must return a dict of updates (only the changed keys), + not mutate the input state in-place and return the full state object. Returning + the full mutated state can cause LangGraph to double-apply updates when merging + state, leading to duplicate messages in the messages list. + + See CONTRIBUTING.md > Bug Fix Workflow > TDD Issue Test Tags. + + Scenario: Bug #10494 — _analyze_error returns the same state object instead of an update dict + Given an AutoDebugAgent with a FakeListLLM for the mutation test + And a state with an empty messages list for the mutation test + When _analyze_error is called with the state for the mutation test + Then the analyze_error result should not be the same object as the input state + And the original messages list should not have been mutated by analyze_error + And the analyze_error result should be a plain dict of updates