features/plan_generation_validation_fix.feature

@@ -0,0 +1,25 @@
+Feature: Plan Generation Validation Fix
+  Regression test for bug where _validate always passed for code longer than 10 characters.
+
+  Scenario: Validation properly fails when LLM response contains FAIL and code is long
+    Given a PlanGenerationGraph instance
+    And generated code longer than 10 characters
+    And the LLM validation response is "FAIL: issues found"
+    When the validation node runs
+    Then the validation status should be "FAIL"
+    And the bug where length over 10 characters forced PASS should be fixed
+
+  Scenario: Validation properly fails when LLM response contains REJECTED
+    Given a PlanGenerationGraph instance
+    And generated code longer than 10 characters
+    And the LLM validation response is "REJECTED: unsafe patterns detected"
+    When the validation node runs
+    Then the validation status should be "FAIL"
+    And the validation should respect LLM rejection regardless of code length
+
+  Scenario: Validation properly passes when LLM response contains PASS
+    Given a PlanGenerationGraph instance
+    And generated code longer than 10 characters
+    And the LLM validation response is "PASS: all checks successful"
+    When the validation node runs
+    Then the validation status should be "PASS"

features/steps/plan_generation_validation_fix_steps.py

@@ -0,0 +1,92 @@
+"""
+Step definitions for plan generation validation fix tests.
+
+These steps verify that the _validate method properly respects the LLM
+validation response and no longer incorrectly passes validation based on
+code length.
+"""
+
+from typing import Any
+from unittest.mock import MagicMock, patch
+
+from behave import given, then, when
+from langchain_community.llms import FakeListLLM
+
+from cleveragents.agents.graphs.plan_generation import PlanGenerationGraph
+
+
+@given("a PlanGenerationGraph instance")
+def step_given_plan_generation_graph_instance(context: Any) -> None:
+    """Create a PlanGenerationGraph instance with a FakeListLLM."""
+    llm = FakeListLLM(
+        responses=[
+            "Requirements: test",
+            "Generated code",
+            "PASS: all checks successful",
+        ]
+    )
+    context.graph = PlanGenerationGraph(llm=llm, max_retries=1)
+    context.validation_response = None
+
+
+@given("generated code longer than 10 characters")
+def step_given_generated_code_longer_than_10_chars(context: Any) -> None:
+    """Set up generated changes with code longer than 10 characters."""
+    long_code = "def some_function():\n    return True  # this is long enough"
+    context.generated_code = long_code
+
+
+@given("the LLM validation response is {response}")
+def step_given_llm_validation_response(context: Any, response: str) -> None:
+    """Set up the validation response that the LLM will return."""
+    # Strip surrounding quotes if present (Gherkin passes quoted strings with quotes)
+    response_value = response.strip('"').strip("'")
+    context.validation_response = response_value
+
+
+@when("the validation node runs")
+def step_when_validation_node_runs(context: Any) -> None:
+    """Invoke the _validate method with the generated changes."""
+    state = {
+        "generated_changes": [
+            MagicMock(file_path="test.py", new_content=context.generated_code),
+        ],
+        "validation_result": {},
+        "retry_count": 0,
+    }
+    # Patch the chain to return the desired validation response
+    validation_response = context.validation_response or "PASS: all checks successful"
+    mock_chain = MagicMock()
+    mock_chain.invoke.return_value = validation_response
+    with patch.object(context.graph, "_chain_with_retry", return_value=mock_chain):
+        context.result = context.graph._validate(state)
+
+
+@then("the validation status should be {status}")
+def step_then_validation_status_should_be(context: Any, status: str) -> None:
+    """Assert the validation result matches the expected status."""
+    # Strip surrounding quotes if present (Gherkin passes quoted strings with quotes)
+    expected_status = status.strip('"').strip("'")
+    actual_status = context.result["validation_result"]["status"]
+    assert actual_status == expected_status, (
+        f"Expected validation status '{expected_status}' but got '{actual_status}'. "
+        f"The bug where code length > 10 forced PASS may still be present."
+    )
+
+
+@then("the bug where length over 10 characters forced PASS should be fixed")
+def step_then_bug_length_over_10_forced_pass_fixed(context: Any) -> None:
+    """Verify the validation correctly rejected the LLM FAIL response."""
+    assert context.result["validation_result"]["status"] == "FAIL", (
+        "Bug still present: validation passed despite LLM saying FAIL. "
+        "The len(all_code) > 10 fallback is still being applied."
+    )
+
+
+@then("the validation should respect LLM rejection regardless of code length")
+def step_then_validation_respects_llm_rejection(context: Any) -> None:
+    """Verify REJECTED responses are properly handled as failures."""
+    assert context.result["validation_result"]["status"] == "FAIL", (
+        "Bug still present: validation passed despite LLM rejecting. "
+        "The fallback length check is overriding the LLM response."
+    )