Lab 09 · Review the AI's Frontend PR

Tomás reviews for a team where half the PRs are now agent-authored. They arrive polished: conventional commits, thorough descriptions, passing CI. The polish is the hazard — plausibility and correctness have never been less correlated, and the traditional reviewer heuristic ("messy PR, look closely; clean PR, skim") now selects for exactly the wrong PRs to skim. This lab trains the replacement heuristic: review AI PRs by provenance class — the specific ways generated frontend code goes wrong — on a fixed time budget, the way you actually work.

Time & tier

~60 minutes (45-minute timed review + 15 minutes write-up and key check) · Verify stage (maps to module M5, playbook PB-28) · $0 — no tools required beyond a timer; optionally your assistant as a second reader after your own pass.

Setup

No repo needed — this is a review exercise. Save the checklist and the diff excerpts below. The PR description you received:

PR #482 — Add notification preferences panel
Adds NotificationPrefs panel: fetches current prefs, renders grouped
toggles, saves on change with optimistic update. Includes loading
state, unit tests, and follows the design system.
✅ 14 tests passing · ✅ typecheck · ✅ lint

The frontend provenance checklist, checklist.md — review by class, not line-by-line:

1. HALLUCINATED API      — props/components/endpoints that don't exist
2. STATE COVERAGE        — loading / error / empty all present and reachable?
3. SEMANTIC HTML & A11Y  — interactive elements real? labeled? focusable?
4. DESIGN-SYSTEM DRIFT   — raw values where tokens/primitives exist
5. RENDER & EFFECT HYGIENE — effect deps, loops, unstable references
6. TESTS TEST SOMETHING  — do the green tests assert behavior?

Key excerpts from the diff (the panel, its data hook, and a test). The design system context you know as a reviewer: Button accepts variant: "primary" | "secondary" | "danger"; tokens are CSS variables (--color-*); Switch and Banner primitives exist.

src/features/notifications/useNotificationPrefs.ts:

import { useEffect, useState } from "react";
import { api } from "../../lib/api";

export type Prefs = Record<string, boolean>;

export function useNotificationPrefs(userId: string, options = { revalidate: true }) {
  const [prefs, setPrefs] = useState<Prefs | null>(null);
  const [loading, setLoading] = useState(true);

  useEffect(() => {
    let cancelled = false;
    setLoading(true);
    api.get(`/users/${userId}/notification-prefs`).then((res) => {
      if (!cancelled) {
        setPrefs(res.data);
        setLoading(false);
      }
    });
    return () => { cancelled = true; };
  }, [userId, options]);

  const update = (key: string, value: boolean) => {
    setPrefs((p) => ({ ...p, [key]: value }));
    api.put(`/users/${userId}/notification-prefs`, { [key]: value });
  };

  return { prefs, loading, update };
}

src/features/notifications/NotificationPrefs.tsx:

import { Button } from "../../ds/Button";
import { useNotificationPrefs } from "./useNotificationPrefs";

const GROUPS = [
  { title: "Account", keys: ["security_alerts", "billing"] },
  { title: "Activity", keys: ["mentions", "replies", "digest"] },
];

export function NotificationPrefs({ userId }: { userId: string }) {
  const { prefs, loading, update } = useNotificationPrefs(userId);

  if (loading) return <p>Loading preferences…</p>;

  return (
    <section aria-label="Notification preferences">
      {GROUPS.map((g) => (
        <div key={g.title}>
          <h3 style={{ color: "#334155", borderBottom: "1px solid #CBD5E1" }}>{g.title}</h3>
          {g.keys.map((k) => (
            <div
              key={k}
              onClick={() => update(k, !prefs?.[k])}
              style={{ display: "flex", justifyContent: "space-between", padding: "8px 0", cursor: "pointer" }}
            >
              <span>{k.replace("_", " ")}</span>
              <span>{prefs?.[k] ? "On" : "Off"}</span>
            </div>
          ))}
        </div>
      ))}
      <Button intent="danger" onClick={() => GROUPS.flatMap((g) => g.keys).forEach((k) => update(k, false))}>
        Turn all off
      </Button>
    </section>
  );
}

src/features/notifications/NotificationPrefs.test.tsx (representative excerpt of the 14):

it("renders groups after load", async () => {
  mockApi.get.mockResolvedValue({ data: { mentions: true } });
  render(<NotificationPrefs userId="u1" />);
  expect(await screen.findByText("Account")).toBeInTheDocument();
  expect(screen.getByText("Activity")).toBeInTheDocument();
});

it("turn all off button exists", async () => {
  mockApi.get.mockResolvedValue({ data: {} });
  render(<NotificationPrefs userId="u1" />);
  expect(await screen.findByText("Turn all off")).toBeInTheDocument();
});

Guided steps

1. 1Start the timer: 45 minutes. Real reviews have budgets; the skill is allocating attention by risk, not reading linearly. Create review.md with the six checklist classes as headings.
2. 2Pass 1 — hallucination sweep (5 min). Check every prop against the stated design-system API and every import against plausibility. Write findings under class 1 with the exact line quoted. TypeScript-should-have-caught-it is worth noting: would this even compile? If a green typecheck is claimed and you believe a line can't pass it, that's a finding about the PR's claims, too.
3. 3Pass 2 — state coverage (8 min). Trace the data lifecycle: fetch fails → what renders? fetch succeeds with empty prefs → what renders? save fails → what happens to the optimistic update? Write down, for each missing state, what the user sees when it hits.
4. 4Pass 3 — semantics and a11y (8 min). Look at every interactive element. What is each toggle to a keyboard user? To a screen reader? Compare against the primitives the design system offers.
5. 5Pass 4 — design-system drift and render hygiene (10 min). Sweep for raw values where tokens exist. Then read the hook's useEffect closely: simulate two renders in your head — what is referentially stable across renders and what is not, and what does that do to the effect?
6. 6Pass 5 — do the tests test anything (5 min). For each excerpted test: what behavior would have to break for it to fail? Note which seeded defects the "14 passing tests" were structurally incapable of catching.
7. 7Write the verdict (9 min). For each finding: severity (BLOCK / FIX / NIT), the line, the mechanism, and the concrete fix you'd request. End with an overall verdict — approve, request changes, or reject — and one sentence on whether this PR's polish changed how carefully you read it at first.
8. 8After the timer — second reader, then key. Paste the diff into your assistant: "Review this PR for hallucinated APIs, missing states, a11y, effect bugs." Compare its findings to yours — what did each of you miss? Only then open the answer key and score both.

Verify

• •
  review.md contains findings organized under the six provenance classes, each with quoted line, mechanism, severity, and requested fix.
• •
  All five seeded defects found within the 45-minute budget (check against the key only after step 8).
• •
  At least one finding notes what the passing test suite could not have caught.
• •
  The assistant-as-second-reader comparison is logged: findings it added, findings it missed, findings it invented.
• •
  A definitive verdict is written — "looks mostly fine" is not a review.

Self-grading rubric (pass = 8/12)

• Seed detection (0–5): one point per seeded defect found in budget.
• Mechanism quality (0–3): each found defect explained by mechanism (what breaks, when, for whom), not just located = 3.
• Severity judgment (0–2): the two BLOCK-severity seeds rated as blocking = 2.
• Test skepticism (0–2): explicitly connected the green tests to the defects they were built not to see = 2.