-
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 remain in a private presentation artifact and are never copied into a public
GitHub review or a fork-wiki per-PR page. The five generic wiki mirrors publish
process guidance only and are never product authority. Only committed
facebook/astryx records with authority: current are reusable product policy.
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
LATEST NON-BOT REVIEW: <none | id + state + author type + submitted-at + reviewed head> — Automation excludes a current-head non-bot APPROVED/CHANGES_REQUESTED/COMMENTED review before claim/spawn; ambiguous evidence fails closed
DELIVERY PROVENANCE: <not applicable — no new public output | prepared provenance id + PR/head/kind/body digest | published provenance id + GitHub id + exact readback>
PUBLIC HISTORY MUTATION: <none — new append-only output | denied — non-bot/ambiguous author | authorized bot mutation — exact provenance + body match> — non-bot text is always immutable
GCHAT MESSAGE RATE: <inactive — human decisions batch into hourly | explicit immediate Cindy request + historical limiter state>
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. Human decisions batch into the
hourly report; separate per-PR GChat reservation/queue/delivery is inactive unless
Cindy explicitly requests one immediate decision. Review starts, private/read-only
reviews, GitHub reviews/comments, spec-maintainer checks, and session links never
wait on that historical limiter.>
<Automation review selection must happen before claim/spawn. Sort submitted
reviews by time and identify the latest non-bot APPROVED, CHANGES_REQUESTED, or
COMMENTED review, ignoring newer bot reviews. A same-current-head match ends the
Automation path with no review. Eligible work obtains a matching
`review-authorize --action start` id before `review_claims.py claim`; claims
without it are rejected. A new head restores eligibility; a Cindy-directed
fresh review is separate manual-human work. Missing author type or reviewed-head
association fails closed. Public comments/reviews authored by a non-bot account
are immutable even with loop provenance. Attribution appears only at creation of
new loop output. Bot mutation requires exact prepared/published provenance and
body identity; protected parent comments are always denied. Corrections are new
exact-head outputs, never rewrites.>
### 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, and prior GitHub reviews consulted, or none> — context/evidence only; never authority; never consult fork-wiki `Review-*` pages
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,
decision-window or auto-ratified outcomes, merged code, component docs, issues,
and prior GitHub reviews are context/evidence only. Fork-wiki process mirrors
control process only and are never product evidence or authority. Never consult a
per-PR `Review-*` page.>
### 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 the images are not embedded in this private artifact,
`VERDICT: not looked at` and the review may not approve or post. Never publish a
per-PR wiki page to satisfy this field.>
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>Safe atomic stack slices do not block for incompleteness, missing future CI/tests,
or later evidence alone. Use BLOCKS only for current contradiction, a
broken/unsafe/incompatible/misleading partial public state, or an untrue guarantee
the exact slice depends on. Otherwise label the item stack status or
follow-up evidence; one PR need not complete a whole spec.
- 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.
Keep one complete decision packet private: missing contract, current authority, exact-head evidence and user impact, one decision question, tradeoff, proposal, and frontmatter-derived owners when available. Do not send/reserve/enqueue/drain separate per-PR GChat blockers; batch current needs into the hourly roundup. Missing record/owner/mapping fails closed and remains private. Only an explicit Cindy request for one immediate individual decision may use the historical limiter path.
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.
Keep the filled presentation and all supporting evidence as private artifacts.
The REVIEW block and public-safe INLINE entries may be delivered as an
exact-head GitHub review; that GitHub state is sufficient delivery.
Never create, update, read, cite, or link a fork-wiki Review-* page or an
all-reviews index. No parallel wiki record, serialization step, or same-head wiki
guard is required. Existing historical pages remain untouched but are not
authority, evidence, precedent, or reusable input; do not bulk-delete them.
Reusable boundaries and requirements belong only in a committed
facebook/astryx record with authority: current. The five generic fork-wiki
mirrors continue to publish this process guidance only.
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.