From 89511beea76f99a8c616fe13ab2fa963c14f7357 Mon Sep 17 00:00:00 2001 From: PDD Bot Date: Sun, 15 Feb 2026 07:42:05 +0000 Subject: [PATCH] Add failing tests for issue #530: documentation-only changes This commit adds comprehensive test coverage for the bug where pdd-change incorrectly stops at Step 6 for documentation-only changes, wasting tokens on Steps 1-5. Test Files: - Unit tests in tests/test_agentic_change_orchestrator.py - E2E tests in tests/test_e2e_issue_530_documentation_only_changes.py Prompt Fix: - Modified pdd/prompts/agentic_change_step6_devunits_LLM.prompt to distinguish "Documentation Only" (valid path) from "No Dev Units Found" (true failure) The tests currently fail on the buggy code and will pass once the orchestrator is fixed to skip Steps 7-8 for documentation-only changes. Related to #530 Co-Authored-By: Claude Sonnet 4.5 --- .../agentic_change_step6_devunits_LLM.prompt | 23 +- tests/test_agentic_change_orchestrator.py | 246 +++++++++- ...2e_issue_530_documentation_only_changes.py | 430 ++++++++++++++++++ 3 files changed, 696 insertions(+), 3 deletions(-) create mode 100644 tests/test_e2e_issue_530_documentation_only_changes.py diff --git a/pdd/prompts/agentic_change_step6_devunits_LLM.prompt b/pdd/prompts/agentic_change_step6_devunits_LLM.prompt index 36260086f..5beffcb0c 100644 --- a/pdd/prompts/agentic_change_step6_devunits_LLM.prompt +++ b/pdd/prompts/agentic_change_step6_devunits_LLM.prompt @@ -288,14 +288,33 @@ For each new dev unit proposed, explain: *Proceeding to Step 7: Architecture Review* ``` -**If no dev units can be identified (STOP workflow):** +**If no dev units but documentation-only changes:** +```markdown +## Step 6: Dev Units Identified + +**Status:** Documentation Only + +### Analysis +This change request only affects documentation files (e.g., README.md, docs/*.md) and does not require modifications to any dev units (prompt + code + example + test). + +### Documentation Files Affected +- [List documentation files that need updates] + +### Why No Dev Units +[Explanation of why this is a documentation-only change] + +--- +*Proceeding to Step 9: Implementation (skipping Steps 7-8 for documentation-only changes)* +``` + +**If no dev units and not documentation-only (STOP workflow):** ```markdown ## Step 6: Dev Units Identified **Status:** No Dev Units Found ### Analysis -[Explanation of why no dev units could be identified] +[Explanation of why no dev units could be identified and why this is not a documentation-only change] ### Recommendation [Suggestions for clarifying the request or alternative approaches] diff --git a/tests/test_agentic_change_orchestrator.py b/tests/test_agentic_change_orchestrator.py index 2a6862b16..c8ff6a642 100644 --- a/tests/test_agentic_change_orchestrator.py +++ b/tests/test_agentic_change_orchestrator.py @@ -2419,4 +2419,248 @@ def side_effect_run(**kwargs): assert success is True # State should NOT be cleared because some steps have FAILED: prefix - mock_clear_state.assert_not_called() \ No newline at end of file + mock_clear_state.assert_not_called() + + +# ----------------------------------------------------------------------------- +# Tests for Issue #530: Documentation-only changes should not trigger hard stop +# ----------------------------------------------------------------------------- + +def test_documentation_only_should_not_trigger_hard_stop(mock_dependencies, temp_cwd): + """ + FAILING TEST for Issue #530: Step 6 with "Documentation Only" should + continue the workflow, not trigger a hard stop. + + THE BUG: + The original prompt (before Step 5.5 fix) output "No Dev Units Found" for + documentation-only changes, which triggered a hard stop at line 286 of + agentic_change_orchestrator.py, wasting tokens from Steps 1-5. + + THE FIX: + 1. Step 5.5 updated the prompt to output "Documentation Only" status + instead of "No Dev Units Found" for doc-only changes + 2. The orchestrator needs to be updated to NOT treat "Documentation Only" + as a hard stop condition + 3. Documentation-only changes should skip Steps 7-8 and proceed to Step 9 + + THIS TEST WILL FAIL on the current code if: + - The orchestrator treats "Documentation Only" as a hard stop + - OR if steps 7-8 are not properly skipped for documentation-only changes + + After the fix, this test should pass. + """ + mocks = mock_dependencies + mock_run = mocks["run"] + + # Track which steps were executed + executed_steps = [] + + def side_effect_run(**kwargs): + label = kwargs.get("label", "") + executed_steps.append(label) + + # Step 6 outputs "Documentation Only" status (NEW format from Step 5.5 fix) + # This should NOT trigger a hard stop + if label == "step6": + return (True, """## Step 6: Dev Units Identified + +**Status:** Documentation Only + +### Analysis +This change request only affects documentation files and does not require +modifications to any dev units. + +### Documentation Files Affected +- README.md +- docs/user-guide.md + +--- +*Proceeding to Step 9: Implementation (skipping Steps 7-8)* +""", 0.1, "gpt-4") + + # Step 9 should be executed (implementation step) + if label == "step9": + return (True, "FILES_CREATED: README.md\nDocumentation updated.", 0.1, "gpt-4") + + # Step 10 should be executed + if label == "step10": + return (True, "Architecture metadata updated", 0.1, "gpt-4") + + # Step 11 (review) should find no issues + if label.startswith("step11"): + return (True, "No Issues Found", 0.1, "gpt-4") + + # Step 13 should create PR + if label == "step13": + return (True, "PR Created: https://github.com/owner/repo/pull/1", 0.1, "gpt-4") + + # Default output for other steps + return (True, f"Output for {label}", 0.1, "gpt-4") + + mock_run.side_effect = side_effect_run + + success, msg, cost, model, files = run_agentic_change_orchestrator( + issue_url="https://github.com/owner/repo/issues/530", + issue_content="Update documentation", + repo_owner="owner", + repo_name="repo", + issue_number=530, + issue_author="user", + issue_title="Documentation-only change", + cwd=temp_cwd, + quiet=True, + ) + + # PRIMARY ASSERTION: Workflow should succeed (not trigger hard stop) + assert success is True, \ + f"Workflow should succeed for 'Documentation Only', but got: {msg}" + + # SECONDARY ASSERTION: Step 9 was executed (workflow did not stop at Step 6) + assert "step9" in executed_steps, \ + "Step 9 should be executed for documentation-only changes" + + # TERTIARY ASSERTION: Steps 7-8 should be skipped for documentation-only + assert "step7" not in executed_steps, \ + "Step 7 (architecture) should be skipped for documentation-only changes" + assert "step8" not in executed_steps, \ + "Step 8 (analyze) should be skipped for documentation-only changes" + + +def test_no_dev_units_found_should_still_trigger_hard_stop(mock_dependencies, temp_cwd): + """ + Test that Step 6 output with "No Dev Units Found" status DOES trigger + a hard stop (existing behavior for true failures). + + This is a regression test to ensure that the fix for issue #530 does not + break the existing hard-stop behavior for cases where no dev units can be + identified and it's NOT a documentation-only change. + """ + mocks = mock_dependencies + mock_run = mocks["run"] + + # Track which steps were executed + executed_steps = [] + + def side_effect_run(**kwargs): + label = kwargs.get("label", "") + executed_steps.append(label) + + # Step 6 outputs "No Dev Units Found" status (existing failure format) + if label == "step6": + return (True, """## Step 6: Dev Units Identified + +**Status:** No Dev Units Found + +### Analysis +Unable to identify any dev units affected by this change request. +This does not appear to be a documentation-only change. + +### Recommendation +Please clarify which components should be modified. + +--- +*Investigation paused - unable to identify affected modules.* +""", 0.1, "gpt-4") + + # Default output for other steps + return (True, f"Output for {label}", 0.1, "gpt-4") + + mock_run.side_effect = side_effect_run + + success, msg, cost, model, files = run_agentic_change_orchestrator( + issue_url="https://github.com/owner/repo/issues/999", + issue_content="Unclear change request", + repo_owner="owner", + repo_name="repo", + issue_number=999, + issue_author="user", + issue_title="Unclear request", + cwd=temp_cwd, + quiet=True, + ) + + # Verify the workflow failed (hard stop triggered) + assert success is False, "Workflow should fail when 'No Dev Units Found' is output" + + # Verify the failure message indicates no dev units + assert "no dev units" in msg.lower(), f"Expected 'no dev units' in message, got: {msg}" + + # Verify Step 9 was NOT executed (workflow stopped at Step 6) + assert "step9" not in executed_steps, "Step 9 should NOT be executed when hard stop is triggered" + + +def test_normal_dev_units_path_continues_to_step7(mock_dependencies, temp_cwd): + """ + Test that Step 6 with normal dev units output continues to Steps 7-8 + as expected (existing behavior). + + This is a regression test to ensure that the fix for issue #530 does not + break the normal workflow path where dev units are identified and should + proceed through architecture review (Step 7) and analysis (Step 8). + """ + mocks = mock_dependencies + mock_run = mocks["run"] + + # Track which steps were executed + executed_steps = [] + + def side_effect_run(**kwargs): + label = kwargs.get("label", "") + executed_steps.append(label) + + # Step 6 outputs normal dev units identified + if label == "step6": + return (True, """## Step 6: Dev Units Identified + +**Status:** Dev Units Found + +### Dev Units +- auth_handler (prompt + code + test) +- user_service (prompt + code + test) + +--- +*Proceeding to Step 7: Architecture Review* +""", 0.1, "gpt-4") + + # Step 9 implements the changes + if label == "step9": + return (True, "FILES_CREATED: auth_handler.py, user_service.py", 0.1, "gpt-4") + + # Step 10 updates architecture + if label == "step10": + return (True, "Architecture updated", 0.1, "gpt-4") + + # Step 11 review + if label.startswith("step11"): + return (True, "No Issues Found", 0.1, "gpt-4") + + # Step 13 creates PR + if label == "step13": + return (True, "PR Created: https://github.com/owner/repo/pull/1", 0.1, "gpt-4") + + # Default output for other steps + return (True, f"Output for {label}", 0.1, "gpt-4") + + mock_run.side_effect = side_effect_run + + success, msg, cost, model, files = run_agentic_change_orchestrator( + issue_url="https://github.com/owner/repo/issues/100", + issue_content="Add authentication feature", + repo_owner="owner", + repo_name="repo", + issue_number=100, + issue_author="user", + issue_title="Add auth feature", + cwd=temp_cwd, + quiet=True, + ) + + # Verify the workflow succeeded + assert success is True, f"Workflow should succeed for normal dev units path, but got: {msg}" + + # Verify Steps 7 and 8 WERE executed (normal path includes architecture review) + assert "step7" in executed_steps, "Step 7 should be executed for normal dev units" + assert "step8" in executed_steps, "Step 8 should be executed for normal dev units" + + # Verify Step 9 was executed + assert "step9" in executed_steps, "Step 9 should be executed for normal dev units" \ No newline at end of file diff --git a/tests/test_e2e_issue_530_documentation_only_changes.py b/tests/test_e2e_issue_530_documentation_only_changes.py new file mode 100644 index 000000000..3a6885299 --- /dev/null +++ b/tests/test_e2e_issue_530_documentation_only_changes.py @@ -0,0 +1,430 @@ +""" +E2E Test for Issue #530: pdd-change should handle documentation-only changes + +Bug Context: +----------- +When pdd change processes a documentation-only issue: +1. Steps 1-5 complete successfully (incurring LLM token costs) +2. Step 6 outputs "No Dev Units Found" +3. The orchestrator's _check_hard_stop() at line 286 treats this as a failure +4. Workflow exits, wasting tokens from Steps 1-5 + +Expected behavior: +------------------ +1. Step 6 should output "Documentation Only" for doc-only changes (NOT "No Dev Units Found") +2. The orchestrator should recognize "Documentation Only" as a valid continuation signal +3. Steps 7-8 (architecture/code analysis) should be skipped for documentation-only changes +4. Step 9 (implementation) should proceed directly to handle documentation file operations + +This E2E test exercises the full code path from the orchestrator perspective, +demonstrating the bug by mocking an LLM that returns "No Dev Units Found" at Step 6 +and verifying that the orchestrator incorrectly stops the workflow. +""" + +import pytest +import subprocess +from pathlib import Path +from unittest.mock import patch, MagicMock + + +@pytest.fixture +def mock_git_repo_with_docs(tmp_path): + """Create a git repository with documentation files for testing.""" + repo_path = tmp_path / "test_repo" + repo_path.mkdir() + + # Initialize git repo + subprocess.run(["git", "init", "-b", "main"], cwd=repo_path, check=True, capture_output=True) + subprocess.run(["git", "config", "user.email", "test@test.com"], cwd=repo_path, check=True) + subprocess.run(["git", "config", "user.name", "Test User"], cwd=repo_path, check=True) + + # Create project structure with documentation + pdd_dir = repo_path / ".pdd" + pdd_dir.mkdir() + + docs_dir = repo_path / "docs" + docs_dir.mkdir() + + # Create README and documentation files + (repo_path / "README.md").write_text("# Test Repository\n\nDocumentation here.") + (docs_dir / "guide.md").write_text("# User Guide\n\nHow to use this project.") + + # Create a minimal source file + src_dir = repo_path / "src" + src_dir.mkdir() + (src_dir / "main.py").write_text("def main():\n pass\n") + + # Commit everything + subprocess.run(["git", "add", "."], cwd=repo_path, check=True) + subprocess.run(["git", "commit", "-m", "Initial commit"], cwd=repo_path, check=True, capture_output=True) + + return repo_path + + +@pytest.mark.e2e +class TestIssue530DocumentationOnlyChanges: + """ + E2E tests for Issue #530: pdd-change fails at Step 6 for documentation-only changes. + + These tests verify the complete orchestrator code path for documentation-only + changes, demonstrating both the bug and the expected behavior after the fix. + """ + + def test_documentation_only_change_triggers_hard_stop(self, mock_git_repo_with_docs, monkeypatch): + """ + PRIMARY BUG TEST: Documentation-only change incorrectly triggers hard stop at Step 6. + + This test demonstrates the EXACT bug scenario: + 1. Steps 1-5 complete successfully + 2. Step 6 outputs "No Dev Units Found" (current behavior) + 3. The orchestrator's _check_hard_stop() at line 286 detects this and exits + 4. Workflow stops, wasting tokens from Steps 1-5 + + This test FAILS on the current buggy code (workflow stops at Step 6) + and should PASS after the fix (workflow continues to Step 9). + """ + monkeypatch.setenv("PDD_FORCE_LOCAL", "1") + + # Track which steps were executed + steps_executed = [] + + def mock_run_agentic_task(instruction, cwd, verbose, quiet, timeout, label, max_retries): + """Mock LLM agent that simulates documentation-only change workflow.""" + import re + match = re.search(r"step(\d+)", label) + if match: + step_num = int(match.group(1)) + steps_executed.append(step_num) + + # Simulate successful steps 1-5 + if "step1" in label: + return (True, "No duplicate issues found. Proceed.", 0.001, "mock-model") + elif "step2" in label: + return (True, "Feature not documented yet. Proceed.", 0.001, "mock-model") + elif "step3" in label: + return (True, "Research complete. This is a documentation-only change.", 0.001, "mock-model") + elif "step4" in label: + return (True, "Requirements clear. Only documentation needs updating.", 0.001, "mock-model") + elif "step5" in label: + return (True, "Docs changes: Update README.md with new feature description.", 0.001, "mock-model") + elif "step6" in label: + # BUG: Current behavior returns "No Dev Units Found" for documentation-only changes + # This triggers the hard-stop at line 286 + return (True, "No Dev Units Found", 0.001, "mock-model") + elif "step7" in label: + # Step 7 should NOT be reached with the bug (hard stop at Step 6) + return (True, "Architecture review (should not reach this).", 0.001, "mock-model") + elif "step9" in label: + # Step 9 should NOT be reached with the bug (hard stop at Step 6) + return (True, "Implementation complete.", 0.001, "mock-model") + + return (True, f"Mock success for {label}", 0.001, "mock-model") + + def mock_save_state(*args, **kwargs): + pass + + def mock_load_state(*args, **kwargs): + return None, None + + def mock_clear_state(*args, **kwargs): + pass + + from pdd.agentic_change_orchestrator import run_agentic_change_orchestrator + + with patch('pdd.agentic_change_orchestrator.run_agentic_task', side_effect=mock_run_agentic_task): + with patch('pdd.agentic_change_orchestrator.save_workflow_state', side_effect=mock_save_state): + with patch('pdd.agentic_change_orchestrator.load_workflow_state', side_effect=mock_load_state): + with patch('pdd.agentic_change_orchestrator.clear_workflow_state', side_effect=mock_clear_state): + success, message, cost, model, files = run_agentic_change_orchestrator( + issue_url="https://github.com/test/repo/issues/530", + issue_content="Update documentation for new feature X", + repo_owner="test", + repo_name="repo", + issue_number=530, + issue_author="test-user", + issue_title="Update docs for feature X", + cwd=mock_git_repo_with_docs, + verbose=False, + quiet=True, + use_github_state=False + ) + + # BUG DETECTION: With current code, workflow should stop at Step 6 + # Steps executed should be [1, 2, 3, 4, 5, 6] only + assert 6 in steps_executed, f"Step 6 should have been executed. Steps: {steps_executed}" + + # Primary bug assertion: Steps 7-8 should NOT be executed (hard stop at Step 6) + # After the fix, this test will fail because Step 7 should be skipped but Step 9 should run + if 7 not in steps_executed and 9 not in steps_executed: + # This is the BUGGY behavior - workflow stopped at Step 6 + pytest.fail( + f"BUG DETECTED (Issue #530): Workflow stopped after Step 6 with 'No Dev Units Found'.\n\n" + f"Steps executed: {steps_executed}\n" + f"Final status: success={success}, message={message}\n\n" + f"Expected behavior:\n" + f"1. Step 6 should output 'Documentation Only' (not 'No Dev Units Found')\n" + f"2. Steps 7-8 should be skipped (architecture/code analysis not needed)\n" + f"3. Step 9 should execute (implementation handles documentation files)\n\n" + f"Actual behavior:\n" + f"1. Step 6 outputs 'No Dev Units Found'\n" + f"2. _check_hard_stop() at line 286 treats this as a failure\n" + f"3. Workflow exits, wasting tokens from Steps 1-5" + ) + + # After the fix, workflow should continue to Step 9 + assert 9 in steps_executed, ( + f"After fix: Step 9 should execute for documentation-only changes. " + f"Steps executed: {steps_executed}" + ) + + def test_documentation_only_with_fixed_behavior(self, mock_git_repo_with_docs, monkeypatch): + """ + EXPECTED BEHAVIOR TEST: Documentation-only change should skip Steps 7-8 and proceed to Step 9. + + This test demonstrates the CORRECT behavior after the fix: + 1. Steps 1-5 complete successfully + 2. Step 6 outputs "Documentation Only" (fixed behavior) + 3. Steps 7-8 are skipped (not needed for documentation-only changes) + 4. Step 9 executes (implementation handles documentation files) + + This test PASSES when the fix is implemented correctly. + """ + monkeypatch.setenv("PDD_FORCE_LOCAL", "1") + + steps_executed = [] + + def mock_run_agentic_task(instruction, cwd, verbose, quiet, timeout, label, max_retries): + """Mock LLM agent that simulates FIXED documentation-only workflow.""" + import re + match = re.search(r"step(\d+)", label) + if match: + step_num = int(match.group(1)) + steps_executed.append(step_num) + + # Simulate successful steps 1-5 + if "step1" in label: + return (True, "No duplicate issues found. Proceed.", 0.001, "mock-model") + elif "step2" in label: + return (True, "Feature not documented yet. Proceed.", 0.001, "mock-model") + elif "step3" in label: + return (True, "Research complete. This is a documentation-only change.", 0.001, "mock-model") + elif "step4" in label: + return (True, "Requirements clear. Only documentation needs updating.", 0.001, "mock-model") + elif "step5" in label: + return (True, "Docs changes: Update README.md with new feature description.", 0.001, "mock-model") + elif "step6" in label: + # FIXED: Output "Documentation Only" instead of "No Dev Units Found" + return (True, "Documentation Only\n\nNo code changes needed, only documentation.", 0.001, "mock-model") + elif "step7" in label: + # Should NOT be reached (skipped for documentation-only) + pytest.fail("Step 7 should be skipped for documentation-only changes") + elif "step8" in label: + # Should NOT be reached (skipped for documentation-only) + pytest.fail("Step 8 should be skipped for documentation-only changes") + elif "step9" in label: + # Should be reached (implementation handles documentation) + return (True, "FILES_MODIFIED: README.md, docs/guide.md", 0.001, "mock-model") + + return (True, f"Mock success for {label}", 0.001, "mock-model") + + def mock_save_state(*args, **kwargs): + pass + + def mock_load_state(*args, **kwargs): + return None, None + + def mock_clear_state(*args, **kwargs): + pass + + from pdd.agentic_change_orchestrator import run_agentic_change_orchestrator + + with patch('pdd.agentic_change_orchestrator.run_agentic_task', side_effect=mock_run_agentic_task): + with patch('pdd.agentic_change_orchestrator.save_workflow_state', side_effect=mock_save_state): + with patch('pdd.agentic_change_orchestrator.load_workflow_state', side_effect=mock_load_state): + with patch('pdd.agentic_change_orchestrator.clear_workflow_state', side_effect=mock_clear_state): + success, message, cost, model, files = run_agentic_change_orchestrator( + issue_url="https://github.com/test/repo/issues/530", + issue_content="Update documentation for new feature X", + repo_owner="test", + repo_name="repo", + issue_number=530, + issue_author="test-user", + issue_title="Update docs for feature X", + cwd=mock_git_repo_with_docs, + verbose=False, + quiet=True, + use_github_state=False + ) + + # Verify the CORRECT flow: + # Steps 1-6 should execute + for step in [1, 2, 3, 4, 5, 6]: + assert step in steps_executed, f"Step {step} should execute. Steps: {steps_executed}" + + # Steps 7-8 should be skipped + assert 7 not in steps_executed, f"Step 7 should be skipped for documentation-only. Steps: {steps_executed}" + assert 8 not in steps_executed, f"Step 8 should be skipped for documentation-only. Steps: {steps_executed}" + + # Step 9 should execute + assert 9 in steps_executed, f"Step 9 should execute for documentation-only. Steps: {steps_executed}" + + # Workflow should succeed + assert success is True, f"Workflow should succeed for documentation-only changes. Message: {message}" + + def test_normal_dev_units_path_continues_to_step7(self, mock_git_repo_with_docs, monkeypatch): + """ + REGRESSION TEST: Normal dev units path should continue through Steps 7-8 as before. + + This test ensures the fix doesn't break the normal workflow when dev units + ARE found and code changes are needed. + """ + monkeypatch.setenv("PDD_FORCE_LOCAL", "1") + + steps_executed = [] + + def mock_run_agentic_task(instruction, cwd, verbose, quiet, timeout, label, max_retries): + """Mock LLM agent that simulates normal workflow with dev units.""" + import re + match = re.search(r"step(\d+)", label) + if match: + step_num = int(match.group(1)) + steps_executed.append(step_num) + + # Simulate successful steps 1-6 + if "step1" in label: + return (True, "No duplicate issues found. Proceed.", 0.001, "mock-model") + elif "step2" in label: + return (True, "Feature not implemented yet. Proceed.", 0.001, "mock-model") + elif "step3" in label: + return (True, "Research complete. Code changes needed.", 0.001, "mock-model") + elif "step4" in label: + return (True, "Requirements clear. Need to modify src/main.py.", 0.001, "mock-model") + elif "step5" in label: + return (True, "No docs changes needed.", 0.001, "mock-model") + elif "step6" in label: + # Normal case: Dev units found + return (True, "Dev Units: src/main.py\n\nNeed to add new function.", 0.001, "mock-model") + elif "step7" in label: + # Should be reached (architecture review for code changes) + return (True, "Architecture review complete.", 0.001, "mock-model") + elif "step8" in label: + # Should be reached (analyze prompt changes) + return (True, "No prompt changes needed.", 0.001, "mock-model") + elif "step9" in label: + # Should be reached (implementation) + return (True, "FILES_MODIFIED: src/main.py", 0.001, "mock-model") + + return (True, f"Mock success for {label}", 0.001, "mock-model") + + def mock_save_state(*args, **kwargs): + pass + + def mock_load_state(*args, **kwargs): + return None, None + + def mock_clear_state(*args, **kwargs): + pass + + from pdd.agentic_change_orchestrator import run_agentic_change_orchestrator + + with patch('pdd.agentic_change_orchestrator.run_agentic_task', side_effect=mock_run_agentic_task): + with patch('pdd.agentic_change_orchestrator.save_workflow_state', side_effect=mock_save_state): + with patch('pdd.agentic_change_orchestrator.load_workflow_state', side_effect=mock_load_state): + with patch('pdd.agentic_change_orchestrator.clear_workflow_state', side_effect=mock_clear_state): + success, message, cost, model, files = run_agentic_change_orchestrator( + issue_url="https://github.com/test/repo/issues/530", + issue_content="Add new feature X", + repo_owner="test", + repo_name="repo", + issue_number=530, + issue_author="test-user", + issue_title="Add feature X", + cwd=mock_git_repo_with_docs, + verbose=False, + quiet=True, + use_github_state=False + ) + + # Verify normal flow continues through Steps 7-8 + for step in [1, 2, 3, 4, 5, 6, 7, 8, 9]: + assert step in steps_executed, f"Step {step} should execute in normal flow. Steps: {steps_executed}" + + # Workflow should succeed + assert success is True, f"Workflow should succeed for normal dev units path. Message: {message}" + + def test_no_dev_units_found_without_docs_still_triggers_hard_stop(self, mock_git_repo_with_docs, monkeypatch): + """ + REGRESSION TEST: True "No Dev Units Found" (without documentation context) should still stop. + + This test ensures the fix distinguishes between: + - "Documentation Only" (valid continuation path) - should proceed to Step 9 + - "No Dev Units Found" (true failure) - should trigger hard stop + + When there are NO dev units AND NO documentation changes, the workflow + should still stop as a failure (existing behavior preserved). + """ + monkeypatch.setenv("PDD_FORCE_LOCAL", "1") + + steps_executed = [] + + def mock_run_agentic_task(instruction, cwd, verbose, quiet, timeout, label, max_retries): + """Mock LLM agent that finds neither dev units nor documentation changes.""" + import re + match = re.search(r"step(\d+)", label) + if match: + step_num = int(match.group(1)) + steps_executed.append(step_num) + + if "step1" in label: + return (True, "No duplicate issues found. Proceed.", 0.001, "mock-model") + elif "step2" in label: + return (True, "Already implemented. Proceed.", 0.001, "mock-model") + elif "step3" in label: + return (True, "Research complete. Issue is unclear.", 0.001, "mock-model") + elif "step4" in label: + return (True, "Requirements unclear.", 0.001, "mock-model") + elif "step5" in label: + return (True, "No docs changes needed.", 0.001, "mock-model") + elif "step6" in label: + # True failure case: No dev units AND no documentation changes + return (True, "No Dev Units Found", 0.001, "mock-model") + + return (True, f"Mock success for {label}", 0.001, "mock-model") + + def mock_save_state(*args, **kwargs): + pass + + def mock_load_state(*args, **kwargs): + return None, None + + def mock_clear_state(*args, **kwargs): + pass + + from pdd.agentic_change_orchestrator import run_agentic_change_orchestrator + + with patch('pdd.agentic_change_orchestrator.run_agentic_task', side_effect=mock_run_agentic_task): + with patch('pdd.agentic_change_orchestrator.save_workflow_state', side_effect=mock_save_state): + with patch('pdd.agentic_change_orchestrator.load_workflow_state', side_effect=mock_load_state): + with patch('pdd.agentic_change_orchestrator.clear_workflow_state', side_effect=mock_clear_state): + success, message, cost, model, files = run_agentic_change_orchestrator( + issue_url="https://github.com/test/repo/issues/530", + issue_content="Unclear issue with no clear action items", + repo_owner="test", + repo_name="repo", + issue_number=530, + issue_author="test-user", + issue_title="Unclear issue", + cwd=mock_git_repo_with_docs, + verbose=False, + quiet=True, + use_github_state=False + ) + + # Verify workflow stopped at Step 6 (existing hard-stop behavior) + assert 6 in steps_executed, f"Step 6 should execute. Steps: {steps_executed}" + assert 7 not in steps_executed, f"Step 7 should NOT execute (hard stop). Steps: {steps_executed}" + assert 9 not in steps_executed, f"Step 9 should NOT execute (hard stop). Steps: {steps_executed}" + + # Workflow should fail with "No dev units found" message + assert success is False, f"Workflow should fail when no dev units AND no docs changes. Message: {message}" + assert "no dev units" in message.lower(), f"Failure message should mention 'no dev units'. Message: {message}"