Repository navigation
fix: PikeVM capture route compiled lazy quantifiers as greedy - #137
Conversation
compilePikeVm built its NFA with a non-lazy-aware ThompsonBuilder, so lazy quantifiers got greedy priority and the extracted capture spans took the longest split instead of the lazy one, diverging from the JDK for both match() and findMatch*() group spans. Boolean results were unaffected (priority-independent), so only group-extracting callers were exposed. Every other PikeVM compile site already builds lazy-aware.
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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #137 +/- ##
=======================================
Coverage 85.4% 85.4%
Complexity 1 1
=======================================
Files 163 163
Lines 48759 48759
Branches 6970 6970
=======================================
+ Hits 41645 41649 +4
+ Misses 5105 5103 -2
+ Partials 2009 2007 -2
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
![datadog-prod-us1-3[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: done
and removed
mergequeue-status: waiting
labels
2 participants
What does this PR do?
One-line fix:
RuntimeCompiler.compilePikeVmnow builds its NFA withnew ThompsonBuilder(true)(lazy-aware), matching every other PikeVM compile site in the file.Motivation
compilePikeVmis the fallback the annotation processor emits for patterns its native strategies refuse (e.g.^(.+?)\.([^/]+)). It built the NFA with the non-lazy-awareThompsonBuilder, so lazy quantifiers (+?,*?,??) compiled with greedy thread priority. The extracted capture spans therefore took the longest split instead of the lazy (shortest) one and diverged from the JDK — for^(.+?)\.([^/]+)onnet/http.(*ServeMux).ServeHTTPthe JDK yields group1 =net/http, the PikeVM yieldednet/http.(*ServeMux).Boolean
matches()/find()are priority-independent and were never affected, which is why the existing boolean differential suites did not catch this. The PikeVM boolean DFA fast paths (findDfa/matchesDfa) are existence checks over the same NFA and remain priority-independent — no behavior change there.Related Issue(s)
None filed yet (regression present in 0.5.0; discovered during backend adoption).
Change Type
Checklist
./gradlew build)Performance Impact
None:
lazyAwareonly changes epsilon-transition ordering for lazy quantifier splits (thread priority), not state count or engine complexity. The rebuilt NFA is cached inPIKEVM_NFA_CACHEas before.Additional Notes
New test
PikeVMLazyGroupParityTest: differential JDK parity formatch()group strings andfindMatchFromper-group spans over 6 lazy patterns x 6 inputs on thecompilePikeVmroute.Companion PR on
fix/apt-findlongestmatchend-missingfixes the APT rich-API (findAll/replaceAll/split) crashes; the two are independent and both were found by the profiling-backend wave-2 differential gate.An external model review (gpt-5.6-terra) of this diff returned approve-with-comments; the one actionable finding (weak find-span oracle) was addressed by comparing all group spans.