Repository navigation
feat(core): Share Sentry's search grammar across the CLI and MCP - #1532
Conversation
Add @sentry/toolkit-core/search-syntax, which runs an unchanged copy of the Sentry frontend's searchSyntax/grammar.pegjs and builds the frontend's token tree. The structural TokenConverter skips validation and parsed values; products subclass it to add them. The parser is compiled into src/generated/ and committed, with pre-commit and check:generated keeping it in sync with the grammar. Sentry's shared fixtures/search-syntax cases are vendored and all 231 pass. The CLI's query sanitizer now parses with it, and its simplified search-query.pegjs fork and generator are gone. MCP's looksLikeSentrySearchSyntax parses with it instead of scanning for key: tokens with a regex. Co-Authored-By: Burak Yigit Kaya <burak.kaya@sentry.io>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
We should remove/move any redundant tests in CLI and MCP modules to the new shared module now. |
Aggregate (count():10), array-includes (foo[*]:a), and other typed filters have no in-list rule in Sentry's grammar, so rewriting 'count():10 OR count():5' to 'count():[10,5]' produced an invalid query. Only text and numeric filters are now mergeable; the rest stay opaque and the OR is rejected as before. Co-Authored-By: Burak Yigit Kaya <burak.kaya@sentry.io>
The CLI and MCP each tested how Sentry's grammar reads regex patterns and key shapes. That's the shared parser's job now, so those cases live in one table in search-syntax.test.ts. The CLI keeps the sanitizer tests (AND stripping and OR rewriting around patterns) and MCP keeps its detection heuristics.
|
Done in a010a36. Moved to Removed:
Kept: tests of each package's own logic.
Tests: toolkit-core 331, CLI search-query 111, MCP search-events 67, all passing. Lint and format are clean. |
#1512 landed on main with a call to the old generated parser's parse(), which this branch removes. Use parseSearchNodes, which returns null for queries that don't parse.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit eb706b7. Configure here.
| return checkAggregate((name) => this.isPercentageKey(name)); | ||
| default: | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Aggregate size filters skip key checks
Medium Severity
predicateFilter gates AGGREGATE_DURATION, AGGREGATE_DATE, and AGGREGATE_PERCENTAGE on argument types, and simple SIZE keys on isSizeKey, but AGGREGATE_SIZE falls through to true. Any aggregate whose value looks like a size therefore matches as aggregateSize, even when no argument is a size key. The duration analog falls through to free text instead.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit eb706b7. Configure here.
There was a problem hiding this comment.
Not changing this: it matches Sentry on purpose. The frontend's searchSyntax/parser.tsx predicateFilter falls through to true for AGGREGATE_SIZE the same way, and the backend's event_search.py parses aggregate_key:size without checking the function's result type. The duration check only exists because m means both minutes and millions; size units don't clash with numbers like that. Diverging here would make toolkit disagree with both. If it should change, that belongs upstream in getsentry/sentry. Details in #1534.
…1534) Follow-up to the [Cursor Bugbot review](#1532 (review)) on #1532. ## Fixed: MCP regex scanner misses canonical array keys Sentry's grammar (`array_includes_tag_key`) writes array membership on an explicit tag inside the brackets: `tags[k[*],array]`. `REGEX_FILTER_KEY_SOURCE` in `mcp-core/.../search-events/utils.ts` only accepted `tags[k,array][*]`, which the grammar doesn't parse. As a result `searchFilterOccurrences` skipped canonical array-includes regex filters, and `isSemanticFilterDowngrade` couldn't tell when one was rewritten into a wildcard or full-text match. The key pattern now takes `[*]` inside the brackets, matching the grammar and the CLI's `REGEX_FILTER_KEY_SOURCE`. New tests cover `tags[k[*],array]://^a b//` rewritten to `tags[k[*],array]:*a*` and to `message:"*a b*"`; both fail without the fix. ## Not changed: aggregate size filters skip key checks `predicateFilter` returning `true` for `AGGREGATE_SIZE` is deliberate parity, not a bug in the port: - The frontend's `searchSyntax/parser.tsx` `predicateFilter` has the same fall-through. - The backend's `event_search.py` parses `aggregate_key:size` without checking the function's result type. `visit_aggregate_duration_filter` only checks because `m` is both minutes and millions; size units don't collide with numeric suffixes. Changing it here would make toolkit disagree with both. If it should change, that belongs upstream in getsentry/sentry, and toolkit would follow. ## Checks - mcp-core: 1823 tests pass - `oxlint` and `oxfmt` clean on the changed files <!-- junior-request-attribution:start --> via **byk**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:slack%3AC010884QHLG%3A1792178565.603129 --> -- [View Junior Session](https://junior-prod.sentry.dev/conversations/slack%3AC010884QHLG%3A1792178565.603129) [[Sentry]](https://sentry.sentry.io/explore/conversations/slack%3AC010884QHLG%3A1792178565.603129/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Burak Yigit Kaya <burak.kaya@sentry.io>
This branch was successfully deployed
1 participant
This is the first step toward one Peggy parser for Sentry search syntax, shared by the CLI, MCP, and eventually the Sentry frontend.
What changed
New
@sentry/toolkit-core/search-syntaxgrammar/search-syntax.pegjsis a byte-for-byte copy of getsentry/sentrystatic/app/components/searchSyntax/grammar.pegjs.parseSearch/parseSearchOrThrowreturn the same token tree as the frontend'sparseSearch: the sameToken,FilterTypeandTermOperatorvalues, and the same filter classification from the default key sets.TokenConverteronly builds structure. It never setsinvalid, warnings, orparsedvalues, because those depend ont(),moment, and tag metadata. A product that wants them subclassesTokenConverter(checkInvalid*hooks,is*Keypredicates) and passes it asconverter. That's how the frontend could use this package without losing its validation.fixtures/search-syntax/*.jsonare copied unchanged intotest/fixtures/(the frontend andevent_search.pyboth test against them). All 231 cases match once location, text, invalid and parsed are stripped.How the generated parser is kept in sync
toolkit-core has no build step, so the compiled parser is committed to
src/generated/.pnpm --filter @sentry/toolkit-core generaterebuilds it, and the output is deterministic.pnpm run check:generated(already in CI) now fails if the committed parser doesn't match the grammar.CLI
sanitizeQuerynow parses with the shared grammar and flattens the tokens into the sameSearchNodelist it used before, so the OR/AND rewrite logic is unchanged.script/search-query.pegjs(a 143-line fork of the frontend grammar),script/generate-parser.ts,generate:parser, and the CLI'speggydependency are all removed.count():>10used to fail to parse. They're now comparison filters.level:) and empty list items stay as written.tags[k[*],array]. One test usedtags[k,array][*], which Sentry's grammar doesn't accept as a key, so I updated it. I also taught the normalizer's preserved-regex pattern the canonical form.MCP
looksLikeSentrySearchSyntaxparses with the shared grammar instead of matchingSENTRY_SEARCH_TOKEN_PATTERN. The heuristics for telling prose from filters are the same as before: URL schemes,Note:,10:30, and//values. Regex filters are still only honored on logs.(level:error)used to return false only because the regex required whitespace before a key.count():>10now count too.searchFilterOccurrences, the semantic-downgrade comparison, still scans with regexes. Moving it to the parser is a follow-up.Size
The minified parser is about 38 KB bigger than the old CLI fork, about 8 KB gzipped, in the CLI and the MCP worker.
Not in this PR
private. The plan is to publish it, probably as@sentry/toolkit(the name is free on npm), so getsentry/sentry can depend on it. That needs a build step todist/and a craft target, which I'll do in a separate PR.arithmeticInput/grammar.pegjscan move here the same way once something uses it.Checks
search-syntax238 tests (231 fixtures plus API tests);tsc --noEmitcleantelemetry.test.ts › shows readonly warning when auto-repair fails, which fails onmainin this sandbox too.tscshows no errors except the unbuilt@sentry/mcp-server-mocksimportsoxlintandoxfmtclean on the touched filesvia byk.
--
View Junior Session [Sentry]