Source: merge commit 374ddfe7d080db96d88aa9e3a9214e88ba578e06 — review-heavy (2 substantive review rounds).
Proposed learning: a scaling guard is only meaningful if you have measured that the pre-fix implementation would actually trip its bound. This change deleted performance guards rather than fixing them, and the reviewer validated the deletion by reconstructing the pre-fix implementations and measuring them directly (input doubling at two size scales, CPU-time sampling, 20 repetitions). The results decided it: the retained regex guard measured a ~4.0 ratio against a bound of 3 — it fires on the real defect, so it encodes a real contract. The removed guard measured ~1.2 for the pre-fix regex it was supposedly protecting against. A guard that cannot reach its own threshold on the very code it was written to catch is not a guard; it is a random number generator wired into CI.
The decisive detail is the third measurement. The current implementation measured a worse ratio (~1.7) than the regex it replaced. That proves the metric was never observing algorithmic scaling on that path at all — it was observing allocation and garbage collection on a large single-character run, and the ratio drifts with whatever the runner's memory pressure happens to be doing. That is how a bound of 3 gets a reading of 4.63 with nothing algorithmically wrong underneath. Same shape for the second removed guard: the function reaches its target via a linear scan, and the regex it replaced was start-anchored with a single greedy backtrack — linear against linear, with a multi-megabyte slice allocated per iteration doing all the actual talking.
What made the deletion safe, and what to check before any similar removal. Behavioral coverage did not move: the extraction path kept its five-case table including the tricky edges, and the sanitization path kept its own cases. Those are the tests that pin behavior; the scaling guards were pinning noise. The shared scaling helper retained three other callers, so nothing rotted into dead code. Typecheck clean, three consecutive green runs of the file.
Suggested capture: docs/solutions/testing/. Scope: the calibration requirement for any scaling or performance guard (measure the pre-fix implementation against the bound before trusting the guard), the allocation/GC confound that makes large-input ratio tests lie, and the checklist for safely deleting a guard — behavioral coverage unchanged, helper still has callers, deletion argument reproduced rather than asserted.
Source: merge commit
374ddfe7d080db96d88aa9e3a9214e88ba578e06— review-heavy (2 substantive review rounds).Proposed learning: a scaling guard is only meaningful if you have measured that the pre-fix implementation would actually trip its bound. This change deleted performance guards rather than fixing them, and the reviewer validated the deletion by reconstructing the pre-fix implementations and measuring them directly (input doubling at two size scales, CPU-time sampling, 20 repetitions). The results decided it: the retained regex guard measured a ~4.0 ratio against a bound of 3 — it fires on the real defect, so it encodes a real contract. The removed guard measured ~1.2 for the pre-fix regex it was supposedly protecting against. A guard that cannot reach its own threshold on the very code it was written to catch is not a guard; it is a random number generator wired into CI.
The decisive detail is the third measurement. The current implementation measured a worse ratio (~1.7) than the regex it replaced. That proves the metric was never observing algorithmic scaling on that path at all — it was observing allocation and garbage collection on a large single-character run, and the ratio drifts with whatever the runner's memory pressure happens to be doing. That is how a bound of 3 gets a reading of 4.63 with nothing algorithmically wrong underneath. Same shape for the second removed guard: the function reaches its target via a linear scan, and the regex it replaced was start-anchored with a single greedy backtrack — linear against linear, with a multi-megabyte slice allocated per iteration doing all the actual talking.
What made the deletion safe, and what to check before any similar removal. Behavioral coverage did not move: the extraction path kept its five-case table including the tricky edges, and the sanitization path kept its own cases. Those are the tests that pin behavior; the scaling guards were pinning noise. The shared scaling helper retained three other callers, so nothing rotted into dead code. Typecheck clean, three consecutive green runs of the file.
Suggested capture:
docs/solutions/testing/. Scope: the calibration requirement for any scaling or performance guard (measure the pre-fix implementation against the bound before trusting the guard), the allocation/GC confound that makes large-input ratio tests lie, and the checklist for safely deleting a guard — behavioral coverage unchanged, helper still has callers, deletion argument reproduced rather than asserted.