Tianyi Cui
7702a33531
Applies the documentation standard to its biggest offender. Every rule survives as one to three lines plus a link to its durable home; the stories, duplicate statements, and re-narrations go: - Situational clusters evict to new homes: docs/testing.md (tiers, with-key policy, real-over-mock, world-verification, real-entry-path guards), docs/defensive-patterns.md (the bug-class rules), and docs/cookbook/responding-to-pr-review-on-a-stack.md (the stacked-PR review procedure). - Doc-authoring rules consolidate into docs/AGENTS.md § Writing rules (current-state-never-history, md-wrap, ts-block compilation, @mode, catalog same-change, pair same-change). - packages/README.md drops to the group table + the extension-vs-bundle dependency rule; the hand ASCII graph yields to the generated module-graph.md; group READMEs are the canonical per-package map. - packages/AGENTS.md keeps only its packages-specific rules (export shape, ctx.get, real-Loader coverage); examples/AGENTS.md repoints its with-key-policy link; rfc/README.md loses a narrated-history aside; dsh-code-review / dsh-find-simplifications / verify-md-wrap references follow the moved content. - Budget manifest ratchets: AGENTS.md 8200 -> 1500 (now 1,495 words), packages/README.md 1900 -> 600, packages/AGENTS.md 600 -> 450; the two new eviction docs join the budget set (testing 800, defensive 550); docs/AGENTS.md raises 1000 -> 1250 for the absorbed writing rules (the one justified increase). The doc-tiers RFC's deferred list prunes the two items this change ships.
2.6 KiB
2.6 KiB
Responding to review across a stacked PR chain
A wave of review comments lands across several PRs in a dependent stack (A ← B ← C …). This is the discipline for resolving it without corrupting the stack. The two invariants it rests on are standing orders in the root AGENTS.md § Conventions: merge commits only, and never rewrite a pushed branch.
Ground rules
- One worktree per PR branch. Each PR's fixes happen in that PR's own worktree; parallel fixes never share a checkout.
- Bring a child up to date by merging the parent down (
git merge <parent-branch>into the child, a new merge commit). Never rebase/amend/force-push a pushed branch: rewriting diverges it from what the parent PR and GitHub recorded, breaks the stacked-merge graph, and erases the review-fix history. - A fix lands on the PR that INTRODUCED the issue, then flows down. When a comment on PR
Bpoints at codeBintroduced, fix it onBand mergeBintoC— even ifCalso carries the file. Originating the fix downstream leavesBshipping the unfixed code and hides the fix fromB's reviewer. - Each review fix is a separate commit, never an amend. The "fix review findings" commit documents what the review caught. Amending is fine only for your own not-yet-pushed, not-yet-reviewed work.
Working the wave
- Triage every comment on the merits before acting: verify the claim against the code — a reviewer flagging the right symptom can still mis-diagnose the cause.
- Map each accepted finding to its originating PR, fix it there, then merge down the chain in order.
- Delegated fixes are trust-but-verify: a sub-agent's report describes intent, not necessarily what landed. Re-run the gates yourself on the actual tree, and for a regression guard, prove it FAILS on the unfixed code (introduce the regression, watch red, revert) — a guard that passes both ways guards nothing. A sub-agent that reframes a problem as already-handled is a signal to dig in personally.
- Reply in the review thread (
gh api repos/{owner}/{repo}/pulls/{pr}/comments/{id}/replies), not as a top-level comment, stating the fix and the commit that carries it. - Before merging the stack, check dependents: deleting a PR's base branch auto-closes the dependent PR —
gh pr list --json number,baseRefNamefirst, and merge without--delete-branchwhere a child still bases on the branch.
Verify
- Every fixed PR shows a new commit (no force-push icon in the PR timeline).
- Each child PR's diff against its parent still shows only its own changes.
- The gates pass on every PR in the stack, not just the top.