-
Notifications
You must be signed in to change notification settings - Fork 0
Review Presentation
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 GChat 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 separately
authorizes a resume.
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.
### 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: <explicit human manual | Automation scheduled queue | Automation on-demand queue> — <request/task reference>
AUTOMATION TEAM ELIGIBILITY: <not applicable — explicit human manual | eligible — author + exact `.github/ENGOWNERS`/`.github/DESIGNOWNERS` source | out-of-scope/skip — absent/unknown>
REVIEW AUTHORIZATION: <review only | none — Automation queue skip> — review scope never authorizes merge
GCHAT MESSAGE RATE: Astryx GChat group only — at most 2 human-review blocker messages in any rolling 3600s
GCHAT MESSAGE RESERVATION: <not applicable — no blocker send | reservation/idempotency identity + reserved-at epoch | pending — no capacity>
GCHAT QUEUED DELIVERY: <not queued | queued — blocker identity + earliest eligible time | delivered/transport retry — same idempotency identity>
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>
<`out-of-scope/skip` ends only an Automation-selected review; explicit human
manual scope is author-neutral. Automation membership comes only from the exact
committed roster union; never infer it from branch, topic, activity, reviewers,
permission, past merges, or review history. GChat capacity never blocks review
work. Before each human-review blocker message to the Astryx GChat group, reserve
one message slot; at most two may be reserved in any rolling 3600 seconds. A transport-only
retry reuses the same identity. Excess messages queue without downtime credit or
burst. Review starts, private/read-only reviews, GitHub reviews/comments,
spec-maintainer checks, and session review links consume no GChat slot.>
### 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 |
|---|---|---|
|  |  |  |
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, Automation team
eligibility, GChat rate/reservation/queue state, 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>- 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 behuman-reviewwith IMPACT, EVIDENCE, CONSTRAINTS, and DECISION NEEDED; AUTHOR CAN PROCEED isno, 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
clearornote; 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.
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.
Limiter scope. Only human-review blocker messages sent to the Astryx GChat
group count. Immediately before each send, atomically reserve one message slot in
GCHAT MESSAGE RESERVATION; at most two may be reserved in any rolling 3600
seconds. A transport-only retry reuses the same idempotency identity and no new
slot. Excess messages stay in GCHAT QUEUED DELIVERY until capacity exists; do
not combine blockers. Downtime creates no credit or burst.
Review starts, private/read-only reviews, GitHub reviews/comments, spec-maintainer checks, and showing review links in session are outside this budget. They neither reserve nor wait.
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 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.
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.
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.