agents/graphs/plan_generation: Add test for `_should_retry` incorrectly mutating state in conditional edge function #10397

New issue

Open

opened 2026-04-18 09:28:49 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-18 09:28:49 +00:00

Owner

Metadata

Commit: test(agents/graphs/plan_generation): add expected-fail test for _should_retry state mutation
Branch: test/plan-generation-should-retry-state-mutation

Background and Context

PlanGenerationGraph._should_retry() is registered as a LangGraph conditional edge function but incorrectly mutates state["retry_count"] directly. LangGraph conditional edge functions must be pure routing functions — they should only inspect state and return a routing key. This TDD issue captures the failing test that exposes the bug before the fix is applied.

Expected Behavior

The test test_should_retry_does_not_mutate_state should initially fail (exposing the bug), and pass once the fix is applied (moving the retry_count increment to a proper node function).

Acceptance Criteria

Test test_should_retry_does_not_mutate_state exists and is decorated with @tdd_issue, @tdd_issue_1, and @tdd_expected_fail
Test verifies that _should_retry() does not mutate state["retry_count"]
Test confirms the routing key returned is "retry" when validation fails
Test is marked as expected-fail until the corresponding bug fix is applied

Subtasks

Add test_should_retry_does_not_mutate_state test to the plan generation graph test suite
Decorate with @tdd_issue, @tdd_issue_1, @tdd_expected_fail
Verify test fails against current implementation (confirming the bug)
Verify test passes after the fix is applied

Definition of Done

This issue is closed when the test exists, is correctly decorated, fails against the buggy implementation, and passes after the fix described in the linked bug issue is applied.

Test Description

Add a test that verifies PlanGenerationGraph._should_retry() does not mutate the state dict (LangGraph conditional edge functions must be pure routing functions that only return a routing key, not modify state).

Failing Scenario

@tdd_issue
@tdd_issue_1
@tdd_expected_fail
def test_should_retry_does_not_mutate_state():
    """_should_retry() is a conditional edge function and must not mutate state."""
    from unittest.mock import MagicMock
    from langchain_community.llms import FakeListLLM
    from cleveragents.agents.graphs.plan_generation import PlanGenerationGraph
    from cleveragents.domain.models.core import Plan, Project

    mock_llm = FakeListLLM(responses=["a"] * 10)
    graph = PlanGenerationGraph(llm=mock_llm)

    state = {
        "project": MagicMock(),
        "plan": MagicMock(),
        "contexts": [],
        "context_summary": "",
        "context_dependencies": {},
        "context_relevance": {},
        "context_analysis_error": None,
        "actor_name": None,
        "actor_options": {},
        "actor_graph_descriptor": None,
        "actor_initial_context": {},
        "prompt": "test",
        "analyzed_requirements": {},
        "generated_changes": [],
        "validation_result": {"status": "FAIL", "message": "failed"},
        "retry_count": 0,
        "error": None,
    }

    original_retry_count = state["retry_count"]
    routing_key = graph._should_retry(state)

    # Conditional edge functions must NOT mutate state
    # The retry_count should remain unchanged - state mutation is LangGraph's job
    assert state["retry_count"] == original_retry_count, (
        f"_should_retry() mutated state['retry_count'] from {original_retry_count} "
        f"to {state['retry_count']}. Conditional edge functions must be pure."
    )
    assert routing_key == "retry"

Root Cause

In src/cleveragents/agents/graphs/plan_generation.py, the _should_retry() method is used as a LangGraph conditional edge function but incorrectly mutates the state:

def _should_retry(self, state: PlanGenerationState) -> str:
    validation = state.get("validation_result", {})
    retry_count = state.get("retry_count", 0)

    if validation.get("status") == "FAIL" and retry_count < self.max_retries:
        state["retry_count"] = retry_count + 1  # BUG: mutates state in conditional edge!
        return "retry"

    return "end"

In LangGraph, conditional edge functions are routing functions — they must only inspect state and return a routing key. State mutations in conditional edge functions are not processed by LangGraph's state management system and can cause inconsistent behavior. The retry_count increment should be done in a proper node function that returns a state update dict.

Expected Fix

Move the retry_count increment to a dedicated node or to the _analyze_requirements node:

def _should_retry(self, state: PlanGenerationState) -> str:
    """Pure routing function - must not mutate state."""
    validation = state.get("validation_result", {})
    retry_count = state.get("retry_count", 0)

    if validation.get("status") == "FAIL" and retry_count < self.max_retries:
        return "retry"
    return "end"

def _increment_retry_count(self, state: PlanGenerationState) -> dict:
    """Node function to increment retry count."""
    return {"retry_count": state.get("retry_count", 0) + 1}

Automated by CleverAgents Bot
Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

## Metadata - **Commit:** `test(agents/graphs/plan_generation): add expected-fail test for _should_retry state mutation` - **Branch:** `test/plan-generation-should-retry-state-mutation` ## Background and Context `PlanGenerationGraph._should_retry()` is registered as a LangGraph conditional edge function but incorrectly mutates `state["retry_count"]` directly. LangGraph conditional edge functions must be pure routing functions — they should only inspect state and return a routing key. This TDD issue captures the failing test that exposes the bug before the fix is applied. ## Expected Behavior The test `test_should_retry_does_not_mutate_state` should initially fail (exposing the bug), and pass once the fix is applied (moving the `retry_count` increment to a proper node function). ## Acceptance Criteria - [ ] Test `test_should_retry_does_not_mutate_state` exists and is decorated with `@tdd_issue`, `@tdd_issue_1`, and `@tdd_expected_fail` - [ ] Test verifies that `_should_retry()` does not mutate `state["retry_count"]` - [ ] Test confirms the routing key returned is `"retry"` when validation fails - [ ] Test is marked as expected-fail until the corresponding bug fix is applied ## Subtasks - [ ] Add `test_should_retry_does_not_mutate_state` test to the plan generation graph test suite - [ ] Decorate with `@tdd_issue`, `@tdd_issue_1`, `@tdd_expected_fail` - [ ] Verify test fails against current implementation (confirming the bug) - [ ] Verify test passes after the fix is applied ## Definition of Done This issue is closed when the test exists, is correctly decorated, fails against the buggy implementation, and passes after the fix described in the linked bug issue is applied. --- ## Test Description Add a test that verifies `PlanGenerationGraph._should_retry()` does not mutate the state dict (LangGraph conditional edge functions must be pure routing functions that only return a routing key, not modify state). ## Failing Scenario ```python @tdd_issue @tdd_issue_1 @tdd_expected_fail def test_should_retry_does_not_mutate_state(): """_should_retry() is a conditional edge function and must not mutate state.""" from unittest.mock import MagicMock from langchain_community.llms import FakeListLLM from cleveragents.agents.graphs.plan_generation import PlanGenerationGraph from cleveragents.domain.models.core import Plan, Project mock_llm = FakeListLLM(responses=["a"] * 10) graph = PlanGenerationGraph(llm=mock_llm) state = { "project": MagicMock(), "plan": MagicMock(), "contexts": [], "context_summary": "", "context_dependencies": {}, "context_relevance": {}, "context_analysis_error": None, "actor_name": None, "actor_options": {}, "actor_graph_descriptor": None, "actor_initial_context": {}, "prompt": "test", "analyzed_requirements": {}, "generated_changes": [], "validation_result": {"status": "FAIL", "message": "failed"}, "retry_count": 0, "error": None, } original_retry_count = state["retry_count"] routing_key = graph._should_retry(state) # Conditional edge functions must NOT mutate state # The retry_count should remain unchanged - state mutation is LangGraph's job assert state["retry_count"] == original_retry_count, ( f"_should_retry() mutated state['retry_count'] from {original_retry_count} " f"to {state['retry_count']}. Conditional edge functions must be pure." ) assert routing_key == "retry" ``` ## Root Cause In `src/cleveragents/agents/graphs/plan_generation.py`, the `_should_retry()` method is used as a LangGraph conditional edge function but incorrectly mutates the state: ```python def _should_retry(self, state: PlanGenerationState) -> str: validation = state.get("validation_result", {}) retry_count = state.get("retry_count", 0) if validation.get("status") == "FAIL" and retry_count < self.max_retries: state["retry_count"] = retry_count + 1 # BUG: mutates state in conditional edge! return "retry" return "end" ``` In LangGraph, conditional edge functions are **routing functions** — they must only inspect state and return a routing key. State mutations in conditional edge functions are not processed by LangGraph's state management system and can cause inconsistent behavior. The `retry_count` increment should be done in a proper node function that returns a state update dict. ## Expected Fix Move the `retry_count` increment to a dedicated node or to the `_analyze_requirements` node: ```python def _should_retry(self, state: PlanGenerationState) -> str: """Pure routing function - must not mutate state.""" validation = state.get("validation_result", {}) retry_count = state.get("retry_count", 0) if validation.get("status") == "FAIL" and retry_count < self.max_retries: return "retry" return "end" def _increment_retry_count(self, state: PlanGenerationState) -> dict: """Node function to increment retry count.""" return {"retry_count": state.get("retry_count", 0) + 1} ``` --- **Automated by CleverAgents Bot** Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

HAL9000 referenced this issue

2026-04-18 09:29:26 +00:00

agents/graphs/plan_generation: _should_retry mutates state["retry_count"] in a LangGraph conditional edge function, bypassing state management #10400

HAL9000 added the