Review order and the verdict

A team chat's MR !4772 adds a typing indicator. Ana reviewed its files in the order the tool lists them and left eight line comments:

• room-view.js line 8: pass the room, not just its id
• typing-indicator.js line 2: IDLE needs its unit
• typing-indicator.js line 4: name it createTypingIndicator
• typing-indicator.js line 5: a template literal reads better
• typing-indicator.js line 6: last → lastSentAt
• typing-indicator.js line 9: invert the if
• typing-indicator.test.js line 6: sent → messages
• typing-indicator.test.js line 11: also check the event's fields

// src/typing/typing-indicator.js (MR !4772)
// Typing indicator: its own channel, one per room.
const IDLE = 3000;

export function startTyping(socket, roomId) {
  const channel = socket.channel('typing:' + roomId);
  let last = 0;
  return {
    keypress(userId, now) {
      if (now - last < IDLE) return;
      last = now;
      channel.send({ userId, typing: true });
    },
    stop(userId) {
      channel.send({ userId, typing: false });
    },
  };
}

Only then did she open presence.js:

// src/presence/presence.js (main, unchanged by the MR)
// Per-room presence over the app's one open socket.
export function createPresence(socket) {
  return {
    online(roomId, userId) {
      socket.send({ kind: 'presence', roomId, userId, state: 'online' });
    },
    away(roomId, userId) {
      socket.send({ kind: 'presence', roomId, userId, state: 'away' });
    },
  };
}

Her ninth comment reads: "issue (blocking): presence.js already sends per-room events over the open socket; send typing as a presence state there, rather than opening a second channel in a new module." The author agrees and pushes a rework that does exactly that. It deletes typing-indicator.js and typing-indicator.test.js and changes presence.js and room-view.js:

// src/rooms/room-view.js (the rework)
import { createPresence } from '../presence/presence.js';

// Opens a room: marks the user online and reports typing through presence.
export function openRoom(socket, room, me) {
  const presence = createPresence(socket);
  presence.online(room.id, me.id);
  return { onKey: () => presence.typing(room.id, me.id) };
}

A box office's MR !4774 adds a sweep that releases expired seat holds:

// eslint.config.js (main, unchanged by the MR)
export default [
  {
    files: ['src/**/*.js'],
    rules: { curly: 'error', 'prefer-const': 'error', 'no-unused-vars': 'error' },
  },
];

CI runs ESLint with this config on every MR and fails the pipeline on any error. You have drafted four comments:

1. line 5: put the if body in braces.
2. line 3: released is never reassigned; declare it with const.
3. line 5: a hold that ends exactly at nowMs stays held; compare with <=.
4. line 4: h says little; call it hold.

A video platform's MR !4775 changes only src/player/subtitles.js and src/player/subtitles.test.js, so the player falls back to a viewer's second language. You have reviewed it and found nothing to block. Its pipeline runs 9 tests and 1 fails: src/billing/renewal.test.js › "renews a 31 January subscription on 28 February". It fails the same way on every retry. renewal.test.js imports only ./renewal.js.

Issue #4120 on a payments ledger: a payment of 5,000 cents already refunded 3,000 must refuse a second refund of 3,000. MR !4776 changes one function:

// src/refunds/refund.js (MR !4776)
// Refuses a refund larger than what is left of the payment after earlier refunds.
export function canRefund(payment, amountCents) {
  const refunded = payment.refunds.reduce((sum, r) => sum + r.amountCents, 0);
  return amountCents > 0 && amountCents <= payment.amountCents - refunded;
}

Your drafts:

1. The sum of earlier refunds could live on Payment as refundedCents(), so the statement page can use it too.
2. r would read better as refund.
3. After two refunds of 2,500, can a third refund of 1 cent still go through?

MR !4779 lets a viewer resume each episode where they stopped. It links no issue; its description and its tests say what resuming must do. It adds a resume_positions table, an endpoint and three changes to the player.

A teammate opens a 2,000-line merge request on ExamHall's grading, written mostly with an AI assistant. Its description is one line: "Grading v2: big refactor + new rubric editor + fixes". The reply is a short text file.

A team chat's MR adds emoji reactions to messages, stored in a new column, rx, on the messages table. Its code is clean and its tests pass. ADR-14, agreed last month, moves everything that annotates a message (edits, pins, reactions) into the new message-events service, and the messages table's extra columns are being removed this quarter. The author was on leave that month. Your first reply goes out before any line comment.

A box office's MR !4777 lets staff extend a seat hold. Ben reviewed readability first and left six comments:

• line 1: say how many minutes
• line 2: EXTRA → EXTRA_MINUTES
• line 4: nowMs → now, as in the other hold functions
• line 5: 60 * 1000 reads better as 60_000
• line 6: t → newEnd
• line 7: fold t into this line

Then he found the bug: an extension starts from now, not from the hold's current end, so a hold extended early gains less than 10 minutes, or even ends sooner. The author's fix:

// src/holds/extend.js (the author's fix)
// Extends a seat hold by the box office's extra minutes.
const EXTRA = 10;

export function extendHold(hold, nowMs) {
  const endsAtMs = Math.max(hold.endsAtMs, nowMs) + EXTRA * 60 * 1000;
  return { ...hold, endsAtMs };
}