[tsapeta]

Why

expo/expo's reviewer config wants the classes its deep-code-review skill uses (critical, design, suggestion, nit), but the engine only has critical, warning and suggestion. So expo/expo's general reviewer reports design problems as warning with category quality, and nits as suggestion with a Nit: title prefix. The skill has no level for a non-critical bug, so warning stays for those, and the scale is now critical, warning, design, suggestion, nit.

How

SEVERITIES and SEVERITY_RANK in src/core/schema.ts gain design (rank 2) and nit (rank 4), and the model-facing JSON schema follows from the zod enum, so providers accept the new values. applyReviewPolicy drops nit together with suggestion when includeSuggestions is false, while design is never dropped by severity, like warning. Decision logic, verification, the inline cap and the dismissal floors key only on critical or on rank, so they need no change.

Comment headings now use the skill's icons (🚨 Critical, ⚠️ Warning, 🏗️ Design, 💡 Suggestion, 🐧 Nit), so the Critical, Warning and Suggestion icons change from 🔴 🟡 🔵.

The templates ask agents for critical, warning and design findings, and both shared.md and coordinator.md let a design finding name a concrete cost in place of a failure path, since a design problem rarely has one. Suggestions and nits stay off. Only ecr init reads the templates, so an existing adopter that doesn't change its prompts only sees the new heading icons.

Finding ids don't include severity (fingerprintFinding hashes file, category and evidence), so moving a finding from warning to design keeps its id and existing dismissals still apply. Downgrading is the one catch: /dismiss re-renders the comment from the stored state, and an engine without this change has no design or nit group in groupBySeverity, so after a review that posted those levels it would break /dismiss on that PR. I got this from reading the code, not from running it.

Test Plan

bun test (798 pass), bun run typecheck, bun run lint, bun run fmt:check and bun run llp:check all pass. The new tests fail before the change and pass after: the reviewer parser accepts design and nit, applyReviewPolicy drops nits with suggestions and keeps design, and the comment renders the groups in rank order with the new headings. One existing assertion changed for the new Warning icon. I haven't run it against a live PR yet.

[github-actions]

🤖 AI code review

Decision: Ready for human review

Overall PR risk: Medium. This change modifies existing behavior by adding design and nit severities to the SEVERITIES enum and SEVERITY_RANK, which affects all review output paths: filtering, sorting, rendering, markdown headings, and template prompts. The core decision logic remains unchanged—blocking still keys only on critical. Material concerns exist: older engines without this change would crash when re-rendering stored state containing design or nit findings, heading emoji changes will alter existing comment text on re-render, and adopters using copied templates will not receive design findings. Rollback is straightforward, though any state already written with these severities would require older engines to tolerate them.

🔗 Review setup:

• This PR changes code the reviewer prompts cite (src, src/tests, src/core/prompts.ts, src/core/render.ts, templates, and 1 more). Check that the guidance quoting it is still correct.

No findings.

---

This review is advisory — it never blocks a merge and never auto-approves.

[brentvatne]

Review notes from a multi-model review (Codex, Grok and Claude, each finding then re-checked against the code). Fixes are in f099a82, pushed to this branch.

1. Engines older than this PR crash on design and nit (holds). parseReviewState doesn't schema-validate stored findings, so an old engine re-rendering this PR's comment state throws in groupBySeverity (groups[finding.severity].push). That hits /dismiss//undismiss and a partial --scopes run. A full ecr ci run on an old engine doesn't crash, since it only reads dismissed, feedback and pins.

• The biggest exposure was templates/dismiss.yml defaulting to ^0.2.3, which on 0.x resolves to 0.2.3 itself. That's fixed separately in fix: default the /dismiss workflow to the latest engine #93.
• Fix here: knownSeverity() renders a severity this engine doesn't know as a warning and keeps the stored value, so the next new severity can't do this again. It only protects engines from this version on.

2. The rubric made design findings hard to report (holds). shared.md defines High and Medium confidence by "the failure or exploit", so a design finding rates Low confidence and the coordinator drops it. The verifier's "the described failure/exploit cannot actually happen" partly applies too, for findings it sees. Fix: a design finding rates by how directly the code shows its cost and how large that cost is (shared.md, correctness.md, buildVerifierSystem).

3. Tests. Added: an unknown stored severity re-renders (single, aggregate and inline), a new terminal.test.ts for the DESIGN and NIT labels, colors and order, and the verifier wording. Each test fails with its fix reverted.

Checks: typecheck, bun test, lint, format and llp:check pass, also with #87 and its fixes on top.