Repository navigation
Hook tests: a budget measures the hook, not the machine - #257
Conversation
The two wall-clock tests of the identity hook, and the one #249 added, failed a local make verify at load 6 to 8 while CI passed the same heads. Each reading that grows is now run at half its size and at its size in the same run: the second must keep to the hook's reading budget, READ_SECONDS of CPU time counting the git it runs, and cost at most three times the first, which is linear and not quadratic. Time by the clock may be twice the budget, more by as much as the load is above the cores, so a hook that waits still fails. A slow hook fails on any machine; a loaded one does not fail a fast hook. The judgments are pure functions in tests/support.py with a test of written-down spends that runs under the mutation check, a test that a child's CPU is counted, and a test beside busy siblings on every core. The marking test counts the helpers that read the clock. Five mutants. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 36 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8eace596c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…he process may use, after Muse and Codex linear() chose its sizes in glue no test under the mutation check reached, and its allowance let a quadratic reading under a quarter-second pass. A reading now runs at sizes 0, half and whole, as sizes() gives them: what it costs past the cheaper of the first two, which is what does not grow, may be three times what the half costs past it and a tenth of a second for noise. The budget and the clock ceiling are held at their edges. The marking test reads support.within(...) as within(). On a runner with a quota, cores() reads the CPUs the process may use, and the busy siblings are at most four. Six mutants, eleven in all. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e61a3006f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… mutant live The cases had no quadratic reading behind a large fixed cost, so counting the fixed cost as growth passed them all, and none whose first run cost more than its half, so taking the first run alone as the floor did too. A second of fixed cost with 0.2 s and 0.8 s past it fails, and 0.3, 0.1 and 0.15 s passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… plain import, after Codex `import tests.support` and then tests.support.within(...) escaped the marking check, which read only a bare name or an alias; it reads a dotted receiver now, so a test in any of the four import forms that calls a timing helper unmarked is caught (probed on a copy with each form). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… of its size, not from size 0 Size 0 can take another path: the nested arithmetic the hook refuses at every size but 0 ran git at size 0, so it cost more than the half, and a linear 0.1 s to 0.2 s from half to whole read as growth and failed a local verify. The three runs are a quarter, half and whole now, on one path. Past the quarter, linear growth is three times the half's and quadratic five, and the limit is four. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f7dcef923
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ngs cleaned up as each starts A quarter-size run that paid a one-time warm-up made a linear 1.3, 1.0 and 1.5 s read as growth (Codex). A reading now runs once untimed before its three timed runs, and when a run costs less than a smaller one the measures disagree and no growth is told; a quadratic reading never does that. Measuring each timed reading at a quarter, half and whole showed one that grows as n to the 1.6: escaped backticks in double quotes, 1.8 s of CPU at 20,000. Every other reading is at 2 to 3.3. The line is at four and a half, about n to the 1.8, so that reading passes and a quadratic one still fails; "Identity hook: escaped backticks in double quotes are read in more than linear time" (issue 260) is filed to make it one pass and bring the line back to four. The busy siblings' cleanup is registered before the first starts, so a failed start leaves none spinning (Codex). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Why
"Hook tests: the two wall-clock budgets fail a loaded machine, not a slow hook" (issue 253). The identity hook's timed tests held each reading to 5 seconds by the clock. With four builder sessions on the Mac (load 6 to 8) they failed a local
make verifyat least four times while CI passed the same heads. A test of the hook's budget is right; a test that fails on load it did not cause is not measuring the hook.Measuring CPU time alone would not have been enough. The case that failed most, 16,000
cd -, is linear (about 0.35 ms each) and simply sized near the old budget: it spent 5.6 s of its own CPU at load 5, and load inflates CPU time too.What changed
READ_SECONDS(10 s) of CPU time, the git it runs counted. The reading runs once untimed first, so what it warms is not counted. What the whole costs past the quarter may be four and a half times what the half costs past it, plus a tenth of a second for the noise of a measure: past the quarter, linear growth gives three times, n to the 1.6 four, and quadratic five, and what does not grow, such as git, falls away. When a run costs less than a smaller one, the measures disagree and no growth is told; a quadratic reading never does that. A quarter and not size 0, because a reading of nothing can take another path: the nested arithmetic the hook refuses at every size but 0 runs git at 0. A loaded machine slows all three runs alike. The line is at four and a half, about n to the 1.8, and not four, because measuring every timed reading showed one that grows as n to the 1.6: escaped backticks in double quotes, 1.8 s of CPU at 20,000, where every other reading is at 2 to 3.3. "Identity hook: escaped backticks in double quotes are read in more than linear time" (issue 260) is filed to make it one pass and bring the line back to four.tests/support.py(over,faster_than_linear,sizes,slack,cores), with the assertionswithin,grows_linearlyandlineararound them andSpentmeasuring CPU and clock. Three tests in a newBudgetsclass: written-down spends against the judgments, which runs under the mutation check; a child's CPU counted, by a subprocess that spins; and a fast reading beside spinning siblings, one to a CPU and at most four, with a reading that spins past its budget still failing.within,linearorgrows_linearlyfromtests/support.py, bare or through the module, as reading the clock. Eleven mutants, one per rule, each held at its edge, and checked as the shards run them.Measured
make verifypassed on agit archiveexport of each head with three spinning siblings beside the other sessions' work, its 1-minute load average sampled every 30 s through the run. At the first head, 8eace59, it was 5.3 at the start, never below 8.4 after that, and peaked at 38.7. At the fix round, e61a300, it rose from 3.7 to 10.9 and held between 8.1 and 10.9 for the second half of the run. At 7f7dcef, with six siblings, it was between 8.0 and 9.3 after the first two samples, and at the final head, 7d9d2ba, between 9.1 and 10.1 after the first three. No test failed in any of the three. One run between them, at 3bc1b07 and a load of 2.7 to 5.9, failed the nested-arithmetic case, which is why the runs are a quarter, half and whole now: size 0 had taken the other path. The eleven mutants were killed by hand on scratch copies with their named tests.Who read it
Muse Wonder Full read the first head, 8eace59, as the Maintainer's ruling of 2026-10-20 has it for a hook pull request, and found four things, each fixed in the one round:
linear()'s choice of sizes was glue no test under the mutation check reached, so measuring both runs at full size would pass unseen (sizes()now, with its test and a mutant); the quarter-second allowance let a quadratic reading under it pass (growth is measured past a size-0 run now, with a tenth of a second for noise); no comparison was held at its edge (each is now, with a mutant); and the marking test missedsupport.within(...)(it reads it now). Codex found thatos.cpu_count()reports the host's CPUs on a runner with a quota, so the siblings could swamp it (cores()reads the affinity, and the siblings are at most four). Muse did not read the fix round. CI's shards then showed two mutants of the floor alive, which my hand-check had counted killed: it copied only part of the tree, so a test that could not import read as a kill. Two spends that tell the floor apart kill them now, and every mutant oftests/support.pywas re-run the way the shards run it. Codex then found the marking check missedimport tests.supportwith a dotted call; it reads every import form now, probed with each. Its last read found a warm-up the quarter run could pay, read as growth, and siblings left spinning if a start failed partway; a warm-up run and a cleanup registered before the first start answer them. The Claude review found the first form, CPU time alone, would still have failed the longcd -case under load, and that the hook's whole-check deadline is 45 s, which would have let a 10-second hook pass; both were fixed before the push. The security pass found nothing to report: the change is to tests, and the spinning siblings are killed in a cleanup.Closes #253.
🤖 Generated with Claude Code