rijuls-review

Rijul's Review

A four-hat adversarial review — each hat an independent agent — that ends in per-role confidence scores and ranked P0–P3 findings, reconciled by a fifth deduplicator agent into one master list. Its purpose is to catch what a single-perspective read misses and to force the reviewer to justify both low confidence (what's broken/risky) and high confidence (why it's actually solid, plus residual risk) — never a bare number. A clean pass here is necessary-not-sufficient: the four hats are the same model reading the same author's framing (correlated, not statistically independent), so an independent-model (Codex) pass remains the real merge gate — this review front-loads and cheapens that pass, it does not replace it.

Each hat's full method + output shape lives in its own references/role-*.md file (passed verbatim as that hat's sub-agent prompt); the shared severity scale, per-finding required fields, and verdict vocabulary live in references/finding-format.md. This SKILL.md is the orchestrator: the Hard Rules every hat applies, the hats table, and the five-agent execution mode. See Targeting & relationship to /code-review at the bottom for the artifact forms it accepts.

Hard rules (non-negotiable)

1. A review is static inspection only unless Rijul explicitly asks for verification. Do NOT run tests, lint, format, typecheck, knip, builds, quality gates, browser smoke, database probes, dependency installation, or any other executable verification during a review. Read the diff, source, callers, tests, configs, and existing check results; reason from that evidence. You may identify a missing or inadequate test/gate as a finding, but do not execute it and do not make review completion contingent on running it. Only cross this boundary when Rijul separately and explicitly asks to test, verify, reproduce, smoke, or run gates.
2. Read everything first. No file left unread. Before scoring, open every file the artifact names or depends on — the files to be modified, the files to be created (read their siblings/templates), the files referenced as "unchanged" reference points, the call sites, the test fixtures, and the immediate collaborators of each. If the artifact cites file:line, open that file and verify the line says what's claimed. Grep to confirm "no other importers" / "only consumer" / "already does X" claims rather than trusting them. List the files you read in the output so the user can audit coverage.
3. Four hats, four independent scores. Score from each role separately. Do not average into one number — each hat sees different risk.
4. Justify low AND high.
   • < 80%: you MUST explain why — the specific gaps, unverified assumptions, missing tests, edge cases, or risks that hold the score down, and what would raise it.
   • > 90%: you MUST also explain why — what concretely makes it strong (verified claims, covered edge cases, reversibility, test coverage) AND the residual risk that keeps it from 100%.
   • 80–90%: explain the main reservation in one or two lines.
5. Verify, don't trust. Every load-bearing claim in the artifact gets independently checked against the code. Call out any claim that turns out to be wrong, stale, or unverifiable. A confidently-worded claim that you could not verify is a confidence deduction, not a pass.
6. Pyramid principle. Lead each role with the score + the one-sentence verdict, then the justification.
7. Cost, not just correctness. A change that compiles, passes every test, and is logically correct can still be a defect if it is slow by construction — most often an N+1: a per-item query or await inside a loop over a collection. Lint / typecheck / unit tests are structurally blind to it (an N+1 is correct, just slow), and it hides behind small local/test data, surfacing only at real volume. For any read over a collection, the reviewer MUST ask: does the query count stay constant as the collection grows, or scale with it? If it scales — a per-row await, an unbatched lookup, an unbounded SELECT, a filtered column with no index — that is a finding to flag, even when nothing fails.
8. jsdom-green ≠ browser-correct for native DOM/event semantics. A change that passes a jsdom unit test can still be wrong in a real browser whenever it leans on native behavior jsdom models loosely: event propagation across the React root, focus, showModal() / ::backdrop / inert, native document/window listeners, layout/measurement, or UA pseudo-classes (:modal). jsdom false-passes these. So a load-bearing claim about such behavior is verified in a real browser (Chrome MCP), not accepted because a jsdom test is green — and a jsdom test asserting such behavior is itself suspect (a "spec-faithful fixture" trap; a green test proving a browser fact can be a false negative for bugs). Corollary bug class the Engineer hat MUST check whenever a native <dialog> modal is introduced: keyboard shortcuts still fire behind it. showModal() blocks POINTER input but NOT keydowns, and React's synthetic stopPropagation() does NOT reliably stop native document/window listeners (React 18 delegates at the root container) — so every shortcut that can be live while the modal is open must stand down (guard each handler on dialog[open], e.g. isModalDialogOpen(); a dialog-boundary stopPropagation is not enough). Audit BOTH source-sets: (1) top-level global shortcuts, AND (2) shortcuts on components that stay mounted behind the modal — most-missed: an editor left mounted while a delete confirm opens over it (its Cmd+S / Cmd+K leak). The only handler you leave unguarded is one registered by the modal itself — and verify a handler's consumer scope by grep before calling it "modal-scoped," don't assume. Ordering matters: for an app-owned chord that overrides a browser-reserved key (Cmd+O / Cmd+S / Cmd+K, "/"), call preventDefault() on the matched chord FIRST, THEN the isModalDialogOpen() early-return — a guard that returns before preventDefault suppresses the app action but lets the chord fall through to the browser's native action (Open File / Save Page / address-bar / quick-find) behind the modal. Second corollary — focus-return on close. When an overlay/dialog conditionally renders its OWN trigger (!open ? <trigger> : <overlay>), do NOT set the focus-restore target by reading document.activeElement inside the open-effect — by the time that effect runs the closed-state trigger has already unmounted, so it captures <body> and closing DROPS keyboard focus. The restore target must be captured synchronously (before the trigger unmounts) or be a stable ref to the remounting trigger; also latch "was ever open" so page-load doesn't steal focus, and suppress the refocus on a route-change close (navigation shouldn't yank focus back to the trigger). jsdom implements focus()/activeElement enough to pin the ref wiring in a test, but the visible focus ring + route-change-close behavior are a real-browser check. Third corollary — custom (non-native) modals are invisible to a dialog[open] guard. A shortcut guard that queries only dialog[open] (native showModal() dialogs) does NOT see a custom role="dialog"/aria-modal overlay (a portaled <div>), so every global shortcut (Cmd+O go-to-file, "/", Cmd+S, tree-nav) still fires BEHIND it. When a change introduces a custom modal overlay, it MUST opt into the guard (e.g. a data-app-modal marker the guard also queries) — AND any shortcut that is the overlay's OWN control (the chord that toggles/closes it) must use a native-only probe, or it stands down against its own overlay and can no longer close it.