Skip to content

Running It

Jump to bottom
Cindy Zhang edited this page Aug 31, 2026 · 7 revisions

Running the loop

The kit on the Mac is what runs. ~/astryx/review-loop-kit/ — the wiki is its published mirror. Corrections go in the kit; editing a mirror page here desyncs it. Reviewer Brief, Critic Rules, Loop Mechanics and Evidence Templates are mirrors. Everything else on this wiki is wiki-native.

export KIT=~/astryx/review-loop-kit
export PATH=/opt/homebrew/bin:$PATH

One agent per PR. Spawn it with:

Read $KIT/review-brief.md and $KIT/review-critic.md in full and follow them. Read $KIT/measurement-harness.md before you build anything.

Review facebook/astryx PR #NNNN. Do the loop: presentation → gate → new presentation, until a gate comes back clean. Reviewer, critic, and evidence are roles you perform in this session: do not spawn, delegate, or hand any pass to another agent or session. Read-only: post nothing, push nothing.

Work in a dedicated worktree under ~/astryx/worktrees/<name> — never main, never a shared checkout. No corepack; use npx --yes pnpm@11.10.0. Other agents share this machine: pick your own Storybook port and say which.

Write every gate to ~/astryx/review-artifacts/pr-<NNNN>/gate-<round>.md in the brief's output shape. One file per round, never overwritten.

Time the run and fill the TIME slot, including the waste line. Return the brief's output block verbatim — every slot, in order, no preamble.

Do not add "be sure to check X" to the prompt. It contaminates the result: you can no longer tell whether the loop found X or whether you did. When a review misses something, write the rule in the kit and re-run stock.

The pipeline

  rules  ─┐
          ├─▶  REVIEW  ─▶  PRESENTATION  ─▶  gate  ─▶  PRESENTATION  ─▶ … clean
  rubric ─┘                                (critic)      (a new one)

The presentation is the product, not a formatting step at the end.

1. The review produces the presentation

Every slot filled, in this order: PROBLEM → SOLUTION → ARCHITECTURE → IMPACT → API → THEMING → BREAKING → PERFORMANCE & RESOURCES → VISUAL EVIDENCE → A11Y & I18N → JUDGEMENT → REVIEW → TIME.

What each slot owes, and the skeleton for the wiki page: Review Presentation. A missing slot is a fail — "no API change" is an answer, silence is not.

Two inputs, both required:

  • The rules. Reviewer Brief is the job; Critic Rules is what is already settled.
  • The component audit. The Component Audit Rubric owns what good CODE is; the brief owns what good REVIEW is. Its trigger table turns what the diff touches into the checks this review owes — themeProps → §2, aria-*/role → §1, useEffect → §7, a physical CSS property → §9. Brief STEP 1b runs it, then adds two tables the rubric cannot supply: semantic triggers (what the change means) and the radius table (what a component-audit instrument structurally cannot see). Origin and the vibe test that promoted it: Rubric-integrated loop.

The rubric is private. No check id, grade, tier or severity ever reaches a posted comment — R12c. It may decide whether to raise a finding; it may never justify one. Translate every finding into what it costs a real person.

2. VISUAL EVIDENCE means a real browser

Filling that slot is where the run spends its time, so read Measurement Harness before cutting a worktree — warm main on port 6100, storybook dev rather than a rebuild, the shared probe kit. Skipping it costs ~35 minutes, measured.

Layout, API and behavior evidence go in a separate bot-attributed comment, tables only — the formats are on Evidence Templates. Gap-filler only: CI already posts bundle size, the a11y audit and preview links. Screenshots are usually the real gap.

Every frame is opened with the read tool. A frame you did not look at is not evidence.

3. The gate reads the presentation, not the diff

The critic never re-reviews the code. It grades the artifact against Critic Rules R0–R42, plus the brief's STEP 7 — the five checks every draft has passed its own critique while getting wrong:

check
7a every entry in SOLUTION and API traces to a sentence in the PR saying why the old behaviour was wrong — quote it or write "none" (R1j)
7b the closing line matches the verdict; the author acts on the last thing you asked (R29)
7c every change you are asking about is named, with a file:line. A count is not actionable
7c2 grep the diff for the fix you are about to ask for. If the author already did it, the finding dies here
7d do the findings share a parent? Three findings about a guard means the subject is whether to guard at all (R30)

A gate does not emit a critique. It emits a new presentation. The document is the unit of iteration: gate-1.md, gate-2.md, and on a hard one gate-r5. Two failures on the same rule = escalate; a loop that cannot converge is a signal about the rules, not the PR.

Worked example — #5391, tonight. Gate 1 proposed sizing a table column by an 8px-per-character estimate. Its own gate found three blocking defects in it: the probe had counted a legitimate hyphen break as a defect, the estimate left one name exactly zero margin, and the widening made the table 280px taller at 375px. Gate 2 is a different solution entirely — a <wbr /> after each solidus, column unchanged. The first pass's answer did not survive its own gate.

4. Then the run acts

The reviewing agent stays read-only, always. The run that owns the PR is what touches GitHub:

  • posts the REVIEW block, signed [Reviewed by Robohands]
  • approves and merges a contributor PR unless there is something we cannot live with (R1g) — three exceptions: new permanent surface, a changed meaning on existing surface, and a wrong mechanism (R30 supersedes R1g)
  • arms auto-merge when only CI is left (R21), then sweeps at the end of the run — an armed merge that never fired is work that looks finished and is not
  • records the page and the index row (R17)

Tiers — not every PR gets the full loop

tier when cost
triage every incoming PR ~4 min, no browser
full loop new surface · a changed default · an a11y or interaction claim ~17 min, browser evidence

The backlog quota is 6 a night, inside the nightly's 18 — Aged PRs owns that arithmetic and why it is 18.

Scale the evidence to the claim. A rushed full loop is worse than an honest triage.

Three consumers

who what it uses the loop for
the nightly component auditor reviews its own audit PRs before Cindy sees them
the gaps loop runs it as a gate on the PRs it opens
the release gate a loop approval plus green CI authorizes an npm self-merge

Self-review is the weakest form of the loop — nobody outside checks it, so "it found nothing" is a claim rather than a result. Weakest is not worthless: #5391 is the counter-example, where the self-gate blocked our own work three times and changed what merged.

What still needs Cindy

  • Owner questions remain owner questions until an applicable current in-repo spec settles them. The fork-wiki Rulings page tracks history and queue state only. Age never authorizes the loop to take its own recommendation.
  • Only authority: current specs inside facebook/astryx settle review judgement. Prior reviews, merged behavior, fork-wiki rulings, and decision windows are context only.
  • Aged PRs never merge unattended. An approve there means "I would merge this" — she merges from the table.
  • Subsystems are never approved-and-merged by the loop (R26).

The three ideas worth stealing even if you hate the rest

1. Order of consequence. 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, where behavior lives, what surface it adds?
  3. Does it actually work?
  4. What else does it need — docs, tests, changeset?

When level 2 is open, level-4 findings are churn against code that may not survive. Say that instead of listing them.

2. Run the thing. Every genuinely useful bug this found came from executing code, not reading it:

  • a mask engine that turned 555 into 11555, because a literal digit in the pattern round-tripped as data
  • a number input that committed 1239 when you typed a then 9 — the rejected keystroke threw the caret to the end
  • a token retune that made three shipped themes unreadable (contrast 8.33 → 1.40)
  • an text-overflow: ellipsis that never fired once across 11 layout cases, because the parent flex child could not shrink

None were visible in the diff.

3. Hard word caps. Approve summary ≤30 words, request-changes ≤150, inlines ≤20. Drafts fail this constantly, and the fix is always to delete a finding, never to compress the prose.

What it is bad at

  • Taste in what to flag. It converges on form fast and on judgment slowly. On one PR it missed the finding a maintainer caught in ten seconds.
  • It over-blocks. Left alone it turns "would be nice" into "must", demands refactors before merge, and leads with duplication nobody cares about. Half the rules in the critic exist to stop that.
  • It re-derives what is already settled. #4743 requested changes against the wiki's own written theming principle and cost a contributor 16 days. 7c2 greps the diff; nothing greps the siblings.
  • It escalates too readily. A reviewer that sends every design question to the TL is the same bottleneck with extra steps.

Astryx conventions the loop relies on

  • Author bucket drives the verdict. .github/ENGOWNERS and .github/DESIGNOWNERS → approve-with-nits, because they merge and follow up. A contributor → approve and merge unless there is something we cannot live with (R1g) — waiting on a follow-up means waiting to merge. Cindy's own PRs still get a real review written to another person.
  • The rubric is the bar, and it is private. The Component Audit Rubric wiki page is the single source of truth for what counts as a defect. It informs the finding; it never appears in the comment, and never argues for it (R12c).
  • The PR Analysis Report already posts bundle size, the a11y audit and preview links, so any evidence comment is gap-filler only. Screenshots are usually the real gap.
  • Never review from a stale checkout. git fetch origin and read via git show origin/main:<path>.
  • Screenshots publish to an assets/pr-<NNNN> orphan branch on your own fork, never on the upstream repo.

Attribution

Every posted review ends with exactly:

[Reviewed by Robohands]

Nothing else — no "generated by", no disclaimer, no model name.

Conflicting PRs — resolve the set, not the PR

The gate is whether every PR in the set has been reviewed, not a night boundary (R6d). Hold every judgement until the last one is in, then post them all at once, winner first.

  1. On detecting a competitor, promote it. The set is the unit now, and finishing it beats starting anything new. Room in the pass? Review it immediately and resolve tonight — that is the best outcome, not an exception.
  2. Carrying over. Record each page, post nothing, and log the pair in a Conflicting PRs report section: both numbers, the shared issue, the axis they differ on, which are reviewed, expected winner.
  3. Read that section before picking work, every pass. An unreviewed partner outranks anything new. A set waiting two passes is the failure this prevents.

The competitor gets a real review on its own merits — the bar goes UP on the one you are choosing, because a decline rides on it.

Exception: a competitor that already merged is not a set — same-night close naming the merged commit.

Never post a decline that names a replacement nobody has read.

Clone this wiki locally