fix: harden linear token sequence matcher - #111
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 42960 +38
Branches 5845 5851 +6
=======================================
+ Hits 36213 36260 +47
+ 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:
|
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.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![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: 6d47a27d3f
ℹ️ 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".
Restores the allocation-free contract for matches()/find()/findFrom() by reusing a ThreadLocal<MatchWorkspace> instead of allocating per call. Result-returning methods (match()/findMatchFrom()) copy arrays before returning, preserving thread-safety without aliasing.
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