diff --git a/features/plan_generation_validation_fix.feature b/features/plan_generation_validation_fix.feature new file mode 100644 index 000000000..c629e2443 --- /dev/null +++ b/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" diff --git a/features/steps/plan_generation_validation_fix_steps.py b/features/steps/plan_generation_validation_fix_steps.py new file mode 100644 index 000000000..4849af423 --- /dev/null +++ b/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." + )