fix(agents/graphs/plan_generation): _validate always passes for code longer than 10 characters

Applied a Ruff format fix in features/steps/tdd_plan_generation_validate_logic_steps.py by merging two f-string lines into one.

Updated two integration tests in robot/plan_generation_graph.robot to use FakeListLLM(responses=['PASS: code looks good']*10) instead of FakeListLLM(responses=['test']*3). This is necessary because the validation logic fix makes 'test' responses fail validation (they don't contain 'PASS'), causing the workflow to retry indefinitely. The tests now provide passing responses to reflect the new behavior.

features/steps/tdd_plan_generation_validate_logic_steps.py

@@ -0,0 +1,155 @@
+"""Step definitions for tdd_plan_generation_validate_logic.feature.
+
+This test captures bug #10480 (TDD counterpart #10477):
+PlanGenerationGraph._validate() always passes for code longer than 10
+characters, making LLM validation ineffective.
+
+The bug was:
+    is_valid = "PASS" in validation.upper() or len(all_code) > 10  # BUG
+
+The fix:
+    is_valid = "PASS" in validation.upper() and "FAIL" not in validation.upper()
+
+These tests verify that _validate() correctly respects the LLM's response
+regardless of the length of the generated code.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+from typing import Any
+
+from behave import given, then, when
+from langchain_community.llms import FakeListLLM
+
+from cleveragents.domain.models.core import Change, OperationType, Plan, Project
+
+
+def _make_change_with_long_code() -> Change:
+    """Create a Change with code longer than 10 characters."""
+    return Change(
+        id=None,
+        plan_id=1,
+        file_path="test.py",
+        operation=OperationType.CREATE,
+        original_content=None,
+        new_content="def broken_function():\n    syntax error here\n    return None",
+        applied=False,
+        applied_at=None,
+        new_path=None,
+    )
+
+
+def _make_state_with_changes(changes: list[Change]) -> dict[str, Any]:
+    """Create a minimal PlanGenerationState dict with the given changes."""
+    return {
+        "project": Project(id=1, name="test", path=Path("/tmp/test")),
+        "plan": Plan(id=1, project_id=1, name="plan", prompt="test"),
+        "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": changes,
+        "validation_result": {},
+        "retry_count": 0,
+        "error": None,
+    }
+
+
+@given("a PlanGenerationGraph with an LLM that always returns FAIL")
+def step_graph_with_fail_llm(context: Any) -> None:
+    """Create a PlanGenerationGraph whose LLM always returns FAIL."""
+    from cleveragents.agents.graphs.plan_generation import PlanGenerationGraph
+
+    context.graph = PlanGenerationGraph(
+        llm=FakeListLLM(responses=["FAIL: this code is completely broken"])
+    )
+
+
+@given("a PlanGenerationGraph with an LLM that always returns PASS")
+def step_graph_with_pass_llm(context: Any) -> None:
+    """Create a PlanGenerationGraph whose LLM always returns PASS."""
+    from cleveragents.agents.graphs.plan_generation import PlanGenerationGraph
+
+    context.graph = PlanGenerationGraph(
+        llm=FakeListLLM(responses=["PASS: code looks good"])
+    )
+
+
+@given('a PlanGenerationGraph with an LLM that returns "FAIL: syntax error on line 5"')
+def step_graph_with_specific_fail_llm(context: Any) -> None:
+    """Create a PlanGenerationGraph whose LLM returns a specific FAIL message."""
+    from cleveragents.agents.graphs.plan_generation import PlanGenerationGraph
+
+    context.llm_response = "FAIL: syntax error on line 5"
+    context.graph = PlanGenerationGraph(
+        llm=FakeListLLM(responses=[context.llm_response])
+    )
+
+
+@given("a state with generated changes containing code longer than 10 characters")
+def step_state_with_long_code(context: Any) -> None:
+    """Create a state with a Change whose code is longer than 10 characters."""
+    change = _make_change_with_long_code()
+    # Verify the code is indeed longer than 10 characters (sanity check)
+    assert len(change.new_content or "") > 10, (
+        f"Test setup error: new_content must be > 10 chars, "
+        f"got {len(change.new_content or '')} chars"
+    )
+    context.state = _make_state_with_changes([change])
+
+
+@when("I call _validate on the state")
+def step_call_validate(context: Any) -> None:
+    """Call _validate() on the graph with the prepared state."""
+    context.result = context.graph._validate(context.state)
+
+
+@then("the validation result status should be FAIL")
+def step_validation_status_fail(context: Any) -> None:
+    """Assert that the validation result status is FAIL.
+
+    This is the core assertion for bug #10480: when the LLM returns FAIL,
+    the validation must fail regardless of the code length.
+
+    Before the fix, ``or len(all_code) > 10`` caused is_valid to always be
+    True for any real code, so this assertion would fail (status would be
+    PASS instead of FAIL).
+    """
+    validation = context.result.get("validation_result", {})
+    status = validation.get("status")
+    assert status == "FAIL", (
+        f"Expected validation status FAIL but got {status!r}. "
+        f"Bug #10480: The `or len(all_code) > 10` condition incorrectly "
+        f"overrides the LLM's FAIL response when code is longer than 10 chars. "
+        f"Full validation result: {validation}"
+    )
+
+
+@then("the validation result status should be PASS")
+def step_validation_status_pass(context: Any) -> None:
+    """Assert that the validation result status is PASS."""
+    validation = context.result.get("validation_result", {})
+    status = validation.get("status")
+    assert status == "PASS", (
+        f"Expected validation status PASS but got {status!r}. "
+        f"Full validation result: {validation}"
+    )
+
+
+@then("the validation message should contain the LLM response")
+def step_validation_message_contains_llm_response(context: Any) -> None:
+    """Assert that the validation message contains the LLM's response text."""
+    validation = context.result.get("validation_result", {})
+    message = validation.get("message", "")
+    llm_response = getattr(context, "llm_response", "FAIL")
+    assert llm_response in message, (
+        f"Expected validation message to contain {llm_response!r} but got: {message!r}"
+    )

features/tdd_plan_generation_validate_logic.feature

@@ -0,0 +1,43 @@
+@tdd_issue @tdd_issue_10477
+Feature: TDD Issue #10477 — PlanGenerationGraph._validate() always passes for code longer than 10 characters
+  As a developer relying on LLM-based code validation
+  I want _validate() to respect the LLM's FAIL response
+  So that invalid, broken, or insecure generated code is correctly rejected
+
+  This test captures bug #10480 (and its TDD counterpart #10477).
+  The _validate() method had a logic error:
+
+      is_valid = "PASS" in validation.upper() or len(all_code) > 10  # BUG
+
+  The ``or len(all_code) > 10`` condition made validation always pass for
+  any generated code longer than 10 characters, completely bypassing the
+  LLM's validation response.
+
+  The fix removes the ``or len(all_code) > 10`` condition so that
+  validation correctly reflects the LLM's response:
+
+      is_valid = "PASS" in validation.upper() and "FAIL" not in validation.upper()
+
+  See CONTRIBUTING.md > Bug Fix Workflow > TDD Issue Test Tags.
+
+  @tdd_issue @tdd_issue_10477
+  Scenario: _validate() fails when LLM returns FAIL regardless of code length
+    Given a PlanGenerationGraph with an LLM that always returns FAIL
+    And a state with generated changes containing code longer than 10 characters
+    When I call _validate on the state
+    Then the validation result status should be FAIL
+
+  @tdd_issue @tdd_issue_10477
+  Scenario: _validate() passes when LLM returns PASS
+    Given a PlanGenerationGraph with an LLM that always returns PASS
+    And a state with generated changes containing code longer than 10 characters
+    When I call _validate on the state
+    Then the validation result status should be PASS
+
+  @tdd_issue @tdd_issue_10477
+  Scenario: _validate() fails when LLM returns FAIL with detailed message
+    Given a PlanGenerationGraph with an LLM that returns "FAIL: syntax error on line 5"
+    And a state with generated changes containing code longer than 10 characters
+    When I call _validate on the state
+    Then the validation result status should be FAIL
+    And the validation message should contain the LLM response

robot/plan_generation_graph.robot

@@ -375,7 +375,7 @@ Workflow Invoke Method Returns Complete State
     ...    from cleveragents.agents.plan_generation import PlanGenerationGraph
     ...    from langchain_community.llms import FakeListLLM
     ...    from cleveragents.domain.models.core import Project, Plan, Context
-    ...    graph = PlanGenerationGraph(llm=FakeListLLM(responses=['test']*3))
+    ...    graph = PlanGenerationGraph(llm=FakeListLLM(responses=['PASS: code looks good']*10))
     ...    project = Project(id=1, name='test_project', path=Path('/tmp/test_project'))
     ...    plan = Plan(id=1, project_id=1, name='Logging Plan', prompt='Add logging')
     ...    contexts = [Context(plan_id=plan.id, path='app.py', content='def main(): pass')]
@@ -405,7 +405,7 @@ Workflow Stream Method Yields Events
     ...    from cleveragents.agents.plan_generation import PlanGenerationGraph
     ...    from langchain_community.llms import FakeListLLM
     ...    from cleveragents.domain.models.core import Project, Plan, Context
-    ...    graph = PlanGenerationGraph(llm=FakeListLLM(responses=['test']*3))
+    ...    graph = PlanGenerationGraph(llm=FakeListLLM(responses=['PASS: code looks good']*10))
     ...    project = Project(id=1, name='test_project', path=Path('/tmp/test_project'))
     ...    plan = Plan(id=1, project_id=1, name='Feature Plan', prompt='Add feature')
     ...    contexts = [Context(plan_id=plan.id, path='app.py', content='# app')]

src/cleveragents/agents/graphs/plan_generation.py

@@ -527,8 +527,12 @@ class PlanGenerationGraph:
             )
             validation = str(result)
 
-            # Simple validation check (in real implementation, parse the LLM response)
-            is_valid = "PASS" in validation.upper() or len(all_code) > 10
+            # Parse the LLM response: PASS only when the response contains PASS
+            # and does not contain FAIL. This ensures the LLM's judgment is
+            # respected regardless of the generated code length.
+            # Bug fix for #10480: removed the erroneous `or len(all_code) > 10`
+            # condition that caused validation to always pass for any real code.
+            is_valid = "PASS" in validation.upper() and "FAIL" not in validation.upper()
 
             return {
                 "validation_result": {