Repository navigation
fix(scripts): three more whole-repo scans were racing a 5s budget, and main went red - #111
Conversation
…d main went red `d44fa95` raised the budget on the repo-scanning tests that had been observed failing under an 8-way local split. CI shards SIX ways — the gate's own fix: line says `x test unit --workers 6 --worker 3` — which is a different split, different files per shard, different contention. So a test nobody had seen fail crossed the line on main: `manifest.test.ts`'s byte-identical check, whose body runs a second full `buildManifest(repoRoot())` on top of the one `beforeAll` already pays for. Raised with it, on the same reasoning rather than on observation, because a budget fixed one test at a time just relocates the failure to another shard on another run: the `registeredErrorCodes()` test, which dynamically imports every package exactly as `verify.test.ts`'s error-reference test does, and the `at` check, which is a couple hundred real file reads scattered across every package. Left alone deliberately: every test asserting over the already-built fixture, the single-file reads, and `boundaries.test.ts`'s `collectSharedFiles` / `collectAdminFiles` — measured at 155ms, 125ms and 145ms, two orders of magnitude under the default. The budget moves only where the scan is real. No test weakened, skipped or narrowed; every scan still runs in full. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RBwWKBJkiogA4mDaJiJf3D
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 35 minutes Limit details: You’ve used all 1 included review currently available under your plan. You completed 78 included PR reviews in the past 7 days; at that activity level, included reviews refill at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Comment |
Main is red on
d44fa95. This makes it green.What happened
d44fa95raised the timeout on the repo-scanning gate tests that had been observed failing under an 8-way local split. CI shards six ways — the gate's ownfix:line saysx test unit --workers 6 --worker 3— which is a different split: different files per shard, different contention. So a test nobody had seen fail crossed the line:Its body runs a second full
buildManifest(repoRoot())on top of the onebeforeAllalready pays for.The PR's own CI passed on identical content. Nothing structural explains that: the test was already sitting near the 5s line, and #110's twelve new files pushed it over. A test at ~5.0s against a 5.0s budget is a coin flip, not a pass.
What changed
Three sites in
scripts/manifest.test.ts, each raised because of what it calls, not because it was seen failing — fixing one test at a time only relocates the failure to another shard on another run:two builds of one tree are byte-identicalbuildManifest(repoRoot())on top of the hook'severy code x errors explain answers for…registeredErrorCodes()dynamically imports every package — the same costverify.test.tsalready carries a raised budget forevery 'at' names a real file…Bun.file().exists()+.text()per shipped code, a couple hundred reads across every packageLeft alone deliberately, so the budget moves only where the scan is real: every test asserting over the already-built fixture, the single-file reads, and
boundaries.test.ts'scollectSharedFiles/collectAdminFiles— measured at 155ms, 125ms and 145ms, two orders of magnitude under the default.No test weakened, skipped or narrowed. Every scan still runs in full; only the budget moved.
Honest limit on the evidence
The 6-way failure could not be reproduced locally, including under
tasksetrestricting all six shards to three cores. A 12-core dev box is less contended than a free runner. The CI log above is the ground truth for "before"; after the fix, six concurrent shards pass twice, with shard 3 carrying the same 127-file count CI reported — confirming it is the same split CI uses.Follow-up, deliberately not in this PR
Six sites now carry a bare
30_000with a near-verbatim comment. That repetition is the usual signal, and there is a natural home for a named constant —scripts/lib/run.ts, which already ownsrepoRoot(). It is not folded into a hotfix for a red main; it lands with the next gate PR.