Code Review, Properly

Tests catch what you thought to test. Review catches what you didn't think of at all. This chapter is about getting a review that's actually independent — from a model that doesn't share the author's blind spots.

The problem

An author reviewing its own work shares its own blind spots — that's as true for models as for people. And here's the part most setups miss: two models from the same family share training, so "ask the model to review itself" (or ask its sibling) doesn't catch the bias — it compounds it. If sonnet systematically forgets authorization checks, sonnet-as-reviewer systematically doesn't notice they're missing.

The hard way

> review your code from this session

  The code looks solid overall. A few suggestions:
  - Consider renaming $noteData to $attributes for clarity
  - The archive method could use a docblock
  - Nice use of syncWithoutDetaching!

Cosmetic nits, a renamed variable, and a compliment for its own fix from Chapter 7. The systematic mistakes — the ones baked into how this model writes Laravel code — sail through, because the reviewer makes the same ones.

The Elyra way: /review

/review runs an independent review of your changes and ends with a go/no-go verdict — not a vibes summary. It takes flags: --cross for a cross-vendor reviewer, --session to review what changed in this session, or a path to review specific files.

Cross-vendor review

This is the moat feature, and it's only possible because Elyra is provider-agnostic:

/review --cross

--cross picks the reviewer from a different vendor than the model that wrote the code. Sonnet wrote Freddy's tags feature, so the review goes to gpt-5.6 from your Chapter 3 lineup. (If only one provider is configured, it falls back to the current model — still independent context, just not independent training.) Different vendor, different training data, different habitual mistakes — which is exactly what a reviewer is for:

  Review by gpt-5.6 (cross-vendor)

  CRITICAL
  - NoteController@archive has no authorization check. Any
    request can archive any note. Fine while Freddy is
    single-user, but SPEC.md says "auth-ready" - this is a
    hole the day auth is shared. Add a policy now while the
    surface is small.

  WARNING
  - Browse page loads tags per note (N+1). Note::query() is
    missing ->with('tags'); at a few thousand notes this is
    a visible slowdown.

  SUGGESTION
  - Tag create-or-attach logic is inlined in the controller.
    Extract to a method on Note; the search feature will
    want it too.

  Verdict: NO-GO until CRITICAL is addressed.

Notice what happened: the authorization gap is precisely the kind of thing the author-model skipped every time, and precisely the kind of thing a same-family reviewer would have skipped too. The N+1 is a classic that pattern review misses because each file looks fine in isolation.

Hand the findings back:

> Fix the critical and the warning. Leave the suggestion for
  the search chapter.

The agent adds a NotePolicy, wires up ->with('tags'), and the tests from Chapter 7 confirm nothing regressed. Then:

/review --cross

  Review by gpt-5.6 (cross-vendor)

  Previous CRITICAL and WARNING resolved. No new findings.

  Verdict: GO.

Project health with doctor

Review looks at changes; something should look at the whole project on a schedule. Install it:

/ext

Select @elyracode/doctor, then /reload. Now:

/doctor

  Security audit .... 1 advisory (dev dependency, low)
  Outdated deps ..... 3 minor, 1 major (vite)
  Code debt ......... 2 TODOs older than 30 days

And for the safe subset, let it act:

/doctor --heal

--heal auto-fixes what's safe to fix (minor dependency bumps, the low-risk advisory) and reports what it left alone. The vite major stays your decision.

What you learned

• Same-family review compounds blind spots. A reviewer that shares the author's training makes the author's mistakes look normal.
• /review --cross breaks the bias. A different vendor's model has different habitual mistakes — provider-agnostic Elyra is what makes this possible at all.
• Review before every merge. A go/no-go verdict is a gate, not a suggestion — the archive endpoint hole proved it.
• Doctor is the physical, review is the surgeon. /doctor for recurring whole-project health, --heal for the safe fixes, /review for changes.