Skip to content

Review Checklist — semantic guidance reviewers must apply

Status: Reference

AGENTS.md covers the mechanical gates (what an AST or test can prove). This file covers the semantic gates (what only a reader can judge). Apply at PR-time. If a PR violates one of these, request changes — these are not honor rules, they are review requirements.

Why split these out: mixing "an AST can detect this" rules with "a human must judge this" rules turns AGENTS.md into a wishlist where every rule feels optional. Keeping the mechanical set strict and the judgment set explicit elsewhere protects both. Items here are tracked separately for adoption / drift.


Components

CHK-COMP-1 — Loading state must show a skeleton, never let EmptyState flash

Any screen that fetches via useQuery (or similar) must render SkeletonCard / SkeletonHero / SkeletonRow while isLoading is true. Don't render EmptyState until the query has resolved with empty data. (Was COMP-7.)

Why review-only: "during loading" depends on the screen's data composition; a generic AST rule would over- or under-fire.

CHK-COMP-2 — SegmentedTabs preload choice for hook-bearing panes

If multi-tab content has panes that each fire their own useQuery / useState hooks, use SegmentedTabs preload="all". Conditional render {tab === 'x' && <Pane/>} on hook-bearing panes is forbidden because it remounts hooks on every chip tap and blocks the JS thread. (Was COMP-9.)

Why review-only: "hook-bearing pane" requires reading the pane component to see whether it calls hooks.

CHK-COMP-3 — Use SectionBlock for titled content groups on detail screens

Detail screens with multiple titled groups (e.g., "Members", "Recent matches", "Settings") use SectionBlock from @twomore/ui. Section titles always live outside the card surface. (Was COMP-11.)

Use SectionHeader only when a title row cannot wrap its body, such as a virtualized list section header or a FeedList header slot. Do not hand-roll section-label typography in feature code.

Section title rows are text-first semantic breaks. Do not add decorative leading icons; use the right slot only for real counts, actions, or configuration controls that affect that section.

Why review-only: recognizing what counts as a "titled group" is judgment.

CHK-COMP-4 — Pressable cards/rows that navigate must wire onPressIn to a prefetchX helper

Press intent populates the cache before navigation completes. Pressable cards/rows pointing to a detail screen wire onPressIn to the matching prefetchX helper from @/presentation/cache. (Was COMP-12.)

Why review-only: identifying nav-pressable surfaces requires reading the JSX.


Data / hooks

CHK-DATA-1 — Optimistic mutations when the target row is already cached

AGENTS.md DATA-2 says mutations either use createMutationHook factory OR manual useMutation paired with applyOptimisticPatches. The PR-time judgment is which — if the row being mutated is already in cache somewhere, use optimistic. Otherwise factory is fine. (Was DATA-3.)

Why review-only: determining whether the target row is already cached requires reading the cache surface area.

CHK-DATA-2 — Use useMemo, not useQuery, for pure CPU derivations

If a value is a pure CPU derivation of an upstream useQuery result (no I/O, no async), it must use useMemo, not its own useQuery. useQuery keys for pure compute churn the cache and add observer overhead. (Was DATA-4.)

Why review-only: classifying a derivation as "pure CPU" requires understanding the inputs.

CHK-DATA-3 — Don't fire the same useQuery key from two component levels

If the same query is observed by both a parent and a descendant, lift state to the parent and pass it down. Multiple observers cost extra subscription work per render with no caching benefit. (Was DATA-5.)

Why review-only: observer-tree analysis spans multiple files.


Persist stores

CHK-STORE-1 — Provider gating on _hasHydrated

AGENTS.md STORE-1 enforces the store-side contract (every store has _hasHydrated). The consumer-side discipline is: any provider whose props depend on persisted state gates root render on _hasHydrated (or useAllHydrated() for global render-gating stores). Skipping the gate causes end-to-end re-renders after async hydration. (Was STORE-3.)

Why review-only: identifying which providers have props derived from a persist store requires cross-file reading.


Migrations and adapters

CHK-MIG-1 — New table migration shape

A new-table migration must include in the same file: CREATE TABLE, ALTER TABLE … ENABLE ROW LEVEL SECURITY, all required RLS policies, and any trigger protections. (Was CONV-4.)

Why review-only: "in the same file" is a structural shape gate that the SQL planner can't enforce; reviewers verify.

CHK-ADAPTER-1 — New adapter shape

A new Supabase adapter file must contain: an explicit mapper in mappers/ rather than inline shape coercion, handleError() on every catch, column projection (.select('a, b, c'), never .select('*') on user-facing reads), and a LIMIT on every list query. (Was CONV-5.)

Why review-only: verifying these structural properties requires reading the entire adapter file.


Architecture / code organization

CHK-ARCH-1 — Cross-feature code promotion target

AGENTS.md ARCH-2 forbids feature packages from importing siblings. The PR-time judgment is where to put code that needs to be shared: domain-aware → @twomore/app; domain-free UI primitive → @twomore/ui. Don't put it in a "shared" feature package. (Was CONV-6.)

Why review-only: the placement decision is a judgment about whether code is domain-aware.

CHK-ARCH-2 — Single source of truth

One implementation per pattern. Use the existing component / utility / interface. If no variant fits, extend it — don't build a custom replacement next to it. (Was CONV-7.)

Why review-only: detecting "this duplicates pattern X" requires recognizing X.


How to use this file

  • As a reviewer: open every PR with this checklist visible. For each item, ask "does this PR touch this surface? if so, does it comply?"
  • As an author: before requesting review, scan this file for items relevant to your change and self-check. PRs that fail multiple items often signal that the change should be re-scoped.
  • When a CHK-* item becomes mechanically detectable: move it to AGENTS.md and remove the entry here. The boundary between "mechanical" and "review" should keep shrinking as eslint-plugin-twomore and ArchUnitTS rules ship.

Markdown remains the source of truth. Run yarn docs:check before handoff.