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": {