DEV Community

Codzee.io
Codzee.io

Posted on

A Label Isn't a Code Review

Consider two comments on the same piece of code.

The first:

“Consider refactoring this function for readability.”

The second:

“This returns null when no active subscription exists, but the caller immediately dereferences .plan. That turns a valid ‘no subscription’ state into an exception. Handle the empty case before accessing plan.”

The first comment might be correct.

The function might genuinely be difficult to read.

But it leaves an important question unanswered:

What is the engineer supposed to do with that information?

The second comment contains considerably more information. It identifies a behavior, connects it to another part of the system, describes the failure mode, and suggests a direction.

That seems straightforward.

But it raises a more interesting question:

What information does a code review comment actually need to contain to be useful?


Code review isn't just defect detection

It is tempting to think about code review as a simple pipeline:

code
  ↓
reviewer
  ↓
problem detected
  ↓
comment
  ↓
code changed
Enter fullscreen mode Exit fullscreen mode

But modern code review is doing several things simultaneously.

It can catch defects.

It can question design decisions.

It can communicate project conventions.

It can transfer knowledge between engineers.

It can document decisions.

It can expose assumptions that aren't visible in the code itself.

Research on modern code review has found that reviewers and authors value activities beyond defect detection, including knowledge sharing and increasing awareness of the codebase.

A Microsoft Research study of peer review also found that review is a form of knowledge exchange in which design rationale can be surfaced during the discussion.

So a review comment isn't merely a defect marker.

It is a small communication channel between two people who may have very different amounts of context.

That changes what “good” means.


Flagging isn't explaining

Consider:

“Potential race condition.”

There may indeed be a race condition.

But the comment assumes the author will reconstruct the reviewer's reasoning.

Where is the race?

Under what interleaving?

Does it produce incorrect data?

Duplicate work?

A crash?

Is it harmless?

Now compare:

“Two requests can both observe that the key is absent before either calls set(). That means fetchValue() can execute twice and the second result can overwrite the first. If fetchValue() has side effects, the current check-then-set sequence isn't sufficient; use an atomic get-or-create operation.”

The second comment isn't merely more verbose.

It exposes the reasoning.

That distinction matters.

A label such as:

[bug]
[performance]
[security]
[style]
Enter fullscreen mode Exit fullscreen mode

answers:

What category did the reviewer assign this observation?

It doesn't necessarily answer:

Why should I change the code?

A label is metadata.

An explanation contains a model of the problem.


What information should a review comment contain?

I don't think there is a universal template.

But useful comments often contain some combination of:

WHAT
What behavior or property is concerning?

WHY
Why does it matter?

CONTEXT
What assumption, constraint, or system behavior makes it important?

CONSEQUENCE
What can happen if it remains unchanged?

NEXT
What should the author investigate or change?
Enter fullscreen mode Exit fullscreen mode

A comment doesn't need all five.

For an obvious typo:

recievereceive.”

is probably sufficient.

For a concurrency bug:

“Two requests can both pass this check…”

may need considerably more context.

The amount of explanation should depend on the amount of ambiguity.


Correctness is not the same as usefulness

This is an important distinction.

Suppose a reviewer writes:

“This function is too complex.”

And suppose the function really is too complex.

The statement can be correct while the review remains unhelpful.

The author still has to determine:

  • Which complexity?
  • Why is it a problem?
  • What boundary should change?
  • Is the reviewer concerned about readability?
  • Testing?
  • Runtime behavior?
  • Future modification?
  • Something else?

Compare:

“This function now validates input, persists the record, and publishes an event. These operations have different failure semantics, so changing the event behavior now requires modifying persistence logic. Consider separating event publication from the persistence operation.”

The second comment doesn't necessarily prescribe the correct refactoring.

It does something more fundamental:

It makes the reviewer's concern inspectable.

The author can agree.

The author can disagree.

The author can explain why the coupling is intentional.

That is a healthy property of a review comment.


Why “why” matters

Code frequently contains decisions that are not obvious from the code itself.

Imagine:

if user.IsDeleted() {
    return ErrNotFound
}
Enter fullscreen mode Exit fullscreen mode

A reviewer might ask:

“Why return not found instead of forbidden?”

That question could be perfectly reasonable.

But suppose the actual reason is security-sensitive API behavior:

“The public endpoint intentionally returns 404 for deleted users so callers can't distinguish between an existing deleted account and an account that never existed. This is consistent with the behavior of the other account lookup endpoints.”

Now the comment isn't just about an error code.

It transfers context.

This is why established review guidelines emphasize explaining why rather than simply describing what the code does. GitLab's review guidance explicitly recommends explaining rationale and making comments actionable.

Google's code-review guidance similarly asks whether comments are clear and useful and distinguishes substantive concerns from optional style feedback.

The “why” matters because code often cannot contain the entire reasoning behind itself.


But there is a danger in over-explaining

Here is the obvious counterargument:

Could this framework cause reviewers to over-explain everything?

Absolutely.

Imagine receiving this review comment:

“This variable is named i, which could potentially reduce semantic clarity because variable names are an important part of communicating intent. Consider using index instead, since…”

Nobody needs a paragraph for that.

The problem is not lack of context.

The problem is that the context is already obvious.

A useful principle is therefore not:

“More explanation is better.”

It is:

“Enough explanation to remove meaningful ambiguity.”

For a typo, that's almost nothing.

For a distributed-system failure mode, it might be several sentences.

Comment length should follow uncertainty.


Signal versus noise

This becomes especially important when review comments are cheap to generate.

A human reviewer has a natural constraint: attention.

An automated reviewer can potentially produce many observations.

That creates a dangerous feedback loop:

more detection
     ↓
more comments
     ↓
more things to inspect
     ↓
less attention per comment
     ↓
more review fatigue
Enter fullscreen mode Exit fullscreen mode

The problem isn't hypothetical in the broader code-review literature.

Researchers have studied reviewer participation, usefulness of comments, review size, reviewer experience, and other factors that affect modern code-review effectiveness.

For example, Bosu, Greiler, and Bird analyzed approximately 1.5 million review comments from five Microsoft projects to investigate characteristics associated with useful review feedback. Their work found relationships between usefulness and factors including reviewer experience and change characteristics.

The implication isn't that automated review is bad.

It's that review bandwidth is finite.

If a system produces ten additional comments, the relevant question isn't whether all ten are technically defensible.

It's whether the engineer's attention was spent better because they received them.


The preference problem

There is another source of noise that has nothing to do with AI.

Human reviewers do it constantly.

Consider:

“I'd use a map here.”

That might be a good suggestion.

But why?

Maybe the map makes lookup complexity clearer.

Maybe it avoids repeated scans.

Maybe it is simply the reviewer's preferred style.

Those are different claims.

Compare:

“I'd use a map here.”

with:

“We're doing a linear scan for every lookup, so the cost grows with the number of entries. Since this collection is accessed repeatedly, an indexed structure would avoid repeated scans.”

Now the author can evaluate the reasoning.

This distinction between preference and engineering necessity is important.

Google's review guidance essentially argues that when multiple approaches are consistent with sound engineering principles, reviewers should accept the author's choice rather than treating their own preferred implementation as mandatory.

A review culture that treats every preference as a defect creates noise even when every reviewer is acting in good faith.


Five examples

1. Nullability

Weak

“Handle null.”

Stronger

getSubscription() returns null when the user has no subscription, but this caller immediately reads .plan. That turns a valid empty state into a runtime exception. Handle the empty case before dereferencing the result.”

The second comment exposes the contract and failure mode.


2. Database queries

Weak

“N+1.”

Stronger

getCustomer() runs a database query inside this loop. With 500 orders, this can result in hundreds of additional queries. Load the required customers before the loop and reuse them here.”

The label tells an experienced engineer what to investigate.

The explanation tells them why this particular code has the problem.


3. Error semantics

Weak

“Don't swallow the error.”

Stronger

“Returning nil here makes a failed request indistinguishable from a valid empty response. The caller treats nil as success, so a network failure could be interpreted as ‘no data.’ Preserve the error so the caller can decide whether to retry or surface it.”

The concern isn't merely “errors are important.”

It's that information is being destroyed.


4. Authorization

Weak

“Security issue.”

Stronger

“Authentication establishes that the user has a valid session, but it doesn't establish that they can modify this document. Since the document isn't checked against the user's project, this path could allow cross-project modification. Verify authorization before the update.”

The reviewer is identifying the missing security boundary.


5. Maintainability

Weak

“This should be refactored.”

Stronger

“This method now handles validation, persistence, retries, and notification. Those operations have different failure modes and are already changing independently. Separating the retry/notification path would keep future notification changes from touching persistence logic.”

The second comment explains why the proposed structural change matters.


What if the reviewer doesn't know the author's intent?

This is where the framework gets uncomfortable.

Suppose a reviewer sees:

if response.status_code == 404:
    return []
Enter fullscreen mode Exit fullscreen mode

They might write:

“This is wrong. A 404 should be an error.”

But perhaps the API deliberately uses 404 to represent “no matching resources.”

The reviewer has made an assumption about intent.

A more useful comment might be:

“Is 404 expected to mean ‘no matching resources’ for this endpoint? If it represents a missing resource instead, returning an empty list here would hide an API error.”

That's a question rather than a declaration.

And sometimes that is the better review comment.

Good review doesn't require the reviewer to pretend they know things they don't know.

Uncertainty should be communicated as uncertainty.

There is a major difference between:

“This is wrong.”

and:

“I may be missing context, but if X is possible, doesn't this create Y?”

The second invites the author to supply missing information.


Review comments are conversations, not verdicts

This is easy to forget when reviews are represented as annotations attached to lines.

The interface makes a comment look like a judgment.

But a review is usually a conversation.

The reviewer proposes an interpretation.

The author provides context.

The two converge on a decision.

That means a useful review comment doesn't necessarily need to prove that the reviewer is right.

It needs to make the disagreement productive.

For example:

“I think this could race if two workers execute this block concurrently. Is there a lock around this operation elsewhere that I'm missing?”

That might be a better comment than a confidently incorrect:

“This has a race condition. Fix it.”

The first comment contains uncertainty.

The second hides it.


What about AI-generated review comments?

AI makes this problem more interesting.

An automated system can examine code at a scale that humans can't.

It can notice patterns.

It can compare implementations.

It can identify suspicious data flows.

It can explain some classes of issues.

But there is a fundamental limitation:

The code is not the entire system.

Intent may live in:

  • product requirements,
  • architecture documents,
  • incident history,
  • database constraints,
  • undocumented operational assumptions,
  • conversations between engineers,
  • downstream consumers,
  • or simply someone's knowledge of why the system works the way it does.

An AI looking only at a pull request may not have access to that context.

So consider an automated comment:

“This validation is redundant.”

Maybe it is.

Or maybe the validation exists because an external system violates the contract once every few thousand requests.

Without broader context, the model may correctly understand the local code and incorrectly understand the system.

That's an important distinction.

Local correctness is not necessarily system correctness.


Can AI generate specific comments without understanding the whole system?

Sometimes.

For straightforward cases, specificity may not require deep system-level intent.

For example:

“This value is checked for None here, but dereferenced unconditionally three lines later.”

The evidence is local.

The model doesn't need to understand the entire architecture.

But consider:

“This retry count is too low.”

That might require knowing:

  • upstream service behavior,
  • timeout characteristics,
  • SLA requirements,
  • idempotency guarantees,
  • traffic patterns,
  • and production failure modes.

The more a comment depends on external intent, the less safe it is to infer that intent from code alone.

This suggests a useful boundary for automated review:

The system should distinguish observations supported directly by code from conclusions that depend on missing context.

That could mean expressing uncertainty.

It could mean asking a question.

It could mean requesting additional repository context.

It could mean not commenting at all.

Silence is sometimes better than a plausible but unsupported explanation.


Explanation isn't evidence

There's another subtle failure mode.

A detailed comment can sound authoritative simply because it is detailed.

Consider an AI-generated comment:

“This implementation creates a race condition because the cache is not synchronized across worker threads, which could cause stale reads and inconsistent state…”

It sounds convincing.

But does the system actually know that the workers share the same cache?

Does it know the operation is concurrent?

Does it know stale reads matter?

A long explanation can create false confidence.

So review quality cannot simply become:

“More detailed comments are better.”

The real requirement is:

The explanation should be proportional to the evidence.

A concise, well-supported observation can be more trustworthy than a sophisticated explanation built on assumptions.


The unit of review isn't always the line

Another limitation of line-level comments is that many engineering problems aren't located on one line.

Consider an authorization bug.

The problem might be:

controller
    ↓
service
    ↓
repository
    ↓
database
Enter fullscreen mode Exit fullscreen mode

The individual lines can all look reasonable.

The vulnerability emerges from how the components interact.

Likewise with:

  • distributed systems,
  • caching,
  • transactions,
  • retries,
  • consistency,
  • API contracts,
  • concurrency,
  • lifecycle management.

Sometimes the most useful review comment needs to refer to a relationship, not a line.

For example:

“The transaction ends before the event is published, so a successful response can be returned even if event publication fails. Is that intentional? If the event is part of the operation's contract, the transaction/outbox boundary probably needs to move.”

That's difficult to express as a simple label.

It requires a model of the interaction between components.


So what makes a review comment good?

I don't think the answer is “make every comment more detailed.”

Nor is it “always explain the why.”

There are cases where the why is obvious.

Nor is it “never use labels.”

Labels are useful.

The more interesting answer is:

A good review comment contains the minimum information necessary for another engineer to correctly evaluate the concern.

Sometimes that's:

“Typo: recievereceive.”

Sometimes it's:

“This returns null when X happens, but Y dereferences it immediately, turning a valid state into an exception.”

Sometimes it's a question:

“Is this 404 intentional? If not, are we hiding an API failure by returning an empty list?”

Sometimes the right review is no comment at all.

That last case matters.

A reviewer should not be rewarded simply for finding something to say.


A small framework: WHAT → WHY → NEXT

If I had to give reviewers a practical heuristic, it would be:

WHAT?

What exactly are you observing?

“This can return null.”

WHY?

Why does that matter?

“The caller dereferences it immediately.”

NEXT?

What should the author investigate or change?

“Handle the empty case before accessing the property.”

Together:

“This can return null, but the caller immediately dereferences it, which can turn a valid empty state into a runtime exception. Handle the empty case before accessing the property.”

But the framework should be treated as a diagnostic tool, not a writing template.

If the issue is obvious, don't force an explanation.

If the reviewer is uncertain, don't manufacture certainty.

If the concern depends on system context the reviewer doesn't have, ask.

If the implementation is simply a matter of preference, say so.


The deeper problem

There is a tendency to think of software engineering communication as secondary to the “real” engineering work.

The code is the artifact.

The review comment is just discussion around it.

I'm not convinced that's true.

A codebase contains decisions.

Reviews contain the reasoning behind some of those decisions.

When that reasoning is missing, future engineers have to reconstruct it.

When it is vague, authors have to interpret it.

When it is excessively noisy, people stop paying attention.

And when it is confidently wrong, the review system can actively make engineering decisions worse.

That applies equally to human and automated reviewers.

The hard problem isn't merely detecting something suspicious.

It's knowing what information another engineer needs to evaluate the observation.


Open questions

I don't think we've solved this.

A few questions seem particularly interesting.

How much context is enough?

Can review systems estimate when a comment needs one sentence versus five?

Can usefulness be measured without reducing it to “did the author change the code”?

A correct review may result in no change because the author provides missing context.

How should automated reviewers represent uncertainty?

Should a system say:

“This is a bug.”

or:

“If X is possible, this may produce Y. Is X guaranteed not to happen?”

How much repository context should an AI reviewer consume before making a claim?

More context can improve reasoning, but it can also increase cost and introduce irrelevant information.

Can automated systems distinguish preference from correctness reliably?

Or will they reproduce the same subjective review behavior humans already struggle with?

What happens to review quality when the cost of generating comments approaches zero?

If comments become effectively free, attention becomes the scarce resource.

And perhaps the most basic question:

Should the objective of code review be to find more problems, or to help engineers make better decisions?

Those are related goals.

They aren't necessarily the same.

A label tells us that someone thinks there is a problem.

A useful review comment gives us enough reasoning to decide whether they are right.

That's why a label isn't a code review.

Top comments (0)