Repository navigation
test(console): pin long-name lookarounds and warm the rule before timing it - #2951
Conversation
…ing it
The shape checks dropped lookaround bodies, so a nested quantifier inside a
lookahead (\p{M}* -> (?:\p{M}+)*) matched the same names, passed every case
and took seconds on a 28-character name (finding 2037). Each lookaround
branch must now be one class or escape after at most one run of marks, and
five timing names put 25 combining marks after a hyphen, an en dash, a thin
space and an ideograph.
The 50 ms bound also timed the rule's first build and its tier-up compile
(finding 2038). The rule now runs three times before any case is timed, and
a call over the bound is retried twice, the fastest counting, so a stalled
runner cannot flake a linear rule while a blowup still fails by seconds.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: test(console): pin long-name lookarounds and warm the rule before timing it This is item 1/1 in the current shard. Shard 0/1. This temporary status tracks the active review worker. The completed review will appear in the durable ClawSweeper review comment. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
clawsweeper
Bot
added
clawsweeper:intake:bug
Review nit: the mutant costs 0.6 to 4 s on the new names and a linear call about 0.1 ms, not the 8.7 s and 1 ms the comments said.
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
clawsweeper
Bot
added
P3
|
Codex review: needs maintainer review before merge. What this changesThis PR checks long-name regex lookaround structure, adds five combining-mark timing cases, and warms the matcher before measuring it. Example: A developer changes a lookahead to use nested combining-mark quantifiers.
Review scores
ProductKind: Test only · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review This PR adds useful regression coverage without changing product behavior; no actionable defect was found, and required CI passes for the reviewed head. Priority: P3 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherThe console classifies Agent names before rendering links; the conformance suite exercises that production classifier and checks its regex structure and execution time. flowchart LR
A[Agent names] --> B[Agents list renderer]
B --> C[Long-name classifier]
C --> D[Wrapping class]
E[Conformance cases] --> C
E --> F[Structure and timing assertions]
Technical reviewBest possible solution: Retain the focused lookaround guards and targeted timing cases alongside the existing overlap checks. Do we have a high-confidence way to reproduce the issue? Source inspection confirms that current-main structure checks discard lookaround bodies and lack the five new timing inputs; the PR reports mutation comparisons, which this read-only review did not execute. Is this the best way to solve the issue? The change extends the existing test owner, preserves production behavior, and combines deterministic structural rejection with timing coverage for distinct lookahead paths. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 7144acebd52a. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Likely related people:
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 19, 2026, 1:55 PM ET / 17:55 UTC. |
Problem
Two gaps in
tests/conformance/console-agent-name-runs.test.mjs(bughunt findings 2037 and 2038, follow-up to #2945):list.mjs:284-(?=\p{M}*[…])->-(?=(?:\p{M}+)*[…])matches the same names, passes all 21 cases, and takes seconds ona-+ 25 combining marks +x(0.03 ms on main). No timing name had a run of marks after a break character.Change
Test-only; no product code changes.
\p{M}*, or^.splitLookaroundsnow returns the lookaround bodies along with the kept source.withoutLookaroundsand the overlap check behave as before.Verification
node --test tests/conformance/console-agent-name-runs.test.mjs: 27/27; 20/20 pinned to 2 CPUs (taskset -c 2,3).Under load (7 busy threads on the same single CPU): passes 3/3, slowest case about 25 ms including harness overhead.
Mutants (
\p{M}*->(?:\p{M}+)*in each lookahead):Probe at 2 CPUs: calls 1 to 3 take 6.0, 1.0 and 0.09 ms. The warm-up covers both the build and the tier-up compile.
prettier --checkandeslint --max-warnings 0on the touched file: clean.No open PR touches this test file, so it adds no new conflicts.
Internal review: approve. Its nits are applied: the comments now state the measured costs.
Risks
Low; test-only. The new shape test refuses a future lookaround that holds a group or several mark runs. Such a rule change has to restate the bound in this test, which is intended.