williamthorsen · williamthorsen · Mar 3, 2026 · Mar 2, 2026 · Mar 2, 2026 · Mar 2, 2026
diff --git a/packages/agents/content/skills/review-criteria/SKILL.md b/packages/agents/content/skills/review-criteria/SKILL.md
@@ -1,6 +1,6 @@
 ---
 name: review-criteria
-description: Evaluation criteria for code review — what to examine and what to skip
+description: Evaluation criteria and finding classification for code review
 user-invocable: false
 ---
 
@@ -33,5 +33,69 @@ Evaluation criteria for code review. Apply proportionally — match depth to ris
 
 ## Distinguish author work from legacy
 
-- Focus findings (F/W/T) only on code authored in the current change
-- Frame observations in pre-existing code as Legacy (L), not defects
+Focus findings (F/W/T) only on code authored in the current change — observations in pre-existing code belong in Legacy (L).
+
+## Finding scheme (F/W/T/R/S/L)
+
+Used by review-producing skills and agents for structured code review findings. Also documented in [artifact conventions](../_data/artifact-conventions.md#finding-scheme-fwtrsl) for human reference. Every finding (F/W/T/R/S) must include a concrete action the author can take. Non-actionable observations belong in prose sections (e.g., Technical Assessment), not in numbered findings.
+
+| ID     | Category       | Severity       | Merge-blocking?                                                    |
+| ------ | -------------- | -------------- | ------------------------------------------------------------------ |
+| `F{n}` | FIXME          | critical       | Yes — must fix before merge                                        |
+| `W{n}` | Warning        | warning        | May block — questionable decisions requiring justification         |
+| `T{n}` | TODO           | todo           | No — should fix, can wait for next PR                              |
+| `R{n}` | Recommendation | recommendation | No — advisable but discretionary                                   |
+| `S{n}` | Suggestion     | suggestion     | No — optional improvement                                          |
+| `L{n}` | Legacy         | legacy         | No — observation in pre-existing code, not authored in this branch |
+
+### Category criteria
+
+**FIXME (F)** — must fix before merge:
+
+- Bugs: incorrect logic, unhandled error paths, data loss risks
+- Security: injection, auth bypass, exposed secrets
+- Contract violations: breaking API changes, type unsafety
+- Test failures: tests that don't pass or don't test what they claim
+
+**Warning (W)** — questionable, may block merge:
+
+- Missing edge case handling that could cause runtime errors
+- Convention violations that affect maintainability
+- Decisions that seem wrong but may be intentional (require justification)
+
+**TODO (T)** — should fix, not in this PR:
+
+- Missing or inadequate tests for new functionality
+- Performance issues with measurable impact
+- Incomplete error handling that won't cause immediate failures
+
+**Recommendation (R)** — advisable but discretionary:
+
+- Better patterns available in the codebase
+- Opportunities to reduce complexity
+- Architectural improvements worth considering
+
+**Suggestion (S)** — optional improvement:
+
+- Better naming or code organization
+- Additional test cases for edge cases
+- Documentation improvements
+
+**Legacy (L)** — pre-existing code observation:
+
+- Issues in code not authored in this branch
+- Frame as future opportunities, not current defects
+- Never count against the review score
+
+### Overall criticality mapping
+
+| Findings present                   | Criticality | Meaning                                      |
+| ---------------------------------- | ----------- | -------------------------------------------- |
+| None, or only S/R/L                | `none`      | Ready to merge                               |
+| W and/or T, but no F               | `low`       | Acceptable to merge with optional follow-ups |
+| 1–2 F (straightforward), or many W | `medium`    | Needs fixes but approach is sound            |
+| Multiple F, or structural issues   | `high`      | Needs significant rework                     |
+
+### Re-review severity escalation
+
+`S → R → T → W → F`. L findings are never escalated.
diff --git a/packages/agents/content/subagents/aspect-code-reviewer.md b/packages/agents/content/subagents/aspect-code-reviewer.md
@@ -7,6 +7,7 @@ skills:
   - anti-patterns
   - common-mistakes
   - get-default-branch
+  - review-criteria
 ---
 
 # Code aspect reviewer
@@ -60,45 +61,19 @@ Do NOT flag:
 
 Each finding must include:
 
-- **ID**: Sequential within category:
-
-| ID     | Category       | Severity       | Merge-blocking?                                                    |
-| ------ | -------------- | -------------- | ------------------------------------------------------------------ |
-| `F{n}` | FIXME          | critical       | Yes — must fix before merge                                        |
-| `W{n}` | Warning        | warning        | May block — questionable decisions requiring justification         |
-| `T{n}` | TODO           | todo           | No — should fix, can wait for next PR                              |
-| `R{n}` | Recommendation | recommendation | No — advisable but discretionary                                   |
-| `S{n}` | Suggestion     | suggestion     | No — optional improvement                                          |
-| `L{n}` | Legacy         | legacy         | No — observation in pre-existing code, not authored in this branch |
-
+- **ID**: sequential within category (F/W/T/R/S/L — see `review-criteria` skill for the full finding scheme)
 - **Location**: `file/path.ts:42` (file and line number)
 - **Description**: what the issue is
 - **Recommendation**: what to do about it
 
-## Overall criticality levels
-
-Classify the overall review into exactly one level:
-
-### `none`
-
-- No findings, or only S/R/L findings
-- Code complies with project guidelines and contains no bugs
-
-### `low`
-
-- W and/or T findings, but no F findings
-- Minor guideline deviations or potential issues
-
-### `medium`
-
-- 1-2 F findings that are straightforward to fix
-- OR many W findings that collectively indicate a quality concern
+## Criticality classification
 
-### `high`
+Classify the overall review into exactly one level (none/low/medium/high) per the `review-criteria` skill. Domain context for this reviewer:
 
-- Multiple F findings
-- OR F findings that require significant rework
-- OR systematic guideline violations indicating the developer missed key conventions
+- `none`: Code complies with project guidelines and contains no bugs
+- `low`: Minor guideline deviations or potential issues
+- `medium`: 1-2 straightforward guideline/bug findings, or many W findings collectively indicating a quality concern
+- `high`: Systematic guideline violations indicating the developer missed key conventions
 
 ## Output format
 

diff --git a/packages/agents/content/subagents/aspect-silent-failure-reviewer.md b/packages/agents/content/subagents/aspect-silent-failure-reviewer.md
@@ -6,6 +6,7 @@ maxTurns: 15
 skills:
   - anti-patterns
   - get-default-branch
+  - review-criteria
 ---
 
 # Silent-failure aspect reviewer
@@ -57,45 +58,19 @@ Do NOT flag:
 
 Each finding must include:
 
-- **ID**: Sequential within category:
-
-| ID     | Category       | Severity       | Merge-blocking?                                                    |
-| ------ | -------------- | -------------- | ------------------------------------------------------------------ |
-| `F{n}` | FIXME          | critical       | Yes — must fix before merge                                        |
-| `W{n}` | Warning        | warning        | May block — questionable decisions requiring justification         |
-| `T{n}` | TODO           | todo           | No — should fix, can wait for next PR                              |
-| `R{n}` | Recommendation | recommendation | No — advisable but discretionary                                   |
-| `S{n}` | Suggestion     | suggestion     | No — optional improvement                                          |
-| `L{n}` | Legacy         | legacy         | No — observation in pre-existing code, not authored in this branch |
-
+- **ID**: sequential within category (F/W/T/R/S/L — see `review-criteria` skill for the full finding scheme)
 - **Location**: `file/path.ts:42` (file and line number)
 - **Description**: what the issue is
 - **Recommendation**: what to do about it
 
-## Overall criticality levels
-
-Classify the overall review into exactly one level:
-
-### `none`
-
-- No error-handling code in the change, or no findings
-- Only S/R/L findings
-
-### `low`
-
-- W and/or T findings, but no F findings
-- Error handling is present but has minor issues
-
-### `medium`
-
-- 1-2 F findings that are straightforward to fix
-- OR many W findings that collectively indicate a quality concern
+## Criticality classification
 
-### `high`
+Classify the overall review into exactly one level (none/low/medium/high) per the `review-criteria` skill. Domain context for this reviewer:
 
-- Multiple F findings
-- OR F findings that require significant rework
-- OR systematic error suppression that indicates a fundamental approach problem
+- `none`: No error-handling code in the change, or no findings
+- `low`: Error handling is present but has minor issues
+- `medium`: 1-2 F findings that are straightforward to fix, or many W findings that collectively indicate a quality concern
+- `high`: Systematic error suppression that indicates a fundamental approach problem
 
 ## Output format
 

diff --git a/packages/agents/content/subagents/aspect-test-reviewer.md b/packages/agents/content/subagents/aspect-test-reviewer.md
@@ -7,6 +7,7 @@ skills:
   - anti-patterns
   - common-mistakes
   - get-default-branch
+  - review-criteria
 ---
 
 # Test aspect reviewer
@@ -60,45 +61,19 @@ Do NOT flag:
 
 Each finding must include:
 
-- **ID**: Sequential within category:
-
-| ID     | Category       | Severity       | Merge-blocking?                                                    |
-| ------ | -------------- | -------------- | ------------------------------------------------------------------ |
-| `F{n}` | FIXME          | critical       | Yes — must fix before merge                                        |
-| `W{n}` | Warning        | warning        | May block — questionable decisions requiring justification         |
-| `T{n}` | TODO           | todo           | No — should fix, can wait for next PR                              |
-| `R{n}` | Recommendation | recommendation | No — advisable but discretionary                                   |
-| `S{n}` | Suggestion     | suggestion     | No — optional improvement                                          |
-| `L{n}` | Legacy         | legacy         | No — observation in pre-existing code, not authored in this branch |
-
+- **ID**: sequential within category (F/W/T/R/S/L — see `review-criteria` skill for the full finding scheme)
 - **Location**: `file/path.ts:42` (file and line number)
 - **Description**: what the issue is
 - **Recommendation**: what to do about it
 
-## Overall criticality levels
-
-Classify the overall review into exactly one level:
-
-### `none`
-
-- No source files requiring test coverage in the change, or no findings
-- Only S/R/L findings
-
-### `low`
-
-- W and/or T findings, but no F findings
-- Tests exist but have minor gaps
-
-### `medium`
-
-- 1-2 F findings that are straightforward to fix
-- OR many W findings that collectively indicate inadequate coverage
+## Criticality classification
 
-### `high`
+Classify the overall review into exactly one level (none/low/medium/high) per the `review-criteria` skill. Domain context for this reviewer:
 
-- Multiple F findings
-- OR new critical functionality with no tests at all
-- OR tests that fundamentally don't test what they claim
+- `none`: No source files requiring test coverage in the change, or no findings
+- `low`: Tests exist but have minor gaps
+- `medium`: 1-2 F findings that are straightforward to fix, or many W findings collectively indicating inadequate coverage
+- `high`: New critical functionality with no tests, or tests that fundamentally don't test what they claim
 
 ## Output format
 

diff --git a/packages/agents/content/subagents/orchestrated-reviewer.md b/packages/agents/content/subagents/orchestrated-reviewer.md
@@ -40,84 +40,14 @@ You will receive:
 
 Each finding must include:
 
-- **ID**: Sequential within category:
-
-| ID     | Category       | Severity       | Merge-blocking?                                                    |
-| ------ | -------------- | -------------- | ------------------------------------------------------------------ |
-| `F{n}` | FIXME          | critical       | Yes — must fix before merge                                        |
-| `W{n}` | Warning        | warning        | May block — questionable decisions requiring justification         |
-| `T{n}` | TODO           | todo           | No — should fix, can wait for next PR                              |
-| `R{n}` | Recommendation | recommendation | No — advisable but discretionary                                   |
-| `S{n}` | Suggestion     | suggestion     | No — optional improvement                                          |
-| `L{n}` | Legacy         | legacy         | No — observation in pre-existing code, not authored in this branch |
-
+- **ID**: sequential within category (F/W/T/R/S/L — see `review-criteria` skill for the full finding scheme)
 - **Location**: `file/path.ts:42` (file and line number)
 - **Description**: what the issue is
 - **Recommendation**: what to do about it
 
-### Category criteria
-
-**FIXME (F)** — must fix before merge:
-
-- Bugs: incorrect logic, unhandled error paths, data loss risks
-- Security: injection, auth bypass, exposed secrets
-- Contract violations: breaking API changes, type unsafety
-- Test failures: tests that don't pass or don't test what they claim
-
-**Warning (W)** — questionable, may block merge:
-
-- Missing edge case handling that could cause runtime errors
-- Convention violations that affect maintainability
-- Decisions that seem wrong but may be intentional (require justification)
-
-**TODO (T)** — should fix, not in this PR:
-
-- Missing or inadequate tests for new functionality
-- Performance issues with measurable impact
-- Incomplete error handling that won't cause immediate failures
-
-**Recommendation (R)** — advisable but discretionary:
-
-- Better patterns available in the codebase
-- Opportunities to reduce complexity
-- Architectural improvements worth considering
-
-**Suggestion (S)** — optional improvement:
-
-- Better naming or code organization
-- Additional test cases for edge cases
-- Documentation improvements
-
-**Legacy (L)** — pre-existing code observation:
-
-- Issues in code not authored in this branch
-- Frame as future opportunities, not current defects
-- Never count against the review score
-
-## Overall criticality levels
-
-Classify the overall review into exactly one level:
-
-### `none`
-
-- No findings, or only S/R/L findings
-- Code is ready to merge
-
-### `low`
-
-- W and/or T findings, but no F findings
-- Code is acceptable to merge with optional follow-ups
-
-### `medium`
-
-- 1–2 F findings that are straightforward to fix
-- OR many W findings that collectively indicate a quality concern
-
-### `high`
+## Criticality classification
 
-- Multiple F findings
-- OR F findings that require significant rework
-- OR structural issues that indicate the approach needs rethinking
+Classify the overall review into exactly one level (none/low/medium/high) per the `review-criteria` skill.
 
 ## Output format