From a611c7af7ba9b15afa6bd99ffa4adda725617321 Mon Sep 17 00:00:00 2001 From: BigSimmo Date: Sat, 22 Aug 2026 04:28:21 +0800 Subject: [PATCH 1/3] docs: preserve the review-coverage decision and the auto-fixer collision finding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both artefacts existed only in volatile locations and would have been lost to an ordinary cleanup. Bundled because they are the same investigation: why automated review is not happening, and why two systems answered one review comment. - docs/decisions/ccz4hb-review-coverage.md — the decision document for ledger #CCZ4HB (PR churn has exhausted the review-bot budget, so PRs are landing with no automated review). Measured from local git history only, with an explicit method-and-limits table: 285 of the last 1,190 merged PRs (23.9%) changed only documentation and 190 (16.0%) changed nothing but this repo's own ledger and issues records, while CodeRabbit refills at 24 reviews/day against a measured mean of 25.4 merged PRs/day over 31 days. Recommends, in order: exclude documentation-only PRs via .coderabbit.yaml path_filters, make the existing bookkeeping-bundling rule mechanical, then size vendor spend against the residual. Rejects the "same scope" branch gate on evidence — 87.4% of merged PRs share a file with another merged within a day, so it would refuse roughly seven in ten legitimate branches. Three open questions need the user and are listed in section 6. It had been written only into an agent worktree under .claude/worktrees/, which is wiped by routine cleanup. - docs/outstanding-issues-inbox/0dd8fe98… — the inbox request for #V1ENZC (P2 rec): two independent auto-fixers race on the same PR review comment, and only one is documented. On #2249 a single Codex comment had three eligible responders — the repo's codex-autofix-review-comments.yml, the app-level "Autofix pull requests" watcher (which fixed and resolved it), and a live session the same watcher instructed to do the identical work. Two complete fixes were built; one was discarded. docs/agents-guide.md already assigns automatic resolve to Codex, whose workflow carries trusted-bot gating, a per-PR dedup marker, one repair pass per PR lifetime and a skip label; the app-level watcher has none of these and is in neither the map nor the repo. Sibling of #292, which is the same failure between human sessions. The canonical docs/outstanding-issues.md table is deliberately NOT touched: inbox requests travel with ordinary branches and only issues:reconcile edits the table. Co-Authored-By: Claude Opus 5 --- docs/decisions/ccz4hb-review-coverage.md | 520 ++++++++++++++++++ .../0dd8fe98-c2c5-4180-abd6-eab0622171f7.json | 14 + 2 files changed, 534 insertions(+) create mode 100644 docs/decisions/ccz4hb-review-coverage.md create mode 100644 docs/outstanding-issues-inbox/0dd8fe98-c2c5-4180-abd6-eab0622171f7.json diff --git a/docs/decisions/ccz4hb-review-coverage.md b/docs/decisions/ccz4hb-review-coverage.md new file mode 100644 index 0000000000..9613730bca --- /dev/null +++ b/docs/decisions/ccz4hb-review-coverage.md @@ -0,0 +1,520 @@ +# Decision: restoring automated review coverage (`#CCZ4HB`) + +- **Status:** recommendation, awaiting the user's decision +- **Issue:** `#CCZ4HB` (P1, recommendation), recorded 2026-08-18 +- **Written:** 2026-08-22 +- **Scope:** analysis only. No product code, workflow, or configuration was changed by this document. + +--- + +## Plain-language summary + +**What happened.** Two robot reviewers check every pull request in this repo — CodeRabbit +and the Codex connector. Both ran out of budget on 18 August, so PR #2113 was merged with no +automated review at all, and every PR after it is in the same position until the budget +refills or you buy more. + +**Why it happened.** It is not really a billing problem. CodeRabbit's own message says its +included reviews refill at **one per hour**, which is 24 reviews a day. Over the last month +this repo merged **25.4 pull requests a day on average**, with single days as high as 67. +The repo has been asking for more review than the plan can produce, every day, for weeks. +Adding credits without changing that would be buying capacity to keep doing the same thing. + +**The most useful thing I found.** A quarter of the pull requests are not code at all. Using +the repo's own tested classifier, **285 of the last 1,190 merged PRs (24%) changed only +documentation**, and **190 (16%) changed nothing except the repo's own record-keeping files** +— the review ledger and the outstanding-issues list. Every one of those spent a review credit +on files a code-review robot has nothing useful to say about. That is roughly six wasted +reviews a day, out of a budget of twenty-four. + +**What I recommend, in order.** + +1. **Tell CodeRabbit to skip documentation-only pull requests.** This is a small + configuration change, it takes back about a quarter of the review budget immediately, and + it removes no safety at all — no required check is touched. +2. **Stop the robots opening a pull request whose only content is a bookkeeping record.** + The rule that says to fold those records into the pull request they belong to already + exists in writing and is plainly not being followed; this repo has learned before that a + written rule does not hold but a blocked command does. +3. **Only then decide whether to raise the spending cap.** After steps 1 and 2 the gap + between what the repo needs and what the plan provides is small, and you will be paying to + close a real gap rather than to fund waste. + +**What I recommend against.** The idea of blocking a new branch when an open pull request +already covers the same ground sounds right but cannot be built here: **87% of pull requests +touch a file that another pull request touched within a day**, so such a gate would block +roughly seven out of every ten legitimate branches. And simply accepting unreviewed merges is +not acceptable on its own — this is a clinical reference tool, there is one maintainer, and +the robot review is currently the only review that happens at all. + +**What I need from you.** Three things, listed in full at the end: whether documentation +pull requests may go unreviewed, what you are willing to spend with each of the two vendors, +and permission to check GitHub for the numbers I could not obtain offline. + +--- + +## 1. The problem + +Recorded on `#CCZ4HB`, 2026-08-18: + +> CodeRabbit on PR #2113: "101 included PR reviews in the past 7 days; at that activity +> level, included reviews refill at 1 review per hour. Your organization has reached its +> usage spending cap." The Codex connector reported its own usage limit on the same PR. + +Net effect: PR #2113 received zero automated review, and subsequent PRs will too until the +cap resets or credits are added. The pull requests most likely to need review are exactly +the ones landing during a churn spike, so the failure mode selects for the worst case. + +This is the second bill for one underlying behaviour — one-task-one-PR churn. `AGENTS.md` +already measured the CI half on 2026-07-30 (437 PR-triggered runs over ~3 days, ~40% +cancelled mid-run, ~12 Production-UI-hours burned on runs that never completed). CI waste +is money. Missing review is undetected defects. + +### The failure is silent by design + +`.coderabbit.yaml` was added on 2026-07-13 by PR #601 specifically so that a CodeRabbit +outage cannot post a red commit status: + +```yaml +reviews: + commit_status: false + fail_commit_status: false +``` + +That was the right call for its own purpose — a credit-exhaustion message should not act as +a merge barrier. But it means budget exhaustion produces **no signal on the pull request at +all**. Nothing in the repo distinguishes "reviewed and clean" from "never reviewed". That is +option (c) already in force, arrived at by accident rather than by decision. + +--- + +## 2. Measured evidence + +### 2.1 Method, and what it cannot see + +All figures below come from **local git history only**. No GitHub API, GitHub CLI, OpenAI, +Supabase, Railway, or hosted CI call was made; provider access requires the user's explicit +confirmation under `AGENTS.md` "API and provider confirmation boundary", and it was not +given for this task. + +The repo squash-merges, so each merged pull request appears as exactly one first-parent +commit on `origin/main` whose subject ends `(#N)`. Counts were taken over +`git log origin/main --first-parent`, with per-file `--numstat` for size and path +classification. Window: 2026-07-11 to 2026-08-22, giving **1,190 merged pull requests**. + +Four limits, and they all bias the same way — **every number here is a floor, not a ceiling**: + +| Limit | Effect on the numbers | +| ---------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------- | +| Only **merged** PRs are visible. Closed-without-merge PRs left no commit. | True PR count, and therefore true review demand, is **higher** than measured. | +| Review demand is per **push**, not per PR. CodeRabbit re-reviews incremental pushes; a PR with three pushes can consume several reviews. | Review demand is **higher** than the PR count, by an unknown multiplier. | +| Squash merge collapses each branch to one commit, so per-branch commit counts, push counts, and CI cancellation rates are unrecoverable. | The 2026-07-30 "~40% cancelled" CI figure could not be refreshed. | +| Risk classification below re-implements `scripts/pr-policy.mjs` patterns in awk rather than importing them. | Approximate; the docs-only figure (§2.3) does not have this problem, see there. | + +### 2.2 Volume: demand structurally exceeds supply + +Merged pull requests per ISO week on `origin/main`: + +| Week | Merged PRs | Week | Merged PRs | +| -------- | ---------- | ----------------------------- | ---------- | +| 2026-W27 | 75 | 2026-W31 | 246 | +| 2026-W28 | 92 | 2026-W32 | 160 | +| 2026-W29 | 216 | 2026-W33 | 108 | +| 2026-W30 | 237 | 2026-W34 (partial, to 22 Aug) | 184 | + +Daily, over the 31 days 2026-07-23 to 2026-08-22: **787 merged PRs, mean 25.4/day**, range +4 to 92. The seven days ending 2026-08-18 — the window CodeRabbit was reporting on — merged +**184 PRs**. + +Against that, CodeRabbit's stated refill rate is **1 review per hour = 24 reviews/day = 168 +per week**. + +> **This is the decisive number.** Mean demand is **106% of refill capacity** counting only +> merged PRs and only one review each. Counting closed PRs and re-reviews on incremental +> pushes, real demand is plainly a multiple of supply. The message "101 included PR reviews +> in the past 7 days" against 184 merged PRs in that same window is consistent with +> CodeRabbit already having been throttled partway through the week. +> +> A repo cannot be a little over an hourly refill rate. Either the PR rate comes down, or +> the plan changes, or reviews are permanently rationed by whichever PR happens to arrive +> when a credit is free — which is exactly the current behaviour. + +### 2.3 Composition: a quarter of the budget is spent on the repo's own paperwork + +Classified against `docPatterns` in `scripts/ci-change-scope.mjs` — the repo's **own, +unit-tested** `docs_only` definition (`docs/`, `mockups/`, `*.md`, `*.mdx`, `README*`), +so this is not a new judgement call: + +| Category | PRs | Share of 1,190 | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | -------------- | +| **Documentation-only** (repo's own `docs_only` classifier) | **285** | **23.9%** | +| **Bookkeeping-only** — touched nothing but `docs/outstanding-issues*`, `docs/branch-review-ledger*`, `docs/branch-review-records/`, `docs/outstanding-issues-inbox/` | **190** | **16.0%** | +| Prose-only, widened to include `.claude/`, `.agents/`, `.cursor/`, `.codex/` | 290 | 24.3% | +| Touch a high-risk path (approximating `pr-policy.mjs` clinical/operational/RAG patterns) | 448 | 37.6% | +| Low-risk **and** small (≤10 files, ≤300 changed lines) | 550 | 46.2% | +| Low-risk **and** tiny (≤3 files, ≤80 changed lines) | 296 | 24.9% | +| Bundling candidates (prose-only **or** low-risk-and-tiny) | 387 | 32.5% | + +Sample of the bookkeeping-only class, all merged in W34 alone: + +``` +docs(issues): reconcile 38 queued ledger requests into the canonical ledger (#2229) +docs(ledger): record the prlanded verification for PR #2210 (#2231) +docs(ledger): record prlanded verification for PR #2223 (#2230) +docs(issues): queue the #DREDWA residual test-coverage gap (#2224) +docs(issues): reconcile 29 queued ledger requests into the canonical ledger (#2217) +``` + +**190 pull requests — 16% of all merged PRs, about 4 per day — contained nothing but the +repo's own record-keeping.** Each consumed one CodeRabbit review credit and one full +`pr-required` CI aggregate, to review Markdown records that a code-review bot has no +purchase on. `AGENTS.md` "PR bundling" already says these "should travel with their owning +product PR instead of receiving a dedicated ledger-only branch". They are not doing so. + +Title-based filtering would **not** be a reliable way to catch this class: among those 190, +the title prefixes are `docs(issues)` (54), bare `docs` (46), `docs(ledger)` (31), `issues` +(30), `chore(ledger)` (6), `chore(issues)` (5) — plus stragglers titled `fix(services)`, +`fix(deps)` and `feat(factsheets)` that were nonetheless bookkeeping-only diffs. **Path-based +classification is the only mechanical route**, and the repo already has one. + +### 2.4 Scope overlap: why a "same scope" gate cannot be built + +For each merged PR, I checked whether any PR merged within roughly one day (±26 positions at +~26 merges/day) touched at least one of the same files: + +| Window | PRs sharing ≥1 file with a near neighbour | +| -------------------------------------------------- | ----------------------------------------- | +| ±1 day | **1,041 / 1,191 (87.4%)** | +| ±3 days | 1,100 / 1,191 (92.4%) | +| ±1 day, **excluding** the 8 hottest shared files\* | 843 / 1,191 (70.8%) | +| ±3 days, excluding those same 8 | 928 / 1,191 (77.9%) | + +\* excluded: `docs/branch-review-ledger.md`, `docs/outstanding-issues.md`, `docs/site-map.md`, +`docs/codebase-index.md`, `docs/scripts-index.md`, `AGENTS.md`, +`docs/design-system/adoption-manifest.json`, `docs/design-system/COMPONENTS.md`, plus the two +record directories. + +The hottest shared files, by number of PRs touching them in the window: + +``` + 423 docs/branch-review-ledger.md 91 src/app/globals.css + 201 docs/outstanding-issues.md 80 src/components/ClinicalDashboard.tsx + 123 tests/ui-smoke.spec.ts 77 .../master-search-header.tsx + 108 package.json 73 AGENTS.md + 93 tests/ui-tools.spec.ts 66 .../global-search-shell.tsx +``` + +This kills option (a) as posed, and §3.1 explains why. + +### 2.5 What existed before this, and still holds + +- `.coderabbit.yaml` (2026-07-13, PR #601) — review is advisory, never a blocking status. + **Still current.** Exhaustion degrades silently. +- `.claude/hooks/pr-handoff-stop.sh` — the precedent for a mechanical gate. Its own header: + _"prose rules in AGENTS.md have not held, a denied tool call does."_ +- `AGENTS.md` "PR bundling (reduce one-task-one-PR churn)" and the `newtask` skill's + "Before you start" — both ask the bundling question **in prose**. +- Bundling is not a total failure: PRs #2180, #2183, #2186 landed in W34 as explicit + multi-item `feat(hardening): … bundle` PRs. The prose works when a session chooses to + apply it. Nothing makes it choose. +- **No gate addressing this has been added since the issue was filed.** I checked commits on + `origin/main` since 2026-08-18. The premise of the issue is not stale in that respect. + Whether the _cap itself_ has since reset is unknown offline — see §6. + +### 2.6 Cross-check, and a worsening trend + +The §2.3 figures were produced twice, by **two independently written implementations** — one +looping `git show --numstat` per commit in bash, one parsing a single bulk `git log --numstat` +dump in awk. On the same window (the newest 1,001 merged PRs) they agree on documentation-only +PRs to within nine out of 1,001: **269 vs 260, a 0.9-point gap**, attributable to differing +rename handling between `--name-only` and `--numstat`. The parsing behind §2.3 is corroborated +rather than single-sourced. + +The cross-check also surfaced something the headline figures understate. Both wasted-review +categories are **larger in recent windows than across the full period**: + +| Window (newest N merged PRs) | Documentation-only | Bookkeeping-only | +| --------------------------------------------------------- | ------------------ | ---------------- | +| All 1,191 (since 2026-07-11) — the figures quoted in §2.3 | 23.9% | 16.0% | +| Newest 1,001 (since 2026-07-18) | 26.9% | 18.7% | +| Newest 500 | — | 16.6% | +| Newest 250 | — | **22.0%** | + +Over the most recent 250 merged pull requests, **more than one in five contained nothing but +the repo's own record-keeping**. This document quotes the full-window numbers throughout, which +is the conservative choice: the behaviour being addressed has been getting worse, not better, +so the expected win from step 1 is more likely understated than overstated. + +--- + +## 3. The three recorded options + +### 3.1 Option (a) — a gate refusing a new branch when an open PR of the same scope exists + +**Recommended against, as posed.** Two independent reasons. + +**It cannot be made mechanical without an unacceptable false-positive rate.** "Same scope" +has to reduce to something a script can compute, and the only available signal is the changed +file set. §2.4 measures what that costs: a gate keyed on file overlap with a recently-active +PR would refuse **87% of new branches** — 71% even after excluding the eight hottest shared +files, which is itself a concession that already erodes the gate's usefulness (five of those +eight are the exact bookkeeping/index files a duplicate-work gate would most want to catch). +This repo genuinely has many sessions working concurrently on a shared surface: +`ClinicalDashboard.tsx` was touched by 80 separate PRs and `tests/ui-smoke.spec.ts` by 123. +Legitimate parallel work on one file is the norm here, not the exception. A gate that blocks +seven in ten legitimate branches will be unlocked reflexively within a day and will then be +worse than nothing, because it will also be trusted. + +**Even if it worked, it addresses a different problem.** Refusing a _branch_ prevents +duplicate concurrent work — that is issue `#292` (PR #1766 / #1767 shipping the same +conversion four hours apart), and it is a real problem worth solving. It does not reduce +review demand: the work still gets done, and still becomes a pull request, just later. What +reduces review demand is **fewer pull requests for the same work** — bundling at the point +the PR is opened, not gating at the point the branch is created. + +The narrow, high-value slice of (a) that _is_ achievable is folded into the recommendation +as step 2: not "same scope", but the far cruder and completely unambiguous test **"this +branch's entire diff is bookkeeping records"**. + +### 3.2 Option (b) — raise the bot spending cap / add credits + +**Recommended, but third, and only after steps 1 and 2.** + +First, a clarification the issue flags and that matters for the decision: **these are two +separate vendors with two separate limits.** + +| | CodeRabbit | Codex connector | +| ---------------------------------- | ------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------- | +| Limit reported | "organization has reached its usage spending cap"; included reviews refill at 1/hour | its own usage limit, unspecified in the record | +| Configured in-repo | `.coderabbit.yaml` (advisory only) | `.github/workflows/codex-autofix-review-comments.yml` routes _repair_; the _review_ itself comes from the connector app | +| Controlled by | CodeRabbit org billing settings | OpenAI/Codex plan settings | +| Repo-side throttle already present | none | yes — the autofix workflow routes only high-risk or ≥10-file/≥300-line PRs, and allows one automatic repair pass per PR lifetime | + +Raising either is a settings change outside this repository, and it treats the symptom. The +arithmetic in §2.2 is the argument for ordering: at 25.4 merged PRs/day against a 24/day +refill, **and with a quarter of that volume being documentation**, buying capacity first +means paying to review the repo's own paperwork. Do steps 1 and 2, re-measure, then size the +purchase against the residual. + +It should not be dropped, though. Steps 1 and 2 together remove roughly a quarter of demand; +they do not remove a burst day of 67 PRs. Some paid headroom is the right answer for the +remainder, and it is the only one of the three options that increases actual review _supply_. + +### 3.3 Option (c) — accept unreviewed merges deliberately rather than by accident + +**Recommended against as a standalone answer. One component of it is worth keeping.** + +To be safe, "deliberately accepting unreviewed merges" would need at minimum: + +1. **A visible signal per PR** that review did not happen. Today there is none, by design + (§1). "Deliberate" is not a state of mind; it has to be legible on the pull request. +2. **A defined class where it is acceptable** — and by construction that class is + documentation and bookkeeping, which is precisely step 1 of the recommendation. Extending + it to code is a different proposition. +3. **A compensating human review** for anything else. This is where it fails. `docs/agents-guide.md` + describes a single maintainer; the bot review is not a supplement to human review here, it + _is_ the review. Accepting unreviewed code merges means accepting genuinely unreviewed code + into a clinical reference tool. +4. **It must not be paid for by weakening any required check.** That is the binding stop rule + on `#CCZ4HB`, and it is respected throughout this document: nothing recommended here + touches `pr-required`, `static-pr`, `scripts/pr-policy.mjs`, the clinical-governance + preflight, or the `RAG impact:` gate. + +Worth stating plainly for the record: the deterministic safety net **does** survive a bot +outage intact. Per-PR, independent of any review budget, the repo still runs lint, typecheck, +the full offline unit suite, coverage, `pr-policy` (clinical governance + RAG impact), the +`ingestion-sast` job inside `pr-required`, secret scanning (`secret-scan.yml`, on +`pull_request`), Chromium UI journeys, migration replay, and the `pr-required` aggregate +itself. Semgrep (`sast.yml`) also runs per-PR but is advisory by its own header, so count it +as signal rather than as a gate. What is lost when the +bots go dark is specifically the **judgement** layer — the reviewer that notices a plausible +but wrong design, a missing edge case, or a governance claim that reads fine and is untrue. +That is not a layer the deterministic gates replace, which is why (c) is not sufficient alone. + +The component to keep: **make the absence of review visible.** See step 4 of the recommendation. + +--- + +## 4. Recommendation + +**Do all three of the following, in this order. Step 1 is the one that matters most per unit +of effort; step 3 should not be sized until steps 1 and 2 have been running for a week.** + +### Step 1 — Stop spending review credits on documentation. _(do first; hours, not days)_ + +Add path exclusions to `.coderabbit.yaml` so CodeRabbit does not review a PR whose entire +diff is documentation or bookkeeping. + +_Why this first:_ it is the largest single win (23.9% of PR volume), it is a configuration +change with no code risk, it weakens no required check, and it removes review only from files +where a code-review bot produces no value. Documentation-only PRs still get the repo's own +docs contracts in `static-pr` (`docs:check-index`, `docs:check-inventory`, `docs:check-scripts`, +`docs:check-links`), which are the checks that actually catch defects in documentation. + +### Step 2 — Make the bookkeeping-bundling rule mechanical. _(do second; a day)_ + +Deny `gh pr create` when the branch's entire diff versus `origin/main` is bookkeeping +records, with a deny message pointing at the `AGENTS.md` rule that already says to bundle +them into the owning product PR. + +_Why this, and not the general gate:_ "is every changed path a bookkeeping record" is a +mechanical test with essentially no ambiguity, unlike "same scope". It targets 16% of PR +volume — the class that most obviously should never have been its own PR. And it follows the +precedent this repo has already validated: `pr-handoff-stop.sh` exists because a written rule +did not hold and a denied tool call did. + +### Step 3 — Then size the vendor spend against what is left. _(after a week of 1 and 2)_ + +Re-measure the merge rate, compare it to the refill rate, and buy headroom for the residual — +principally for burst days, which steps 1 and 2 do not address. Handle CodeRabbit and the +Codex connector separately; they are separate bills with separate controls. + +### Step 4 (optional, cheap) — Make "not reviewed" visible. + +Keep the one useful piece of option (c). A **non-blocking** annotation on any PR that merged +without a bot review turns silent degradation into a fact you can see and, later, count. It +must stay non-blocking: `.coderabbit.yaml`'s existing intent is deliberate and correct, and a +new required check is not what this problem needs. + +**Not recommended:** option (a) as posed (§3.1), and option (c) as a standalone (§3.3). + +**Expected effect, stated honestly.** Steps 1 and 2 together address about a quarter of +review demand — roughly 6 of 24 daily credits — and do not by themselves bring a 25.4/day +merge rate under a 24/day refill. They make step 3 a proportionate purchase instead of a +blank cheque, and they stop the most indefensible category of waste. Anyone claiming this +combination alone restores full review coverage is over-reading the numbers. + +--- + +## 5. Implementation sketch + +Not implemented. This is the shape the work would take, for sizing only. + +### 5.1 Step 1 — `.coderabbit.yaml` path filters + +```yaml +reviews: + commit_status: false + fail_commit_status: false + # Documentation and the repo's own bookkeeping records carry no code for a + # code-review bot to review, and consumed ~24% of PR volume (measured + # 2026-08-22, docs/decisions/ccz4hb-review-coverage.md §2.3). Excluding them + # returns that share of the review budget to code. + path_filters: + - "!docs/**" + - "!mockups/**" + - "!**/*.md" + - "!**/*.mdx" +``` + +Key names verified against CodeRabbit's published YAML reference: `reviews.path_filters` +(default `[]`, glob patterns, `!` excludes). `reviews.auto_review.labels` (negative matches +with `!`) is available as a per-PR manual override if one is wanted. + +Two things to settle before writing it: + +- **Does a fully path-filtered PR still consume a review credit?** Unverified — see §6. + If it does, the filter buys nothing and the lever becomes `auto_review.labels` with a + negative match plus a label applied at PR-creation time, which is more machinery. +- **Keep the pattern list honest.** Add a Vitest contract test that reads `docPatterns` from + `scripts/ci-change-scope.mjs` and asserts the `.coderabbit.yaml` filter list stays + consistent with it, so the two definitions of "documentation" cannot drift. The repo + already pins config this way (`tests/ci-cache-safety.test.ts`, `tests/session-start-hook.test.ts`). + +### 5.2 Step 2 — a bookkeeping-only PR-creation gate + +Model it directly on `.claude/hooks/pr-handoff-stop.sh`, which is already registered in +`.claude/settings.json` and already intercepts PR-creating calls: + +- **Hook point:** `PreToolUse`, matching the same call shapes the existing hook matches in + `post` mode — the `gh pr create` CLI and GitHub MCP `create_pull_request`. +- **Test:** compute `git diff --name-only origin/main...HEAD`; deny only if the result is + non-empty **and** every path is under `docs/outstanding-issues.md`, + `docs/outstanding-issues-inbox/`, `docs/branch-review-ledger.md`, + `docs/branch-review-records/`, or `docs/archive/branch-review-ledger-*`. +- **Deny message:** name the `AGENTS.md` "PR bundling" rule, say the record should travel + with its owning product PR, and name the escape hatch. +- **Escape hatch:** `CLAUDE_ALLOW_LEDGER_PR=1`. This is **required**, not optional — the + `npm run issues:reconcile` transaction is deliberately serialized onto its own fresh-base + branch by `AGENTS.md` and `check:ledger-write-discipline`, and legitimately must be its own + PR. The gate must not fight a rule the repo already enforces. +- **Contract, inherited from the existing hook:** exit 0 on any parse problem, make no + decision, never fail a tool call by accident. Register as + `bash "$CLAUDE_PROJECT_DIR/.claude/hooks/.sh"`, set an explicit `timeout`, commit + mode `100755` via `git update-index --chmod=+x` (`core.fileMode=false` on this Dev Drive + makes a local `chmod` a silent no-op), LF endings. `tests/session-start-hook.test.ts` + already fails any hook that is not `100755` or that carries CR bytes. +- **Known gap to state up front:** `core.hooksPath` and `.claude/settings.json` are local, so + this binds Claude Code sessions in this checkout and nothing else — not Codex, not Cursor, + not a Cloud agent pushing from its own environment. It is a strong nudge, not an + enforcement boundary. Given that ~16% of PR volume comes from exactly the sessions this + hook governs, that is still most of the target. + +### 5.3 Step 4 — review-coverage visibility + +Lowest-machinery version: a scheduled, non-blocking job that lists merged PRs carrying no bot +review and appends the count to the existing `ops-digest.yml` output. Deliberately _not_ a +per-PR required check. + +### 5.4 What must not change + +Binding stop rule from `#CCZ4HB`, restated because it constrains any future variant of this +work: **do not weaken any required check to compensate for missing bot review.** Not +`pr-required`, not `static-pr`, not `scripts/pr-policy.mjs`, not the clinical-governance +preflight, not the `RAG impact:` gate. Nothing in this document does; anything built from it +must not either. + +--- + +## 6. Open questions + +**For the user to decide:** + +1. **May documentation-only pull requests go entirely unreviewed by the bots?** This is the + premise of step 1 and it is a judgement call, not a measurement. `AGENTS.md` and + `CLAUDE.md` are themselves agent-behaviour-changing files — a bad edit to them has real + consequences, even though a code-review bot is poorly placed to catch it. My + recommendation is yes, because the repo's docs contracts in `static-pr` are the checks + that actually catch defects there. If you would rather keep review on `AGENTS.md`, + `CLAUDE.md` and `docs/` prose while excluding only the bookkeeping records, the exclusion + list narrows to the four record paths in §5.2 and the win drops from 23.9% to 16.0%. + +2. **What are you willing to spend, with each vendor?** Two separate bills: CodeRabbit's org + spending cap, and the Codex connector's plan limit. Step 3 cannot be sized without a + ceiling from you. + +3. **May I query GitHub to close the gaps below?** Everything in §2 came from local git + history because provider access needs your explicit confirmation and I did not have it. + +**Unanswerable offline — each needs GitHub, or the vendor dashboards:** + +- **Has the cap actually reset since 2026-08-18?** Unknown. Four days have passed. If it has, + the urgency drops but not the analysis: §2.2 shows the repo re-exhausts it structurally. +- **How many pull requests are open right now**, and how many were closed without merging? + Every count in §2 is merged-PRs-only and therefore a floor. +- **How many reviews does one PR actually consume?** Reviews are per push; squash history + destroys push counts. This multiplier is the single biggest uncertainty in §2.2, and it can + only make the picture worse. +- **Is the 2026-07-30 CI figure still current** (437 runs / ~40% cancelled / ~12 Production-UI + hours)? Not recoverable from squashed history. +- **Does a path-filtered CodeRabbit PR consume a review credit?** Blocks the sizing of step 1 + (§5.1). Answerable either from CodeRabbit's usage dashboard or by observing one filtered PR + after the change lands. +- **What exactly is the Codex connector's limit** — per-day, per-month, or plan-wide — and + when does it reset? The `#CCZ4HB` record does not say, and the repo cannot see it. + +--- + +## Sources + +Read for this analysis: `AGENTS.md` ("PR bundling", "Anti-conflict and CI-speed operating +procedure", "Codex GitHub review behavior", "Codex review throttling and routing", "Babysit +the pull request, then stop"); `CLAUDE.md`; `.coderabbit.yaml`; `.github/workflows/ci.yml` +(`changes` and `pr-required` jobs); `.github/workflows/codex-autofix-review-comments.yml`; +`.github/workflows/sast.yml`; `.github/workflows/secret-scan.yml`; `.github/workflows/claude.yml`; +`scripts/pr-policy.mjs`; `scripts/ci-change-scope.mjs`; `.claude/hooks/pr-handoff-stop.sh`; +`.claude/skills/newtask/SKILL.md`; `.claude/skills/handoff/SKILL.md`; +`docs/outstanding-issues.md` (row `#CCZ4HB`); `docs/agents-guide.md`. CodeRabbit YAML key +names and defaults confirmed against CodeRabbit's public configuration reference. +Quantitative figures derived from `git log origin/main --first-parent` over 2026-07-11 to +2026-08-22; method and limits in §2.1. diff --git a/docs/outstanding-issues-inbox/0dd8fe98-c2c5-4180-abd6-eab0622171f7.json b/docs/outstanding-issues-inbox/0dd8fe98-c2c5-4180-abd6-eab0622171f7.json new file mode 100644 index 0000000000..9c4f12f8af --- /dev/null +++ b/docs/outstanding-issues-inbox/0dd8fe98-c2c5-4180-abd6-eab0622171f7.json @@ -0,0 +1,14 @@ +{ + "version": 2, + "id": "0dd8fe98-c2c5-4180-abd6-eab0622171f7", + "createdOn": "2026-08-21", + "action": "add", + "payload": { + "pri": "P2", + "type": "issue", + "summary": "Five unit tests fail on a clean, unmodified main (local Windows run)", + "detail": "Full 'npm run test' on main at cdfcbaccd with an empty working tree: 4 test files failed, 5 tests failed, 680 files / 7423 tests passed, duration 601.68s, exit code 1. Failures: (1) tests/codex-cloud-setup.test.ts > 'writes managed shell policy behaviorally and preserves unrelated Codex config'; (2) tests/hosted-migration-role-guard.test.ts > 'accepts the current repository state'; (3) tests/http-readiness.test.ts > 'honours the overall deadline when every response remains open'; (4) and (5) tests/document-viewer-page-virtualization.dom.test.tsx > 'reads exactly one page ahead and one behind, and only once idle' and > 'collapses render-ahead to the readers page alone when one canvas costs the whole budget' (both fail at flushIdle waitFor idleCallbacks.length > 0). Items 3-5 are deadline/idle-callback timing shapes and are the likeliest environmental Windows failures; items 1-2 assert repository state and look substantive rather than timing-dependent, so they should be reproduced in CI before being dismissed. Next: confirm against a CI run of the same SHA to split genuine breakage from local-environment flake, then either fix or add to the flake ledger per docs/testing.md (zero-retry policy, three reproductions on the same SHA before quarantine). Note the known local-Windows failures previously recorded were session-start-hook and worker-observability, which both PASSED in this run, so the failing set has changed.", + "source": "session 2026-08-22; clean main cdfcbaccd; npm run test exit 1", + "issueUlid": "01M0JTM0ECNQGB13STGM2MRNPQ" + } +} From e8442c1efdc8a521a2ba3bbbcc9f5e1550cdf9c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 20:52:35 +0000 Subject: [PATCH 2/3] fix(issues): file the auto-fixer collision request the PR intended The inbox request added by a611c7af carried the wrong payload. It queued the five Windows unit-test failures at cdfcbaccd, which origin/main already records in docs/outstanding-issues-inbox/67bf71cf-25cb-40d9-a8a2-bbf993bf6b29.json with newer Linux evidence showing they do not reproduce and the suite is green. Reconciling both would have created a second, stale row asserting live breakage. Worse, the auto-fixer collision finding named in this PR's title and body was absent from the diff entirely, so the artefact this PR exists to preserve would have been lost anyway. Replace the payload with the intended request, grounded in local git history rather than narrative: 687b166d and c9d990aa edit the same hook on the same branch from two different identities, b9414fa0 and 94b6c291 are two separate merges of main into that one branch minutes apart, and 828106a5 merges the branch into itself to reconcile the divergence. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ApB8NBygQn9cosxCQ8omk9 --- .../0dd8fe98-c2c5-4180-abd6-eab0622171f7.json | 14 -------------- .../7c0f035c-2b63-47c2-b5a6-6df0118fdb9f.json | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 14 deletions(-) delete mode 100644 docs/outstanding-issues-inbox/0dd8fe98-c2c5-4180-abd6-eab0622171f7.json create mode 100644 docs/outstanding-issues-inbox/7c0f035c-2b63-47c2-b5a6-6df0118fdb9f.json diff --git a/docs/outstanding-issues-inbox/0dd8fe98-c2c5-4180-abd6-eab0622171f7.json b/docs/outstanding-issues-inbox/0dd8fe98-c2c5-4180-abd6-eab0622171f7.json deleted file mode 100644 index 9c4f12f8af..0000000000 --- a/docs/outstanding-issues-inbox/0dd8fe98-c2c5-4180-abd6-eab0622171f7.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "version": 2, - "id": "0dd8fe98-c2c5-4180-abd6-eab0622171f7", - "createdOn": "2026-08-21", - "action": "add", - "payload": { - "pri": "P2", - "type": "issue", - "summary": "Five unit tests fail on a clean, unmodified main (local Windows run)", - "detail": "Full 'npm run test' on main at cdfcbaccd with an empty working tree: 4 test files failed, 5 tests failed, 680 files / 7423 tests passed, duration 601.68s, exit code 1. Failures: (1) tests/codex-cloud-setup.test.ts > 'writes managed shell policy behaviorally and preserves unrelated Codex config'; (2) tests/hosted-migration-role-guard.test.ts > 'accepts the current repository state'; (3) tests/http-readiness.test.ts > 'honours the overall deadline when every response remains open'; (4) and (5) tests/document-viewer-page-virtualization.dom.test.tsx > 'reads exactly one page ahead and one behind, and only once idle' and > 'collapses render-ahead to the readers page alone when one canvas costs the whole budget' (both fail at flushIdle waitFor idleCallbacks.length > 0). Items 3-5 are deadline/idle-callback timing shapes and are the likeliest environmental Windows failures; items 1-2 assert repository state and look substantive rather than timing-dependent, so they should be reproduced in CI before being dismissed. Next: confirm against a CI run of the same SHA to split genuine breakage from local-environment flake, then either fix or add to the flake ledger per docs/testing.md (zero-retry policy, three reproductions on the same SHA before quarantine). Note the known local-Windows failures previously recorded were session-start-hook and worker-observability, which both PASSED in this run, so the failing set has changed.", - "source": "session 2026-08-22; clean main cdfcbaccd; npm run test exit 1", - "issueUlid": "01M0JTM0ECNQGB13STGM2MRNPQ" - } -} diff --git a/docs/outstanding-issues-inbox/7c0f035c-2b63-47c2-b5a6-6df0118fdb9f.json b/docs/outstanding-issues-inbox/7c0f035c-2b63-47c2-b5a6-6df0118fdb9f.json new file mode 100644 index 0000000000..57b452313f --- /dev/null +++ b/docs/outstanding-issues-inbox/7c0f035c-2b63-47c2-b5a6-6df0118fdb9f.json @@ -0,0 +1,14 @@ +{ + "version": 2, + "id": "7c0f035c-2b63-47c2-b5a6-6df0118fdb9f", + "createdOn": "2026-08-21", + "action": "add", + "payload": { + "pri": "P2", + "type": "rec", + "summary": "Two independent auto-fixers can answer the same PR review comment, and only one of them is governed or documented", + "detail": "On PR #2249 a single Codex review comment on .claude/hooks/push-format-guard.sh drew more than one responder: this repo's .github/workflows/codex-autofix-review-comments.yml, and an app-level 'Autofix pull requests' watcher that also instructed a live session to do the identical work. Two complete fixes were built independently and one was discarded. Evidence in local git history: commit 687b166d (author 'Claude', 2026-08-21T19:41:56Z) and c9d990aa (author 'BigSimmo', +0800) both edit the same hook on the same branch claude/push-guard-hookspath-relative; b9414fa0 and 94b6c291 are two separate 'merge main into claude/push-guard-hookspath-relative' commits made minutes apart by those two different identities, and 828106a5 merges the branch into itself to reconcile the divergence. The repo-side Codex workflow carries real safeguards: trusted-bot login gating, a per-PR dedup marker, one automatic repair pass per PR lifetime, and a skip-codex-review opt-out. The app-level watcher has none of these, and the string 'Autofix pull requests' appears nowhere in .github/ or docs/ - it is in neither the repository nor the tooling map, while docs/agents-guide.md line 44 assigns 'Primary PR code-review + automatic resolve' to Codex alone. Next: decide which responder owns automatic resolve, then either document and throttle the app-level watcher in docs/agents-guide.md or disable it, so one comment cannot spawn two competing fixes. Sibling of the ledger item recording the same failure between two human-driven sessions.", + "source": "PR #2259 review; verified against local git history at origin/main 226bd32c (687b166d, c9d990aa, b9414fa0, 94b6c291, 828106a5); replaces a mis-filed payload that duplicated inbox request 67bf71cf-25cb-40d9-a8a2-bbf993bf6b29", + "issueUlid": "01M0K1F646JZM7RMCMPM6W5J0H" + } +} From 91da392015a2fb0689a7a15cb4f6a4d9e1361efb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 20:55:53 +0000 Subject: [PATCH 3/3] docs(ccz4hb): report the prose share as merge volume, not spent credits Codex review on #2259 objected that share-of-merged-PRs cannot be converted into share-of-review-credits, and the objection holds. Section 2.1 already concedes that CodeRabbit bills per push and that push counts are unrecoverable from a squash-merged history, yet the summary, the 2.3 heading, the 2.3 body, the Step 1 recommendation, the expected-effect paragraph and the .coderabbit.yaml sketch all still asserted a credit figure derived from the PR share. PRs that merged after exhaustion consumed no credit at all. Restate each as what was actually measured - merge volume - and mark the credit conversion as an unverified estimate to be sized against the observed refill. The direction of the recommendation is unchanged: the underlying reasoning does not depend on the conversion, only the precision of its sizing did. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ApB8NBygQn9cosxCQ8omk9 --- docs/decisions/ccz4hb-review-coverage.md | 48 ++++++++++++++++-------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/docs/decisions/ccz4hb-review-coverage.md b/docs/decisions/ccz4hb-review-coverage.md index 9613730bca..6a1fd560ba 100644 --- a/docs/decisions/ccz4hb-review-coverage.md +++ b/docs/decisions/ccz4hb-review-coverage.md @@ -23,15 +23,26 @@ Adding credits without changing that would be buying capacity to keep doing the **The most useful thing I found.** A quarter of the pull requests are not code at all. Using the repo's own tested classifier, **285 of the last 1,190 merged PRs (24%) changed only documentation**, and **190 (16%) changed nothing except the repo's own record-keeping files** -— the review ledger and the outstanding-issues list. Every one of those spent a review credit -on files a code-review robot has nothing useful to say about. That is roughly six wasted -reviews a day, out of a budget of twenty-four. +— the review ledger and the outstanding-issues list. Those are files a code-review robot has +nothing useful to say about. + +Be careful what that share is, though. It is a share of **merged pull requests**, not a share +of review credits spent, and the two are not interchangeable. CodeRabbit bills per review and +re-reviews incremental pushes, so one PR can consume several credits — or, if it merged after +the budget was already exhausted, none at all. Push counts are unrecoverable from a +squash-merged history (§2.1), so this share cannot be converted into credits without vendor +consumption data the repo does not have. If documentation PRs are pushed to less often than +code PRs the credit share is lower than the merge share; if more often, higher. Read "about a +quarter of merged PRs are prose" as the measured fact, and any credit number derived from it +as an unverified estimate. **What I recommend, in order.** 1. **Tell CodeRabbit to skip documentation-only pull requests.** This is a small - configuration change, it takes back about a quarter of the review budget immediately, and - it removes no safety at all — no required check is touched. + configuration change, it removes review from about a quarter of merged pull requests + immediately, and it removes no safety at all — no required check is touched. How much + credit that returns depends on push counts the repo cannot measure, so size it by + observing the refill after the change rather than by assuming it tracks the PR share. 2. **Stop the robots opening a pull request whose only content is a bookkeeping record.** The rule that says to fold those records into the pull request they belong to already exists in writing and is plainly not being followed; this repo has learned before that a @@ -139,7 +150,7 @@ per week**. > the plan changes, or reviews are permanently rationed by whichever PR happens to arrive > when a credit is free — which is exactly the current behaviour. -### 2.3 Composition: a quarter of the budget is spent on the repo's own paperwork +### 2.3 Composition: a quarter of merged pull requests are the repo's own paperwork Classified against `docPatterns` in `scripts/ci-change-scope.mjs` — the repo's **own, unit-tested** `docs_only` definition (`docs/`, `mockups/`, `*.md`, `*.mdx`, `README*`), @@ -166,10 +177,14 @@ docs(issues): reconcile 29 queued ledger requests into the canonical ledger (#22 ``` **190 pull requests — 16% of all merged PRs, about 4 per day — contained nothing but the -repo's own record-keeping.** Each consumed one CodeRabbit review credit and one full -`pr-required` CI aggregate, to review Markdown records that a code-review bot has no -purchase on. `AGENTS.md` "PR bundling" already says these "should travel with their owning -product PR instead of receiving a dedicated ledger-only branch". They are not doing so. +repo's own record-keeping.** Each opened a review request and one full `pr-required` CI +aggregate against Markdown records that a code-review bot has no purchase on. The CI cost is +firm — `pr-required` runs per PR and is visible in the workflow history. The review cost is +**not** one credit each: billing follows pushes rather than PRs, and any of these that merged +after exhaustion consumed nothing, so treat this row as review _demand_ offered, an upper +bound on credits spent rather than a count of them. `AGENTS.md` "PR bundling" already says +these "should travel with their owning product PR instead of receiving a dedicated +ledger-only branch". They are not doing so. Title-based filtering would **not** be a reliable way to catch this class: among those 190, the title prefixes are `docs(issues)` (54), bare `docs` (46), `docs(ledger)` (31), `issues` @@ -378,9 +393,11 @@ new required check is not what this problem needs. **Not recommended:** option (a) as posed (§3.1), and option (c) as a standalone (§3.3). -**Expected effect, stated honestly.** Steps 1 and 2 together address about a quarter of -review demand — roughly 6 of 24 daily credits — and do not by themselves bring a 25.4/day -merge rate under a 24/day refill. They make step 3 a proportionate purchase instead of a +**Expected effect, stated honestly.** Steps 1 and 2 together remove review from about a +quarter of merged pull requests. What that is worth in credits is genuinely unknown — the +conversion needs per-push consumption data the repo cannot recover (§2.1, §2.3) — so the +honest statement is a direction, not a number: it reduces demand materially, and it does not +by itself bring a 25.4/day merge rate under a 24/day refill. They make step 3 a proportionate purchase instead of a blank cheque, and they stop the most indefensible category of waste. Anyone claiming this combination alone restores full review coverage is over-reading the numbers. @@ -397,9 +414,10 @@ reviews: commit_status: false fail_commit_status: false # Documentation and the repo's own bookkeeping records carry no code for a - # code-review bot to review, and consumed ~24% of PR volume (measured + # code-review bot to review, and were ~24% of merged PR volume (measured # 2026-08-22, docs/decisions/ccz4hb-review-coverage.md §2.3). Excluding them - # returns that share of the review budget to code. + # returns review capacity to code; the credit saving tracks pushes, not PR + # share, so confirm it against the observed refill rather than assuming 24%. path_filters: - "!docs/**" - "!mockups/**"