Lesson 2: Writing a review lens

Last lesson ended with a finding that never arrived. The 404 in the new endpoint sends a bare string where this codebase requires apiError(), and no default lens said a word about it.

That isn't a bug, and tuning wouldn't fix it. The rule exists in one repository and in the heads of the people who work on it, and a reviewer shipped to everyone has no way to know it. This lesson writes it down.

What you'll build

• A lens encoding one convention your codebase actually keeps.
• A review that runs it beside the defaults and finds what they missed.
• A test of the lens in both directions: it fires on the bad case and goes quiet on the good one.

A lens is a skill

The four default lenses aren't settings inside a product you can't open. Each one is a skill: a SKILL.md with a name, a description, and a body of review instructions, published in the tessl/code-review plugin.

The design follows from that. If review criteria are a file, you can read them, change them, version them, and point them at a different repository next week. The sources are published so you can fork one and tune it instead of starting from nothing.

You name a lens three ways: a local path to a SKILL.md or its directory, the name of a skill you've installed, or a registry reference in the form workspace/plugin[@version]#skill-name.

tessl code review \
  --skill ./review-lenses/review-api-error-shape/SKILL.md \
  --skill tessl/code-review@0.1.0#review-security-and-privacy

Order is preserved, and a review supports at most eight lenses. Pin the version on registry references. An unpinned lens means your review can change because someone else published, which is a strange way to find out your standards moved.

--skill replaces, it doesn't add

This is the one that catches people. Passing --skill states the complete set of lenses you want. It does not append to the profile's defaults; it replaces them.

So the command above runs exactly two lenses. Correctness, maintainability, and scale are gone, because you didn't ask for them. Anything from the defaults you still want has to be in your list.

That's deliberate, because a review you can't fully account for isn't one you can trust in a gate. But it means a first custom run usually finds less than the run before it, which reads as a regression until you know why.

Step-by-step

1. Write the lens

The rule is already in the codebase, just not written down: every non-2xx response from a handler under src/api/ goes through apiError(). orders.ts follows it. The new endpoint doesn't.

Create review-lenses/review-api-error-shape/SKILL.md:

---
name: review-api-error-shape
description: Review changed HTTP handlers for error responses that bypass the shared apiError() helper, which every client depends on for a consistent error shape.
---

# API error shape

This service answers every error through one helper, `apiError(res, status, code)`
in `src/api/errors.ts`. Clients parse `{ error: { code } }` and nothing else, so a
handler that sends its own error body breaks them.

## What to flag

A changed handler that returns a non-2xx response without going through
`apiError()`. That includes `res.status(...).send(...)`, `res.status(...).json(...)`
with an ad-hoc body, and a thrown error that reaches the client unshaped.

Correct:

    if (!order) return apiError(res, 404, 'order_not_found');

Incorrect:

    if (!order) return res.status(404).send('order not found');

## What not to flag

- Success responses. `res.json(order)` on the happy path is fine.
- Files outside `src/api/`. This rule is about HTTP handlers.
- Error handling inside the helper itself.

Report the line and name the code the handler should have used. If every changed
handler uses the helper, say so in one line rather than reaching for something else.

Four things make that work, and none of them is length.

One concern. This lens knows about error shape and nothing else. A lens that also has opinions on naming and logging will produce findings that collide with the maintainability lens.

The good case beside the bad case. A rule stated in the abstract gets applied in the abstract. Two three-line examples do more than a paragraph of description.

An explicit quiet list. "What not to flag" is the difference between a lens that earns its place and one people mute. Say what's out of scope, or it will find something there eventually.

A description that says when it applies. That field is how the lens gets selected and understood. "Reviews code" tells nobody anything.

Prefer to be walked through it? tessl/code-review-lens-creator ships inside the CLI, so you can ask your agent to draft a lens with nothing to install. It'll ask what the concern is and check the lens against changes that should and shouldn't trip it. Write this one by hand first, because a lens you've read in full is one you can debug later.

2. Run it beside the defaults

Same branch, same command, one addition:

tessl code review \
  --skill ./review-lenses/review-api-error-shape/SKILL.md \
  --skill tessl/code-review@0.1.0#review-correctness-and-data-integrity \
  --skill tessl/code-review@0.1.0#review-security-and-privacy

Three lenses now: yours, plus the two defaults that earned their keep last lesson.

Expect a third finding this time: the 404 sending a bare string where the codebase requires apiError(). Nothing about that line is wrong in general. It's wrong here, and it took a rule you wrote for a reviewer to see it.

Notice what you dropped, too. Maintainability and scale didn't run, because you didn't ask for them. Get in the habit of reading your --skill list as the whole review, because that's what it is. If a finding you expected doesn't appear, check the list before you suspect the lens.

Notice what you pinned. Both registry references carry @0.1.0. Drop the version and the lens resolves to whatever is current, so tomorrow's review can differ from today's for reasons that have nothing to do with your code. Your own lens is pinned differently: it's a path in the repository, so git versions it along with everything else. That's a real advantage of keeping review criteria in the repo they judge.

3. Try the negative case

Fix the error shape on the branch by swapping the bare string for apiError(res, 404, 'invoice_not_found'), then run the same command again.

The finding should go. A lens that fires on the bad case and stays quiet on the good one is a lens you can put in front of other people. One that keeps complaining after the fix is one they'll route around.

Put the defect back before moving on. Lesson 3 routes this lens by path, and it needs something to find.

Is this lens worth running?

Run these checks before you put any lens in front of colleagues:

• It fires on the bad case. Point it at a change that breaks the rule and confirm the finding appears, on the right line.
• It stays quiet on the good case. Fix the violation, run it again, and watch the finding go. A lens that keeps complaining after the fix is worse than no lens, because it teaches people the reviewer is wrong.
• It names one concern. If you can't state what it checks in a sentence, it's two lenses. Split it before its findings start colliding with a neighbor's.
• It says what it won't flag. An explicit quiet list is what keeps a lens from drifting into taste. Without one it will eventually find something outside its remit and lose the room.
• Its description says when it applies. That field is how the lens gets selected and how a reader knows what it's for.

Then count. Eight is the maximum, and the useful number is usually lower. Every extra lens is more findings competing for the same attention, and the point is the important issue getting read, not the comment count going up.

If a lens fails the quiet-on-the-good-case check, the cause is nearly always a "what to flag" section describing a shape rather than a rule. Give it the correct code beside the incorrect code and rerun.

Verify

• The lens file exists at review-lenses/review-api-error-shape/SKILL.md with name and description frontmatter.
• A review naming all three lenses reports the error-shape finding on src/api/invoices.ts.
• Fixing the error shape makes that finding disappear; putting it back brings it up again.
• You can explain why dropping --skill changes which lenses run.

What you keep

The lens is a file in your repository. It goes into commits, shows up in diffs, and can be pointed at another project tomorrow, which is the argument for keeping review criteria in a repo rather than a settings screen.

More to the point, one of your team's unwritten rules is now written. The reviewer holds it whether or not the person who remembers it is looking at the pull request.

Right now you're naming three lenses on the command line every time, which works for one person and falls apart for a team. Lesson 3 moves that into the repository, and adds something the command line can't do: sending each lens only to the paths its rule governs.