Skip to content

Loop Mechanics

Jump to bottom
Cindy Zhang edited this page Sep 1, 2026 · 18 revisions

The Astryx review loop

Built and calibrated with Cindy on 2026-08-16, against seven real PRs. This is the single reference: how the loop runs, why it is shaped this way, and every rule it learned from her corrections.

The goal, in her words: "a reviewer that's just me always having a good day in a good mood and can work 24/7." Handle the volume without her worrying about what it said.

Where this file lives, and what to do when you change it

The kit on Cindy's Mac is the SOURCE OF TRUTH. These five files are it:

~/astryx/review-loop-version.md  one version for the whole kit → wiki Review-Loop-Version
~/astryx/review-critic.md        the rules                     → wiki Critic-Rules
~/astryx/review-brief.md         the reviewer's job            → wiki Reviewer-Brief
~/astryx/review-loop.md          how the loop runs             → wiki Loop-Mechanics
~/astryx/review-loop-kit/review-presentation.md
                                  required output contract       → wiki Review-Presentation

Read review-loop-version.md first. Every presentation names the version active when gate 1 starts; never infer it from the date.

Every change to one of the five process files propagates to the fork wiki — https://github.com/cixzhang/astryx.wiki.git — in the same turn it is made. Not the public facebook/astryx wiki.

The five wiki pages above are byte-identical process mirrors. They govern how the loop operates and are never product or design authority. Only a committed record inside facebook/astryx declaring authority: current is reusable policy for product judgement. Therefore:

  • Edit the kit, then copy the five process files over and push. Never hand-edit a process mirror; runs read the local kit, and the next propagation overwrites mirror-only edits.
  • A drifted process mirror is a bug, not a difference. diff each pair; the local kit wins.
  • Never create, read, cite, or link a fork-wiki Review-* page or an all-reviews index. Existing historical pages stay in place but are not authority, evidence, precedent, or reusable input. Do not bulk-delete them.
  • Other wiki-native process pages are outside this five-file propagation and are never product authority.
cp ~/astryx/review-critic.md /tmp/forkwiki/Critic-Rules.md
cd /tmp/forkwiki && git add -A \
  && git commit -m "<rule id>: <what changed, in one line>" \
  && git pull --rebase -q && git push
diff -q ~/astryx/review-critic.md Critic-Rules.md   # prove the mirror is in sync

Pull before you push — several runs write this wiki on a busy night.

Why the direction matters. A rule only takes effect where the runs read it, and they read the kit. The wiki is how a person reads the loop and how it survives this machine. Publishing late is how the two stopped matching before, and a wiki that lags is worse than no wiki: it is confidently wrong to whoever finds it first.

0. Version the run before reviewing

Read ~/astryx/review-loop-version.md and the Current line in the official Component Audit Rubric at gate 1. Every private presentation names both. The versions stay fixed through critic passes; a fresh rerun reads them again. Historical reviews before loop 1.0.0 remain unversioned.

Metrics compare reviews within one loop/audit version pair. Never describe a rate change across versions as reviewer improvement or regression without separating the populations.

0a. Activation, scope mode, and GChat delivery gate

The review Automations remain disabled. This guidance is effective now, but publishing it does not resume or mutate them. The parent must separately authorize any resume.

  1. Explicit human manual scope is author-neutral. A human may directly select any PR, including for a manual spec-maintainer task. Author identity does not change evidence, authority, severity, or verdict.
  2. Scheduled/on-demand Automation queues are team-only. Queue selection may include only authors whose exact @handle appears in the union of committed .github/ENGOWNERS and .github/DESIGNOWNERS. Unknown or outside is out-of-scope/skip. Never infer membership from branch, topic, activity, reviewers, permission, past merges, or review history.
  3. Scope authorizes review only. It never authorizes merge.
  4. Human/spec decisions are recorded privately and batched into the hourly report. Separate per-PR GChat blocker messages and the historical rolling limiter queue stay inactive unless Cindy explicitly requests one immediate decision.

Review starts, private/read-only reviews, GitHub reviews/comments, spec-maintainer checks, and showing review links in session are not limited. The private presentation records scope mode, Automation eligibility, review-only authorization, latest non-bot review evidence, delivery provenance, immutable public-history disposition, and spec-owner routing. The kit specifies these controls; the private helper enforces selection and provenance.

0b. Immutable GitHub history and pre-claim human precedence

Before an Automation claim or reviewer spawn, fetch submitted GitHub reviews, sort by submission time, and identify the latest non-bot APPROVED, CHANGES_REQUESTED, or COMMENTED review. Ignore later bot reviews/checks. When that non-bot review is attached to the current head, the human disposition satisfies review: exclude the PR before claim/spawn and produce no findings, public action, or GChat escalation. A new head restores eligibility. Missing or ambiguous author type or commit association fails closed. For an eligible PR, review-authorize --action start runs with the same evidence before claim; review_claims.py claim requires its authorization id and matching PR/head/mode/owner. A direct Cindy request is separate manual-human work; it is not an Automation exception.

Public review/comment history from a non-bot account is append-only. Never edit, replace, delete, dismiss, resolve, supersede, or append attribution to it—even when cixzhang authored it and durable Review Loop provenance exists. The visible [Reviewed by Robohands] marker belongs only on newly created loop output; it is necessary but not sufficient provenance and is never added to parent/human text. Bot-authored output is mutable only when the private prepare→publish ledger matches its PR, head, kind, GitHub id, body digest, and provenance id. Missing or ambiguous provenance fails closed. #5806 issuecomment-5498903070 and #5543 issuecomment-5488965637 are explicitly protected.

A conflict or correction creates a new exact-head loop delivery when policy calls for one. Old human, parent-process, and loop text stays untouched; refer to stale loop feedback by its reviewed head.

1. The loop

Reviewer, critic, and evidence are phases of one agent in one session. They are not subagents. The agent that receives the review task performs every pass itself; it never spawns, delegates, or hands the review to another session.

Every run chooses a lane at STEP 1:

  • Fast (target 5–10 minutes): all eligibility conditions in the reviewer brief hold; one decisive check; one critic pass; same evidence and verdict bar.
  • Full: default whenever any condition is uncertain or any promotion trigger appears.

Fast lane is conditional depth, not weaker review. A promoted run records why and continues from the evidence already collected.

Before drafting, every lane fills CONTEXT & ROUTING. Context is loaded with progressive disclosure: changed code → nearest authority: current specification inside facebook/astryx → relevant current family/system specification. Stop when the question is resolved.

Authority gate — literal and non-negotiable. Only current in-repo specs can settle judgement. At the start of every review, discard all cached and prior non-repo decisions as authority and record AUTHORITY CACHE: discarded. A decision source is authoritative only when it is a committed specification inside the facebook/astryx repository and declares authority: current. Changed code, merged behavior, component docs, issues, prior GitHub reviews, review acceptance, decision-window outcomes, and auto-ratified recommendations are context or evidence only. Draft and archived specs are also context only. Fork-wiki process mirrors govern this review process only; they are never product evidence or authority, and per-PR Review-* pages are never consulted.

Every candidate issue is classified as exactly one of preserves, settled, novel-human, or out-of-scope. Preserves needs regression evidence and creates no finding. settled requires an applicable current in-repo spec and current-head conformance evidence. When no such spec settles an owner-level system, API, theme, compatibility, ownership, or design choice, classify it novel-human, ask exactly one private owner question, set AUTHOR CAN PROCEED: no, write no contributor- facing REVIEW, and do not post or merge. Prior reviews remain evidence and checklists; they can never promote an unresolved choice to settled. Out-of-scope is not charged to the author and is routed separately only when consequential. The public REVIEW is then written as a separate, user-impact-first translation with none of those labels or workflow jargon.

  ┌──────────┐  draft   ┌────────┐  PASS   ┌──────┐
  │ REVIEWER │────────▶ │ CRITIC │────────▶│ POST │
  └──────────┘          └────────┘         └──────┘
       ▲                     │
       └──── FAIL: rewrite ──┘        max 3 passes
                             │
                             └── ESCALATE ──▶ Cindy
Role File Sees Never
Reviewer review-brief.md the PR, the repo, the rubric posts
Critic review-critic.md ONLY the draft + the rules + scope/risk re-reviews the code
Evidence review-evidence-templates.md the PR, a real browser writes prose

The critic is deliberately blind to the reviewing. It grades the artifact. Two failures on the same rule = escalate. A loop that cannot converge is a signal about the rules, not the PR.

Never auto-post: a verdict the critic disputes · a finding with no real file:line · a major API change or large behavior refactor · any approval. A wrong "looks good" is the expensive failure — approvals stay Cindy's until the loop earns them.

Literal template gate — before critic and again before publication

The full review is the filled review-presentation.md, not an equivalent summary. Custom headings such as “Problem and direction” or “Architecture budget” do not replace the required slots. Before the critic sees a draft, save it as a private artifact and verify every literal heading/field below exists; repeat against that private artifact immediately before posting the public GitHub review comment:

for required in \
  'LOOP VERSION:' '### OPERATING CONTROLS (PRIVATE)' \
  'REQUEST SCOPE:' 'AUTOMATION TEAM ELIGIBILITY:' 'REVIEW AUTHORIZATION:' \
  'LATEST NON-BOT REVIEW:' 'DELIVERY PROVENANCE:' 'PUBLIC HISTORY MUTATION:' \
  'GCHAT MESSAGE RATE:' 'GCHAT MESSAGE RESERVATION:' \
  'GCHAT QUEUED DELIVERY:' 'SPEC OWNER SOURCE:' 'SPEC OWNER MENTIONS:' \
  '### CONTEXT & ROUTING' 'CHANGED CODE:' 'AUTHORITY CACHE:' \
  'AUTHORITY CHECK:' 'NEAREST CURRENT COMPONENT CONTRACT:' 'CURRENT FAMILY:' \
  'CURRENT API/THEMING/SYSTEM RECORDS:' 'DRAFT/HISTORY CONTEXT:' 'STOPPED AT:' \
  'REGRESSION EVIDENCE:' 'OWNER QUESTION (PRIVATE):' \
  '| candidate issue | classification | evidence | disposition |' \
  '### PROBLEM' '### SOLUTION' '### ARCHITECTURE' \
  'COMPLEXITY BUDGET:' 'ACTUAL BURDEN:' 'BURDEN TREND:' 'RESET TRIGGER:' \
  '| domain fact | one authoritative writable source |' \
  '### IMPACT' '### API' 'API ROW:' 'NON-DERIVABLE NEED:' 'MEANING:' \
  'PREDICTABILITY:' 'CAPABILITY:' 'DOCS OBLIGATION:' \
  '### THEMING' '### BREAKING' \
  '### PERFORMANCE & RESOURCES' '### VISUAL EVIDENCE' '### REMEDY SEARCH' \
  '### A11Y & I18N' '### JUDGEMENT' 'AUTHOR CAN PROCEED:' \
  'WORST OUTCOME:' '### REVIEW'; do
  grep -Fq "$required" "$REVIEW_FILE" || {
    echo "missing required review field: $required" >&2
    exit 1
  }
done

A failure here is not stylistic and cannot be waived by saying the prose contains the same idea. The control fields are semantic gates too: explicit human manual scope is author-neutral; Automation queue scope requires an exact roster-union match; and GChat reservation/queue fields apply only to a blocker message sent to the Astryx GChat group. Unresolved required owner mappings hold that message privately. Draft frontmatter may route a draft-spec blocker but cannot settle judgement. The classifier table has one row per candidate issue, and the public REVIEW is graded separately for ordinary contributor language and absence of private classification, scope, Automation eligibility, GChat delivery, spec-owner-routing, rule, and spec-workflow labels. The review is unfinished; keep the full presentation private and do not post its GitHub comment. No fork-wiki per-PR record is created. For an architecture BLOCK, also read the public REVIEW alone: it must name the model-level defect, consequence and contraction direction. A question like “where should this live?” is valid only as unresolved human judgement, in which case REVIEW must be not written and nothing is posted.

Human decisions — private packets, hourly batch

Do not send separate per-PR human/spec GChat messages. A human hold records one complete private decision packet: missing contract, current authority checked, exact-head evidence and user impact, one decision question, tradeoff, proposal link/none, and frontmatter-derived owners when a governing current/draft record exists. Missing canonical owner routing remains private; never infer it.

Release the claim and continue reviewing. The next hourly report batches new or changed decisions under New this hour and keeps unchanged ones compact under Current human judgements. The historical two-message rolling ledger and queue are inactive. Only an explicit Cindy request for one immediate individual decision may use them.

After Cindy settles a hold, incorporate any reusable boundary into a committed authority: current in-repo spec, rerun JUDGEMENT, and create a new exact-head GitHub disposition when policy calls for one. Never edit/dismiss/resolve old non-bot reviews or comments. None of this supplies merge authorization.

2. One public review, private presentation and evidence

The GitHub review — her voice, short, human, user-impact-first, with exact-head code pointers and inline suggestions. It is a separate public translation: no private classification, scope, Automation eligibility, GChat delivery controls, spec-owner-routing, slot/rule, or spec-workflow labels. Exact-head findings may be delivered here. Never ask a contributor to edit a spec.

The full presentation and evidence — private artifacts only. They carry the slot analysis, matrices, receipts, screenshots, omitted facts, and critic trail. Do not publish them as a bot evidence comment, a fork-wiki Review-* page, or an all-reviews index. A selected public-safe fact or frame may support the GitHub finding when needed; the complete evidence stays private.

GitHub delivery requires no parallel wiki record, serialization step, or same-head wiki guard. Only committed facebook/astryx records with authority: current become reusable product policy.

3. Order of consequence — the spine

Four levels. The review spends itself on the highest unsettled one.

  1. Is this the right thing to do at all?
  2. Is this the right way? Shape — component vs hook, where behavior lives, what surface it adds.
  3. Does it actually work? Correctness on every input, without corrupting data or breaking a path.
  4. What else does it need? Docs, tests, stories, changeset, polish.

Behavior must be correct before anything at level 4 is worth a word. When level 2 is unsettled, level-4 findings are churn against code that may not survive. Level 3 is the exception — a correctness bug outlives the reshape.

Finding level-3 defects usually means running the thing, not reading it. The InputMask engine was caught turning 555 into 11555 by executing it against the PR's own documented pattern. Bug-fix and regression work must show the same contract-boundary case failing before and passing at the reviewed head. Consumer docs change only when usage or a documented promise changes, or current docs become false—not because implementation or tests changed.

4. Verdict

Manual review judgement is author-neutral after an explicit human scope request passes. Scheduled/on-demand Automation queues may select only team authors from the exact committed owner-roster union; unknown or outside authors are skipped before judgement. For every eligible review, current-spec authority, verified correctness, user/builder impact, risk, and unresolved novel-human decisions supply the verdict—not author identity.

  • approve — no verified blocker and no unresolved human-owned decision;
  • request changes — one or more verified blockers with outcome-based acceptance criteria;
  • needs human — no applicable current in-repo spec settles one owner-level choice; AUTHOR CAN PROCEED: no, no public REVIEW/post/merge.

Risk decides what gates. Low-risk → a missing story/test/doc may be a nit. Higher-risk → required. Judge blast radius before deciding what holds.

5. Length — the hardest rule

Verdict Summary Each inline
approve-with-nits ≤ 30 words ≤ 15 words
request-changes ≤ 150 words ≤ 20 words
RFC / shape redirect ≤ 120 words —

The anchor — her real review of #4769, a +1042/-48 PR across 13 files:

Thanks this is good. Will likely merge as is then do the nits separately

Plus three inlines: "Might need isRenderable check" · "Hmm some of this seems redundant. Probably can apply the focus outline without a variant check" · "I need to double check this with other recent updates to fix this across the board for menus"

52 words, whole review. Count words, not characters. Over the cap = rewrite by deleting, not compressing.

On an approve, everything is a nit and a nit is a sentence fragment. No warm-open paragraph, no code block, no closing question longer than a clause.

6. Voice

  1. Warm open naming the hard part of the problem, not flattery of the person.
  2. Verdict as a preference — "ideally I'd like to avoid…" — never a ruling.
  3. Code blocks do the work prose would do: suggested shape, then consumer usage.
  4. Considerations listed in one sentence, not argued.
  5. Ends with a real question.

Register is plain and unhedged: "Might need X." Not "The one I'd like fixed is the pair of…". Delete any clause that only softens.

Banned: checklist output · severity headers · emoji signal lines · rubric ids (T1, A8) · scaffolding headers ("Risk first", "Gates", "Follow-ups") · a "what I could not verify" paragraph · evidence chains for a finding already accepted.

The voice does not change on her own PRs. "Notes to self" bullet-dumps were rejected twice.

7. Judgment rules

  • State the problem at the level it exists. "The guard doesn't reach ComplexSelector" is a symptom; "the guard only reaches callers that go through toggle()" is the problem. Name the class, cite the instance.
  • Never let one instance justify a system change. A local finding is insight, not a reason. System questions go in a separate note framed as "would this remove a class of footgun".
  • Consolidation only where it belongs. A wrong consolidation is worse than none. Presentation and behavior do not share a home — suggesting the ghost trigger's styles move into usePopover, a behavior hook, was wrong. If you cannot name the right owner confidently, say the duplication exists and stop.
  • Don't lead with duplication. "Third byte-identical copy" is a follow-up.
  • Never gate on prerequisite refactors. Say the nit, let it merge.
  • Half-baked APIs do not go public. Unexported types, ad-hoc props, unwired consumers → internal until finished. Non-negotiable.
  • Never charge a contributor for inherited debt. Say plainly when it is pre-existing.
  • Merge conflicts get one sentence.
  • Confidence gate: every claim needs a real file:line actually read. Never assert in the comment anything the draft lists as unverified.

8. What she cares about, in order

  • New API surface is the expensive thing. Read progressively: changed code, nearest current component contract, current landed family, then only the relevant current API/theming/system record. Drafts are context only. A new public API needs a need the current contract cannot fulfil, plus understandable meaning, predictable behavior, and a mechanism capable of its stated purpose.
  • Component vs hook, three probes: (a) the last noun is what the thing IS — if you must rename it to a functionality word, smell; (b) functionality-first defaults to a hook, but a component is fine when it is the ergonomic answer — builder-first beats taxonomy; (c) if it is <Base>-with-a-type it should ride the base's prop evolution, not fork it. Say "smells hook-shaped, here's why" — never "this must be a hook".
  • React effects are disliked, layout effects especially. Can it be done in the event handler while the DOM edit is still the browser's own?
  • Theming: hardcoded colors/spacing/radius/shadow, removed themeable surfaces, raw CSS where StyleX works.
  • Accessibility: accessible name, exposed state, focus management, keyboard.
  • i18n: hardcoded user-facing or AT-facing strings.
  • Code comments are rare. Never suggest adding explanatory ones — and flag a PR that deletes a why-comment while keeping the code it explains.

9. Layout evidence needs a matrix, not a pair

Parents and grandparents change how an element renders. Name the containment assumptions and test each.

Assumptions: parent display · which box actually shrinks · width source (container-defined vs auto-sizing) · min-size defaults · ancestor overflow, grid tracks, flex ancestors.

Matrix: baseline · long content at each width source · the component's own layout variants · narrow (320px) · mixed long + short siblings · RTL · 200% text zoom · forced colors · icon-only.

Probe column when the fix does not achieve its goal — the minimal delta that does.

Worked example, #5035: the PR added text-overflow: ellipsis to an inner span, but the parent flex child had min-width: auto and could not shrink, so the ellipsis never fired once in 11 cases — and an auto-width grandparent hid the bug entirely. Look at every image. Never infer appearance from CSS.

Keep every image in the private evidence artifact. Do not publish a fork assets branch merely to serialize review evidence. A selected public-safe frame may be attached directly to an exact-head GitHub finding when necessary; the full matrix remains private.

10. Remediation and repair

Safe prep-for-merge only: merge main in, run the formatter, add a changeset. No code fixes. Always says it did, one line on the PR.

Only committed facebook/astryx records with authority: current preserve reusable boundaries and requirements. Private presentations may retain exact-head reasoning for Cindy, but they do not become policy and are never serialized to a per-PR wiki page.

Public review/comment history is append-only for every non-bot author. Never edit, dismiss, delete, resolve, supersede, or append attribution to it. A correction is a new exact-head loop delivery with new prepare→publish provenance; old text stays untouched. Bot-authored mutation still requires exact provenance and body identity.

She delegates her own nits: after merging, a subagent opens the follow-up PR off main; an inline she reserved for herself ("I need to double check this") becomes a read-only investigation task, not a code change.

Reciprocal spec-maintainer flow

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

Never broaden scope, promote authority: draft to authority: current, or make a comment authoritative by repetition. Feedback becomes reusable authority only after it is committed in facebook/astryx with authority: current and that exact head is approved.

11. Trust program

All review phases remain paused until the parent verifies the durable guards; this document does not resume them.

  • Phase 0 — approval queue. Bot drafts; she says ship / edit / kill.
  • Phase 1 — style memory built from the diff between draft and shipped.
  • Phase 2 — graduated auto-post, starting at 5%, on docs / tests / dependabot / lab-only, judged on communication style rather than taste.

Agreement = same verdict AND same findings AND "nothing that would embarrass me."

12. Delivery

Twice a day into a session she checks. Each message: the full list of PRs awaiting her review, a top-impact subset, and her own PRs including drafts. Other people's drafts excluded unless they mention her. Replaces the old session-bound "Community PR Cron"; rebuild as a durable Metamate Automation so it cannot silently die.

13. Scoreboard from the first session

PR Draft said She said Loop after correction
#4769 request-changes, led with duplication, 185 w approve, 52 w approve, 47 w, matched one finding exactly
#5035 approve-with-nits approved, then flipped screenshots proved the fix does not fire
#4977 argued caret re-renders — found 555 → 11555 by running the engine

Where it is strong: form converges fast, it verifies its own claims, it self-corrects ("fourth copy" → third, "25 props" → 23), and it drops bad suggestions when the rules conflict.

Where it is weak: taste in what to flag. It missed isRenderable on #4769, which she caught immediately. Form is cheaper to teach than judgment.

14. Files

File Role
~/astryx/review-brief.md reviewer
~/astryx/review-critic.md critic, R1–R56 with her quotes
~/astryx/review-evidence-templates.md layout / API / behavior evidence
~/astryx/review-loop.md the mechanics
~/astryx/REVIEW-LOOP.md this document
memory astryx-review-loop-design.md the design record

15. Evidence behind the design

Clone this wiki locally