test: give the scaling discrimination guard headroom under contention

[marcusrbrown]

The proves the scaling helper discriminates quadratic work guard fails intermittently under CPU load. The failure is a timeout, not an assertion:

× proves the scaling helper discriminates quadratic work 10403ms
Error: Test timed out in 10000ms.

vitest.config.ts sets a global testTimeout: 10000. This test is deliberately expensive — a timing measurement needs enough work to mean anything — and takes 3.4s isolated and unloaded, 7.4s isolated under load. Running alongside the other 70 test files under contention pushes it past the ceiling.

The fix is a 30-second timeout on that one test. The global ceiling stays at 10s so every other test keeps the tighter bound.

What this is not

My first pass at this deleted the linearMeasurement.ratio < 3 assertion, on the theory that it was the flaky check — the same shape as the two guards removed in #3810, which measured 4.63 against a bound of 3.

That was wrong. The observed ratio at failure was 1.94, passing comfortably. Removing an assertion also cannot make a test faster, so the change addressed nothing; it only looked like it worked because the verification ran under a raised timeout that was never committed.

Both assertions are restored. They do different jobs: the separation check (quadratic.ratio >= linear.ratio * 1.5) proves the estimator discriminates superlinear work, and survives contention because inflation largely cancels across a ratio of ratios. The absolute bound anchors the synthetic linear control against an estimator that uniformly inflates — a failure the separation check cannot see, since both terms would scale together.

Verification

• Reproduced the timeout under a 12-process load with the global ceiling in place.
• 5/5 full-suite runs pass under the same load with the per-test timeout applied.
• The restored assertion still fails against a genuinely quadratic implementation, confirmed by temporarily swapping the linear helper: expected 4.062083268723833 to be greater than or equal to 6.025179357912523.
• pnpm check-types, pnpm lint, pnpm test: 2846 passed, 3 todo, 71 files.

The comments now record the timeout as the failure mode, so the next person reading this doesn't repeat the misdiagnosis.

[fro-bot]

Reproduced the failure mode locally before judging it, because the previous pass at this PR was a misdiagnosis and I wasn't going to take the second one on faith.

Isolated on a 4-core runner: proves the scaling helper discriminates quadratic work completes in 2412ms. Under 12 spinning processes on those same 4 cores: 7061ms, file duration 13.17s. That matches the numbers in the description and it confirms the mechanism — measure() samples process.cpuUsage(), so the assertions are on CPU-time ratios that stay stable under contention, while the wall clock Vitest enforces the timeout against inflates ~3x. The ratio assertions were never the flaky part. The clock was.

That also settles the "does this mask a real regression" question, which is the only thing that would make a raised timeout dangerous. The work is fixed: the calibration loop bounds by CPU-ms, both operations exceed the 50ms floor on the first repetition, so repetitions stays at 1 and the sample count is constant. Nothing about a genuine superlinear regression could route through the timeout instead of the assertion — and this is a meta-test on synthetic closures anyway, so it carries no production surface. 30s is a wall-clock-only knob on a deterministic workload.

Restoring both assertions is the right call. They are not redundant: the separation check survives contention because inflation cancels across a ratio of ratios, and the absolute bound catches uniform inflation that the separation check is structurally blind to. Deleting the bound would have left the estimator able to lie in one direction with nothing watching.

Verdict: PASS

Blocking issues

None.

Non-blocking concerns

packages/wiki-write-core/src/regex-redos-regressions.test.ts:224 — scales malformed wiki log header parsing linearly runs 731ms isolated and 2742ms under the same 12-process load I used to reproduce this. That is the next-shortest fuse in the file, same class of failure, same global 10s ceiling. Full-suite contention across 71 files is harsher than my synthetic load, and when it goes it will present as the identical timeout-not-assertion confusion this PR just spent a cycle untangling. Either extend the headroom to the three expectLinearScaling callers now, or record why their margin is considered sufficient — otherwise the reasoning you just wrote down gets rediscovered from scratch.

packages/wiki-write-core/src/regex-redos-regressions.test.ts:75-78 and :102-105 — two comment blocks now state the contention-causes-timeout rationale. The describe-level one is the useful placement; the inline one repeats it. Minor drift risk if one is later edited alone.

A per-test timeout is the smallest correct diff here, and I prefer it to a Vitest project split or forced sequencing for one test. Noting only that if a third test needs the same treatment, the pattern is a signal to isolate the timing suite rather than to keep sprinkling literals.

Missing tests

None required. This is a test-only change with no production code touched — the diff adjusts an execution bound and restores an assertion that already existed. The verification in the description (5/5 full-suite runs under load, plus the swapped-implementation check confirming the restored bound still fails against genuinely quadratic work: expected 4.062083268723833 to be greater than or equal to 6.025179357912523) is the right shape of evidence for a deflake, and it demonstrates the guard can still catch the regression it names — which is precisely the standard #3810 used to delete the two guards that couldn't.

Risk assessment: LOW

Single test file, +9/-3, no production code, no dependencies, no workflow or permission surface, no any / @ts-ignore / class / console introductions. The one behavioral change is a 3x wall-clock ceiling on a single deterministic test; the global 10s bound holds for the other 2845. Worst realistic case is that a future timing pathology in this synthetic test takes 30s to surface instead of 10s — contained, visible, and cheaper than the flake it retires.

The comments now name the failure mode, which is the part that actually compounds. The next person to see a red X on this test reads "timeout under contention" instead of reaching for the assertion, the way this PR's first pass did.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/.github
Run ID	33597803264
Cache	hit
Session	ses_f9f409ba6ffezK78qR03VhsrIF

[fro-bot]

Both items from the last pass are addressed. The comment duplication is resolved cleanly — the describe-level block now owns the contention/timeout story and the inline comment was narrowed to the assertion rationale, with which a ratio of ratios cannot see because both terms scale together doing real explanatory work rather than restating the header. The sibling-guard question got answered with measurements instead of hand-waving, and the closing instruction (isolate the timing suite rather than scattering more per-test literals) captures the escalation rule correctly.

I re-measured before accepting the recorded figures, since a number written into a comment as a durable record is load-bearing in a way prose isn't. Two unloaded runs and three under 12 spinning processes on 4 cores:

Test	Unloaded	Loaded (3 runs)	Median loaded
proves the scaling helper discriminates quadratic work	2718 / 2716ms	5821 / 6865 / 5625ms	5821ms
scales wikilink parsing linearly (:163)	148 / 159ms	683 / 794 / 766ms	766ms
scales exported wikilink validation linearly (:178)	282 / 273ms	839 / 1449 / 1202ms	1202ms
scales malformed wiki log header parsing linearly (:224)	752 / 772ms	2228 / 2413 / 2733ms	2413ms

The headline number checks out. My earlier 7061ms was a single noisy sample; ~5.4s for the meta-test is the accurate figure and the comment records it correctly. The first two guards land close enough to ~1.1s to be fair.

The third does not. The comment records ~0.5s for the malformed-wiki-log guard; I measure ~2.4s loaded and 752ms even unloaded. It is the slowest of the three production guards in all five of my runs, loaded and unloaded, by a factor of 3-5x over the wikilink guard. Recorded as the fastest, it inverts exactly the risk ordering the comment exists to communicate.

Verdict: PASS

Blocking issues

None. The executable change — }, 30_000) on a single deterministic test — is correct, reproduces as a fix, and is unchanged from the commit I already validated.

Non-blocking concerns

packages/wiki-write-core/src/regex-redos-regressions.test.ts:79-80 — the ~0.5s figure for the third guard should read ~2.4s. Correct it before merging. Two consequences follow from the real number. The ordering inversion is the important one: a reader triaging a future timeout consults this comment, sees the log guard listed as the cheapest, and looks elsewhere first — at the one test most likely to have failed. The margin claim also softens. Against the 10s ceiling the meta-test had ~1.7x headroom when it started flaking; the log guard has ~4.1x. That is roughly 2.4x more room, not "several times," and full-suite contention across 71 files is harsher than my synthetic load. The conclusion still holds — the guards are meaningfully safer and scattering literals is the wrong response — but state it on the true margin so it survives the next person checking your work.

This is the failure shape in docs/solutions/best-practices/test-the-integration-seam-not-the-endpoints-2026-07-06.md, transposed to documentation: a number that carries the authority of a measurement without having been re-walked reads as harder evidence than prose, so it gets trusted more and questioned less. A wrong recorded measurement is worse than the absence it replaced. The other three figures are sound, which is what makes the fourth costly — nothing about its presentation flags it as the one to re-derive.

I can't tell you where ~0.5s came from. My box may differ from yours in core count and contention ratio, and absolute timings are not portable. But the relative ordering is, and on this hardware it is stable and inverted from what the comment claims. If your run genuinely produced that ordering, the measurement is worth repeating before it is trusted — something was mismatched.

Not blocking the merge on a comment's decimal point while a live flake burns CI runs. Patch the figure and ship it.

Missing tests

None required. Still a test-only change — the delta since my last review is comment text plus the same 30_000 timeout literal. No production code, no new behavior to cover. The description's verification (5/5 full-suite runs under load, plus the swapped-implementation check at expected 4.062083268723833 to be greater than or equal to 6.025179357912523) remains the right evidence shape for a deflake, and it demonstrates the restored guard still fails against the regression it names.

Risk assessment: LOW

One test file, +12/-3, no production code, no dependencies, no workflow or permission surface, no any / @ts-ignore / class / console. The only behavioral change is a 3x wall-clock ceiling on a single test whose workload is fixed — the calibration loop bounds by CPU-ms and repetitions stays at 1, so the assertions measure CPU-time ratios while only the wall clock moves under contention. The global 10s bound still governs the other 2845 tests. The inaccurate figure is a comprehension hazard, not a runtime one; it cannot fail a build, only misdirect the person reading it.

The durable value here is that the failure mode is now named at the top of the suite. Someone hitting a red X on this test reads "timeout under contention" and skips the assertion hunt that cost this PR its first pass. Get the third number right and that record holds up.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/.github
Run ID	33598539320
Cache	hit
Session	ses_f9f409ba6ffezK78qR03VhsrIF

[fro-bot]

The ordering inversion is fixed, and the fix is better than the one I asked for. I wanted ~0.5s patched to ~2.4s. Instead b8cd4a3 dropped the absolute figures entirely and rewrote the claim in relative terms, with Absolute timings are not portable across machines, but that ordering is stating explicitly which part of the record is durable and which was never going to survive a different runner. That is the correct lesson from the defect rather than the minimum edit that closes the ticket — the number I flagged was wrong precisely because absolute milliseconds don't travel, so removing the load-bearing dependency on them fixes the class, not the instance.

The rewrite also names the triage target directly: the log header guard is the one to check first if a production guard ever does time out. That is the entire operational payload of the comment, and it now points at the right test.

Verified the new claims against the measurements from my last pass. The delta 993e5cd..b8cd4a3 touches comment lines only — the executable content is byte-identical to what I timed — so those figures carry over without re-measurement:

Claim	Measured (12 spinners, 4 cores, median of 3)	Holds
meta-test by far the most expensive	5821ms vs 2413ms next	yes, 2.4x
consumes over half the 10-second budget	5821ms = 58% (all 3 samples >50%)	yes
log guard is the costliest production guard	2413ms vs 1202 / 766ms	yes
the two wikilink guards less again	1202ms, 766ms	yes

Re-ran the file on this pass: 23 passed, meta-test 2726ms unloaded, guards 146 / 277 / 705ms. Comment lines land at 90-99 chars, in line with the surrounding block.

Verdict: PASS

Blocking issues

None.

Non-blocking concerns

packages/wiki-write-core/src/regex-redos-regressions.test.ts:78 — well under a quarter is the one phrase running slightly ahead of my data. The log guard measures 2228 / 2413 / 2733ms against the 10s ceiling: 22.3%, 24.1%, 27.3%. Median at a quarter, one sample over it. On a 4-core box with 3x oversubscription I am plausibly harsher than your runner, and the sentence's actual job — establishing that the log guard has substantially more room than the meta-test — is true by a wide margin either way (24% vs 58%). Take it as calibration, not a change request. comfortably under half would be unimpeachable; well under a quarter is defensible and I am not going to keep circling a hedge word.

Nothing else outstanding. Both items from the first pass and the one from the second are closed.

Missing tests

None required. Test-only change across all three commits. The delta since my last review is comment prose; the executable diff remains }, 30_000) on one deterministic test, already validated by reproduction. The description's evidence — 5/5 full-suite runs under load, plus the swapped-implementation check at expected 4.062083268723833 to be greater than or equal to 6.025179357912523 — still demonstrates the restored guard catches the regression it names, which is the standard #3810 used to delete the two guards that couldn't.

Risk assessment: LOW

One test file, +14/-3, no production code, no dependencies, no workflow or permission surface, no any / @ts-ignore / class / console. The single behavioral change is a 3x wall-clock ceiling on a test whose workload is fixed — the calibration loop bounds by CPU-ms and repetitions stays at 1, so the assertions read CPU-time ratios while only the wall clock moves under contention. The global 10s bound still governs the other 2845 tests.

Three passes to land nine lines of comment is not waste. The first commit fixed the flake; the next two fixed what the next person will believe about it. The suite now carries its own failure mode, its own cost ordering, and an explicit note about which of its claims are portable — a guard that explains why it is shaped the way it is, instead of leaving that to be excavated by whoever next sees it go red at an inconvenient hour.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/.github
Run ID	33599469390
Cache	hit
Session	ses_f9f409ba6ffezK78qR03VhsrIF

[fro-bot]

7013894 takes the wording I offered and the reasoning behind it. comfortably under half measures 22.3% / 24.1% / 27.3% against the 10s ceiling on my three loaded samples — true with roughly 2x margin, and still true on a runner nearly twice as slow as mine. That is the point the commit message makes explicitly: a bound chosen to survive hardware it was never measured on, rather than the tightest one the author's box happened to support. Precision that doesn't travel is what put the wrong number in this comment two commits ago, so trading a sharper claim for a portable one is the consistent call, not a retreat.

Confirmed the executable content is unchanged since the baseline I measured at 993e5cd — every commit after it is comment-only. 23 passed on this pass, meta-test 2417ms unloaded, log guard 742ms. Comment lines 89-99 chars, matching the block above them.

Nothing outstanding. All four findings across the review cycle are closed.

Verdict: PASS

Blocking issues

None.

Non-blocking concerns

None.

Missing tests

None required. Test-only change across all four commits. The executable diff is still a single }, 30_000) on one deterministic test, validated by reproduction on the first pass. The description's evidence — 5/5 full-suite runs under load, plus the swapped-implementation check at expected 4.062083268723833 to be greater than or equal to 6.025179357912523 — demonstrates the restored guard still fails against the regression it names, which is the standard #3810 used to delete the two guards that couldn't.

Risk assessment: LOW

One test file, +14/-3, no production code, no dependencies, no workflow or permission surface, no any / @ts-ignore / class / console. The single behavioral change is a 3x wall-clock ceiling on a test whose workload is fixed — the calibration loop bounds by CPU-ms and repetitions stays at 1, so the assertions read CPU-time ratios while only the wall clock moves under contention. The global 10s bound still governs the other 2845 tests.

One commit retired the flake; three more fixed what the next person will believe about it. The compounding artifact is the last sentence of that comment block — it now says which of its own claims are portable and which aren't. Most comments record a conclusion; this one records the shelf life of its evidence, which is the only reason the wrong number got caught rather than inherited. Ship it.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/.github
Run ID	33599785639
Cache	hit
Session	ses_f9f409ba6ffezK78qR03VhsrIF