Repository navigation
Identity hook: a word is read in linear time however long it is - #262
Conversation
Escaped backticks in a double-quoted commit message were read as n to the 1.6: 1.8 s of CPU at 20,000 pairs, and past the 10-second reading budget near 80,000, so a long harmless message was refused. Two readings copied the whole word at each character. shlex grows a token with self.token += c, which CPython cannot extend in place on an attribute; the hook now reads with Lexer, a shlex whose token is held in pieces and given back as a string, so it reads what shlex reads. And _held grew its current word inside a tuple assignment, which copies it; it grows it with a statement of its own now. 20,000 pairs take 0.89 s and 80,000 take 3.6 s, a ratio of 2.8 past the quarter. The growth line in tests/support.py goes back to four. A test holds that the lexer gives every word shlex gives, on a set of hard spellings, and grows a word in pieces; two mutants, and a row on decision 0040. _held's change gives the same value faster, so the wall-clock growth test holds it. 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 6 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 4 files. (1 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly describes the main change: identity-hook word parsing now reads words in linear time regardless of word length. Linked Issues check Active linked issue #260requires linear-time reading for escaped backticks, restoration of thefaster_than_linearthreshold to 4, and regression and mutation coverage.tools/hooks/shell.pyadds `…Out of Scope Changes check The changed files stay within issue #260scope.tools/hooks/shell.pyfixes hook parsing cost.tests/support.py,tests/test_hooks.py, andtools/mutants.pyadd or adjust the required performance…Full details: Linked Issues check
-
Explanation
Active linked issue
#260requires linear-time reading for escaped backticks, restoration of thefaster_than_linearthreshold to 4, and regression and mutation coverage.tools/hooks/shell.pyaddsLexerand_Tokenpiece accumulation, and_heldappends the word separately.tests/support.pyenforces the 4× threshold. The supplied PR evidence reports 2.8 growth at 80,000 pairs, lexer and wall-clock tests, decision-0040 coverage, two lexer mutants, and zero reported corpus regressions or new refusals.
Full details: Out of Scope Changes check
-
Explanation
The changed files stay within issue
#260scope.tools/hooks/shell.pyfixes hook parsing cost.tests/support.py,tests/test_hooks.py, andtools/mutants.pyadd or adjust the required performance and mutation coverage. The decision-0040 documentation records the related boundary rule. No unrelated functional change is established.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 4 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
…ord dense with substitutions is linear too, after Muse Each ) gave back the word before its ( with the character after it, and each word's end hashed it against the keywords, so "a$(x)" repeated 80,000 times took 9.1 s in _held. The reading compares a word only with the keywords, "in" and "case", so it keeps WORD_KEPT, 64, characters and one more: grown() does it at every place the word grows, with a test of its edges and a mutant. The same input takes 0.51 s now, and a case of it joins the growth test. The lexer test asserts the lexer's own words are strings, not shlex's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # tools/mutants.py
|
Why
"Identity hook: escaped backticks in double quotes are read in more than linear time" (issue 260), found by the growth test of "Hook tests: a budget measures the hook, not the machine" (pull request 257). A double-quoted commit message of escaped backticks was read as n to the 1.6: 1.8 s of CPU at 20,000 pairs, and past the hook's 10-second reading budget near 80,000, so a long harmless message would be refused
command_unreadable.What changed
Two readings copied the whole word at every character:
shlexgrows a token withself.token += c. CPython cannot extend a string held in an attribute in place, so a word of n characters costs about n squared over two copies.tools/hooks/shell.pynow reads withLexer, ashlexwhose token is held as its pieces and given back as a string, so it reads exactly whatshlexreads._heldgrew the current word inside a tuple assignment,starts, comments, word = …, word + c, which also copies it, and each)gave back the whole word before its(. It compares the word only with its keywords,inandcase, so it now keeps the firstWORD_KEPT(64) characters and one more, throughgrown(), at every place the word grows.Measured best of two, CPU:
The growth past the quarter is 2.8, linear, where it was 4.0. The growth line in
tests/support.py, set at four and a half for this reading in #257, goes back to four.Lexergives every wordshlexgives, in order and as its own strings, on a set of hard spellings (escapes, quotes, punctuation, an open quote that both refuse), that it grows a word in pieces, and thatgrown()keeps a word toWORD_KEPTand one more, longer than every keyword. Three mutants: the hook reading withshlexitself, a word given back in pieces rather than as a string, andgrown()keeping a word whole. A word dense with substitutions,a$(x)repeated, joins the growth test.Regression corpus
hook_regressions.pyand the 10,722-command fresh corpus, main ab53e4f against this head, at the first head and again after the fix round: 0 regressions from the reviewers' attack inputs; 8,842 corpus commands, 0 newly refused; 10,722 fresh commands, 0 newly refused and 0 newly passed, no exception on either side.Measured
make verifypassed on agit archiveexport of each head. The three new mutants and every mutant oftests/support.pywere killed by hand as the shards run them: a whole copy of the tree,MUTATING=1, each named test byunittest.Who read it
Muse Wonder Full read the first head, 485bc5d, as the Maintainer's ruling of 2026-10-20 has it for a hook pull request, and found three things, each fixed in the one round:
_heldwas still quadratic on a word dense with substitutions,a$(x)repeated 80,000 times taking 9.1 s, since each)gave back the whole word and each word's end hashed it (0.51 s now, withgrown()); the in-place growth had no mutant (grown()has one, and a test of its edges); and the lexer test checkedshlex's words were strings, not the lexer's. Muse also fuzzed the lexer againstshlexon 30,000 inputs and found no difference. Muse did not read the fix round. The Claude review found the second copy, in_held, after the first fix left the ratio at 3.45; the security pass found nothing: what the hook reads and refuses is unchanged, as the corpus shows.Closes #260.
🤖 Generated with Claude Code