Repository navigation
fix: route DFA_UNROLLED_WITH_GROUPS group-span divergences (A1+A2) to PIKEVM_CAPTURE - #90
Conversation
This comment has been minimized.
This comment has been minimized.
a392ffb to
e137c29
Compare
|
@codex review |
![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: 7e1db7bdcc
ℹ️ 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".
Squash-merge of 78 commits from fix/optimized-nfa-backref-perconfig: - Per-config captures for backref NFAs (Tasks 6-7) - 64-bit NFA contentHashCode + verify-on-hit for structural cache - Cache key NUL separator in RuntimeCompiler.cacheKeyFor() - PikeVM per-call compilation (no cached volatile field) - B16 guard: needsFallback check before PIKEVM_CAPTURE early return - epsilonGroup flag on DFA.GroupAction for zero-width group tracking - CRLF/end-anchor fixes (\Z/$ NEL/LS/PS) across all bytecode generators - BackrefBacktrackMatcher and per-config worklist for backref NFAs - Fix requiresBacktrackingForGroups false positive on anchor-only suffix Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extend hasStringEndAnchorInAlternation to cover $ (end-of-line anchor) in addition to \Z. Add two routing tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
B5: add missing broad-charset guard to hasGroupWithVariableLengthAlternationBody
B3b: restrict $ trigger to leading alternation branches; keep \Z triggering unconditionally;
return PIKEVM_CAPTURE (not OPTIMIZED_NFA) for cap group path — needsFallback rejects the latter
B6: restore suffix-decline logic at call sites; remove it from inside detectFixedRepetitionBackref
B4: restore PIKEVM_CAPTURE routing before requiresBacktrackingForGroups; restore .+ decline in
detectGreedyBacktrackPattern for non-group literal suffix (GREEDY_BACKTRACK overshoots on
trailing-newline inputs); update contradicting test to expect PIKEVM_CAPTURE
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add FuzzRunner.Config.patternSkip so sweeps can skip already-verified pattern ranges without re-running the oracle. Expose via -Dreggie.fuzz.skip=N in largeSweepConfig(). Add divergenceGate_extended (skip=25k, budget=43) covering patterns 25001–50000 at depth 3 which surfaces 22 unique pre-existing bugs (E1–E6); see doc/2026-06-29-fuzz-extended-findings.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… instructions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-06-29.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…0k patterns Remove KNOWN_FINDINGS_BUDGET_EXTENDED and divergenceGate_extended. Raise default sweep to 50k patterns (covers full validated range), budget=43 (E1-E6 findings from doc/fuzz/2026-06-29.md). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… raw, 29 repros) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
7494b35 to
efb797e
Compare
|
@codex review |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #90 +/- ##
=========================================
+ Coverage 82.4% 82.5% +0.1%
Complexity 1 1
=========================================
Files 128 128
Lines 38962 39226 +264
Branches 4962 5094 +132
=========================================
+ Hits 32136 32400 +264
+ Misses 5276 5208 -68
- Partials 1550 1618 +68
... and 19 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efb797ecaf
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4142fd5ebd
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e32904ce15
ℹ️ 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".
|
Auto-expanded scope: 1 sibling issue fixed in the same pass:
|
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bb9205ec8
ℹ️ 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".
| if (ast instanceof GroupNode g && g.capturing) { | ||
| if (isAnchorOnlyBody(g.child)) return true; | ||
| return hasAnchorOnlyCapturingGroup(g.child); |
There was a problem hiding this comment.
When the only anchor is hidden by a non-capturing wrapper, e.g. ((?:$)) or (?:($)), this branch does not inspect that wrapper: isAnchorOnlyBody(g.child) rejects a non-capturing GroupNode, and the recursion here only starts from a capturing group. Fresh evidence in the current code is that there is no later non-capturing GroupNode recursion path for this predicate, so these patterns skip the B3a PIKEVM route and, when OnePass-eligible, can hit the same zero-width capture span bug fixed for ($).
Useful? React with 👍 / 👎.
| * before the check, so both forms are correctly detected. | ||
| */ | ||
| public static boolean hasGreedyDotPlusGroupWithSuffix(RegexNode ast) { | ||
| if (!(ast instanceof ConcatNode concat)) return false; |
There was a problem hiding this comment.
For a transparent wrapper around the entire B4 shape, such as (?:(.+)_), the AST is a non-capturing GroupNode, so this early return false runs before the child concat is inspected. Fresh evidence in the current code is that unwrapNonCapturing is only applied to concat children after this check, not to ast itself. The semantically equivalent unwrapped (.+)_ routes to PIKEVM_CAPTURE to avoid TDFA extending group 1 into the suffix; the wrapped form can remain on the unsafe DFA-with-groups path.
Useful? React with 👍 / 👎.
2 participants
What does this PR do?
Routes two classes of
DFA_UNROLLED_WITH_GROUPSgroup-span bugs toPIKEVM_CAPTURE, reducing the fuzz divergence budget from 65 to 13.Motivation
The differential fuzzer (
AlgorithmicFuzzTest.divergenceGate) reported 65 pre-existing divergences, allDFA_UNROLLED_WITH_GROUPSgroup-span bugs. Two root-cause classes were identified:(a)|bmatches via the branch without group 1, TDFA incorrectly binds group 1 instead of leaving it at[-1,-1).a?,{0,n}), TDFA fires the group-start tag at an epsilon-reachable state, recordingmatch-startinstead ofgroup-start.Routing these patterns to
PIKEVM_CAPTURE(linear-time NFA simulator) gives correct spans with no backtracking penalty.Related Issue(s)
N/A
Change Type
Checklist
./gradlew build)Performance Impact
Patterns with groups absent from some alternation branch or with nullable first group elements now route to
PIKEVM_CAPTUREinstead ofDFA_UNROLLED_WITH_GROUPS. PikeVM runs in O(n·states) vs O(n) for TDFA, but these patterns were already producing wrong results on TDFA — correctness takes priority. All other patterns are unaffected.Additional Notes
Implementation:
FallbackPatternDetector.hasCapturingGroupAbsentFromSomeAlternative()— detects A2 by collecting group numbers per alternative and checking for non-uniform setsFallbackPatternDetector.hasCapturingGroupWithNullableFirstElement()— detects A1 by inspecting the first element of each capturing group's body via the existingisNullable()helper!hasNamedGroups && !hasAnchorInNfablock, before theDFA_UNROLLED_WITH_GROUPSstate-count check, each guarded by!hasNullableGroupContentWithNullableQuantifierRemaining 13 divergences (out of scope for this PR) fall into three distinct classes:
($),$|[^0]{1},$|[^c]{1}— outside the!hasAnchorInNfarouting block(.+)_— TDFA greedy group-end tag not rolled back when suffix character is also matched by the group([1]|1.)[b]_— TDFA group-end ambiguity when sibling alternatives have different lengths(.)\1{2}.— separate engine concern