test(agents/graphs/auto_debug): add expected-fail test for _analyze_error in-place state mutation
Added a TDD-style test for issue #10494 by introducing a new features/tdd_auto_debug_analyze_error_mutation.feature with a scenario tagged @tdd_issue @tdd_issue_10494 @tdd_expected_fail, and implemented Behave steps in features/steps/tdd_auto_debug_analyze_error_mutation_steps.py. The test captures the bug that _analyze_error mutates the state in-place and returns the full state object instead of a dict of updates, violating the LangGraph node contract. The @tdd_expected_fail tag inverts the test outcome so CI remains green while the bug exists. ISSUES CLOSED: #10494
This commit is contained in:
@@ -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}"
|
||||
)
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user