Repository navigation
Publish toolkit-core as @sentry/toolkit so getsentry/sentry can share the search-syntax parser? #1533
Copy link
Copy link
Open
Description
Question
Should we publish packages/toolkit-core (today @sentry/toolkit-core, private: true) to npm as @sentry/toolkit, so that getsentry/sentry's frontend can use the same code as the CLI and MCP server, starting with the search-syntax parser?
@scttcper @gggritso — what do you think? Your take on the frontend side especially: would you add a dependency like this, and is there anything that would block it?
Context
- feat(core): Share Sentry's search grammar across the CLI and MCP #1532 adds
@sentry/toolkit-core/search-syntax. Its grammar (grammar/search-syntax.pegjs) is a copy of the frontend'sstatic/app/components/searchSyntax/grammar.pegjs, and it passes Sentry's sharedfixtures/search-syntaxcases with the same token tree. The plan is for the frontend to import the grammar/parser from a package instead of keeping its own copy. The frontend'sTokenConverter, validation,t(),momentand tag-data logic would stay in the frontend. - fix(cli): Sort every Events dataset through a shared resolver #1521 added
events-fields(a port ofparseFunction/parseArgumentsand the aggregate/equation helpers fromstatic/app/utils/discover/fields.tsx), andevents-sort. Both are candidates to share too. A follow-up would also move the frontend'sarithmeticInput/arithmeticBuildergrammars over. - Without a shared package, these grammars drift. The CLI's old search grammar was a 143-line simplified fork of the frontend's 541-line one before feat(core): Share Sentry's search grammar across the CLI and MCP #1532 replaced it.
- Related: toolkit merge: Phase B — extract @sentry/toolkit-core (shared code) #1469 (Phase B, extracting
@sentry/toolkit-coreas an internal workspace dep).
Why toolkit-core and not a separate package
- No
node:imports. It already runs in the CLI (Node/Bun) and the MCP server (Cloudflare Workers). - Everything is exported by subpath (
./search-syntax,./events-fields,./api-request, …), so the frontend would only bundle the parts it imports. @sentry/toolkitis currently unclaimed on npm.- One published package is simpler than splitting off a
@sentry/search-syntaxpackage and then more packages later.
What it would take
- Build step: toolkit-core ships
.tssource only (exportspoint at./src/*.ts). Publishing means adding adist/build (ESM +.d.ts) and pointing the publishedexportsat it, while keeping source resolution for workspace consumers. - Release: add a
packages/toolkit-coreworkspace to.craft.ymlwith annpmtarget, a tag prefix, versioning and changelog. The CLI and MCP already release from this monorepo with Craft, so this follows the existing setup. - Rename: change
@sentry/toolkit-coreto@sentry/toolkiteverywhere it's imported inpackages/cliandpackages/mcp*. It's mechanical but touches a lot of files, so it should be its own PR. - API surface: once it's published, its subpaths are a public API. We need to decide what's public and what stays internal (for example, the auth/OAuth/region-cache modules), and how strict semver is.
- Frontend PR:
getsentry/sentryswaps its local.pegjsimports for the package, and uses its existingparser.spec.tsx/tokenizer.spec.tsxas parity tests.
Open questions
- Is
@sentry/toolkitthe right name? Alternatives include@sentry/toolkit-coremade public, or a narrower@sentry/search-syntax. - Would the frontend accept grammar changes coming in through toolkit releases instead of being edited in place? Who should own grammar changes?
- Should we publish everything, or only the parser/fields modules at first?
via byk.
--
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels
Activity
linear-code commentedon Oct 16, 2026
MCP-1092
fix(core): Gate aggregate size filters on a size argument
sentry-junior commentedon Oct 17, 2026
Known divergence from the frontend parser (from #1534)
The shared parser no longer matches the frontend on one point. In the frontend's
static/app/components/searchSyntax/parser.tsx,predicateFilterchecks argument types forAGGREGATE_DURATION,AGGREGATE_DATEandAGGREGATE_PERCENTAGE, but returnstrueforAGGREGATE_SIZE. So any aggregate whose value looks like a size (avg(span.duration):>1kb) parses as an aggregate size filter, even when none of its arguments is a size field.#1534 fixes this in
@sentry/toolkit-core/search-syntax:AGGREGATE_SIZEnow requires a size-key argument, like the other typed aggregates. Without one, the term falls back to free text, as a duration value on a non-duration aggregate already does.Before the frontend switches to the shared package, we should either fix this upstream in
getsentry/sentry, or accept the stricter behavior there. With an emptysizeKeys,avg(x):>1kbstops being a typed filter, so frontend callers would need to pass their size fields. For reference, the backend'sevent_search.pydoesn't check the result type for size filters either.nsdeschenes commentedon Oct 21, 2026
Heyo @BYK 👋
We have a couple of concerns with this move:
BYK commentedon Oct 21, 2026
Hey @nsdeschenes - thanks a lot, this is exactly why I pinged you folks! Would it be possible to publish the spec/parser from the Sentry side like we do for the scraps package that @natemoo-re created? The core need is having a shared spec/parser for this. I don't really care where it lives :)
BYK commentedon Oct 21, 2026
And also we should probably incorporate that fix we added here back on the Sentry side
nsdeschenes commentedon Oct 22, 2026
Hrrm, that is a possibility!
We'll have to chat with the design-eng folks to see what that would look like exactly, and how it fits in with things. They're fairly coupled right now, so we'd have to put some work in figuring out how to de-couple things, but this approach would be more favourable overall in my opinion!
BYK commentedon Oct 22, 2026
@nsdeschenes no need for chats or effort, I'll put Junior on it if you agree. If you folks don't like the PR we can iterate on it or simply not merge.
nsdeschenes commentedon Oct 22, 2026
Yea that should be alright, if you want to initially tag me for review, i can tag the others who should take a peek at it too.
sentry-junior commentedon Oct 22, 2026
Opened getsentry/sentry#128514 and requested review from @nsdeschenes.
What it does
@sentry/search-syntaxunderstatic/packages/search-syntax, set up for publishing the same way as@sentry/icons. The frontend grammar moves into it unchanged, along with theToken/TermOperator/FilterType/BooleanOperatorenums. The app now imports both from the package, so there is only one copy of the grammar.TokenConverterandparseSearchto the package. This is the toolkit port, and it passes every case infixtures/search-syntax. Those are the fixtures the frontend parser andevent_search.pyalready test against, so the API↔FE contract covers the package too.AGGREGATE_SIZEgap from fix(core,mcp): Bugbot follow-ups from the shared search parser #1534 in the frontend parser, so typed size aggregates now need a size-key argument. The backend still doesn't check this. That's noted on the PR as a follow-up.Not in this PR
The app keeps its own validating
TokenConverter. Having it subclass the package converter, so the structural code lives in one place, is a follow-up.Next on the toolkit side
Once the package is on npm, swap
@sentry/toolkit-core/search-syntaxand the vendoredgrammar/search-syntax.pegjsfor@sentry/search-syntax/parserand@sentry/search-syntax/tokens. The exported names are the same, but the enums are TS enums rather thanas constobjects.