Repository navigation
📦🔧：keep three pattern matches from blowing up - #140
Merged
Merged
Conversation
|
Warning Review limit reachedEnable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Next included review available in 2 seconds. View limit details
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Block-comment matching and classification tests packages/util-core/src/_internal/_matches-brand-shape.ts, packages/util-types/test/guards/brand-fallbacks.ts, .changeset/comment-scan-linear.md |
The block-comment pattern now stops at the first closing delimiter. Tests cover asterisk-containing and adjacent comments, including classification timing. A patch changeset records the update. |
Tool identity matching
| Layer / File(s) | Summary |
|---|---|
Identity matching and regression tests build/shared/commit-message.mts, build/shared/commit-message.test.mts |
The tool identity pattern groups trailing whitespace with the optional address. A timing test checks a long whitespace run followed by other text. A Git comparison test skips the bare-link fixture for Git 2.56 or newer. |
Example literal parsing
| Layer / File(s) | Summary |
|---|---|
Aside detection and literal parsing tools/check-examples.js |
Aside detection scans for the first whitespace-delimited - or —. readLiteral parses the full claim before retrying after aside removal, and array elements are parsed directly as literals. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Possibly related PRs
- OpenINF/sdk#126: Adds doc-comment
@examplechecking totools/check-examples.js, which is also updated here.
Merge Risk: 
47962
-
The change targets slow pattern matching; no material merge-blocking risk was established.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title accurately summarizes the primary change: preventing three matching or scanning operations from taking superlinear time. It is concise and specific enough to identify the performance fix. Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 5 files.
-
Finishing Touches
-
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
CodeRabbit Verify
- Run CodeRabbit Verify to test the changes in this pull request.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This was referenced Nov 2, 2026
Member
Author
|
@coderabbitai review Generated by Claude Code |
Action performed
|
DerekNonGeneric
added a commit
to OpenINF/.github
that referenced
this pull request
Nov 2, 2026
TOOL_IDENTITY put two runs of \s* side by side after a [bot], so a long run of spaces with something else at the end could be split between them every possible way before the match gave up: 200,000 spaces took 20 seconds, and the landing runner reads contributor commit messages. The address group now takes its own trailing whitespace along, which matches the same strings in linear time. git 2.56 no longer reads a line starting `token://` as a trailer, so on the runners' new git the readTrailers comparison disagreed on a bare link and CI has been red since. The rules keep the older reading, because a history is read by every git and not only the newest, so that one case is only compared against a git before 2.56. This is the same change as OpenINF/sdk#140; the three copies of commit-message.mts stay byte for byte the same. Signed-off-by: Derek Lewis <DerekNonGeneric@inf.is> Assisted-by: Claude-Code:claude-opus-5
DerekNonGeneric
added a commit
to OpenINF/openinf.github.io
that referenced
this pull request
Nov 2, 2026
TOOL_IDENTITY put two runs of \s* side by side after a [bot], so a long run of spaces with something else at the end could be split between them every possible way before the match gave up: 200,000 spaces took 20 seconds, and the landing runner reads contributor commit messages. The address group now takes its own trailing whitespace along, which matches the same strings in linear time. git 2.56 no longer reads a line starting `token://` as a trailer, so on the runners' new git the readTrailers comparison disagreed on a bare link and CI has been red since. The rules keep the older reading, because a history is read by every git and not only the newest, so that one case is only compared against a git before 2.56. This is the same change as OpenINF/sdk#140; the three copies of commit-message.mts stay byte for byte the same. Signed-off-by: Derek Lewis <DerekNonGeneric@inf.is> Assisted-by: Claude-Code:claude-opus-5
Three places matched text in a way whose cost grew far faster than the text did. Each could be made to stall the code running it. The brand checks behind isAsyncFunction, isAsyncGeneratorFunction and isGeneratorFunction read a block comment lazily, so a comment could run on through its own */ into the next one. Adjacent comments before a function's name could then be split every possible way: two dozen took seconds, more never finished. A comment now ends at its first */ and can match no other way. The trailer check's [bot] pattern put two runs of \s* side by side, so a long run of spaces after a [bot], with something else at the end, took time quadratic in its length to refuse: 200,000 spaces took 20 seconds, and the landing runner reads contributor commit messages. The examples checker tried each nested array element both with and without an aside, doubling its work at every level of nesting, and looked for the aside with /\s+[—-]\s+.*$/, which retries its \s+ from every space of a run. Elements are now read as they stand, and the aside is found by a linear scan, only when the whole claim is not a literal. git 2.56 no longer reads a line starting `token://` as a trailer, so on the runners' new git the readTrailers comparison disagreed on a bare link and main has been red since. The rules keep the older reading, because a history is read by every git and not only the newest, so that one case is only compared against a git before 2.56. Signed-off-by: Derek Lewis <DerekNonGeneric@inf.is> Assisted-by: Claude-Code:claude-opus-5
DerekNonGeneric
force-pushed
the
claude/project-thread-fqaph2
branch
from
November 2, 2026 23:28
47962f3 to
c0c0ea8
Compare
DerekNonGeneric
added
the
🚀 Status: Commit Queue
Claude
openinf-commit-queue
Bot
removed
the
🚀 Status: Commit Queue
openinf-commit-queue Bot
pushed a commit
to OpenINF/.github
that referenced
this pull request
Nov 3, 2026
TOOL_IDENTITY put two runs of \s* side by side after a [bot], so a long run of spaces with something else at the end could be split between them every possible way before the match gave up: 200,000 spaces took 20 seconds, and the landing runner reads contributor commit messages. The address group now takes its own trailing whitespace along, which matches the same strings in linear time. git 2.56 no longer reads a line starting `token://` as a trailer, so on the runners' new git the readTrailers comparison disagreed on a bare link and CI has been red since. The rules keep the older reading, because a history is read by every git and not only the newest, so that one case is only compared against a git before 2.56. This is the same change as OpenINF/sdk#140; the three copies of commit-message.mts stay byte for byte the same. Signed-off-by: Derek Lewis <DerekNonGeneric@inf.is> Assisted-by: Claude-Code:claude-opus-5 PR-URL: #948
openinf-commit-queue Bot
pushed a commit
to OpenINF/openinf.github.io
that referenced
this pull request
Nov 3, 2026
TOOL_IDENTITY put two runs of \s* side by side after a [bot], so a long run of spaces with something else at the end could be split between them every possible way before the match gave up: 200,000 spaces took 20 seconds, and the landing runner reads contributor commit messages. The address group now takes its own trailing whitespace along, which matches the same strings in linear time. git 2.56 no longer reads a line starting `token://` as a trailer, so on the runners' new git the readTrailers comparison disagreed on a bare link and CI has been red since. The rules keep the older reading, because a history is read by every git and not only the newest, so that one case is only compared against a git before 2.56. This is the same change as OpenINF/sdk#140; the three copies of commit-message.mts stay byte for byte the same. Signed-off-by: Derek Lewis <DerekNonGeneric@inf.is> Assisted-by: Claude-Code:claude-opus-5 PR-URL: #1931
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by DerekNonGeneric
Before: three text matches took time that grew much faster than their input. With the right input, each one could stall the code running it. CodeRabbit's security deepscan reported all three: CWE-1333 in
_matches-brand-shape.tsandcommit-message.mts, and CWE-407 intools/check-examples.js.After: each match takes time roughly linear in its input, and results on ordinary input are unchanged.
@openinf/util-core):isAsyncFunction,isAsyncGeneratorFunctionandisGeneratorFunctionread block comments with a lazy[\s\S]*?. A lazy match can continue through a comment's own*/into the next comment, so the matcher could split a run of adjacent comments every possible way. With 26 comments, the check took 0.7 s, and each extra comment pair doubled the time. The block comment pattern is now\/\*[^*]*\*+(?:[^/*][^*]*\*+)*\/, which ends at the first*/and can match only one way.build/shared/commit-message.mts): inTOOL_IDENTITY, the[bot]lookahead had two\s*runs next to each other. A long run of spaces after[bot]with a different character at the end took quadratic time to reject; 200,000 spaces took 20 s. The landing runner reads contributor commit messages, so this was reachable. The address group now includes its own trailing whitespace, and the pattern matches the same strings as before.tools/check-examples.js): the checker read each nested array element twice, once as written and once with the aside removed. That doubled the work at every level of nesting. Asides only ever follow a whole claim, so elements are now read once, as written. The whole claim is read a second time only when removing the aside actually changed it. The checker still reports the same 16 README examples and 306 doc-comment claims, with identical output.How: each fix comes with a regression test. One test checks adjacent comments and comments that contain asterisks in the brand checks. Another checks the 200,000-space trailer, which takes 20 s before the fix and 5 ms after. The
util-core,util-typesandtest:queuesuites pass on Node 24. The.githuband portal repos keep their own copies ofcommit-message.mts, so the same trailer fix goes to them in separate PRs.🤖 Generated with Claude Code
Summary by CodeRabbit