Skip to content

Commit 1994f94

Browse files
nhortonclaude
andcommitted
fix: remove redundant review rules and fix strategy/criteria mismatch
Several review cleanups driven by the observation that the `add_documentation_rules` step review was taking forever: - job.yml: change `add_documentation_rules` review from `strategy: individual` to `strategy: matches_together`. Two of its three criteria (Documentation Covered, Efficient Rule Count) are project-wide and cross-cutting, so running them per-file forced each reviewer to redo the full project scan N times. - job.yml: drop the `apply_rule` "Valid Configuration" criterion — the `deepreview` DeepSchema already enforces structural/schema validation via `json_schema_path`. - job.yml: collapse the duplicated rule-YAML blocks in `add_deepwork_native_reviews`. The step previously showed the same two rules twice (once as a file-ref reference example, once as the inline version to emit) and told the agent to consult both, creating a contradiction about which form to use. - Delete `src/deepwork/standard_jobs/deepwork_reviews/.deepreview` (`job_definition_review` rule): fully redundant with the `job_yml` DeepSchema which covers the same files and criteria. - Delete `job_schema_instruction_compatibility` from the top-level `.deepreview`: its whole purpose was verifying job.yml files conform to `job.schema.json`, which is exactly what the `job_yml` DeepSchema's `json_schema_path` already enforces. - Add `review-strategy-matches-criteria-scope` requirement to the `job_yml` DeepSchema so this class of bug is caught going forward: review `strategy: individual` must only be used when every criterion can be evaluated from a single file in isolation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1091181 commit 1994f94

4 files changed

Lines changed: 19 additions & 97 deletions

File tree

.deepreview

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -369,46 +369,6 @@ shell_code_review:
369369
section markers) still accurate after the changes? Flag any comments
370370
that describe behavior that no longer matches the code.
371371

372-
job_schema_instruction_compatibility:
373-
description: "Verify all standard and library job.yml definitions and templates are compatible with the job schema."
374-
match:
375-
include:
376-
- "src/deepwork/jobs/job.schema.json"
377-
- "src/deepwork/standard_jobs/*/job.yml"
378-
- "src/deepwork/standard_jobs/deepwork_jobs/templates/job.yml.*" # Templates users copy — must match schema
379-
- "library/jobs/*/job.yml"
380-
review:
381-
strategy: matches_together
382-
additional_context:
383-
unchanged_matching_files: true
384-
instructions: |
385-
When the job schema, standard job definitions, or library job definitions
386-
change, verify they are still compatible with each other.
387-
388-
Read src/deepwork/jobs/job.schema.json to understand the current schema.
389-
Then read each job.yml (standard and library) and check:
390-
391-
1. **Field references**: Every field name referenced in inline step
392-
instructions must exist in the schema at the correct level.
393-
Pay special attention to step_arguments vs workflow vs step fields.
394-
395-
2. **Required vs optional**: If instructions say a field is required,
396-
verify the schema agrees. If instructions say a field is optional,
397-
verify the schema doesn't require it.
398-
399-
3. **Schema structure**: Any YAML examples shown in inline instructions
400-
must match the schema's property names and nesting.
401-
402-
4. **Terminology consistency**: Instructions should use the same field
403-
names as the schema (e.g., if the schema uses
404-
"common_job_info_provided_to_all_steps_at_runtime", instructions
405-
should not call it "description" or "job_description").
406-
407-
Output Format:
408-
- PASS: All job definitions are compatible with the schema.
409-
- FAIL: Incompatibilities found. List each with the file path, line
410-
reference, the incompatible content, and what the schema actually says.
411-
412372
agents_md_claude_md_symlink:
413373
description: "Ensure every AGENTS.md file has a sibling CLAUDE.md symlink pointing to it, because Claude Code reads CLAUDE.md but ignores AGENTS.md."
414374
match:

src/deepwork/standard_jobs/deepwork_reviews/.deepreview

Lines changed: 0 additions & 1 deletion
This file was deleted.

src/deepwork/standard_jobs/deepwork_reviews/job.yml

Lines changed: 10 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -51,57 +51,15 @@ workflows:
5151
5252
## Task
5353
54-
### 1. Read the example review instruction files
55-
56-
Read these files from the DeepWork plugin:
57-
- `plugins/claude/example_reviews/prompt_best_practices.md`
58-
- `plugins/claude/example_reviews/suggest_new_reviews.md`
59-
60-
These are the full, detailed versions of the two review instruction prompts. Read them to understand the review's intent, checklist, and tone. The inline YAML rules in steps 3 and 4 below are condensed versions — use the example files to fill in any gaps or to judge whether the inline version captures the key points.
61-
62-
For reference, this is how the DeepWork project's own `.deepreview` configures these two rules (using `instructions: { file: ... }` to point at the example files above):
63-
64-
```yaml
65-
prompt_best_practices:
66-
description: "Review prompt/instruction markdown files for Anthropic prompt engineering best practices."
67-
match:
68-
include:
69-
- "**/CLAUDE.md"
70-
- "**/AGENTS.md"
71-
- ".claude/**/*.md"
72-
- ".deepwork/review/*.md"
73-
- ".deepwork/jobs/**/*.md"
74-
review:
75-
strategy: individual
76-
instructions:
77-
file: .deepwork/review/prompt_best_practices.md
78-
79-
suggest_new_reviews:
80-
description: "Analyze all changes and suggest new review rules that would catch issues going forward."
81-
match:
82-
include:
83-
- "**/*"
84-
exclude:
85-
- ".github/**"
86-
review:
87-
strategy: matches_together
88-
instructions:
89-
file: .deepwork/review/suggest_new_reviews.md
90-
```
91-
92-
When creating rules for the target project, adapt the `match.include` patterns to its structure. The inline YAML in steps 3 and 4 uses inline `instructions:` text (suitable for projects that don't have the external instruction files), while the above uses `instructions: { file: ... }` references.
93-
94-
If the example files are not found (e.g., the plugin is installed differently), proceed using the inline YAML in steps 3 and 4 below.
95-
96-
### 2. Check the existing `.deepreview` file
54+
### 1. Check the existing `.deepreview` file
9755
9856
Read the top-level `.deepreview` file if it exists. Check whether `suggest_new_reviews` and/or `prompt_best_practices` rules are already present.
9957
10058
- If both rules already exist with reasonable configurations, skip to the output step — no changes needed.
10159
- If one or both are missing, proceed to add the missing rule(s).
10260
- If a rule exists but has a substantially different configuration (wrong strategy, missing match patterns), update it to match the specification below.
10361
104-
### 3. Add the `prompt_best_practices` rule (if not present)
62+
### 2. Add the `prompt_best_practices` rule (if not present)
10563
10664
Add to `.deepreview`:
10765
@@ -144,7 +102,7 @@ workflows:
144102
145103
Adapt the `match.include` patterns to the project if needed. Check for directories containing `.md` files that appear to be AI instruction files (e.g., `.gemini/`, `.cursorrules`, custom agent directories). If found, add those patterns too. You may add both missing rules in a single edit to the `.deepreview` file. The patterns above are the baseline.
146104
147-
### 4. Add the `suggest_new_reviews` rule (if not present)
105+
### 3. Add the `suggest_new_reviews` rule (if not present)
148106
149107
Add to `.deepreview`:
150108
@@ -175,7 +133,7 @@ workflows:
175133
5. If no rules are warranted, say so. An empty suggestion list is valid.
176134
```
177135
178-
### 5. Validate
136+
### 4. Validate
179137
180138
Ensure the `.deepreview` file is valid YAML. Ensure both rules have all required fields: `description`, `match.include`, `review.strategy`, `review.instructions`.
181139
@@ -347,14 +305,12 @@ workflows:
347305
deepreview_files:
348306
required: true
349307
review:
350-
strategy: individual
308+
strategy: matches_together
351309
instructions: |
352-
Read each .deepreview file and the documentation files its rules protect.
353-
Verify that the trigger scope of each rule is as narrow as possible —
354-
it should only fire when files that could actually affect the doc's accuracy
355-
change. Consider whether having more separate reviews with narrower scope
356-
is actually more efficient than a slightly wider, shared review — each
357-
review spawns a sub-agent with material overhead.
310+
Read all .deepreview files together along with the documentation files their
311+
rules protect. The criteria below are cross-cutting — Documentation Covered
312+
and Efficient Rule Count require a project-wide view of all rules at once,
313+
which is why this review runs once over the full set rather than per file.
358314
359315
Evaluate the output against these criteria:
360316
- Documentation Covered: Every project documentation file that describes the project itself has a corresponding rule (either newly created or pre-existing).
@@ -905,10 +861,8 @@ workflows:
905861
instructions: |
906862
Read the dependency analysis from the previous step to verify the rule
907863
faithfully implements the approved plan. Read the documentation file
908-
referenced in the rule's instructions. Check the .deepreview file for
909-
valid YAML syntax and consistency with existing rules.
864+
referenced in the rule's instructions.
910865
911866
Evaluate the output against these criteria:
912867
- Faithful Implementation: The rule accurately implements the dependency analysis from the previous step — same match patterns, same strategy, same rule name convention.
913-
- Valid Configuration: The .deepreview YAML is syntactically valid and follows the schema. All required fields (description, match.include, review.strategy, review.instructions) are present.
914868
- Effective Instructions: Review instructions clearly tell the reviewer to check whether the documentation file is still accurate given the source file changes. The documentation file path is explicitly referenced. Uses additional_context.unchanged_matching_files: true so the reviewer can read the doc even when only source files changed.

src/deepwork/standard_schemas/job_yml/deepschema.yml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,15 @@ requirements:
7676
auto-pass rather than fail. Avoid criteria that penalize steps for not doing things
7777
outside their scope.
7878
79+
review-strategy-matches-criteria-scope: >
80+
A review `strategy: individual` MUST only be used when every listed criterion can be
81+
evaluated from a single file in isolation. Criteria that require cross-file comparison,
82+
project-wide enumeration, set-level properties (coverage, minimality, deduplication),
83+
or reasoning about the relationship between multiple output files MUST use
84+
`strategy: matches_together` so the reviewer sees the full set at once. Using
85+
`individual` with cross-cutting criteria forces each reviewer to redo the project-wide
86+
scan N times and multiplies latency without improving review quality.
87+
7988
review-no-redundant-context: >
8089
Review `instructions` MUST NOT repeat domain context already in `common_job_info` since
8190
the framework automatically prepends it to review prompts.

0 commit comments

Comments
 (0)