Skip to content

Review Presentation

Cindy Zhang edited this page Aug 31, 2026 · 30 revisions

Review presentation format

This is what the loop produces. It mirrors the current output contract in Reviewer Brief; that brief owns the detailed rules and evidence standards. A critic pass returns a corrected presentation in this same shape.

The review kit on the M5 is the source of truth: {review-root}/review-loop-kit/review-presentation.md.

OPERATING CONTROLS and every section other than REVIEW/INLINE are private. They are never copied into a public GitHub review. Public Review-* records must redact reservation/idempotency identities and real GChat mention mappings; at most they say verified privately. This guidance does not resume or mutate any Automation; all review Automations stay disabled until the parent verifies the durable guards and separately authorizes a resume.

Current slot order

Private preface: OPERATING CONTROLS. Substantive slots: CONTEXT & ROUTING → PROBLEM → SOLUTION → ARCHITECTURE → IMPACT → API → THEMING → BREAKING → PERFORMANCE & RESOURCES → VISUAL EVIDENCE → REMEDY SEARCH → A11Y & I18N → JUDGEMENT → REVIEW → TIME.

Each substantive slot ends with exactly one of:

VERDICT: clear
VERDICT: note — <non-blocking fact>
VERDICT: BLOCKS — <proven defect>
VERDICT: [Needs human judgement] — <one human-owned decision>
VERDICT: not looked at — <missing evidence>

A slot verdict is a floor, not a ceiling. JUDGEMENT may promote it after reading the body and the combined evidence; it may not silently lower it.

The template

### PR
#<number> <title> by <author>

### HEAD REVIEWED
<full SHA>

### VERSIONS
LOOP VERSION: <current Review Loop version>
AUDIT RUBRIC: <current Component Audit Rubric version>

### OPERATING CONTROLS (PRIVATE)
REQUEST SCOPE: <direct user request | scheduled selection | on-demand request | spec-maintainer task> — <request/task reference>
REVIEW AUTHORIZATION: review only — request scope never authorizes merge
RATE POLICY: shared — at most 2 substantive PR review attempts in any rolling 3600s across all entry points
REVIEW ATTEMPT RESERVATION: <attempt reservation/idempotency identity + reserved-at epoch | queued — no capacity>
POST RESERVATION REUSE: <not attempted | same attempt identity reused unchanged + exact-head SHA>
SPEC OWNER SOURCE: <blocker spec path + `authority: current` or `authority: draft` + exact `owners` frontmatter | not applicable>
SPEC OWNER MENTIONS: <all frontmatter owners mapped to real GChat mentions | unresolved — hold privately | not applicable>

<`queued — no capacity` ends the run before review work. The parent-owned durable
helper atomically reserves once immediately before each substantive review
attempt. An initial review, re-review, or substantive retry is a new attempt.
The public GitHub review action consumes no second slot: it reuses the same
attempt identity after the exact head is rechecked. A transport-only retry
reuses the same identity/idempotency key. Author identity never gates review or
changes the verdict. Downtime creates no tokens or burst capacity.>

### LANE
LANE: <fast | full>
WHY: <each fast-lane condition, or the first promotion trigger>

### CONTEXT & ROUTING
CHANGED CODE: <what the diff changes; evidence only, never authority>
AUTHORITY CACHE: discarded — cached and prior non-repo decisions cannot settle judgement
AUTHORITY CHECK: <applicable `authority: current` spec path inside `facebook/astryx`, or none — novel-human>
NEAREST CURRENT COMPONENT CONTRACT: <current in-repo spec path + boundary, or none>
CURRENT FAMILY: <current in-repo family spec path, or none; landed behavior is evidence only>
CURRENT API/THEMING/SYSTEM RECORDS: <applicable current in-repo spec paths only, or none>
DRAFT/HISTORY CONTEXT: <draft or archived specs, code, docs, issues, prior reviews, fork-wiki records consulted, or none> — context/evidence only; never authority
STOPPED AT: <current in-repo spec | none — novel-human> — <why authority resolves here or does not exist>
REGRESSION EVIDENCE: <preserves: current-spec baseline + current-head match | bug fix/regression: before failure + current-head pass at the same boundary | not applicable>
OWNER QUESTION (PRIVATE): <none | exactly one owner-level question; when present AUTHOR CAN PROCEED:no, no REVIEW, no post/merge>

| candidate issue | classification | evidence | disposition |
|---|---|---|---|
| <one row for every candidate> | <preserves | settled | novel-human | out-of-scope> | <current in-repo spec + current-head evidence, or evidence-only context> | <no finding | apply spec | ask owner privately | route separately> |

<Only current in-repo specs can settle judgement. A committed
`facebook/astryx` specification declaring `authority: current` is the only source
that can make a choice settled. Cached or prior non-repo decisions are discarded
as authority at the start of every review. Draft/archived specs, fork-wiki
rulings, decision-window or auto-ratified outcomes, merged code, component docs,
issues, and prior reviews are context/evidence only. Prior reviews never promote
an unresolved choice to settled.>

### PROBLEM
WHY 1: <why the author wants this; immediate failure, not mechanism>
WHY 2: <why that failure hurts the person doing the task>
WHY 3: <why that harm matters in the job this surface exists to support>
USER-FACING PROBLEM: <synthesize person + task + worse outcome>
PROBLEM SEVERITY: <broken task | harmful friction | missing capability | polish> — <evidence>
NEW FEATURE CASE: <not a new feature | named person + current failed task + evidence of demand + why existing mechanisms do not reach>
EARLY STOP: <clear | [Needs human judgement]> — if absent or owner-unverified, stop before lint, types, tests, or implementation mechanics.
<For preventative or maintainer-facing work, name that exception directly. Contributor-authored test/vibe feature additions without an owner-verified case are Needs human.>

VERDICT: <...>

### SOLUTION
<In 2–4 plain sentences: what notices the situation, what changes, where the
responsibility lives, and why that solves the stated problem. Do not use file,
hook, or variable names.>

SOLUTION (<N> decisions · ~<runtime lines> of <total lines>)
1. <independent runtime decision and the problem it serves>
2. <decision> [NO STATED PROBLEM, when applicable]

BURDEN: <low | medium | high> — <state, Effects/listeners/timers, shared coupling,
public surface, maintenance and test burden>
BURDEN MATCH: <proportionate | disproportionate | [Needs human judgement]> — <why>
<High-burden machinery serving only polish is a human decision; AUTHOR CAN PROCEED: no.>

VERDICT: <...>

### ARCHITECTURE
OWNER: <component or primitive that owns the concept>
TIER 1: <applicable shared systems and reused APIs, or none>
TIER 2: <size / semantic icons-indicators / status, or none>
SEAMS: <wrappers / portals / slots / render props / custom surfaces to preserve>
BEHAVIOR UNIT: <inline | useNamedHook | pure utility> — <why it is testable>
COMPLEXITY BUDGET: <problem-derived maximum: runtime decisions, owners, durable state representations, modules/workflows>
ACTUAL BURDEN: <changed files; runtime lines; new owners/modules; durable schemas/maps; workflows; focused test matrix>
BURDEN TREND: <first reviewed head → current head: grew / shrank / flat + justification>
RESET TRIGGER: <not triggered | triggered — second model finding, burden growth, or duplicated truth>

| domain fact | one authoritative writable source | generated / immutable projections | other writable copies |
|---|---|---|---|
| <ownership / identity / plan / state / policy> | <source> | <derived outputs> | <none, or BLOCKS> |

<If 2+ Effects coordinate refs/rAF/timers/events/observers/native DOM state,
inline or missing focused browser tests is an automatic ARCHITECTURE BLOCK. A
focused bug fix defaults to one runtime decision, one owner, and zero new durable
maps/registries/schemas unless the proved problem requires more. Two writable
sources for one domain fact BLOCK unless one-way derivation/migration is named
and driven. Re-measure burden every round. A second model-level architecture
finding, or a fix that adds another owner/representation/guard for the same fact,
triggers `BLOCKS — architecture reset required`; stop patching the local case and
redraw the smaller model.>

| seam | driven result |
|---|---|
| <documented composition seam> | <works / fails + evidence> |

<Does the implementation live with its owner, survive every documented seam,
and use the smallest number of owners and truths that solves the stated problem?
Name reuse, ownership, source-of-truth, burden, behavior-unit, or extension-mechanism findings.>

VERDICT: <...>

### IMPACT
<Who the change reaches and what that person sees, hears, or can no longer do.
Name any newly visible gap in a sibling or paired surface.>
NEW FEATURE IMPACT: <not a new feature | person + task + current worse outcome + observable improvement; provider/support/test-count statements do not count>

VERDICT: <...>

### API
<If changed, begin with the smallest real call site from a story, doc, or test.
Otherwise: no API change.>

| | change | public? | class | doc'd? | verdict |
|---|---|---|---|---|---|
| `+/-/~` | `<exact signature, optionality, default>` | <barrel/internal> | <landed siblings or none> | <en+zh/en only/missing> | <ok/finding/Needs human> |

PUBLIC API GATE: <repeat for every added/changed public row; otherwise not applicable>
API ROW: <exact added/changed public signature this gate judges>
NON-DERIVABLE NEED: <yes/no — why the current contract cannot already fulfil the need>
MEANING: <understandable — what this API means in ordinary builder language>
PREDICTABILITY: <predictable — valid, invalid, default, and composed behavior>
CAPABILITY: <capable — evidence the mechanism fulfils its stated purpose end to end>
DOCS OBLIGATION: <required — usage/promise changes or current docs become false | not required — implementation/tests only>

<A non-derivable need is necessary for new public surface but insufficient.
Meaning, predictability, and capability must each pass. Consumer docs do not
change automatically for implementation or test changes.>

OSSIFICATION: <For every changed line: class or single case; nearest current
mechanism; cost of being wrong. A genuinely new concept needs human judgement
unless a current durable ruling settles the same boundary.>

VERDICT: <...>

### THEMING
<Enumerate changed targets, tokens, variables, and style contracts. Name the
painting element for each target. Verify `<component-kebab>-<part>`, stability,
documentation, token use, and whether an old target has shipped. Otherwise: no
theming change.>

VERDICT: <...>

### BREAKING
BEHAVIOR: <changed reachable states/defaults/conditionals, or evidenced no>
API: <yes/no + evidence>
VISUAL: <yes/no + evidence, including constrained-neighbor checks for growth>
THEME: <yes/no + evidence>

VERDICT: <...>

### PERFORMANCE & RESOURCES
EFFECTS: <enumerate every added/changed/kept/moved Effect, or zero>

| Effect + deps | external system | why render/handler cannot do it | measured render cost | lifetime + cleanup | focused test |
|---|---|---|---|---|---|
| `<kind> [deps] file:line` | `<DOM/browser/network/widget>` | `<why>` | `<before/after counts at realistic N>` | `<attach/read/write/cleanup>` | `<test>` |

RENDER: <measured delta or no reachable change>
LISTENERS/OBSERVERS: <lifetime and cleanup>
LAYOUT: <reads/writes/reflow/compositor evidence>
BUNDLE: <dependency or size delta>

<Added/changed Effects may not end `not looked at`. Missing measurement is
`[Needs human judgement] — Effect cost unmeasured`, AUTHOR CAN PROCEED: no. Two
Effects coordinating refs/rAF/timers/listeners/native DOM state require a named
behavior unit and focused browser tests; otherwise ARCHITECTURE is BLOCKS.>

VERDICT: <...>

### VISUAL EVIDENCE
VISUAL CHECK: <manual frames required | visual gate sufficient | not applicable>
WHY: <evidence-based obligation decision>

#### <claim proved>

| Before | After | Diff when useful |
|---|---|---|
| ![Before](<durable image URL>) | ![After](<durable image URL>) | ![Diff](<durable image URL>) |

SENSOR RECEIPT: <build SHA, story/route, theme, color mode, direction,
viewport/DPR/media, semantic state, target geometry, settled fonts/animation,
zero page/Storybook errors, and exact-head link. Mutated arms also bank an exact
arm diff.>

<Repeat only for decisive/control pairs. Filenames and prose descriptions alone
are not evidence. If durable images are not embedded here, `VERDICT: not looked
at` and the review may not approve, post, or merge.>

VERDICT: <...>

### REMEDY SEARCH
<If no visual defect needs a system/design remedy choice, write exactly one:>
REMEDY SEARCH: <not triggered — no proven visual defect | not triggered — author owns HOW; acceptance criteria are sufficient>

<Otherwise fill every line:>
TRIGGERED BY: <proven visual defect>
ELAPSED: <Nm; <=5m before outcome>
OUTCOME: <existing-pattern | human-review | owner-requested-extension>
EXISTING PATTERN: <landed public pattern + citation, or none>
REQUESTED BY: <person + component/design-owner role; required for owner-requested-extension, otherwise none>
EXTENSION ELAPSED: <Nm; extension only, otherwise 0m>

HUMAN HANDOFF: <required for human-review; otherwise not applicable>
IMPACT: <proven user impact>
EVIDENCE: <current-head evidence>
CONSTRAINTS: <what the remedy must preserve>
DECISION NEEDED: <one exact system/design choice>

<human-review requires AUTHOR CAN PROCEED: no and no public review text. Never
post speculative implementation advice.>

### A11Y & I18N
<Run only applicable Component Audit checks. Drive keyboard, focus, dismissal,
announcement, and composition-sensitive behavior in a browser. Verify translated
whole utterances, locale formatting, direction, and logical layout as applicable.>

VERDICT: <...>

### JUDGEMENT

| slot | verdict |
|---|---|
| PROBLEM | <...> |
| SOLUTION | <...> |
| ARCHITECTURE | <...> |
| IMPACT | <...> |
| API | <...> |
| THEMING | <...> |
| BREAKING | <...> |
| PERFORMANCE | <...> |
| VISUAL | <...> |
| A11Y & I18N | <...> |

GOAL: <met | not met | partly met> — <measured evidence>
DISPOSITION: <each negative finding → blocks now | follow-up link | accepted reason>
ADVICE: <proven existing pattern | bounded outcome criteria | omitted> — <evidence>
AUTHOR CAN PROCEED: <yes | no> — <acceptance criteria or exact human decision>
WORST OUTCOME: “<strongest user-impact sentence>” → <matching verdict>

JUDGEMENT NEEDED: <none | API — one decision | design — one decision>

<comment | request changes | needs human | approve and merge>

1. <finding>
   → <who hits it, in what state, and what they experience> · <file:line>

### REVIEW
<Separate public review comment, in Cindy's voice, concise and user-impact-first.
Use ordinary contributor language. Do not expose `preserves`, `settled`,
`novel-human`, `out-of-scope`, context routing, request scope, rate
reservations, spec-owner routing, slot/rule ids, or spec-workflow jargon. Never
ask a contributor to edit a spec. For unresolved human review,
write `[not written — human decision unresolved]`.
An architecture BLOCK names the model-level defect, maintainer/user consequence,
and contraction direction; one representative file:line only. Do not lead with
the newest guard, variable, JSON map, hash, or test count — those stay in the
private ARCHITECTURE evidence. If the contraction follows from the one-source
analysis, state outcome-based acceptance criteria rather than asking “where
should this live?”. A genuine unresolved owner/source decision means no public
review text.>

### INLINE (0–3, only when genuinely line-anchored)
- <path:line> — <one sentence>

### EVIDENCE I DID NOT SPEND
- <at most three relevant facts omitted from the public comment>

### TIME
TIME  total <N>m
  setup      <N>m  <worktree/build; warm main reused: yes/no>
  reading    <N>m
  measuring  <N>m  <probe count and re-measures>
  writing    <N>m
  waste      <N>m  <work that should not be repeated>

### WHAT I COULD NOT VERIFY
<one or two bullets, or nothing>

Verdict rules

  • comment — PROBLEM, SOLUTION, or IMPACT cannot yet be established.
  • request changes — any slot says BLOCKS, or GOAL is not met.
  • needs human — one candidate is novel-human: current sources cannot answer one genuine owner-level API/design/cost decision, and nothing already blocks. For visual remedy selection, REMEDY SEARCH must be human-review with IMPACT, EVIDENCE, CONSTRAINTS, and DECISION NEEDED; AUTHOR CAN PROCEED is no, REVIEW is not written, and nothing is posted until the decision is incorporated and JUDGEMENT is rerun.
  • approve and merge — every touched slot is evidenced and is clear or note; every negative fact has a disposition; the author can proceed. This is a review verdict, not merge authorization. Review scope never authorizes merge; any merge needs its own independent authority.
  • A touched slot marked not looked at, a missing verdict, unsupported advice, or a mismatch between WORST OUTCOME and the final verdict means the review is unfinished.

Human-review GChat

Send at most one blocker per message, and use exactly this shape:

*<PR link|#NNN> - <PR title>*
New/updated specification for <plain-language subject>. <simply what it does + API added/removed/changed; if none say none>
cc: <real GChat mentions for required spec owners>

Choose the blocker spec first. A blocker governed by a committed authority: current spec uses that spec's exact owners frontmatter. A blocker about a new or updated authority: draft spec uses that draft's exact owners frontmatter. Draft frontmatter routes the question only; it remains evidence and cannot settle judgement or promote the draft.

Never infer an owner from topic, activity, review history, or familiarity. Resolve every frontmatter owner to a real GChat mention and record the spec path, authority, and owners in SPEC OWNER SOURCE plus the complete private mapping in SPEC OWNER MENTIONS. Any missing spec, owner, or mapping fails closed: do not send; hold privately.

Private execution fact, not public policy: before a blocker naming GitHub owner rubyycheung (Ruby) can send, the private resolver must have a verified real-mention mapping for her. The mapping value is private runtime state, never committed or mirrored, and grants no ownership beyond the selected spec's frontmatter. These fields and all rate/eligibility labels stay out of public REVIEW and public Review-* records.

Fast lane

Fast and full use the same template, REMEDY SEARCH checkpoint, and evidence bar. Fast is available only for one runtime decision of roughly 50 lines or fewer, with no changed public/theme surface, state/default, Effect/listener/observer, dependency, architecture owner, uncovered visual endpoint, unresolved review, conflict, subsystem, or trust issue. One decisive check must settle the goal. Any uncertainty promotes to full.

Recording the review

Durable records preserve reusable boundaries and requirements, not dead-PR prop names or failed mechanisms unless the consequence is important and likely to recur. A maintainer or agent records a ruling in the same PR when possible, otherwise in a linked lower spec PR; contributors are never asked to maintain the spec workflow.

Reciprocal spec-maintainer loop

Monitor comments and new commits on our own open spec PRs. Apply valid feedback to the canonical in-repo record, preserve every unresolved human decision, rerun the required checks, and independently review every new exact head. Approval of an earlier head never carries forward.

Do not broaden the PR's scope, promote authority: draft to authority: current, or treat a comment as authority. Feedback becomes reusable authority only after it is incorporated into a committed facebook/astryx spec with authority: current and that exact head is approved.

After the review is settled, save this filled presentation as Review-<number> and add it to All reviews. Keep the same slots and append:

## What changed before posting
<the maintainer's edits, verbatim — or "posted as drafted">

A later round appends ## Round N; it does not replace earlier evidence.

Clone this wiki locally