10 - Review Checklist

The scan list reviewers run before approving a UI change. Authors run it first (self-review), then reviewer runs it again. AI code reviewers load this doc via .claude/rules/design-system.md.

This is the checklist. If a change violates an item, the review is “request changes” — not “approve with comments”. Letting violations through trains everyone that the spec is optional. It isn’t.

Prereqs: know the spec. If you don’t, read 00-overview.md first.

How to use this doc

As an author, before requesting review:

Run pnpm lint:compliance. The Attio compliance sweep is a CI hard gate (.github/workflows/ci.yml § attio-compliance). Local runs match CI exactly. If the script reports a new violation on your branch, fix it before pushing. Exceptions go in as skip rules in scripts/check-attio-compliance.sh, not as // eslint-disable-style suppressions.
Open your diff.
Walk through the sections below top-to-bottom.
Anything you’re not sure about → re-read the referenced spec section.
Don’t push fixes in a hurry — a clean first-pass review is faster than 4 rounds of nits.

As a reviewer:

Read the PR description and look at the screens (Loom / screenshot / dev preview).
Walk through the sections below.
For each violation: comment with the section number and a link to the spec doc. No paraphrasing — link the source.
Approve only when every section passes.

As an AI code reviewer (this file auto-loads via .claude/rules/design-system.md):

Read the changed files.
For each file, run the relevant sections below against the code.
Report findings in file:line — violation — link to spec section format.
Sort findings by severity: blocker (violates a hard rule), nit (style/minor), question (ambiguous).

Review severity scale

Label	Meaning	Action
`blocker`	Violates a hard rule in the spec (semantic tokens, archetype, etc.)	Must fix before merge
`major`	Breaks a pattern but could ship	Fix before merge unless explicitly deferred
`minor`	Style / consistency nits	Fix if trivial; otherwise defer
`question`	Unclear intent; may or may not be a problem	Author clarifies

Bias to blocker for: accessibility, raw-color usage, breaking inline-feedback rules, destructive-action missing confirm, layout archetype violation. Those erode the product fast.

Section 1 — Principles alignment

Check against 01-principles.md.

Doesn’t introduce a new user-facing setting without a justification paragraph (C2 opinionated defaults).
Doesn’t introduce a rejected synonym (Bot/Persona/Skill/Plugin for Agent/Tool, etc. — see 07-voice.md glossary).
Doesn’t add motion without a declared category (Adding / Receding / Normal).
Doesn’t add a new @na/ui primitive when an existing one could stretch (C8 restraint).
If it’s a tradeoff call: is it the option a thoughtful designer would recognize as “obviously right”?

Section 2 — Foundations

Check against 02-foundations.md.

Color

Zero raw Tailwind colors (bg-blue-500, text-gray-700, border-red-200). Semantic tokens only.
Disabled states use opacity-50, not a custom grey.
Works in both light and dark mode (run the dark-mode toggle; no hardcoded bg-white).
On neutral decisions, both buttons are variant="outline".

Typography

No font-bold (700) — font-semibold (600) is max.
Base font size ≥ text-sm for admin, text-base for chat.
Headings use text-base font-semibold; section titles don’t go bigger.
Numbers in columns use tabular-nums and text-right.

Spacing

All spacing on the 8px ladder (gap-2, p-4, space-y-6) or 4px sub-grid.
No off-grid values (p-5, p-7, gap-3.5).
Form-field spacing = space-y-4; label→input = space-y-1.5.

Density, motion, latency

Surface density matches the contract (admin 40px rows, chat 56px thread rows).
Animation categories: each transition declared (Adding 150-200ms ease-out / Receding 80-100ms ease-in / Normal 150-250ms).
Receding is faster than Adding.
No silent wait > 500ms — skeleton, spinner, or optimistic update.

Icons

Icons from lucide-react. No inline SVG for decorative use.
Icons sized per the Size table (16 / 20 / 24 px by context).
Decorative icons have aria-hidden="true".
Same concept → same icon (search always Search, delete always Trash).

Section 3 — Layout

Check against 03-layout.md.

Archetype

Every new page declares exactly one archetype (List / Detail / Form / Workbench / Dashboard / Canvas / Focus). Result and Exception are templates within archetypes, not archetypes themselves.
Detail picks one of 3a Split / 3b Tabbed / 3c Drawer. 3a Split only when the main area has genuine activity/timeline content (not “more properties”). Settings sub-tabs are Form, not Detail.
Multi-step wizards use Focus archetype (<FocusLayout> + <Stepper>), not oversized Dialogs.
Two pages of the same archetype feel like siblings, not cousins.

Focus archetype specifics

Focus route mounted inside <AppLayout /> children — Sidebar stays visible. Routes registered outside AppLayout overlay the nav and are forbidden.
<FocusLayout> header is two rows: row 1 ✕ + breadcrumb, row 2 stepper. Single-row crowding (stepper next to title) is forbidden.
Step controls live in the sticky bottom action bar via the actions prop on <FocusLayout>. Floating-pill clusters (fixed bottom-6 right-6 bordered chip) are forbidden.
Body uses bg-background — no surface tint. The “tinted body rgb(249,250,251)” rule was a misread of old Attio’s sidebar color and is no longer canonical.
No role="dialog" / aria-modal on the focus surface — focus pages are pages, not modals.
Confirm-on-discard wired for every exit path (✕, Cancel, Esc, sidebar/back). A silent no-op (handler returns early with no UI) is a blocker. See 06-flows.md § Exit-path close behavior.
Final commit is one atomic backend call when feasible — wizard does NOT do POST then PUT and call it “atomic with rollback”.

Shell + structure

Page uses <ListPageBar> or <DetailPageBar>, never custom chrome.
Content uses <PageContent>, not a raw <div className="flex-1 overflow-auto">.
<ActionBar> is a sibling of <PageContent>, not inside it.
No <h1> outside the page bar.
No breadcrumbs.
No position: sticky / fixed inside <PageContent>.
No nested scroll containers — only <PageContent> scrolls.
No min-h-screen or whole-page scroll.

Detail page specifics

Back button goes to parent list URL, not history.back().
Max 8 tabs. More → split into two detail pages.
Tabs text-only (no icons in L3).
Max 1 primary action in the page bar.

Action zones

Primary actions in page bar (primary size="sm").
Section actions in <ContentSection> (outline sm).
Row actions in table cells (ghost icon-xs, max 3 visible; overflow → <DropdownMenu>).
Save/Cancel in <ActionBar>, never in sidebars or section headers.
Dialog footers use verb labels (“Delete”, “Create”) — never “OK” / “Confirm”.

Tables + cards

Tables wrapped in <div className="rounded-lg border">.
Numeric columns: text-right tabular-nums on header and cell.
Interactive rows: hover:bg-muted/50 transition-colors.
Cards at rest: border only, no shadow.
Selected card: ring-2 ring-primary, not border-primary.

Section 4 — Components

Check against 04-components.md.

Section 5 — Patterns

Check against 05-patterns.md.

Feedback

Correct errorPlacement per context (toolbar → "tooltip" default, form → "below").
No toast that duplicates inline feedback.
Background-only events use toast (SSE reconnect, async job finished, auth expired).
100ms feedback budget — every action has a visible reaction within 100ms.

Data entry

Labels above inputs (never left-aligned).
Required fields: * on label + aria-required="true" on input.
Validation: on submit for server checks; on blur for format; never while typing (except password strength / char counter).
Help text as <p className="text-muted-foreground text-xs">, not placeholder.
Placeholder is an example, not a label.

Data display + format

Tables for tabular data; <CardGrid> only for heterogeneous / pick-one content.
Relative dates for < 7d, absolute for older, tooltip always has exact timestamp.
Numbers formatted via Intl.NumberFormat — not string concat.
Currency/percent/date formats match the Data Format table.

Button, list, empty

Button labels are verbs (“Save changes”, “Create agent”) — not “OK” / “Submit”.
Lists with multiple actions: three-tier disclosure respected (always / hover / toggle).
Max 3 inline row actions.
Empty states have icon + title (fact) + description (next step) + primary action.
Table empty body picks the right chrome mode — 'standalone' (drop) for primary lists, 'in-table' (keep) only for streaming or stacked sub-tables. See 05-patterns.md → Tables: drop chrome vs keep chrome.
No first-run tour re-shown to returning users.

Error handling

Field error → <FormMessage> with aria-describedby.
Form error → <FormError> above <ActionBar>.
Network error on load → error state with [Retry].
404 / 403 → Exception archetype page with plain-language title + recovery action.

Section 6 — Flows

Check against 06-flows.md.

Create flow matches Pattern A (≤4 fields → Dialog) or Pattern B (many fields → dedicated page).
Update: settings-style uses <ActionBar> with isDirty; inline uses click-to-edit or icon-edit per 3-tier.
Delete: routine → <ConfirmPopover>; catastrophic → <ConfirmDialog> with verb-labeled button; extra friction (type-to-confirm) for truly irreversible.
Unsaved-changes guard present on dirty forms (useBlocker + discard dialog).
Bulk actions: toolbar appears on selection, <N> selected label, “Clear selection” action.
Multi-step wizard only for > 5 logical inputs; never 1-step; never 8+ step.
Save model is consistent on the page (don’t mix explicit Save with auto-save).
Undo offered on reversible mutations, with a 5-10s window.
Long-running jobs: inline progress for < 30s, background job toast for > 30s.

Section 7 — Voice

Check against 07-voice.md.

Section 8 — Accessibility

Check against 08-accessibility.md.

Quick manual check (for any screen with interactions)

Can you complete the primary task keyboard-only?
Does Esc close every overlay you open?
Does the screen hold at 200% zoom with no horizontal scroll?
Run VoiceOver/NVDA for 60 seconds on the screen. Does the structure make sense?

Section 9 — i18n + testing

Check against 09-i18n-testing.md.

i18n

Every user-facing string wrapped in t() — including tooltips, placeholders, aria-labels.
No string concatenation across translated pieces.
Plurals use ICU plural, not ternary expressions.
Dates/numbers via Intl.* with i18n.language.
New keys added to en/common.json with English translation present.
No hardcoded English in new files.

Testing

Every new interactive component has a *.test.tsx file.
Tests use role/label/text queries, not class names or DOM internals.
Each state (idle / pending / success / error) tested.
Mutation stubs use the hoisted vi.hoisted pattern (for TanStack Query mocks).
No regressions in existing test count (baseline: admin 177/15, ui varies).
User events via userEvent.setup(), not synthetic fireEvent.

Section 10 — Code hygiene (cross-cutting)

Not design-system-specific but part of every UI review.

React Rules of Hooks — blocker

No use* hook called after an early return inside a component. Every useState / useEffect / useCallback / useMemo / useRef / useContext / useReducer / custom use* must run on every render in the same order. A hook below if (isLoading) return <Skeleton/> crashes the app at runtime with “Rendered more hooks than during the previous render.”
All component hooks declared in a single block at the top of the function, before any early-return guards.
No hooks inside loops, conditions, or nested functions. (Custom hooks are fine — they’re just functions that contain hooks.)
eslint-plugin-react-hooks must be passing — react-hooks/rules-of-hooks is error at the repo level; CI blocks merges that violate it.

If you spot this in a diff: the fix is almost always “move the offending useState / useEffect up with the other hook declarations at the top of the component.” See the ConversationDetailPage.tsx:196 incident for reference.

Other hygiene

Imports via @na/ui/components/<name> and @/<app-local> — no relative ../../../ cross-package imports.
No dead code, no commented-out code, no console.log left behind.
Feature-flagged? If yes, flag pattern documented and guard consistent.
Error paths covered (not just the happy path).
No new any types without justification in a comment.
TypeScript strict — tsc --noEmit passes for the changed packages.
Lint clean (pnpm lint).
Tests pass (pnpm test).
Prettier clean (hooked via lint-staged).

Section 11 — PR hygiene

PR title uses conventional prefix: feat(<scope>): ..., fix(<scope>): ..., docs(<scope>): ..., refactor(<scope>): ..., chore(<scope>): ....
PR description: what, why, how (screenshot or Loom if visual).
Diff is < 500 lines when possible. If bigger: is there a natural split? If not, call it out in the description.
Commits are logical units (no WIP or asdf messages in final history — squash or rewrite).
No unrelated files staged (don’t commit tsconfig.tsbuildinfo, .env, IDE configs).
No secrets in the diff (API keys, tokens, passwords).
Breaking changes called out in the description with a migration note.

AI-slop blacklist (scan any new UI for these)

If your change introduces UI, scan it for the ten patterns below. Every one of them screams “AI-generated” and erodes the “Attio-grade craft” the rest of this spec is protecting. None of them are outright errors the compliance script catches — they’re taste violations that only a human reviewer (or a careful author) spots. Flag as major when you see them.

Purple / violet / indigo gradient backgrounds — or any “blue-to-purple” hero. nx-agent’s accent is indigo #475BA1 as a solid; no gradient backgrounds on surfaces.
The 3-column feature grid — icon-in-colored-circle + bold title + 2-line description, repeated 3× symmetrically. The single most recognizable AI layout. If you’re tempted to add one, pick one feature and promote it, or use a flat <ContentSection> with a short paragraph.
Icons in colored circles as section decoration — the SaaS-starter-template look. Our icons live next to labels (<PropertyRow icon={Globe} label="Domain">) or inside buttons (<Button><Plus />Add</Button>). Icons don’t need chrome.
Centered everything — text-align: center on all headings, descriptions, and cards. Page content aligns left. Centering is reserved for empty states, landing pages, and focus wizards.
Uniform bubbly border-radius on every element — our radius-md is 6px, radius-lg is 8px. A rounded-2xl on a settings card is a tell.
Decorative blobs, floating circles, wavy SVG dividers — if a section feels empty, add better content, not decoration.
Emoji as design elements — 🚀 in headings, ✨ as bullet points, 🎉 in empty states. Use lucide-react icons everywhere. The one carve-out is a single emoji in <EmptyState icon="🔧"> for warmth — not a garnish elsewhere.
Colored left-border on cards (border-left: 3px solid <accent>). Only <Alert variant="destructive"> and the Danger Zone card use colored borders; other cards use the neutral border-border.
Generic hero copy — “Welcome to [X]”, “Unlock the power of…”, “Your all-in-one solution for…”. nx-agent copy names the product and the verb: “Configure your agent”, “Review conversations”.
Cookie-cutter section rhythm — hero → 3 features → testimonials → pricing → CTA, every section the same height. nx-agent is a workspace, not a landing page; marketing rhythms don’t apply. Sections earn their heights.

Source: adapted from OpenAI “Designing Delightful Frontends with GPT-5.4” (Mar 2026) and gstack’s design methodology. The compliance script can’t catch these (they’re pattern-level, not token-level). Human and AI reviewers must.

Common review patterns (AI reviewers should report these)

Rank these blocker by default. Use lower severity only if the context clearly warrants it.

Smell in diff	Severity	Spec reference
`<button className="...">`	`blocker`	04-components.md → Button
`<table>` / `<thead>` / `<tbody>` without `<Table>`	`blocker`	03-layout.md → Tables
`toast.success('...saved')` / `toast.error('...failed')` after a click	`blocker`	05-patterns.md → Feedback
`window.confirm(...)` / `alert(...)`	`blocker`	04-components.md → ConfirmPopover
`text-gray-` / `bg-blue-` / hex colors	`blocker`	02-foundations.md → Color
`font-bold`	`major`	02-foundations.md → Typography
`<div className="flex-1 overflow-auto">`	`blocker`	03-layout.md → PageContent
`p-5` / `p-7` / `gap-3.5`	`major`	02-foundations.md → Spacing
Button with no tooltip + no visible text label	`blocker`	04-components.md → Button
Dialog with no `<DialogTitle>`	`blocker`	08-accessibility.md → Dialogs
Form input with no `<Label>`	`blocker`	08-accessibility.md → Forms
Hardcoded English string (not in `t()`)	`major`	09-i18n-testing.md → i18n
`confirmLabel="OK"` or `confirmLabel="Confirm"`	`major`	07-voice.md → Destructive button copy
Trailing period on a button or label	`minor`	07-voice.md → Punctuation
New `@na/ui` primitive with no justification	`major`	01-principles.md → C8 Restraint
More than 1 primary button in a page bar	`major`	03-layout.md → Action placement
More than 3 always-visible row actions	`major`	05-patterns.md → Data List
Missing `<EmptyState>` on an empty list	`major`	05-patterns.md → Empty State
Bare `<Table>` + `<TableHeader>` rendered above an empty body (column headers floating above no rows, with no streaming/sub-table justification)	`major`	05-patterns.md → Tables: drop chrome vs keep chrome
`emptyMode="in-table"` on a primary list page (blank-slate / filtered-to-zero) where the default `'standalone'` would do	`minor`	05-patterns.md → Tables: drop chrome vs keep chrome
Full-page spinner	`major`	03-layout.md → Loading
`use*` hook (useState / useEffect / etc.) below an `if (...) return`	`blocker`	Section 10 — React Rules of Hooks
Hook inside an `if` / loop / nested function	`blocker`	Section 10 — React Rules of Hooks
Custom child of `<SheetContent>` without `px-4` (buttons/dividers flush to drawer edge)	`blocker`	04-components.md → Sheet content layout
`mt-*` on a direct child of `<SheetContent>` (stacks on top of parent’s `gap-4`)	`major`	04-components.md → Sheet content layout
`border-b` mid-drawer not wrapped in `px-4` (extends edge-to-edge, misaligns with SheetHeader)	`blocker`	04-components.md → Sheet content layout
`<ScrollArea>` inside `<Sheet>` with fixed `h-[calc(...)]` instead of `min-h-0 flex-1`	`major`	04-components.md → Sheet content layout
Focus route registered outside `<AppLayout />` (overlays Sidebar)	`blocker`	03-layout.md → Template F
`<FocusLayout>` with `fixed inset-0` / `role="dialog"` / `aria-modal`	`blocker`	04-components.md → FocusLayout
Floating-pill action cluster in a focus page (`fixed bottom-6 right-6` bordered chip)	`blocker`	03-layout.md → Forbidden action patterns
Stepper crammed onto the same row as breadcrumb in `<FocusLayout>` header	`blocker`	04-components.md → FocusLayout
Tinted body on a focus page (`bg-[rgb(249,250,251)]` or any non-`bg-background` body bg)	`major`	04-components.md → FocusLayout
Wizard close handler returns early on `isDirty` with no UI (silent no-op on ✕ / Cancel / Esc)	`blocker`	06-flows.md → Exit-path close behavior
`<ConfirmPopover>` with default `align="end"` on a left-side trigger (popover clips off-screen)	`major`	04-components.md → ConfirmPopover
Wizard does two-step submit (`POST agent` then `PUT tools`) when backend supports atomic	`major`	06-flows.md → Multi-step wizard § Multi-API submit

For AI reviewers — output format

When invoked (via Skill tool calling review, design-review, or directly with this file in context), produce findings in this format:

<file>:<line> — <severity> — <brief explanation> — see [spec section](link)

Group by severity (blocker first). End with a summary line:

Summary: 3 blockers, 5 major, 2 minor, 1 question.

If the PR passes clean: Design-system review: PASS. No violations found.

Be specific (file:line, not “somewhere in the component”). Be terse. Be linkable.

When a rule should break

Occasionally a rule doesn’t fit. The override path:

Comment in the PR why the rule doesn’t apply here.
If the reviewer agrees → merge.
If the exception is worth codifying → open a follow-up to update the spec.

Exceptions accumulate into principles. One-off escapes without documentation become tomorrow’s inconsistency.

This is the final doc in the spec. Everything else points here when it’s time to review.