fix: harden linear token sequence matcher#111
Draft
jbachorik wants to merge 2 commits into
Draft
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #111 +/- ##
=======================================
Coverage 84.3% 84.4%
Complexity 1 1
=======================================
Files 147 147
Lines 42922 42955 +33
Branches 5845 5851 +6
=======================================
+ Hits 36213 36255 +42
+ Misses 4919 4916 -3
+ Partials 1790 1784 -6
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
This was referenced Jul 17, 2026
There was a problem hiding this comment.
Pull request overview
This PR hardens the runtime Linear Token Sequence (LTS) matcher to be safer under concurrency and more correct for Grok-style “named-only” capture projection, while adding targeted regression/performance tests around these behaviors.
Changes:
- Make LTS execution state per-invocation (removing shared scratch arrays) and make bracket-tail scanning linear-time.
- Gate runtime LTS routing on whether the plan actually covers all named capture indexes (avoid selecting an LTS route that can’t populate Grok-observable named groups).
- Add new correctness, JDK-differential, and concurrency tests for named-only routing and bracket scanning edge cases.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| reggie-runtime/src/main/java/com/datadoghq/reggie/runtime/RuntimeCompiler.java | Refines LTS routing by requiring plans to cover all named capture indexes before selecting the LTS matcher. |
| reggie-runtime/src/main/java/com/datadoghq/reggie/runtime/LinearTokenSequenceMatcher.java | Removes shared scratch state, introduces per-call workspace, and replaces bracket scanning with a linear pass. |
| reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/LinearTokenSequencePlan.java | Adds coversCaptureIndexes(...) to verify a plan has capture-producing ops for required group indexes (including inside optional sequences). |
| reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/LinearTokenSequencePlanTest.java | Adds unit tests validating capture-coverage semantics (optional nesting, named-group coverage, invalid indexes). |
| reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/LinearTokenSequenceMatcherTest.java | Adds routing regression test for named-only coverage plus new bracket-scanning edge/perf tests. |
| reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/LinearTokenSequenceMatcherConcurrencyTest.java | Introduces concurrent-use stress tests for cached LTS matchers and nested optional rollback behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+69
to
+73
| ExecutorService executor = Executors.newFixedThreadPool(threads); | ||
| CountDownLatch ready = new CountDownLatch(threads); | ||
| CountDownLatch start = new CountDownLatch(1); | ||
| CountDownLatch done = new CountDownLatch(threads); | ||
| ConcurrentLinkedQueue<Throwable> failures = new ConcurrentLinkedQueue<>(); |
Comment on lines
+114
to
+118
| ExecutorService executor = Executors.newFixedThreadPool(threads); | ||
| CountDownLatch ready = new CountDownLatch(threads); | ||
| CountDownLatch start = new CountDownLatch(1); | ||
| CountDownLatch done = new CountDownLatch(threads); | ||
| ConcurrentLinkedQueue<Throwable> failures = new ConcurrentLinkedQueue<>(); |
Comment on lines
+134
to
+136
| private MatchWorkspace newWorkspace() { | ||
| return new MatchWorkspace(groupCount, optionalDepth); | ||
| } |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
LinearTokenSequenceMatcherexecution state per-call and make bracket-tail scanning linearWhy
Grok consumes named capture indexes only. The named-only projection preserves those indexes, but the LTS plan could previously select a route that did not populate every named group. This change declines that route and lets the normal strict compiler choose the implementation instead.
Validation
./gradlew --rerun-tasks :reggie-codegen:test --tests '*LinearTokenSequencePlanTest' :reggie-runtime:test --tests '*LinearTokenSequenceMatcherTest' --tests '*LinearTokenSequenceAccessLogTest'./gradlew jacocoVerify build