Sign inReview today's PR

Code review example: Requirement mismatch: the promised 'jump to last page' control can't be built in Node

Retired from the DiffDojo practice pool in October 2026; the review below is the canonical answer. An AI wrote this pull request for the task underneath. Read the diff and decide what you would say before you scroll to the review.

Node · Requirement fidelity · intermediate

The task

Paginate the invoices endpoint

  1. GET /invoices returns invoices in pages of 20.
  2. Accept a page query param (1-based); default to page 1.
  3. The response must include items, page, and totalPages, so the client can render a 'jump to last page' control.

Adds pagination to the invoices list. Slices the result set by page with a sensible default and returns the items with the current page number. Keeps the response shape minimal.

The admin billing screen; some accounts have thousands of invoices.

The diff

src/routes/invoices.js+13
1+ const PAGE_SIZE = 20;
2+
3+ router.get('/invoices', requireAuth, async (req, res) => {
4+ const page = Math.max(1, parseInt(req.query.page, 10) || 1);
5+ const offset = (page - 1) * PAGE_SIZE;
6+ const items = await db.invoices.findAll({
7+ where: { accountId: req.accountId },
8+ order: [['createdAt', 'DESC'], ['id', 'DESC']],
9+ limit: PAGE_SIZE,
10+ offset,
11+ });
12+ return res.json({ items, page });
13+ });

The review

Issue 1: Requirement mismatch: the promised 'jump to last page' control can't be built

src/routes/invoices.js, line 12

  1. The response contract requires totalPages. Requirement 3 lists items, page, and totalPages, because the client must render a 'jump to last page' control, which needs to know how many pages exist. The code returns only items and page.
  2. Nothing is computed for it. There is no count query, so even the data to fill totalPages is never fetched, the missing field is not a typo, the endpoint simply doesn't produce it.
  3. The mechanics are otherwise correct, pages of 20, 1-based page, the default, and the offset math all match the spec, which is what makes it look done. Only holding the response against requirement 3 reveals that a promised feature cannot be built.

The fix: run a count for the account and return totalPages alongside the items, totalPages: Math.ceil(total / PAGE_SIZE).

The fix

  const total = await db.invoices.count({ where: { accountId: req.accountId } });
  return res.json({ items, page, totalPages: Math.ceil(total / PAGE_SIZE) });

Why it matters

A recognizable pattern (pagination) in the diff → ask: "does every specific number and contract field from the ticket appear in the code?" Half-implemented requirements are the classic AI slip: the pattern gets built, the specifics get dropped.

Real-world stakes: the endpoint works in every manual test, it pages fine, so it merges, and the failure lands on someone else's desk later: the frontend dev picks up the 'last page button' task and discovers the API never gave them totalPages, turning a one-line review catch into a cross-team change.

Read more: api pagination totalPages response contract, sequelize findAndCountAll pagination, acceptance criteria checklist code review, offset limit pagination express

The verdict

Request changes. Requirement mismatch: the promised 'jump to last page' control can't be built

Try a live one

Node PRs you have not seen the answer to.

Leave inline comments and a verdict, then see what you caught. Free, no signup.

Review a Node PR →

How to review Node code · All code review examples