Skip to content

Loop Mechanics

Jump to bottom
Cindy Zhang edited this page Aug 28, 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 four 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

Read review-loop-version.md first. Every gate and every Review-* record names the version active when gate 1 starts; never infer it from the date.

Every change to one of them 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: nothing about the loop goes there (R17).

The wiki pages above are byte-identical MIRRORS. That means:

  • Edit the kit, then copy the file over and push. Never hand-edit a mirror page — an edit made on the wiki is invisible to every run, because runs read the kit, and the next propagation silently overwrites it.
  • A drifted mirror is a bug, not a difference. diff the pair; if they disagree, the kit wins and the wiki gets overwritten.
  • The wiki's other pages — Rulings, Aged-PRs, Calibration, Review-Presentation, Evidence-Templates, Measurement-Harness, the Review-<n> records — are wiki-native and edited there directly. They are not mirrors and nothing in the kit owns them.
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 presentation and Review-* record names both. The versions stay fixed through critic passes; a fresh rerun reads them again. Historical reviews before loop 1.0.0 stay 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.

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.

  ┌──────────┐  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 + bucket/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 to a file and verify every literal heading/field below exists; repeat against the durable review record before posting the public comment:

for required in \
  'LOOP VERSION:' '### PROBLEM' '### SOLUTION' '### ARCHITECTURE' \
  'COMPLEXITY BUDGET:' 'ACTUAL BURDEN:' 'BURDEN TREND:' 'RESET TRIGGER:' \
  '| domain fact | one authoritative writable source |' \
  '### IMPACT' '### API' '### 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 review is unfinished; do not publish its wiki record or GitHub comment. 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.

After Cindy settles a held review, the loop must publish the matching disposition. A request-changes ruling becomes a GitHub CHANGES_REQUESTED review in that cycle; a comment, label, wiki row, or private hold is not delivery. An owner decision to close gets a warm closure comment plus closure, without a redundant review. Read back the PR state before marking the run complete.

2. Two comments per PR, never merged

The review — her voice, short, human, code pointers, inline suggestions. Discussion is fine. Never a checklist dump; checklists are a process, not an output.

The evidence — a separate bot-attributed comment. Tables only, zero prose. Gap-filler only: the repo's PR Analysis Report already posts bundle size, the a11y audit and preview links, so skip anything CI reports and anything the description already says. What survives: screenshots (the real gap), API/prop deltas, bundle-size delta. Often there is no evidence comment at all.

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.

4. Verdict

Author Default Why
Internal (ENGOWNERS/DESIGNOWNERS) approve-with-nits they merge and follow up
External contributor request-changes they cannot merge; the review carries their loop
Cindy comment still a real review written to another person

Risk decides what gates. Low-risk → a missing story/test/doc is 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. Precedent order: nearest sibling → published spec protocol or approved internal twin → full API rethink if the existing code is bad.
  • 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.

Images publish to an assets/pr-<NNNN> orphan branch on cixzhang/astryx, the fork — never facebook/astryx.

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.

A posted review can be edited (gh api -X PATCH .../issues/comments/<id>) and should be, rather than left wrong. A reversal is a one-off — but log it. Twice is a rule, and rules live in the critic.

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.

11. Trust program

  • 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–R29 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