feat(search-events): correctly process attribute context in tests + deprecated attribute evals by nikkikapadia · Pull Request #1440 · getsentry/toolkit

nikkikapadia · 2026-10-07T18:32:05Z

The main purpose of this PR was to make sure that our evals with the attribute context take deprecated attributes into account and pick the replacement instead.

To do this, the attribute context we are providing in the mocks needed to be properly handled, parsed and passed to the agent in a way that it will understand. We've gone through and excluded attributes that have been deprecated from the list of attributes the agent sees and then also list out the deprecated fields and what its replacement attribute is.

I've adjusted some unit tests to reflect these changes as well.

Closes EXP-1288

…eprecated attribute evals

linear-code · 2026-10-07T18:32:46Z

EXP-1288

JoshuaKGoldberg

This generally looks good to me as a toolkit novice! I ran it through Claude and it produced a collection of reports that I narrowed down to a bunch of questions & nitpicks. So you shouldn't trust my review at all, this is just for me learning 😄

JoshuaKGoldberg · 2026-10-07T20:37:21Z

 7. Use datasetAttributes substringMatch, query, and attributeTypes for targeted lookup when broad field discovery is truncated
 8. For non-replay datasets, call validateSearch after constructing the candidate request. If invalid, fix and validate again in this same pass
 9. NEVER replace a structured field:value filter with message/log.body/full-text matching. If an explicit field is unavailable on the dataset, keep it and let validation fail instead of inventing a weaker query
+10. If datasetAttributes lists a field under Deprecated Fields, use its replacement instead, even when the deprecated name appears in the guidance or examples in this prompt


[Question] Nothing in production passes context: true to fetchCustomAttributes. Is that something we handle separately?

yes! we don't want to add it in just yet, i want to make sure all the evals are in and working before turning it on in the mcp

JoshuaKGoldberg · 2026-10-07T20:37:21Z

+ * truncated off the end of the listing, and the swapped-out fields are
+ * returned separately so the agent can be told not to use them.
+ */
+function preferReplacementFields(fields: Record<string, AttributeDescriptor>): {


[Question] I think this swap goes the opposite direction from Sentry's own handling? When a deprecated attribute and its replacement are both in the same RPC page, the attributes endpoint hides the replacement and keeps the source (_replacement_superseded_by_present_source in organization_trace_item_attributes.py). Is that something we need to worry about?

And/or, should this have direct unit tests?

oh? this is news to me i'd have to look into that. Maybe we can chat about this in the DMs

took a look at this! This is true lol but the 🤖 took a look at how it's done in seer and it basically replaces the key/name with whatever the replacement attribute is and passed that to the llm so that it chooses correctly. I'm gonna do the same thing here. So instead of looking for that replacement attribute (which you're right wouldn't exist) i'm gonna change the key of the attribute so it represents the recommended attribute.

That makes sense to me 🚀

JoshuaKGoldberg · 2026-10-07T20:37:21Z

+  if (context?.isDeprecated) {
+    parts.push(
+      context.replacementAttribute
+        ? `DEPRECATED: use ${context.replacementAttribute} instead.`


[Bug?] This hint can steer the agent somewhere unhelpful when the replacement isn't in the list:

13 conventions are deprecated with no backfill/normalize status (e.g. http.host → server.address, route → http.route), so the replacement may have no data

db.params / db.sql.bindings have the templated replacement db.query.parameter.<key>, which can't be queried literally

On logs, numeric attributes come back as tags[http.status_code,number] with replacement http.response.status_code. But the bare/direct http.response.status_code resolves as a string attribute on logs. So http.response.status_code:503 misses the numeric data.

On spans, transaction would read "DEPRECATED: use sentry.segment.name instead.", but the endpoint re-aliases sentry.segment.name to transaction. environment/release get the same label even though they resolve to the same storage as sentry.environment/sentry.release.

Should we maybe...

...give some kind of "if (thing) is possible" treatment? (or do agents just know to not fully trust these hints?)

...if this is legit, treat these mismatches as bugs... somewhere?

hm yes maybe worth putting in there to use the replacement if possible and if it exists. I'll add that in and make an eval for a case like this!

JoshuaKGoldberg · 2026-10-07T20:37:21Z

+${replacedFields.length > 30 ? `\n... and ${replacedFields.length - 30} more deprecated fields` : ""}`
+    : ""
+}
 Recommended Fields for ${dataset}:


[Bug] Recommended Fields and the EXAMPLE QUERIES below aren't filtered against replacedKeys, so the same output can say "do NOT use http.method" and then show has:http.method as an example. Rule 10 covers this for the prompt but just for the Deprecated Fields section.

Filtering or rewriting those here might be more reliable. And either way, something to test maybe?

ya good point. I'll do an audit of those examples

JoshuaKGoldberg · 2026-10-07T20:37:21Z

            name: "datasetAttributes",
            arguments: {
              dataset: "spans",
+              substringMatch: "http.method",


[Testing] With params: "fuzzy", this requires a datasetAttributes call whose substringMatch contains "http.method", so a run that only calls { dataset: "spans" } and still produces the right query scores lower. The with-context case doesn't require it. Was that intentional?

oh yes i actually was testing something and left this in accidentally 😭 i'll clean this up

JoshuaKGoldberg · 2026-10-07T20:37:21Z

    return [
      {
-        // EVENTUALLY Context marks http.method as deprecated in favor of http.request.method
+        // Context marks http.method as deprecated in favor of http.request.method


[Docs] Missing also updating the the suite-level comment above this?

cursor

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.}

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit b4af07d. Configure here.}

nikkikapadia · 2026-10-09T16:35:34Z

I talked to Dom yesterday about the concerns with replacement attributes for things fields that are not backfilled and he said it shouldn't matter because we still coalesce at query time. As for using sentry.segment.name over transaction and other examples like that, this would be following sentry conventions which we've claimed as ideal behaviour in seer so i think it makes sense to keep it like that in the mcp too. I'm not going to update the example queries we put in the prompt just yet because the context is just being used in evals; it will make more sense to change these when we have context digestion out in prod.

JoshuaKGoldberg

feat(search-events): correctly process attribute context in tests + d…

9ef270d

…eprecated attribute evals

nikkikapadia deployed to Actions October 7, 2026 18:32 — with GitHub Actions Active

nikkikapadia marked this pull request as ready for review October 7, 2026 18:56

github-actions Bot added the risk: medium PR risk score: medium label Oct 7, 2026

JoshuaKGoldberg approved these changes Oct 7, 2026

View reviewed changes

address feedback, add more evals, adjust mocks and prompts

1223591

nikkikapadia deployed to Actions October 9, 2026 15:57 — with GitHub Actions Active

Merge branch 'main' into nikki/digest-eval-mock-context

b4af07d

nikkikapadia deployed to Actions October 9, 2026 15:57 — with GitHub Actions Active

cursor Bot reviewed Oct 9, 2026

View reviewed changes

Comment thread packages/mcp-core/src/tools/support/search-events/utils.ts

Comment thread packages/mcp-core/src/tools/support/search-events/utils.ts

cursor fixes

40a8b51

nikkikapadia deployed to Actions October 9, 2026 16:21 — with GitHub Actions Active

nikkikapadia requested a review from JoshuaKGoldberg October 9, 2026 16:35

JoshuaKGoldberg approved these changes Oct 9, 2026

View reviewed changes

Conversation

nikkikapadia commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linear-code Bot commented Oct 7, 2026

Uh oh!

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nikkikapadia commented Oct 9, 2026

Uh oh!

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nikkikapadia commented Oct 7, 2026 •

edited

Loading