The comment that lands

You once left a comment that was right. The query really did skip tenantDb, you saw it, and you flagged it. Three days later the thread has six replies, the author has gotten defensive, and the fix still isn’t in. Nothing was wrong with your judgment, since the last lesson made your judgment good. What was wrong was the shape of the comment.

That’s the gap this lesson closes. The last lesson taught where your eyes go and what holds the merge. This one teaches what you actually write once you’ve decided to leave the comment: the small, repeatable shape an author parses without translation and acts on in seconds, instead of one that burns thirty minutes of thread. You’ll walk away with two things: a four-part anatomy every comment fits into, and a five-label vocabulary that tells the author, at a glance, which comments hold the merge. Then we flip the chair and look at the side you’ll spend far more of your career on, receiving the review.

A comment is a message with a severity header

Start with the picture from the author’s seat, because that’s whose time the comment is spending. You finish a review and drop, say, eight comments on a PR. The author opens it, and their first job isn’t to fix anything. It’s to triage. Which of these eight hold the merge? Which are polish they can take or leave? Which are questions waiting on an answer? Until they’ve sorted that, they can’t plan their next twenty minutes.

You already triage a log without reading it: your eye lands on ERROR versus DEBUG and you know how much to care before you read the message. A review comment should work the same way. The author should be able to triage it from its first token, before reading the body. That first token is the severity label, and it’s the idea the rest of this lesson orbits. The anatomy you’re about to learn exists to make triage cheap, and the labels are the triage vocabulary. The hardest decision in the lesson, whether something blocks the merge, is a decision you make so the author doesn’t have to guess.

blocking: this query goes straight to the table, so it isn't scoped to the org and can read

suggestion: you map then filter in two passes here; a single reduce would do it in one and

question: is there a reason this internal-only path skips the wrapper? if it's reachable from a

praise: good call keeping the helper on the internal path even where it was tempting to skip,

The author triages on the pill, before reading a word of the body.

The diagram above captures the core of the lesson: you read the pill first. Everything that follows is in service of making that pill, and the sentence behind it, worth reading.

The four parts of a comment

This is the first of the lesson’s two deliverables. A comment that lands has four parts, and they go in this order:

1. The severity label. Does this block the merge? It’s one token, and we’ll fill in the full set of labels in the next section.
2. The observation. The line or behavior, named concretely. Not “this is wrong” but “this query goes straight to db.select().” You’re pointing at a specific thing, so the author’s eyes go to the right place.
3. The reason. The principle, pattern, or failure mode it trips, with a link to the lesson that owns it when there is one. This is where the map from the last lesson pays off: the reason is almost always one map-row away. “This leaks across orgs” is the reason; “see pattern #1” is the link.
4. The action or question. Propose the fix, or ask the question the author answers. Then stop. A comment with no ask leaves the author guessing what you want.

Before you picture four bullet points, here’s the catch that makes this practical: four parts, but usually one or two sentences total. The parts compress. They aren’t four sentences, they’re four slots, and a good comment fits all four into a sentence or two. The discipline isn’t that each part is verbose; it’s that each part is present. Here’s how they collapse into a single real comment.

blocking: this query goes straight to db.select(), so it
isn't scoped to the org — it'll read rows across tenants.
Route it through tenantDb(orgId) (pattern #1).

The severity label. This is the header the author triages on, before reading a word of the body. The full label set is the next section.

blocking: this query goes straight to db.select(), so it
isn't scoped to the org — it'll read rows across tenants.
Route it through tenantDb(orgId) (pattern #1).

The observation. The concrete thing, named, so the author’s eyes land on the exact line instead of hunting for what you mean.

blocking: this query goes straight to db.select(), so it
isn't scoped to the org — it'll read rows across tenants.
Route it through tenantDb(orgId) (pattern #1).

The reason and the link. The failure mode plus the map-row that owns it, so the author can read the full context from pattern #1.

blocking: this query goes straight to db.select(), so it
isn't scoped to the org — it'll read rows across tenants.
Route it through tenantDb(orgId) (pattern #1).

The action. Propose the fix, then stop. The whole thing is two sentences, and all four slots are filled.

← Previous 1 / 1 Next →

Two sentences, four slots. Notice what the comment does not do: it doesn’t apologize, doesn’t pile on three paragraphs of context, doesn’t ask a rhetorical question. Once you’ve seen the slots, you start reading them in every comment, and writing them without thinking. Here are three more in the same shape with varied severity, so the slots become a habit rather than a checklist.

blocking: this throws on a validation miss, but actions
return the Result shape — callers expect { ok: false, error },
not an exception (pattern #6). Return the error instead.

suggestion: this maps then filters in two passes; a single
reduce would do it in one. Not blocking — the current version
is correct, just doing extra work on large lists.

question: is there a reason this internal-only path skips
authedAction? If it's reachable from a request I'd want the
wrapper; if it truly can't be, a one-line note would help
the next reader.

The five severity labels

The second deliverable is a small, stable vocabulary for that first token. The course uses exactly five labels, lowercase, with a trailing colon. Here they are, each paired with the question that matters most: does it hold the merge?

| Label | When to use | Blocks the merge? | | --- | --- | --- | | blocking: | Must change before merge. Correctness, security, a principle or pattern violation the codebase already paid to establish. | Yes | | suggestion: | A strong recommendation: the alternative is genuinely better, but the current code isn’t broken. | No (unless the author agrees in discussion) | | question: | You don’t know if it’s right and want the author to explain. | No, on its own | | nit: | Non-blocking polish the author may ignore. | No | | praise: | Genuine, specific praise for a non-trivial good call. | No |

Most of the table is self-explanatory, but a few of these labels have a sharp edge worth naming.

question: is the most dynamic one. It doesn’t resolve into a fix; it resolves into an answer, and the answer decides what happens next. The author explains, and either that’s the end of it because the explanation is fine, or your question upgrades to a blocking: or a suggestion: now that you understand. A question: is a fork in the thread, not a verdict.

nit: is the one to ration. A nit is fine: sometimes a name really is slightly off and it’s worth a word. But nits drown signal. A PR with twelve nit: comments and one blocking: buries the blocker in noise, and the author can’t tell your “this leaks tenant data” apart from your “I’d rename this variable.” Aim for one or two nits a PR, not twelve. If you find yourself writing a third, ask whether the formatter or linter should own it instead. If it should, the real move isn’t a nit; it’s promoting the rule to the linter so no one has to write it again.

That leaves praise:, the most misused of the five, almost always in the same wrong direction. Reflexive praise like “nice work!” or “looks good!” is noise at best and faintly condescending at worst; it reads as a participation trophy stapled to the top of the review. Real praise names the specific good call: “good call keeping authedAction on the internal-only path, which keeps the pattern uniform even where it’s tempting to skip.” That comment teaches. It tells the author and every future reader that the uniform thing was the deliberate thing. Aim for the three praise comments a quarter that someone actually remembers, not the thirty rote ones nobody reads.

Where these labels come from

The five labels aren’t something the course invented. They’re a deliberately trimmed subset of Conventional Comments , a tiny public standard for exactly this: putting a parseable label on the front of every review comment. It’s worth knowing how the course’s subset differs from the full standard, so you’re not surprised when you join a team that uses the whole spec.

The full standard has more labels than five (praise, nitpick, suggestion, issue, todo, question, thought, chore, note), and it does not have a blocking label. Instead, “blocking” is a decoration the spec appends in parentheses: you’d write suggestion (blocking): or issue (non-blocking):. The course flattens this on purpose. The single most important thing the author needs from a label is the merge-hold signal, and a beginner, whether author or reviewer, reads a first-class label faster than a parenthetical decoration tucked after it. So the course promotes blocking from a decoration to a label of its own. That’s a pedagogical choice, not a mistake: on a team using the full spec, you’ll write issue (blocking): where this course writes blocking:, and the underlying skill is identical.

Subset or full spec, the property is the same, and it’s the only one that matters: severity is parseable without reading the body. That’s the whole reason the convention exists.

Conventional Comments

conventionalcomments.org

The public standard the course's five labels are a trimmed subset of — the full label set plus the blocking / non-blocking decoration.

Before the harder distinction in the next section, drill the label-to-situation mapping until it’s reflexive. Sort each review situation into the label you’d reach for.

Sort each review situation into the label you'd reach for. Drag each item into the bucket it belongs to, then press Check.

blocking: Must change before merge

suggestion: Better, but not broken

question: You need an explanation

nit: Optional polish

praise: A specific good call

A list query reads the table with no org filter at all

An as User cast stands in where a runtime parse should validate the shape

A Server Action throws on bad input instead of returning the Result shape

This loop builds an array that a single .map would express more directly

Two state fields could collapse into one discriminated union, but the current code works

You can’t tell why this path does a manual auth check instead of the wrapper

It’s unclear whether this endpoint is reachable from a request or strictly internal

A variable named data2 would read better as pendingInvoices

A tricky error path still returns the Result shape cleanly instead of throwing

The migration was split into expand and contract steps exactly as the pattern calls for

Check Reset

Blocking or suggesting: decide before you type

This is the load-bearing discipline of the whole lesson, so give it room. The single most useful habit a reviewer can build is to decide before you start typing the comment whether the line must change, not after. The cut that decides it is sharp once you name it.

Blocking is an objective failure. There’s a fact of the matter, and the code is wrong against an established decision: the principle violation, the security gap, the contract drift, the test that doesn’t actually test. You’re not expressing a preference. You’re pointing at something the codebase already decided and showing where the diff broke it.

Suggesting is a subjective preference. The code works. You’d have factored it differently, reached for a different pattern, picked a nicer name. It’s a real opinion, often a good one, but it’s yours, and there’s no established decision the current code violates.

Getting the cut wrong has a cost, and that cost is the reason the discipline matters. When you don’t make the cut, the author has to guess, and they guess in one of two failure directions. Either they treat everything as blocking, so they churn the whole PR to satisfy your stylistic asides, over-correct, and quietly start resenting your reviews. Or they treat everything as optional, shrug off your comments as opinions, and merge straight past the one that was a real security hole. Either way the signal is gone. You can be right about every single comment and still have a useless review, because the author couldn’t tell your “must” from your “maybe.”

The reflex that follows is slightly counterintuitive: the discipline is visible in the comments that aren’t blocking. Anyone can mark the scary bug blocking:. The skill is honestly marking the suggestion a suggestion and the nit a nit, because that restraint is exactly what makes your blocking: label trustworthy. An author who has learned that you only write blocking: when you mean it will drop everything when they see it. An author who has watched you slap blocking: on a variable name has learned to ignore the label entirely. You earn the weight of blocking: by spending it carefully.

The stack from the last lesson gives you a strong prior here. Layer-1 findings, which are correctness and security, along with clear principle or pattern violations, are almost always blocking:. Pure style and polish, the bottom of the stack, are nit: or suggestion: at most. The layer suggests the severity, but don’t make it mechanical: a principle question that’s genuinely ambiguous in this diff might be a question: first, and only become a blocking: once the author’s answer confirms the violation is real.

Tip

A quick gut-check before you type blocking:: could you finish the sentence “this is wrong because the codebase already decided ___”? If you can name the decision, it’s blocking. If the honest ending is “…because I’d have done it differently,” it’s a suggestion.

Try the cut on a few real scenarios. For each, pick the severity that fits, and notice why the tempting wrong answer is wrong.

A working function does its job correctly, but the reviewer is convinced it would read better split into two smaller functions, and they hold that view strongly. What severity fits?

suggestion:

blocking:

nit:

question:

The diff includes a list query against a tenant-scoped table with no org filter at all, so running it can hand back rows belonging to a different organization. What severity fits?

blocking:

suggestion:

nit:

question:

A reviewer genuinely can’t tell whether an endpoint is reachable from an outside request. If it is, it’s missing an auth check; if it’s strictly internal, the current code is fine. What severity fits first?

question:

blocking:

nit:

suggestion:

The language of disagreement

You’ve decided what to flag and what severity it carries. Now come the words below the label. The whole craft compresses to three adjectives: opinionated, evidence-led, short. State your position, name the reason it’s right, and make the ask, all in one to three sentences. The voice underneath all of it is one peer talking to another who shares the same codebase and its conventions. You’re not a gatekeeper guarding the merge button, and not a supplicant apologizing for having an opinion. You’re a peer.

The fastest way to calibrate that voice is to watch it fail. Here are the three ways beginners’ comments most often go wrong, each with the rewrite that fixes it.

Hedged into uselessness

maybe we could perhaps consider possibly scoping this to
the org at some point? not sure, just a thought, feel free
to ignore

No position, so no signal. The author can’t tell whether there’s a real concern here or you’re just thinking out loud, so they ignore it, and a tenant leak ships.

Blame-laden

this is wrong. did you even read the docs on tenantDb?
we literally have a helper for exactly this.

Aims at the person, not the code. It puts the author on the defensive and ages into an accusation when someone reads the thread in a year. The defensiveness, not the bug, becomes the topic.

Essay-length

So historically the reason we have tenantDb is that back
when we first built multi-tenancy we had an incident where
[three paragraphs of history] ... anyway, so the upshot of
all that is you probably want to use tenantDb here.

The ask is buried. A one-line ask is lost under context the author has to dig for. The signal is in there somewhere, which is the same as not being there.

Each of those is the same finding, mangled three different ways. Here’s the finding said well: opinionated, evidence-led, short.

blocking: this query isn't org-scoped, so it can read
across tenants. Route it through tenantDb(orgId) — see
pattern #1 for why we never query the table directly.

Position, reason, ask. The history that the essay-length version dumped inline lives behind the pattern #1 link, where the author can read it if they want it and skip it if they don’t.

Talk about the code, not the author

There’s a small phrasing reflex worth building, and the argument for it is not politeness. It’s durability. The comment is about the code, not the person who wrote it: “This bypasses tenantDb, let’s route it through the helper” instead of “You bypassed tenantDb.”

This matters because PR threads are read months later, often by people who weren’t in the room. A “you” comment ages badly. Read cold, eighteen months on, “you didn’t validate this” reads like an accusation in the permanent record, even though in the moment it was a neutral note between teammates. A “the code does X” comment stays neutral forever because it never pointed at a person in the first place. You’re not being nice. You’re writing for an audience that includes strangers reading the archive.

You forgot to scope this to the org.

This isn't scoped to the org — let's route it through tenantDb.

There are two exceptions to “address the code”: praise (“nice call you made keeping the pattern uniform here”) and direct questions (“can you explain why this path skips the wrapper?”). Both are genuinely about the person, so “you” is honest there.

Ask when you’re unsure, assert when you’re sure

The question: label has one honest use and one dishonest one, and the difference is a matter of credibility.

The honest use is when you genuinely don’t know. “Is there a reason this doesn’t go through authedAction?” is a real question when you suspect the author might hold context you lack: maybe this path truly can’t be reached from a request and the wrapper would be dead weight. The question signals that uncertainty, invites the explanation, and resolves fast.

The dishonest use is dressing a position you’re certain about as a question to avoid committing to it: “Can we use authedAction here?” when what you actually mean is “this must use authedAction.” That’s epistemic cowardice , and the name is harsh on purpose, because it’s a credibility issue, not a manners issue. It costs you three ways. It adds a round-trip, because the author answers the literal question (“yeah, we can”) instead of acting on the position you were too timid to state. It softens the signal, so a real requirement reads as an idle musing. And over time it erodes your standing: a reviewer whose “questions” are secretly demands trains the team to distrust both their questions and their assertions.

The cut is clean: questions when you’re uncertain, positions when you’re certain, and never a question used to dodge a position. This is the same problem as an unlabeled blocker, just wearing a different costume. A question: that’s really a blocking: in disguise hides the merge-hold signal exactly the way an unlabeled comment does. Say what you mean, and label it for what it is.

Watch video

Code-review researcher Dr. Michaela Greiler walks through ten phrasing tips (11 min): make it about the code, state it as your perspective, drop condescending words, give the reason. It’s the same craft this section just covered.

One comment, one concern

This one is short and sharp: put one concern in each comment.

A comment that bundles three observations forces the author to swallow or reject all three as a unit. They can’t agree with the first, push back on the second, and ask about the third, because it’s one thread. The conversation can’t converge because there’s no clean thing to converge on. Split them: one concern per comment, each one separately ack-able, fixable, or push-back-able.

Before: one bundled comment. Three concerns the author can only swallow or reject as a unit:

This function is doing a lot — the query isn't org-scoped,
the error path throws instead of returning Result, and the
variable names are pretty unclear. Might be worth a look.

After: three separate comments. Each one is independently ack-able, fixable, or push-back-able:

blocking: this query isn't org-scoped — route it through
tenantDb(orgId) (pattern #1).

blocking: the error path throws on bad input; actions
return the Result shape (pattern #6).

nit: data2 would read better as pendingInvoices.

There’s a useful corollary hiding in that example. If three genuine concerns really do cluster on one line, that’s a signal in itself: the line is doing too much. The honest response then isn’t three comments but one, a level up: “this function is carrying too many responsibilities to address these cleanly; it wants to split first.” That, itself, is exactly one concern.

The suggestion block: when the fix is one click away

There’s a platform feature beginners reliably under-use, and it’s a real latency win. Most review platforms, including GitHub, GitLab, and Graphite, support a special ```suggestion block inside a review comment. Whatever you put in it renders as a proposed diff that the author applies with a single click, and can batch with others into one commit. You’re not describing the fix; you’re handing it over, ready to merge.

The syntax is just a fenced block with suggestion as the language, placed in your comment on the line you’re replacing.

suggestion: scope this to the org so it can't read across tenants.

```suggestion
const invoices = await tenantDb(orgId).select().from(invoicesTable);
```

The multi-line variant works too: put several lines inside the suggestion fence and the author applies all of them as a single commit.

The threshold for reaching for it: under roughly five lines, and one obviously-correct edit. A renamed identifier, a missing await, a one-line wrapper swap, anything mechanical where there’s exactly one right answer. Below that bar, the suggestion block removes a manual step for the author with no downside. Above it, for a forty-line rewrite or anything where the fix involves judgment, write prose instead. A suggestion block is the wrong surface for a redesign: you’d be jamming an architecture conversation into a one-click diff, and the author can’t engage with it as a decision. The syntax here is GitHub’s, but the posture is platform-agnostic: a mechanical fix becomes a suggestion, a design change becomes prose, wherever you review.

Receiving the review

Now flip the chair. You will spend far more career-hours as the author receiving comments than as the reviewer writing them, and the receiving side is where the labels prove they were structural all along, not decorative. Three reflexes carry it.

Respond to every comment. Even a 👍, even “done in a3f9c2.” A comment left with no response leaves the reviewer guessing whether you saw it, agreed, disagreed, or missed it, and that guessing is exactly the latency the whole lesson is trying to kill, now running in the other direction. This is the symmetric payoff of one-comment-one-concern: because each comment is a single thing, each one gets a cheap individual acknowledgment.

Push back with evidence; don’t just defer. A reviewer can have a bad day, and a reviewer can be wrong. An incorrect comment shouldn’t get merged into the codebase just because a reviewer wrote it, any more than a correct one should get ignored because you’d rather not deal with it. Where you have evidence the design is right, defend it neutrally, with the same code-not-person discipline you’d want from the other side. The asymmetry worth internalizing is that you own the code’s correctness as much as the reviewer does. Deferring to a wrong comment is exactly as much a failure as ignoring a right one. Both put bad code in main.

Resolve threads honestly. Resolving a thread with “I’ll get to it next PR” is a small lie the archive remembers: six months later the thread reads “resolved” and the fix never came. “Fixed in a3f9c2” is the honest resolution. And resolving a thread by quietly closing it without addressing the comment at all is the author-side twin of the drive-by approval: it makes the record say a conversation happened that didn’t.

One thing is non-negotiable, and it ties straight back to the core model: a blocking: comment is structural. You don’t 👍 it and merge anyway. The label means the merge waits, which is the entire point of putting it on the front of the comment. If you think the blocker is wrong, you don’t route around it; you argue it down to a non-blocker in the thread, in the open, where the reviewer can agree or hold the line. Disagreeing with a blocker is fine. Pretending it wasn’t there is not.

You’re the author. A reviewer left a blocking: comment, but after a careful re-read you’re confident the current code is correct and the blocker is mistaken. What’s the right move?

Reply in the thread with the evidence for why the code is right, and argue the label down to a non-blocker in the open — merging only once that’s resolved.

Apply the requested change anyway; conceding is faster than a thread, and the reviewer’s call carries more weight on their own comment.

Resolve the thread yourself and merge, since you’ve already satisfied yourself the code is correct.

Take it to a private DM so the disagreement stays off the PR, then merge once you’ve talked it through.

Match the review action to the comments

Most platforms make you pick an action when you submit a review: approve, comment, or request changes. The cut is one sentence. The review action should mirror the severities you wrote, not your mood or your relationship with the author.

| Action | When | What must be true of the comments | | --- | --- | --- | | request changes | At least one blocking: comment exists | The state says, out loud, that the merge waits | | approve (with comments) | Only suggestion: / nit: / resolved question: remain | The author is trusted to address them or not, and can merge | | comment | The default during back-and-forth | No final verdict is warranted yet |

The rule is simple: the review state tracks the comment severities. If you wrote a blocking:, you click request changes, because anything else lies about what you found. If everything left is a suggestion or a nit, you approve and trust the author. The drive-by failures from the last lesson map cleanly onto this table, in both directions. A drive-by approve, clicking approve without really reading, especially on a teammate you trust, decouples the action from the content by skipping the content entirely. A drive-by request changes, blocking a merge over a style preference, decouples it the other way. The bar is the same for everyone: don’t wave the senior teammate through unread, and don’t over-block the junior. The action is a claim about the comments, so make it a true one.

Habits that keep reviews honest

A few meta-disciplines don’t each need a section but compound over a career.

The “what would I want to see” mirror. Before you submit, re-read your own comments as if you were the author about to receive them. Are the severities right? Is every ask actually actionable? Is every reason named and, where it exists, linked? Sixty seconds, every review. This is the single habit that puts the whole lesson into practice: it catches the comment that came out as “I don’t like this” when what you meant was “this conflicts with pattern #7.” If you build one habit from this lesson, build this one.

Escalate at three round-trips. When a thread crosses roughly three exchanges without converging, stop typing. The disagreement is structural, and the async comment box is the wrong surface for it, because you’re litigating a design decision one paragraph at a time. Pull it into a fifteen-minute call, decide, and then capture the decision back in the PR. That last step is the one people skip. A call that isn’t written down didn’t happen, as far as the archive is concerned. If the decision is architectural, it’s a candidate for an ADR, which the next chapter covers in full. The thread is for comments; the big design move belongs somewhere durable.

Mentorship is a side effect of linking. When a comment explains a principle or pattern violation, link the lesson or doc that owns it: “see the tenantDb chapter for why we never query the table directly.” It costs you a minute and saves the author an hour of context-hunting, and it’s the most efficient teaching channel a team has. Feedback on your own code, at the exact moment you got it wrong, teaches faster than any doc you’d ever read cold. The junior who gets three linked comments a week is up to speed in a quarter.

Reviewing in a queue full of agents

Two things change when coding agents are in the loop, and the standing rule applies to both: neither makes the bar move.

An agent-authored PR gets the same bar. When a coding agent opens a PR, the diff lands in your queue like any other and gets the same five-layer pass, the same checklist, the same severities. The bar doesn’t soften because “the AI should have gotten it right,” and it doesn’t tighten into disproportionate nitpicking because “it’s only a bot, who cares.” Same comment shape, same labels, same restraint about what doesn’t get a comment. The checklist works on agent code for the same reason it works on human code: the principles and patterns are stable, and agents trip the exact failure modes the last lesson named, such as re-inventing a wrapper the codebase already has, adding plausible-looking tests that don’t actually exercise the contract, and drifting from the conventions in AGENTS.md.

An agent-authored comment gets calibrated, not obeyed. Review tools like Copilot’s reviewer, CodeRabbit, and Qodo now leave their own comments on PRs. Read them the way you’d read a brand-new junior reviewer’s comments: sometimes genuinely load-bearing, often noise, and your job is to tell which is which. You dismiss the noise, acknowledge the real ones, and you do not outsource the judgment to the bot. The label discipline cuts both ways here: a bot’s blocking: is a claim to verify, not a gate to obey. The tool flagging it doesn’t make it true; you still have to check.

A review, end to end

Here’s the dress rehearsal. The diff below is a small two-file PR built on the canon you already know, so the findings are familiar, and the lesson now is purely the comment. Click any line to leave a review comment, the way you would on a real PR. For each one, label it with one of the five severities and keep it to the four-part shape: label, observation, reason, ask. The grader scores the substance of what you write, not just whether you clicked the right line, so a comment that spots the bug but skips the severity or the ask is only half the job.

Label every comment with one of the five severities (`blocking:` / `suggestion:` / `question:` / `nit:` / `praise:`) and keep each to the four-part shape: label, observation, reason, ask. Click any line to leave a review comment, then press Submit review.

src/lib/invoices/list-invoices.ts

src/lib/invoices/list-invoices.ts

import { eq } from 'drizzle-orm';
import { db, tenantDb } from '@/lib/db';
import { invoicesTable } from '@/lib/schema';

export async function listInvoices(orgId: string, limit = 50) {
  return tenantDb(orgId).select().from(invoicesTable).limit(limit);
}

export async function getInvoiceByNumber(orgId: string, number: string) {
  return db.select().from(invoicesTable).where(eq(invoicesTable.number, number));
}

src/app/invoices/actions.ts

src/app/invoices/actions.ts

import { authedAction } from '@/lib/safe-action';
import { invoiceSchema } from '@/lib/invoices/schema';
import { createInvoice } from '@/lib/invoices/create';
import { getInvoiceByNumber } from '@/lib/invoices/list-invoices';

export const createInvoiceAction = authedAction('member', invoiceSchema, async (input, ctx) => {
  const existing = await getInvoiceByNumber(ctx.orgId, input.number);
  if (existing.length > 0) {
    throw new Error(`Invoice ${input.number} already exists`);
  }
  const invoice = await createInvoice(ctx.orgId, input);
  return { ok: true, data: invoice };
});

Submit review

That exercise is a single focused rehearsal of comment-craft. The next chapter turns it into the real thing: a full PR reviewed start to finish, with the architecture-level decision written up as an ADR.

External resources

Google Engineering Practices — How to write code review comments

google.github.io

Google's vendor-neutral guide: be courteous, comment on the code not the developer, explain the why, and label severity so authors can prioritize — the same craft, from a different shop.