From de84554b1600cecf5d73698353d6cd57fe5c479b Mon Sep 17 00:00:00 2001 From: Noah Horton Date: Mon, 6 Apr 2026 14:20:09 -0600 Subject: [PATCH 1/5] fix: only run .deepreview rules against git-changed output files The quality gate was treating all file_path outputs as "changed files" for .deepreview matching. This caused reviews to run on unchanged reference files (e.g. key_reference_files in exploration steps). Now intersects output file paths with actual git-changed files before matching .deepreview rules. Step-specific (dynamic) reviews still run against all declared outputs since those are explicitly defined. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/deepwork/jobs/mcp/quality_gate.py | 24 +++++++--- tests/unit/jobs/mcp/test_quality_gate.py | 56 ++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/src/deepwork/jobs/mcp/quality_gate.py b/src/deepwork/jobs/mcp/quality_gate.py index e21d0790..cb655578 100644 --- a/src/deepwork/jobs/mcp/quality_gate.py +++ b/src/deepwork/jobs/mcp/quality_gate.py @@ -26,7 +26,7 @@ from deepwork.review.instructions import ( write_instruction_files, ) -from deepwork.review.matcher import match_files_to_rules +from deepwork.review.matcher import get_changed_files, match_files_to_rules from deepwork.utils.validation import ValidationError, validate_against_schema logger = logging.getLogger("deepwork.jobs.mcp.quality_gate") @@ -328,17 +328,27 @@ def run_quality_gate( schema_rules, _schema_errors = gen_schema_rules(project_root) deepreview_rules.extend(schema_rules) - # 4. Get the "changed files" list = output file paths + # 4. Collect output file paths output_files = _collect_output_file_paths(outputs, job) - # 5. Match .deepreview rules against output files + # 5. Match .deepreview rules against output files that are actually changed. + # Output files may include unchanged reference files — .deepreview rules + # should only fire on files that were actually modified (git diff). deepreview_tasks: list[ReviewTask] = [] if deepreview_rules and output_files: - deepreview_tasks = match_files_to_rules( - output_files, deepreview_rules, project_root, platform - ) + try: + git_changed = get_changed_files(project_root) + except Exception: + git_changed = [] + output_set = set(output_files) + changed_output_files = [f for f in git_changed if f in output_set] + if changed_output_files: + deepreview_tasks = match_files_to_rules( + changed_output_files, deepreview_rules, project_root, platform + ) - # 6. Match dynamic rules against output files + # 6. Match dynamic rules (step-specific reviews) against all output files. + # These are explicitly defined for specific outputs and should always run. dynamic_tasks: list[ReviewTask] = [] if dynamic_rules and output_files: dynamic_tasks = match_files_to_rules(output_files, dynamic_rules, project_root, platform) diff --git a/tests/unit/jobs/mcp/test_quality_gate.py b/tests/unit/jobs/mcp/test_quality_gate.py index b7d9fcb0..ec336576 100644 --- a/tests/unit/jobs/mcp/test_quality_gate.py +++ b/tests/unit/jobs/mcp/test_quality_gate.py @@ -664,6 +664,10 @@ def test_merges_deepreview_and_dynamic_tasks(self, tmp_path: Path) -> None: "deepwork.jobs.mcp.quality_gate.load_all_rules", return_value=([deepreview_rule], []), ), + patch( + "deepwork.jobs.mcp.quality_gate.get_changed_files", + return_value=["report.md"], + ), patch( "deepwork.jobs.mcp.quality_gate.match_files_to_rules", side_effect=[[deepreview_task], [dynamic_task]], @@ -697,6 +701,58 @@ def test_merges_deepreview_and_dynamic_tasks(self, tmp_path: Path) -> None: assert all_tasks[1].rule_name == "external_rule" assert result is not None + def test_deepreview_rules_skip_unchanged_output_files(self, tmp_path: Path) -> None: + """Deepreview rules should only match output files that are actually changed in git.""" + arg = StepArgument(name="refs", description="Reference files", type="file_path") + output_ref = StepOutputRef(argument_name="refs", required=False) + step = WorkflowStep(name="explore", outputs={"refs": output_ref}) + job, workflow = _make_job(tmp_path, [arg], step) + + deepreview_rule = ReviewRule( + name="python_lint", + description="Lint Python files", + include_patterns=["**/*.py"], + exclude_patterns=[], + strategy="matches_together", + instructions="Run linting", + agent=None, + all_changed_filenames=False, + unchanged_matching_files=False, + precomputed_info_bash_command=None, + source_dir=tmp_path, + source_file=tmp_path / ".deepreview", + source_line=1, + ) + + with ( + patch( + "deepwork.jobs.mcp.quality_gate.load_all_rules", + return_value=([deepreview_rule], []), + ), + # git says no files changed — output files are just references + patch( + "deepwork.jobs.mcp.quality_gate.get_changed_files", + return_value=[], + ), + patch( + "deepwork.jobs.mcp.quality_gate.match_files_to_rules", + ) as mock_match, + ): + result = run_quality_gate( + step=step, + job=job, + workflow=workflow, + outputs={"refs": ["src/foo.py", "src/bar.py"]}, + input_values={}, + work_summary=None, + project_root=tmp_path, + ) + + # match_files_to_rules should not be called for deepreview since + # no output files are in the git changed set + assert mock_match.call_count == 0 + assert result is None + # --------------------------------------------------------------------------- # TestValidateJsonSchemas — additional coverage From 0bf9e69e0099bd7e3bb3c661a0e27ff2dd582eef Mon Sep 17 00:00:00 2001 From: Noah Horton Date: Mon, 6 Apr 2026 14:32:31 -0600 Subject: [PATCH 2/5] fix: update JOBS-REQ-004.5 for git-changed file intersection and add traceability Update requirements to reflect that .deepreview rules are now matched only against git-changed output files, and that dynamic rules still match all outputs. Documents the fallback behavior when get_changed_files fails. Adds traceability comment to the new test. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../jobs/JOBS-REQ-004-quality-review-system.md | 13 +++++++------ tests/unit/jobs/mcp/test_quality_gate.py | 1 + 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/specs/deepwork/jobs/JOBS-REQ-004-quality-review-system.md b/specs/deepwork/jobs/JOBS-REQ-004-quality-review-system.md index ab68742b..fa9d9ec3 100644 --- a/specs/deepwork/jobs/JOBS-REQ-004-quality-review-system.md +++ b/specs/deepwork/jobs/JOBS-REQ-004-quality-review-system.md @@ -41,12 +41,13 @@ The quality review system evaluates step outputs against defined quality criteri ### JOBS-REQ-004.5: Review Pipeline Integration 1. `run_quality_gate()` MUST load `.deepreview` rules via `load_all_rules()`. -2. `.deepreview` rules MUST be matched against output file paths via `match_files_to_rules()`. -3. Dynamic rules (from step reviews) MUST also be matched via `match_files_to_rules()`. -4. All matched tasks (dynamic + `.deepreview`) MUST be combined. -5. Combined tasks MUST be passed to `write_instruction_files()`, which honors `.passed` marker files. -6. If `write_instruction_files()` returns no task files (all already passed), `run_quality_gate()` MUST return `None`. -7. Remaining task files MUST be formatted via `format_for_claude()`. +2. `.deepreview` rules MUST be matched only against output file paths that also appear in `get_changed_files()` (i.e., files actually modified in git). Unchanged reference files in outputs MUST NOT trigger `.deepreview` rules. +3. Dynamic rules (from step reviews) MUST be matched against all output file paths via `match_files_to_rules()`, regardless of git change status. +4. If `get_changed_files()` fails, `.deepreview` matching MUST be skipped (no `.deepreview` tasks produced). Dynamic rules MUST be unaffected. +5. All matched tasks (dynamic + `.deepreview`) MUST be combined. +6. Combined tasks MUST be passed to `write_instruction_files()`, which honors `.passed` marker files. +7. If `write_instruction_files()` returns no task files (all already passed), `run_quality_gate()` MUST return `None`. +8. Remaining task files MUST be formatted via `format_for_claude()`. ### JOBS-REQ-004.6: Review Guidance Output diff --git a/tests/unit/jobs/mcp/test_quality_gate.py b/tests/unit/jobs/mcp/test_quality_gate.py index ec336576..2dea92eb 100644 --- a/tests/unit/jobs/mcp/test_quality_gate.py +++ b/tests/unit/jobs/mcp/test_quality_gate.py @@ -701,6 +701,7 @@ def test_merges_deepreview_and_dynamic_tasks(self, tmp_path: Path) -> None: assert all_tasks[1].rule_name == "external_rule" assert result is not None + # Validates JOBS-REQ-004.5.2 (deepreview rules matched only against git-changed output files) def test_deepreview_rules_skip_unchanged_output_files(self, tmp_path: Path) -> None: """Deepreview rules should only match output files that are actually changed in git.""" arg = StepArgument(name="refs", description="Reference files", type="file_path") From fd7a3e0e9d84eecfa25220d80820d9c686707768 Mon Sep 17 00:00:00 2001 From: Noah Horton Date: Mon, 6 Apr 2026 14:32:52 -0600 Subject: [PATCH 3/5] docs: update architecture and MCP docs for git-changed file filtering Reflect that .deepreview rules now match against git-changed output files only, while dynamic rules still match all output files. Co-Authored-By: Claude Opus 4.6 (1M context) --- doc/architecture.md | 2 +- doc/mcp_interface.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/architecture.md b/doc/architecture.md index a7ff1fc9..d10db53c 100644 --- a/doc/architecture.md +++ b/doc/architecture.md @@ -1001,7 +1001,7 @@ The quality gate integrates with the DeepWork Reviews infrastructure rather than 3. **Process requirements**: If the step defines `process_requirements`, a review is created that evaluates the `work_summary` against those requirements using RFC 2119 semantics. MUST/SHALL violations cause failure; SHOULD/RECOMMENDED violations fail only if easily achievable; other requirements produce feedback without failure. -4. **Merge with `.deepreview` rules**: The dynamically built rules are merged with any `.deepreview` file-defined rules that match the output files. The changed file list comes from the `outputs` parameter (not git diff). +4. **Merge with `.deepreview` rules**: `.deepreview` file-defined rules and DeepSchema-generated synthetic rules are matched against output files that are *actually changed* (via git diff). Output files that were provided as unchanged references are excluded from `.deepreview` matching. Dynamic rules (from step output `review` blocks) still run against all output files regardless of git status. 5. **Apply review strategies**: Review strategies (`individual`, `matches_together`, etc.) work normally on the merged rule set. diff --git a/doc/mcp_interface.md b/doc/mcp_interface.md index ac8ee00e..ea516956 100644 --- a/doc/mcp_interface.md +++ b/doc/mcp_interface.md @@ -390,7 +390,7 @@ Steps may define quality reviews that outputs must pass. When `finished_step` is 1. JSON schema validation runs first (if any outputs have `json_schema` defined) 2. Dynamic review rules are built from step output `review` blocks and `process_requirements` -3. `.deepreview` rules and DeepSchema-generated synthetic review rules are also loaded and matched against output files +3. `.deepreview` rules and DeepSchema-generated synthetic review rules are loaded and matched against output files that are actually changed (via git diff). Dynamic rules from step `review` blocks run against all output files regardless of git status. 4. If any reviews are needed, `status = "needs_work"` with review instructions 5. If all reviews pass (or no reviews defined), workflow advances 6. There is no maximum attempt limit — the agent can retry `finished_step` indefinitely From 4627a2c90d72648bc96c576e1e16344d84e4e4f5 Mon Sep 17 00:00:00 2001 From: Noah Horton Date: Mon, 6 Apr 2026 14:34:16 -0600 Subject: [PATCH 4/5] fix: add missing RFC 2119 keyword to JOBS-REQ-004.3 item 7 Pre-existing issue: requirement 7 read as a descriptive statement without a MUST/SHOULD/MAY keyword, violating the DeepSchema. Co-Authored-By: Claude Opus 4.6 (1M context) --- specs/deepwork/jobs/JOBS-REQ-004-quality-review-system.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/deepwork/jobs/JOBS-REQ-004-quality-review-system.md b/specs/deepwork/jobs/JOBS-REQ-004-quality-review-system.md index fa9d9ec3..2d9dfa6a 100644 --- a/specs/deepwork/jobs/JOBS-REQ-004-quality-review-system.md +++ b/specs/deepwork/jobs/JOBS-REQ-004-quality-review-system.md @@ -29,7 +29,7 @@ The quality review system evaluates step outputs against defined quality criteri 4. Each rule's `instructions` MUST be prefixed with a preamble containing workflow `common_job_info` and input context (if available). 5. Outputs with no review blocks MUST be skipped. 6. Outputs with `None` values MUST be skipped. -7. Only `file_path` type arguments with actual file paths generate `ReviewRule` objects. +7. Only `file_path` type arguments with actual file paths MUST generate `ReviewRule` objects. ### JOBS-REQ-004.4: Process Requirements From e2146c50af5c8ad0b9a74604e1643c51b24fea59 Mon Sep 17 00:00:00 2001 From: Noah Horton Date: Mon, 6 Apr 2026 14:35:46 -0600 Subject: [PATCH 5/5] fix: update stale requirement IDs, add missing tests for JOBS-REQ-004.5.3/4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Renumber traceability comments (.5.4→.5.5, .5.6→.5.7, .5.7→.5.8) to match spec renumbering - Fix traceability comment format on new test to match project convention - Add test for JOBS-REQ-004.5.3: dynamic rules match all outputs regardless of git status - Add test for JOBS-REQ-004.5.4: get_changed_files failure skips .deepreview matching but dynamic rules still run Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/unit/jobs/mcp/test_quality_gate.py | 136 ++++++++++++++++++++++- 1 file changed, 132 insertions(+), 4 deletions(-) diff --git a/tests/unit/jobs/mcp/test_quality_gate.py b/tests/unit/jobs/mcp/test_quality_gate.py index 2dea92eb..959dc5d4 100644 --- a/tests/unit/jobs/mcp/test_quality_gate.py +++ b/tests/unit/jobs/mcp/test_quality_gate.py @@ -518,7 +518,7 @@ def test_returns_feedback_when_json_schema_fails(self, tmp_path: Path) -> None: assert "JSON schema validation failed" in result assert "finished_step" in result - # THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.1.3, JOBS-REQ-004.5.7, JOBS-REQ-004.6.1). + # THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.1.3, JOBS-REQ-004.5.8, JOBS-REQ-004.6.1). # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES def test_returns_review_instructions_when_reviews_exist(self, tmp_path: Path) -> None: """When dynamic rules produce tasks, review instructions are returned.""" @@ -573,7 +573,7 @@ def test_returns_review_instructions_when_reviews_exist(self, tmp_path: Path) -> assert "Quality reviews are required" in result assert "step_write_output_report" in result - # THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.5.6). + # THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.5.7). # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES def test_returns_none_when_all_reviews_already_passed(self, tmp_path: Path) -> None: """If write_instruction_files returns empty (all .passed), result is None.""" @@ -616,7 +616,7 @@ def test_returns_none_when_all_reviews_already_passed(self, tmp_path: Path) -> N assert result is None - # THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.5.4). + # THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.5.5). # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES def test_merges_deepreview_and_dynamic_tasks(self, tmp_path: Path) -> None: """Both .deepreview rules and dynamic rules are processed together.""" @@ -701,7 +701,8 @@ def test_merges_deepreview_and_dynamic_tasks(self, tmp_path: Path) -> None: assert all_tasks[1].rule_name == "external_rule" assert result is not None - # Validates JOBS-REQ-004.5.2 (deepreview rules matched only against git-changed output files) + # THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.5.2). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES def test_deepreview_rules_skip_unchanged_output_files(self, tmp_path: Path) -> None: """Deepreview rules should only match output files that are actually changed in git.""" arg = StepArgument(name="refs", description="Reference files", type="file_path") @@ -754,6 +755,133 @@ def test_deepreview_rules_skip_unchanged_output_files(self, tmp_path: Path) -> N assert mock_match.call_count == 0 assert result is None + # THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.5.3). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES + def test_dynamic_rules_match_all_outputs_regardless_of_git(self, tmp_path: Path) -> None: + """Dynamic rules run against all output files even when git says nothing changed.""" + review = ReviewBlock(strategy="individual", instructions="Check it") + arg = StepArgument(name="report", description="Report", type="file_path") + output_ref = StepOutputRef(argument_name="report", required=True, review=review) + step = WorkflowStep(name="write", outputs={"report": output_ref}) + job, workflow = _make_job(tmp_path, [arg], step) + + dynamic_task = ReviewTask( + rule_name="step_write_output_report", + files_to_review=["report.md"], + instructions="Check it", + agent_name=None, + ) + instruction_path = tmp_path / ".deepwork" / "tmp" / "instr.md" + instruction_path.parent.mkdir(parents=True, exist_ok=True) + instruction_path.write_text("content") + + with ( + patch( + "deepwork.jobs.mcp.quality_gate.load_all_rules", + return_value=([], []), + ), + patch( + "deepwork.jobs.mcp.quality_gate.match_files_to_rules", + return_value=[dynamic_task], + ) as mock_match, + patch( + "deepwork.jobs.mcp.quality_gate.write_instruction_files", + return_value=[(dynamic_task, instruction_path)], + ), + patch( + "deepwork.jobs.mcp.quality_gate.format_for_claude", + return_value="formatted", + ), + ): + result = run_quality_gate( + step=step, + job=job, + workflow=workflow, + outputs={"report": "report.md"}, + input_values={}, + work_summary=None, + project_root=tmp_path, + ) + + # Dynamic rules matched even though no deepreview rules exist and + # get_changed_files was never called (no deepreview rules to trigger it) + mock_match.assert_called_once() + assert result is not None + + # THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.5.4). + # YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES + def test_deepreview_skipped_when_get_changed_files_fails(self, tmp_path: Path) -> None: + """If get_changed_files() fails, .deepreview matching is skipped; dynamic rules unaffected.""" + review = ReviewBlock(strategy="individual", instructions="Check it") + arg = StepArgument(name="report", description="Report", type="file_path") + output_ref = StepOutputRef(argument_name="report", required=True, review=review) + step = WorkflowStep(name="write", outputs={"report": output_ref}) + job, workflow = _make_job(tmp_path, [arg], step) + + deepreview_rule = ReviewRule( + name="lint", + description="Lint", + include_patterns=["*.md"], + exclude_patterns=[], + strategy="individual", + instructions="Lint it", + agent=None, + all_changed_filenames=False, + unchanged_matching_files=False, + precomputed_info_bash_command=None, + source_dir=tmp_path, + source_file=tmp_path / ".deepreview", + source_line=1, + ) + + dynamic_task = ReviewTask( + rule_name="step_write_output_report", + files_to_review=["report.md"], + instructions="Check it", + agent_name=None, + ) + instruction_path = tmp_path / ".deepwork" / "tmp" / "instr.md" + instruction_path.parent.mkdir(parents=True, exist_ok=True) + instruction_path.write_text("content") + + from deepwork.review.matcher import GitDiffError + + with ( + patch( + "deepwork.jobs.mcp.quality_gate.load_all_rules", + return_value=([deepreview_rule], []), + ), + patch( + "deepwork.jobs.mcp.quality_gate.get_changed_files", + side_effect=GitDiffError("git not available"), + ), + patch( + "deepwork.jobs.mcp.quality_gate.match_files_to_rules", + return_value=[dynamic_task], + ) as mock_match, + patch( + "deepwork.jobs.mcp.quality_gate.write_instruction_files", + return_value=[(dynamic_task, instruction_path)], + ), + patch( + "deepwork.jobs.mcp.quality_gate.format_for_claude", + return_value="formatted", + ), + ): + result = run_quality_gate( + step=step, + job=job, + workflow=workflow, + outputs={"report": "report.md"}, + input_values={}, + work_summary=None, + project_root=tmp_path, + ) + + # match_files_to_rules called only once (for dynamic rules, not deepreview) + mock_match.assert_called_once() + assert result is not None + # --------------------------------------------------------------------------- # TestValidateJsonSchemas — additional coverage