fix(auto_debug): fix LangGraph node contracts and resolve CI failures
- Return state update dict from _analyze_error using iterable unpacking so existing messages are preserved (state.get + [new_message]) and the RUF005 concatenation lint rule is satisfied - Remove @tdd_expected_fail from tdd_auto_debug_analyze_error_mutation feature now that bug #10494 is resolved - Add BDD node-contract tests for _generate_fix, _validate_fix, _finalize verifying each returns only the changed keys, not the full state - Fix typer.Exit propagation in actor_run.py and actor.py: widen the passthrough except clause from click.exceptions.Exit to (click.exceptions.Exit, typer.Exit) so _resolve_actor's typer.Exit(2) is not swallowed and re-raised as Exit(3) - Add typer.Exit to Behave step except clauses in actor_run_signature_resolve_steps.py and actor_run_signature_security_steps.py so test scenarios capture the exit code instead of erroring - Fix SQLChatMessageHistory call in memory_service.py: rename kwarg connection_string to connection per langchain_community 0.4.x API change ISSUES CLOSED: #10496
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -165,7 +165,7 @@ def step_resolve_with_no_config_data(context: Any) -> None:
|
||||
try:
|
||||
resolve_config_files("local/empty-actor", [])
|
||||
context.resolve_exit_code = 0
|
||||
except (SystemExit, click.exceptions.Exit) as exc:
|
||||
except (SystemExit, click.exceptions.Exit, typer.Exit) as exc:
|
||||
context.resolve_exit_code = getattr(
|
||||
exc, "exit_code", getattr(exc, "code", 1)
|
||||
)
|
||||
@@ -209,7 +209,7 @@ def step_resolve_unknown_actor_directly(context: Any) -> None:
|
||||
try:
|
||||
resolve_config_files("nonexistent/actor", [])
|
||||
context.resolve_exit_code = 0
|
||||
except (SystemExit, click.exceptions.Exit) as exc:
|
||||
except (SystemExit, click.exceptions.Exit, typer.Exit) as exc:
|
||||
context.resolve_exit_code = getattr(
|
||||
exc, "exit_code", getattr(exc, "code", 1)
|
||||
)
|
||||
@@ -260,7 +260,7 @@ def step_resolve_with_empty_config_blob(context: Any) -> None:
|
||||
try:
|
||||
resolve_config_files("local/empty-blob-actor", [])
|
||||
context.resolve_exit_code = 0
|
||||
except (SystemExit, click.exceptions.Exit) as exc:
|
||||
except (SystemExit, click.exceptions.Exit, typer.Exit) as exc:
|
||||
context.resolve_exit_code = getattr(
|
||||
exc, "exit_code", getattr(exc, "code", 1)
|
||||
)
|
||||
@@ -356,7 +356,7 @@ def step_resolve_with_unserializable_config_blob(context: Any) -> None:
|
||||
try:
|
||||
resolve_config_files("local/bad-blob-actor", [])
|
||||
context.resolve_exit_code = 0
|
||||
except (SystemExit, click.exceptions.Exit) as exc:
|
||||
except (SystemExit, click.exceptions.Exit, typer.Exit) as exc:
|
||||
context.resolve_exit_code = getattr(
|
||||
exc, "exit_code", getattr(exc, "code", 1)
|
||||
)
|
||||
|
||||
@@ -49,7 +49,7 @@ def step_resolve_with_control_character_name(context: Any) -> None:
|
||||
try:
|
||||
resolve_config_files(actor_name, [])
|
||||
context.resolve_exit_code = 0
|
||||
except (SystemExit, click.exceptions.Exit) as exc:
|
||||
except (SystemExit, click.exceptions.Exit, typer.Exit) as exc:
|
||||
context.resolve_exit_code = getattr(
|
||||
exc, "exit_code", getattr(exc, "code", 1)
|
||||
)
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
@@ -131,11 +131,12 @@ Analyze this error and provide insights."""
|
||||
|
||||
return {
|
||||
"messages": [
|
||||
*state.get("messages", []),
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": analysis,
|
||||
"type": "error_analysis",
|
||||
}
|
||||
},
|
||||
]
|
||||
}
|
||||
|
||||
@@ -280,7 +281,9 @@ Validate this fix."""
|
||||
updates: dict[str, Any] = {"fix_validated": is_valid}
|
||||
|
||||
if not is_valid:
|
||||
current_fix_copy = dict(current_fix) if isinstance(current_fix, dict) else current_fix
|
||||
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
|
||||
|
||||
|
||||
@@ -490,7 +490,7 @@ class MemoryService:
|
||||
BaseChatMessageHistory,
|
||||
SQLChatMessageHistory(
|
||||
session_id=session_id,
|
||||
connection_string=connection_string,
|
||||
connection=connection_string,
|
||||
table_name="message_history",
|
||||
),
|
||||
)
|
||||
|
||||
@@ -185,7 +185,7 @@ def run(
|
||||
except UnsafeConfigurationError as exc:
|
||||
typer.echo(f"Error: {exc}", err=True)
|
||||
raise typer.Exit(code=1) from exc
|
||||
except click.exceptions.Exit:
|
||||
except (click.exceptions.Exit, typer.Exit):
|
||||
raise
|
||||
except CleverAgentsError as exc:
|
||||
typer.echo(f"Error: {exc}", err=True)
|
||||
|
||||
@@ -159,7 +159,7 @@ def run(
|
||||
except UnsafeConfigurationError as exc:
|
||||
typer.echo(f"Error: {exc}", err=True)
|
||||
raise typer.Exit(code=1) from exc
|
||||
except click.exceptions.Exit:
|
||||
except (click.exceptions.Exit, typer.Exit):
|
||||
raise
|
||||
except CleverAgentsError as exc:
|
||||
typer.echo(f"Error: {exc}", err=True)
|
||||
|
||||
Reference in New Issue
Block a user