Pull request review comments need a contract.

What makes a useful pull request review comment?

A useful pull request review comment identifies whether action is required, anchors the concern at the narrowest accurate scope, cites evidence that another reviewer can check, traces the consequence, and proposes the smallest safe next step. It stays attached to a review decision and the exact commit that was reviewed.

That definition is stricter than “be constructive.” Polite wording matters, but a friendly comment can still waste time when the author cannot tell whether it blocks merge, which code proves the claim, or what would resolve it. The same ambiguity is more expensive when a bot can publish dozens of comments in seconds.

GitHub exposes different surfaces for different jobs: general discussion on the Conversation tab, line and file comments in Files changed, and a submitted review with a Comment, Approve, or Request changes decision. Itscommenting documentationalso lets reviewers batch line comments into one pending review. A good comment contract uses those surfaces deliberately instead of treating every thought as an equivalent inline alert.

Use a five-step comment contract

Apply the full contract to blocking findings and automated feedback. A short human question or nitpick can use a lighter version, but it should still declare its intent. This preserves signal without turning review into form-filling.

1. Classify the intent. Say whether the comment is a blocker, issue, question, suggestion, nitpick, or praise before explaining it.
2. Choose the narrowest honest scope. Put a local defect on the changed line, a file-level concern at file scope, and a pull-request-wide decision in the review summary.
3. State the checked claim and evidence. Cite the exact code and the repository contract, test, caller, or documentation that makes the claim checkable.
4. Trace the impact and smallest resolution. Explain the triggering condition, causal path, observable consequence, and the smallest safe fix or verification step.
5. Track disposition on the current head. Record whether the finding is open, fixed, rejected, accepted as risk, or stale, and tie that decision to the reviewed commit.

The first step removes a common source of friction: authors should not have to infer whether “Could we change this?” means a required correction or an optional idea. Google'scode review comment guidancerecommends explaining the reason for a comment and labeling severity, including optional notes and nitpicks. TheConventional Comments specificationformalizes a similar idea with labels such as issue,suggestion, question, and nitpick.

Labels are not proof. A comment marked “blocking” still needs a checked claim. For review automation, pair this grammar with anevidence-bound PR review promptthat requires the reviewer to abstain when the repository does not support the finding.

Choose the narrowest honest review surface

Placement is part of meaning. An inline comment implies that a specific changed line is relevant. A review summary implies that the observation affects the pull request as a whole. When the evidence does not map to a changed line, forcing an inline anchor creates false precision.

Batch related observations rather than emitting one notification per thought. GitHub's pending-review flow exists for this purpose: add comments while inspecting the diff, then submit them together with the overall decision. Consolidation also exposes duplicates before they reach the author.

Make the finding falsifiable

A review comment should give the author enough information to disprove it. Use this order for a blocking defect:

1. Decision: label the finding and say whether it blocks merge.
2. Claim: describe one behavior the patch introduces or worsens.
3. Evidence: quote the changed code and cite the governing contract.
4. Causal path: connect a realistic input or state to the failure.
5. Impact: name the observable user, system, or compatibility effect.
6. Resolution: propose the smallest safe fix or a verification step.

For example, imagine a pull request removes an authorization guard:

issue (blocking): preserve the project membership check. This handler now loads the report by ID and returns it before callingrequireProjectMember. A signed-in user who can guess another report ID can reach the response path without the project-level check used by the sibling handlers. Keep the guard before the lookup, or add a test showing that a non-member still receives 403.

This is an illustrative comment, not a reported ReviewGate vulnerability. Its shape is useful because each clause can be checked: the changed path, the sibling contract, the triggering identity, the consequence, and the proposed regression test.

Tone supports that technical work. A Microsoft Research study of1.5 million review commentsreported that extremely negative comments were less often judged useful than neutral or somewhat negative ones. The study was published in 2015 and should not be treated as a universal benchmark, but it supports a durable practice: discuss the code and its consequences, not the developer's ability or motives.

Automated review comments need a stricter contract

Automation changes the cost structure. A human reviewer naturally has a limited comment rate; a model does not. Before an automated finding becomes visible, test that it is introduced by this pull request, material, supported by repository evidence, non-duplicate, and mapped to an honest scope. Keep confidence separate from severity: a severe hypothetical with weak evidence is still weak evidence.

ReviewGate implements that separation in public source. Itsreview output schemarecords line, file, and pull-request scope. Changed-line findings are published inline when GitHub accepts the anchor; file-level, pull-request-wide, unanchored, or stale-line findings use a fallback anchor, while every finding remains available in the structured JSON artifact. The canonical summary carries the review decision instead of scattering state across standalone comments.

ReviewGate also derives a semantic finding identity from the finding's substance. That lets reruns reconcile the same issue even when wording changes, while the canonical summary and artifact remain the auditable record. This is a product implementation, not a claim that one identity scheme is universal. You can inspect theGitHub publication implementationand adapt the invariant: one issue should have one stable lifecycle.

The broadermerge-safe AI review workflowadds trusted-event, runtime, verdict, and current-head bindings. Precise comments are only one link in that chain; they cannot make stale or untrusted automation a merge gate by themselves.

Resolve comments without erasing the decision

A thread is discussion state, not proof that the latest code is safe. When the author pushes a fix, re-check the triggering path on the new head before marking the finding fixed. When the team rejects a finding or accepts the risk, record the reason. When the relevant line disappears, mark the old anchor stale rather than silently treating the issue as resolved.

Use a small disposition vocabulary:

• Open: evidence still applies and action is required.
• Fixed: the latest commit removes the causal path.
• Rejected: repository evidence disproves the claim.
• Accepted risk: the team understands the impact and chooses to merge.
• Stale: the anchor or reviewed commit is no longer current.

Preserve the reviewed commit SHA with the decision. A green review for commit A is not evidence about commit B. If branch protection depends on review automation, require a successful result for the current pull request head and fail unavailable review closed rather than converting it into a pass.

Turn vague review comments into decisions

Vague: “This could break retries.”

Better: issue (blocking): keep the idempotency key stable across retries.The retry branch generates a new key on each attempt, while the payment client deduplicates by that key. A timeout after the first accepted request can therefore create a second charge. Reuse the original operation key and add a timeout-then-retry test that asserts one provider request.

Vague: “Can we clean this up?”

Better: nitpick (non-blocking): extract the repeated label. The same user-facing string appears in three branches. A constant would make future copy changes safer, but the current behavior is correct, so this should not hold the merge.

Vague: “Missing tests.”

Better: question: which test exercises the new 429 path? The handler now retries 429 responses, but the visible tests cover only 500 and timeout. If an existing case covers the retry budget, please point to it; otherwise add one that proves the final error and attempt count.

These examples are deliberately compact. Add more context only when it helps the author verify the claim. Review comments are an interface for a decision, not a place to display everything the reviewer considered.

Pull request review comment questions