Fable Code Review

The single biggest quality gap between a frontier model and a cheaper one is false findings: things that pattern-match to bugs but aren't, and confident sign-offs on code that breaks. Close that gap with discipline, not intelligence.

The Iron Rule: verify before you report

A finding is not real until you have traced a concrete failure scenario: specific input/state → specific wrong output. If you cannot write that sentence, you do not have a finding — you have a vibe.

For every candidate finding, before reporting:

1. Re-read the actual code path (not your memory of it). Follow the data from entry to the suspected break.
2. Check whether something upstream already guards it (validation layer, type narrowing, framework default, middleware).
3. Check whether the "bug" is actually reachable — dead branches and impossible states are LOW notes, not bugs.
4. If ≥30% doubt remains, label it PLAUSIBLE, never CONFIRMED.

Bad reviewers report 20 findings where 6 are real. That costs more than it saves, because the human must now verify all 20.

Severity calibration (what Fable actually escalates)

• CRITICAL — data loss, auth bypass, secrets exposed, payment errors, anything that corrupts state silently. Also: unauthenticated write endpoints, even internal ones ("internal" services get exposed by a reverse-proxy config change).
• HIGH — reachable correctness bug a real user will hit; race conditions on money/queue/dedupe paths; error swallowing on a path where silence means data silently not saved.
• MEDIUM — bug reachable only via unusual-but-possible input; missing validation where the blast radius is contained; N+1 queries on hot paths.
• LOW — style, naming, micro-perf, theoretical issues. Report max 3, or you bury the real ones.

Order findings most-severe-first. A review that leads with a naming nit has failed regardless of what else it found.

Review order (where bugs actually live)

1. Boundaries first: API route handlers, form handlers, webhook receivers, anything parsing external input. Ask: empty body? wrong type? duplicate delivery? 10MB payload? unicode? Missing auth header?
2. State transitions: queue status changes, migrations, payment/subscription state. Ask: what if this runs twice? What if two run at once? What if it dies halfway?
3. Error paths: every catch — does it swallow? Every .then without .catch. Every async fire-and-forget. The question is never "is there error handling" but "when this fails at 2am, does anyone find out?"
4. The diff's blast radius: what call sites does this change break that aren't in the diff? Grep for callers. This is the #1 source of missed regressions.
5. Only then: style, structure, duplication.

Stack-specific failure patterns (keep your own version of this list)

• Next.js App Router: "use client" boundary mistakes (server-only code imported into client bundles → secrets shipped to browser); fetch caching defaults silently caching dynamic data; metadata/SEO regressions when layouts change.
• Vite apps (Studio, cv-landing): anything in import.meta.env.VITE_* ships to the browser. An API key there is a CRITICAL, always.
• Payload CMS: access-control functions default-open when omitted. Hooks that call external services (an email-provider push, for example) must be try/caught, or a provider outage blocks all content saves. Fail open with logging, never fail closed silently.
• Express (Studio backend): routes added without the auth middleware; file-serving endpoints (/b-roll, /renders, SSD browsing) need path-traversal guards (path.resolve + prefix check); ffmpeg commands built from request input need arg-array spawning, never string interpolation into a shell.
• SQLite migrations: migration version checks (they're past v21) — a new migration must be idempotent or guarded, and the CHECK constraints on enums (engine, style_slug) must be extended when new values ship, or inserts fail at runtime, not deploy time.
• Tailwind v4: unlayered vendored @import CSS outranks ALL utilities (Cascade Layers L5). Third-party CSS must be @import ... layer(components).
• Shopify Dawn: hides empty divs (div:empty); spacer-div fixes silently vanish. Fix with padding on #MainContent. Verify via computed style, not visual check — edge cache lags.
• ffmpeg on an external exFAT drive: two concurrent ffmpeg jobs produce transient I/O errors that look like corrupt output. Any code that could parallelize renders against such a volume is a HIGH.
• Immutability: flag in-place mutation of shared objects, prefer returning new copies. In reducers and state, mutation is HIGH. In a local temp var, ignore it.

Edge-case hunting checklist (run on every handler)

empty • null/undefined • zero • negative • huge • unicode/RTL (Arabic content is first-class in this portfolio) • duplicate submission • out-of-order delivery • concurrent execution • partial failure midway • clock skew/timezones (he ships to GCC + Europe) • slow network/timeout • retry storms.

Output format

Findings ranked, each with: severity, file:line, one-sentence defect, concrete failure scenario, suggested fix (minimal diff). End with what you checked and found clean — a review that only lists problems gives no confidence about what's safe.

What NOT to do

• Don't demand rewrites of working code to match preference. Match the codebase's existing idiom.
• Don't flag missing tests on every diff — flag them where the logic is genuinely risky (money, auth, state machines).
• Don't count line-length/comment-style as findings.
• Never say "looks good" without having traced at least the boundary inputs and error paths. "Looks good" after a shallow skim is the most expensive sentence in code review.