From 1994f9468fa36ecbf970ef2ed98ff14ae5dcef04 Mon Sep 17 00:00:00 2001 From: Noah Horton Date: Wed, 8 Apr 2026 17:16:28 -0600 Subject: [PATCH 1/7] fix: remove redundant review rules and fix strategy/criteria mismatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .deepreview | 40 ----------- .../deepwork_reviews/.deepreview | 1 - .../standard_jobs/deepwork_reviews/job.yml | 66 +++---------------- .../standard_schemas/job_yml/deepschema.yml | 9 +++ 4 files changed, 19 insertions(+), 97 deletions(-) delete mode 120000 src/deepwork/standard_jobs/deepwork_reviews/.deepreview diff --git a/.deepreview b/.deepreview index 93593249..2c1ef60a 100644 --- a/.deepreview +++ b/.deepreview @@ -369,46 +369,6 @@ shell_code_review: section markers) still accurate after the changes? Flag any comments that describe behavior that no longer matches the code. -job_schema_instruction_compatibility: - description: "Verify all standard and library job.yml definitions and templates are compatible with the job schema." - match: - include: - - "src/deepwork/jobs/job.schema.json" - - "src/deepwork/standard_jobs/*/job.yml" - - "src/deepwork/standard_jobs/deepwork_jobs/templates/job.yml.*" # Templates users copy — must match schema - - "library/jobs/*/job.yml" - review: - strategy: matches_together - additional_context: - unchanged_matching_files: true - instructions: | - When the job schema, standard job definitions, or library job definitions - change, verify they are still compatible with each other. - - Read src/deepwork/jobs/job.schema.json to understand the current schema. - Then read each job.yml (standard and library) and check: - - 1. **Field references**: Every field name referenced in inline step - instructions must exist in the schema at the correct level. - Pay special attention to step_arguments vs workflow vs step fields. - - 2. **Required vs optional**: If instructions say a field is required, - verify the schema agrees. If instructions say a field is optional, - verify the schema doesn't require it. - - 3. **Schema structure**: Any YAML examples shown in inline instructions - must match the schema's property names and nesting. - - 4. **Terminology consistency**: Instructions should use the same field - names as the schema (e.g., if the schema uses - "common_job_info_provided_to_all_steps_at_runtime", instructions - should not call it "description" or "job_description"). - - Output Format: - - PASS: All job definitions are compatible with the schema. - - FAIL: Incompatibilities found. List each with the file path, line - reference, the incompatible content, and what the schema actually says. - agents_md_claude_md_symlink: 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." match: diff --git a/src/deepwork/standard_jobs/deepwork_reviews/.deepreview b/src/deepwork/standard_jobs/deepwork_reviews/.deepreview deleted file mode 120000 index ffda0549..00000000 --- a/src/deepwork/standard_jobs/deepwork_reviews/.deepreview +++ /dev/null @@ -1 +0,0 @@ -../deepwork_jobs/template.deepreview \ No newline at end of file diff --git a/src/deepwork/standard_jobs/deepwork_reviews/job.yml b/src/deepwork/standard_jobs/deepwork_reviews/job.yml index 5cab26a3..577232ee 100644 --- a/src/deepwork/standard_jobs/deepwork_reviews/job.yml +++ b/src/deepwork/standard_jobs/deepwork_reviews/job.yml @@ -51,49 +51,7 @@ workflows: ## Task - ### 1. Read the example review instruction files - - Read these files from the DeepWork plugin: - - `plugins/claude/example_reviews/prompt_best_practices.md` - - `plugins/claude/example_reviews/suggest_new_reviews.md` - - 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. - - 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): - - ```yaml - prompt_best_practices: - description: "Review prompt/instruction markdown files for Anthropic prompt engineering best practices." - match: - include: - - "**/CLAUDE.md" - - "**/AGENTS.md" - - ".claude/**/*.md" - - ".deepwork/review/*.md" - - ".deepwork/jobs/**/*.md" - review: - strategy: individual - instructions: - file: .deepwork/review/prompt_best_practices.md - - suggest_new_reviews: - description: "Analyze all changes and suggest new review rules that would catch issues going forward." - match: - include: - - "**/*" - exclude: - - ".github/**" - review: - strategy: matches_together - instructions: - file: .deepwork/review/suggest_new_reviews.md - ``` - - 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. - - 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. - - ### 2. Check the existing `.deepreview` file + ### 1. Check the existing `.deepreview` file Read the top-level `.deepreview` file if it exists. Check whether `suggest_new_reviews` and/or `prompt_best_practices` rules are already present. @@ -101,7 +59,7 @@ workflows: - If one or both are missing, proceed to add the missing rule(s). - If a rule exists but has a substantially different configuration (wrong strategy, missing match patterns), update it to match the specification below. - ### 3. Add the `prompt_best_practices` rule (if not present) + ### 2. Add the `prompt_best_practices` rule (if not present) Add to `.deepreview`: @@ -144,7 +102,7 @@ workflows: 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. - ### 4. Add the `suggest_new_reviews` rule (if not present) + ### 3. Add the `suggest_new_reviews` rule (if not present) Add to `.deepreview`: @@ -175,7 +133,7 @@ workflows: 5. If no rules are warranted, say so. An empty suggestion list is valid. ``` - ### 5. Validate + ### 4. Validate Ensure the `.deepreview` file is valid YAML. Ensure both rules have all required fields: `description`, `match.include`, `review.strategy`, `review.instructions`. @@ -347,14 +305,12 @@ workflows: deepreview_files: required: true review: - strategy: individual + strategy: matches_together instructions: | - Read each .deepreview file and the documentation files its rules protect. - Verify that the trigger scope of each rule is as narrow as possible — - it should only fire when files that could actually affect the doc's accuracy - change. Consider whether having more separate reviews with narrower scope - is actually more efficient than a slightly wider, shared review — each - review spawns a sub-agent with material overhead. + Read all .deepreview files together along with the documentation files their + rules protect. The criteria below are cross-cutting — Documentation Covered + and Efficient Rule Count require a project-wide view of all rules at once, + which is why this review runs once over the full set rather than per file. Evaluate the output against these criteria: - 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: instructions: | Read the dependency analysis from the previous step to verify the rule faithfully implements the approved plan. Read the documentation file - referenced in the rule's instructions. Check the .deepreview file for - valid YAML syntax and consistency with existing rules. + referenced in the rule's instructions. Evaluate the output against these criteria: - Faithful Implementation: The rule accurately implements the dependency analysis from the previous step — same match patterns, same strategy, same rule name convention. - - Valid Configuration: The .deepreview YAML is syntactically valid and follows the schema. All required fields (description, match.include, review.strategy, review.instructions) are present. - 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. diff --git a/src/deepwork/standard_schemas/job_yml/deepschema.yml b/src/deepwork/standard_schemas/job_yml/deepschema.yml index 0600def4..d0b9b0d8 100644 --- a/src/deepwork/standard_schemas/job_yml/deepschema.yml +++ b/src/deepwork/standard_schemas/job_yml/deepschema.yml @@ -76,6 +76,15 @@ requirements: auto-pass rather than fail. Avoid criteria that penalize steps for not doing things outside their scope. + review-strategy-matches-criteria-scope: > + A review `strategy: individual` MUST only be used when every listed criterion can be + evaluated from a single file in isolation. Criteria that require cross-file comparison, + project-wide enumeration, set-level properties (coverage, minimality, deduplication), + or reasoning about the relationship between multiple output files MUST use + `strategy: matches_together` so the reviewer sees the full set at once. Using + `individual` with cross-cutting criteria forces each reviewer to redo the project-wide + scan N times and multiplies latency without improving review quality. + review-no-redundant-context: > Review `instructions` MUST NOT repeat domain context already in `common_job_info` since the framework automatically prepends it to review prompts. From a4dc74c2dbbae8750fe69eb51cc93c2c01f57338 Mon Sep 17 00:00:00 2001 From: Noah Horton Date: Wed, 8 Apr 2026 17:18:00 -0600 Subject: [PATCH 2/7] feat: add no-sentinel-files SHOULD requirement to job_yml DeepSchema Discourages the sentinel-placeholder-file workaround (e.g., writing a NONE.md file when the real result is "nothing to report") used to satisfy required file_path outputs that would otherwise be empty. The fix is to use type: string for outputs whose list can legitimately be empty, since string outputs can naturally carry a narrative "no items found" message. Kept as SHOULD so genuine filesystem-artifact needs (manifests, etc.) can still opt into a sentinel, but the common "required output can't be empty" workaround is explicitly not a sufficient reason. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../standard_schemas/job_yml/deepschema.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/deepwork/standard_schemas/job_yml/deepschema.yml b/src/deepwork/standard_schemas/job_yml/deepschema.yml index d0b9b0d8..a8d70f81 100644 --- a/src/deepwork/standard_schemas/job_yml/deepschema.yml +++ b/src/deepwork/standard_schemas/job_yml/deepschema.yml @@ -138,6 +138,20 @@ requirements: that are too large for inline strings. The existence of a `.deepwork/tmp/` path in a step_argument name is a strong signal the argument should be `type: string`. + no-sentinel-files: > + Steps SHOULD NOT use sentinel placeholder files (e.g., writing a `NONE.md` or + `EMPTY.md` file to satisfy a required `file_path` output when the real result is + "nothing to report") to work around the rule that `file_path` outputs cannot be + empty. If a step's output is a list that can legitimately be empty, the output + SHOULD be `type: string` (containing a narrative summary that naturally handles + the empty case) rather than `type: file_path` with a placeholder file. Sentinel + files leave stray artifacts on disk, confuse downstream steps that treat every + output path as a real file, and mislead reviewers. Exception: a sentinel file + MAY be used when the step genuinely needs a filesystem artifact a later step + will read and parse (e.g., a manifest with "no items" semantics baked into its + schema) — but the common "required output can't be empty" workaround is not a + sufficient reason. + step-visibility-boundary: > A step's instructions MUST NOT reference information from steps that have not yet run in the same workflow, including steps from other workflows in the same job. From 16d38b31c57cf1c17825899e7b824b1df8ec963a Mon Sep 17 00:00:00 2001 From: Noah Horton Date: Wed, 8 Apr 2026 17:32:37 -0600 Subject: [PATCH 3/7] fix: replace migrate_existing_skills sentinel file with empty list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The step previously directed the agent to write a .deepwork/tmp/migrated_skills/NONE.md sentinel file as a workaround for "required file_path output can't be empty" — exactly the anti-pattern the new no-sentinel-files DeepSchema requirement added in a4dc74c prohibits. Verified the MCP validation accepts empty lists for required file_path outputs (no minimum-length check in src/deepwork/jobs/mcp/tools.py), so the sentinel workaround was never necessary. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/deepwork/standard_jobs/deepwork_reviews/job.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deepwork/standard_jobs/deepwork_reviews/job.yml b/src/deepwork/standard_jobs/deepwork_reviews/job.yml index 577232ee..bb75a2cc 100644 --- a/src/deepwork/standard_jobs/deepwork_reviews/job.yml +++ b/src/deepwork/standard_jobs/deepwork_reviews/job.yml @@ -225,7 +225,7 @@ workflows: ### migrated_skill_backups - All backup copies of deleted skills in `.deepwork/tmp/migrated_skills/`. If no skills were migrated, this output should contain a single file `.deepwork/tmp/migrated_skills/NONE.md` stating that no review-like skills were found. + All backup copies of deleted skills in `.deepwork/tmp/migrated_skills/`. If no skills were migrated, this output should be an empty list. inputs: {} outputs: From abff8bbdec79cef9be9ebec6ba51d56c54c31013 Mon Sep 17 00:00:00 2001 From: Noah Horton Date: Wed, 8 Apr 2026 17:36:08 -0600 Subject: [PATCH 4/7] fix: restore reference to example review prompt files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous consolidation commit (1994f94) removed the duplicated rule-YAML blocks but also removed the pointers to plugins/claude/example_reviews/*.md, which contain the full detailed prompts that the inline YAML condenses. Those pointers are legitimately useful — they tell an agent writing condensed inline versions where to read for tone, intent, and checklist context. Restored as a "Reference material" section that clearly resolves the original inline-vs-file-based contradiction: always emit the inline form (target projects don't have the example files), and use the example files only as reference when deciding what the condensed inline text should say. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/deepwork/standard_jobs/deepwork_reviews/job.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/deepwork/standard_jobs/deepwork_reviews/job.yml b/src/deepwork/standard_jobs/deepwork_reviews/job.yml index bb75a2cc..89798800 100644 --- a/src/deepwork/standard_jobs/deepwork_reviews/job.yml +++ b/src/deepwork/standard_jobs/deepwork_reviews/job.yml @@ -49,6 +49,14 @@ workflows: Ensure the project's top-level `.deepreview` file contains the two built-in DeepWork review rules: `suggest_new_reviews` and `prompt_best_practices`. These are general-purpose rules that benefit any project using DeepWork Reviews. + ## Reference material + + The DeepWork plugin ships full, detailed versions of these two review prompts at: + - `plugins/claude/example_reviews/prompt_best_practices.md` + - `plugins/claude/example_reviews/suggest_new_reviews.md` + + Read these for context on each review's intent, checklist, and tone. The inline YAML rules in steps 2 and 3 below are condensed versions suitable for emitting into the target project's `.deepreview` file — always emit the inline form (not `instructions: { file: ... }`), since target projects don't have the example files. Use the example files only as reference when deciding what the condensed inline text should say. + ## Task ### 1. Check the existing `.deepreview` file From 2d567d8bf967a430837f12ca99209b87c077bce4 Mon Sep 17 00:00:00 2001 From: Noah Horton Date: Wed, 8 Apr 2026 17:43:41 -0600 Subject: [PATCH 5/7] fix: soften review-strategy-matches-criteria-scope wording The previous wording forbade any cross-file reasoning from `strategy: individual`, which would disqualify legitimate cases like a Python code review rule where the reviewer may need to consult other project files (imports, references, related code, conventions) while evaluating a single changed file. Reworded so the distinction is about whether the reviewer needs to look at the OTHER CHANGED FILES in the same review set, not whether they need to look at any other file at all. Individual is appropriate when focus is heavily on each file and the reviewer is unlikely to need to see the other changed files; matches_together is for criteria that reason about the set as a whole (coverage, deduplication, consistency across the set). Co-Authored-By: Claude Opus 4.6 (1M context) --- .../standard_schemas/job_yml/deepschema.yml | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/deepwork/standard_schemas/job_yml/deepschema.yml b/src/deepwork/standard_schemas/job_yml/deepschema.yml index a8d70f81..87fa870d 100644 --- a/src/deepwork/standard_schemas/job_yml/deepschema.yml +++ b/src/deepwork/standard_schemas/job_yml/deepschema.yml @@ -77,13 +77,17 @@ requirements: outside their scope. review-strategy-matches-criteria-scope: > - A review `strategy: individual` MUST only be used when every listed criterion can be - evaluated from a single file in isolation. Criteria that require cross-file comparison, - project-wide enumeration, set-level properties (coverage, minimality, deduplication), - or reasoning about the relationship between multiple output files MUST use - `strategy: matches_together` so the reviewer sees the full set at once. Using - `individual` with cross-cutting criteria forces each reviewer to redo the project-wide - scan N times and multiplies latency without improving review quality. + Choose the review `strategy` based on whether the criteria need to reason across the + other changed files in the same review set. Use `individual` when the focus is + heavily on each file itself and the reviewer is unlikely to need to look at the other + changed files in order to evaluate this one — the reviewer MAY still consult unchanged + project files for context (imports, references, related code, conventions), and that + does not disqualify `individual`. Use `matches_together` when the changed files are + likely to need to be looked at alongside each other during review — e.g., criteria + about coverage across the set, deduplication, consistency between the files, or any + property of the set as a whole. Picking `individual` for set-level criteria forces + each spawned reviewer to redo the full cross-file scan N times and multiplies latency + without improving review quality. review-no-redundant-context: > Review `instructions` MUST NOT repeat domain context already in `common_job_info` since From e4fca4e4c3b6fbbb7eb03bdaf51dfe16674a991f Mon Sep 17 00:00:00 2001 From: Noah Horton Date: Wed, 8 Apr 2026 17:44:44 -0600 Subject: [PATCH 6/7] fix: use RFC 2119 keywords in review-strategy-matches-criteria-scope Restructured the requirement to use SHOULD/MAY/MUST NOT explicitly, matching the RFC 2119 style of the other requirements in this schema. The substantive guidance is unchanged: individual is preferred when focus is on each file and the reviewer is unlikely to need to see the other changed files (consulting unchanged project files is explicitly allowed as MAY); matches_together is preferred when changed files need to be looked at alongside each other; and individual is a hard MUST NOT when any criterion is a property of the set of changed files as a whole. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../standard_schemas/job_yml/deepschema.yml | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/deepwork/standard_schemas/job_yml/deepschema.yml b/src/deepwork/standard_schemas/job_yml/deepschema.yml index 87fa870d..5a360a83 100644 --- a/src/deepwork/standard_schemas/job_yml/deepschema.yml +++ b/src/deepwork/standard_schemas/job_yml/deepschema.yml @@ -77,16 +77,18 @@ requirements: outside their scope. review-strategy-matches-criteria-scope: > - Choose the review `strategy` based on whether the criteria need to reason across the - other changed files in the same review set. Use `individual` when the focus is - heavily on each file itself and the reviewer is unlikely to need to look at the other - changed files in order to evaluate this one — the reviewer MAY still consult unchanged - project files for context (imports, references, related code, conventions), and that - does not disqualify `individual`. Use `matches_together` when the changed files are - likely to need to be looked at alongside each other during review — e.g., criteria - about coverage across the set, deduplication, consistency between the files, or any - property of the set as a whole. Picking `individual` for set-level criteria forces - each spawned reviewer to redo the full cross-file scan N times and multiplies latency + The review `strategy` SHOULD match whether the criteria need to reason across the + other changed files in the same review set. `strategy: individual` SHOULD be used + when the focus is on each file itself and the reviewer is unlikely to need to see + the other changed files in order to evaluate this one. A reviewer operating under + `individual` MAY consult unchanged project files for context (imports, references, + related code, conventions); such consultation does NOT require switching to + `matches_together`. `strategy: matches_together` SHOULD be used when the changed + files are likely to need to be looked at alongside each other during review. + `strategy: individual` MUST NOT be used when any criterion is a property of the + set of changed files as a whole (e.g., coverage across the set, deduplication of + the set, cross-file consistency, set-level minimality), because it forces each + spawned reviewer to redo the cross-file scan N times and multiplies latency without improving review quality. review-no-redundant-context: > From 4aeb85ab240059fed7a52f9c2c14391b84328a36 Mon Sep 17 00:00:00 2001 From: Noah Horton Date: Wed, 8 Apr 2026 17:50:28 -0600 Subject: [PATCH 7/7] feat: broaden deepschema matchers to **/deepschema.yml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The meta-DeepSchema governing deepschema files previously only matched .deepwork/schemas/*/deepschema.yml and **/.deepschema.*.yml, which missed the project's own standard schemas in src/deepwork/standard_schemas/ — they were exempt from the rules they define for users. Consolidated both hard-coded directory matchers into a single generic **/deepschema.yml pattern that: - Covers .deepwork/schemas/*/deepschema.yml (user-created named schemas) - Covers src/deepwork/standard_schemas/*/deepschema.yml (built-in schemas) - Covers any other location a user might put a deepschema.yml - Self-matches the meta-schema itself, which is fine — it conforms to its own requirements Co-Authored-By: Claude Opus 4.6 (1M context) --- src/deepwork/standard_schemas/deepschema/deepschema.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deepwork/standard_schemas/deepschema/deepschema.yml b/src/deepwork/standard_schemas/deepschema/deepschema.yml index 90e1db87..d83f711a 100644 --- a/src/deepwork/standard_schemas/deepschema/deepschema.yml +++ b/src/deepwork/standard_schemas/deepschema/deepschema.yml @@ -8,7 +8,7 @@ instructions: | matchers: - - ".deepwork/schemas/*/deepschema.yml" + - "**/deepschema.yml" - "**/.deepschema.*.yml" json_schema_path: "deepschema_schema.json"