Severity-tagged review comments

At the flat	In the review
each defect gets its own entry: the appliance, where it is, what she found	one finding, one comment, on its line (steps 1, 3)
the entry says what it means for the tenant and what to do	impact and suggestion (steps 4, 5)
a boiler that fails the flue spillage test is switched off until fixed, however small the repair	blocking follows the run (step 6)
ugly boxing round the pipes is not a defect and goes on no entry as one	a preference never blocks (step 7)
"boiler dodgy" leaves the landlord unable to tell whether to call someone today or next year	"this is wrong" (the common trap)
she writes about the appliance, never about the last fitter	the code, not the person (step 11)

In real terms: each comment carries a severity the author can trust and the reasoning to act on it, so the blocking finding stands out from the preferences.

Breaks down when: gas classes are fixed by an industry procedure and the record is final; review labels are agreed by the team, and the author can reply and change your mind (step 12).

Contrasting case: a one-person project with no author to inform needs no labels; the code's next reader is you.

4.1 ExamHall EXH-3110: "this is wrong" ships the bug

EXH-3110: a practice review of MR !3412, which regrades one question after its key is corrected. The results page shows each candidate's total, which the store recomputes only when asked:

// src/grading/regrade-question.ts (as merged)
import type { MarksStore } from '../infra/types';

export async function regradeQuestion(store: MarksStore, examId: string, questionId: string, newKey: string) {
  const answers = await store.answersTo(examId, questionId);
  for (const a of answers) {
    const mark = a.choice === newKey ? 1 : 0;
    await store.setMark(a.id, mark);
  }
  return { regraded: answers.length };
}

The first review held three bare comments. The author renamed the variable, answered the question and replied "works for me, the test passes" to the third. The second review labelled the same three findings, and each was run:

4.1 MR's test (q3, key b -> c): marks c1 0 · c2 1 · c3 1 · green
4.1 first review: "this is wrong" (l.9) · "a?" (l.5) · "`a` is a bad name" (l.5) · labelled 0 of 3
4.1 as merged, the results page after the regrade: totals c1 3 · c2 2 · c3 2 (the marks add to 2 · 3 · 3) · red
4.1 issue (blocking): its suggestion applied (recomputeTotals before returning): totals c1 2 · c2 3 · c3 3 · green · MR's test green
4.1 nitpick: rename a -> answer: marks c1 0 · c2 1 · c3 1, totals c1 3 · c2 2 · c3 2 · no result changes, non-blocking
4.1 question: asks for a fact the code does not hold · no run decides it, the author answers
4.1 second review: "issue (blocking): after the loop nothing recomputes the totals, so the results page keeps the old scores (c1 shows 3 where its marks add to 2); call `store.recomputeTotals(examId)` before returning."
4.1 second review: "question (non-blocking): can `answersTo` return a withdrawn candidate's answers? If it can, should they be regraded?"
4.1 second review: "nitpick: `a` would read better as `answer`."
4.1 second review parsed: issue (blocking) · question · nitpick · 1 of 3 blocking

MR !3413 is the author's fix after the second review: the suggested line and the rename. The bare "this is wrong" named no line, input or effect, so the author could check nothing (steps 3–5). Labelled, the one defect is the only blocking comment, its impact is a total the run printed, and its suggestion is the line that turned the test green; the rename changes no mark, so it stays a nitpick (steps 6, 7).

4.2 Proof domain, media streaming: the same finding, written twice

A video platform's MR plays the next episode of a 12-episode season when one ends:

// src/player/next-episode.ts (after the MR)
import type { Episode } from '../infra/types';

export function nextEpisode(season: Episode[], currentId: string): Episode | null {
  const order = season.map((e) => e.number).sort();
  const current = season.find((e) => e.id === currentId);
  if (!current) return null;
  const next = order[order.indexOf(current.number) + 1];
  return season.find((e) => e.number === next) ?? null;
}

4.2 MR's test (a 3-episode season, after episode 1): 2 · green
4.2 as merged, a 12-episode season: after 1 -> 10 · after 9 -> none · after 12 -> 2
4.2 with sort((a, b) => a - b): after 1 -> 2 · after 9 -> 10 · after 12 -> none · MR's test green
4.2 first version: "this sort is wrong." · unlabelled · names no line, input, effect or change
4.2 second version: "issue (blocking): line 4's `sort()` orders episode numbers as text, so in a 12-episode season autoplay goes from episode 1 to 10, stops after 9, and plays 2 after the finale; sort with `(a, b) => a - b`."
4.2 second version: issue (blocking) · each effect it states matches the run (1 -> 10, 9 -> none, 12 -> 2) · its change turns them to 2, 10, none

The first version is a verdict. The second names the line and the input, states the impact the run printed and gives the change, so the author can verify it without a reply (steps 2–5). Why sort() with no compare function orders numbers as text belongs to the JavaScript records.