Repository navigation
feat(datasetAttributes): provide pagination option + attribute description based evals - #1505
Conversation
…ption based evals
…f-of-more-accurate-description
|
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}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ 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 42ab651. Configure here.
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
JoshuaKGoldberg
left a comment
JoshuaKGoldberg
left a comment
There was a problem hiding this comment.
Great stuff! I'm definitely not an area expert but this all looks good and reasonable from a data browsing perspective, I think. 👏
There was a problem hiding this comment.
[Style] A collection of unrelated formatting changes to Markdown - is this maybe an existing tooling issue in the repo?
There was a problem hiding this comment.
ya maybe, honestly idk how it did this because i swear my prettier extension hasn't been working for a while 😢
There was a problem hiding this comment.
Oh we switched from Prettier to Oxfmt, that might have something to do with it 😄
| const pageFields = [...availableFields].slice( | ||
| offset, | ||
| offset + FIELDS_PAGE_SIZE, | ||
| ); |
There was a problem hiding this comment.
[Question] This still means custom attributes are going to take >1 query to get surfaced. Spans have more built-in fields than that (including aggregates).
Should we do something like put custom attributes first and/or put aggregates last? Is there a general sense of which should come first? (genuinely asking, I don't have one myself)
There was a problem hiding this comment.
so they way it works is there are some attributes that are hardcoded because they are the most used and suggested by us. The rest of the attributes come after; more often than not most queries end up using these suggested attributes anyways so it's not really a big deal. Also when we spread the custom attributes on top of the suggested attributes it absorbs all the context and stuff from the attributes endpoint and puts it in the same position (AKA at the top). For attributes that aren't in that first page it will have to query it again.
| expect(result).toContain( | ||
| "call datasetAttributes again with the same arguments and offset: 50", | ||
| ); | ||
| expect(result).not.toContain("- device.memory: The estimated"); |
There was a problem hiding this comment.
[Bug] The list will have device.memory (number), so this isn't asserting anything.
There was a problem hiding this comment.
damn good catch, I'll fix that!
There was a problem hiding this comment.
I wish I could take credit for this one, it was a Claude finding haha
…f-of-more-accurate-description
This branch was successfully deployed
2 participants
This PR creates more evals; this case in particular is to see that the descriptions of attributes make a difference in which attributes the agent chooses. I've added two new cases: (1) chooses a field that has a description over a very similar field that doesn't and (2) chooses the more correct field between two similar fields that have descriptions.
The issue with the
datasetAttributestool is that it only shows the first 50 attributes, but majority of those first 50 attributes are the sentry recommended fields. This means the agent doesn't even see the attributes and the descriptions we need it to see. I've done some rearranging so that the field type is in brackets beside the attribute key so we can get rid of that entire section and i've filtered out the sentry provided attributes based on thesubstringMatch. Unfortunately that isn't quite enough since there are still attributes hidden, so i've added a pagination feature to this tool that will direct the agent to call the tool again to see the next page of results.