feat(op)!: Add gen_ai prefix to existing ops and add embeddings, generate_content, and rerank ops - #543
Conversation
Semver Impact of This PR🔴 Major (breaking changes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Breaking Changes 🛠
New Features ✨Attributes
Op
Other
Internal Changes 🔧Attribute
Attributes
Deps
Other
🤖 This preview updates automatically when you update the PR. |
![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.
Reviewed by Cursor Bugbot for commit 49b319b. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| "name": "gen_ai.chat", | ||
| "description": "A chat interaction with a generative AI model" | ||
| }, | ||
| { |
There was a problem hiding this comment.
Bug: Prefixing field names in gen_ai.json causes breaking changes to both the exported constant names and their underlying string values, affecting downstream consumers and data processing.
Severity: HIGH
Suggested Fix
To avoid breaking downstream consumers, revert the field names in gen_ai.json to their original unprefixed form (e.g., chat, execute_tool). If the prefix is necessary, introduce new fields and deprecate the old ones, or modify the code generator in scripts/generate_op.ts to handle the prefixing logic correctly without creating double-prefixed constant names and provide backward-compatibility aliases for the old constants.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: model/op/gen_ai.json#L6-L9
Potential issue: Changing the field names in `gen_ai.json` from unprefixed (e.g.,
`chat`) to prefixed (e.g., `gen_ai.chat`) introduces two breaking changes. First, it
alters the generated constant names from `GEN_AI_CHAT_SPAN_OP` to
`GEN_AI_GEN_AI_CHAT_SPAN_OP`. Since this is a publicly published package, any downstream
consumer importing the old constant names will receive `undefined` at runtime, causing
silent failures. Second, it changes the underlying string values of these constants from
`'chat'` to `'gen_ai.chat'`. Any system, such as the Sentry backend or SDKs, that
matches or filters spans based on these string values will fail to recognize spans using
the old values, leading to incorrect categorization or missing telemetry data.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
I think this is fine to be honest, given we fix incorrect ops. Now, the other question is, whether the exported constant names make much sense given they now all start with GEN_AI_GEN_AI_* 😅 IMHO grouping span ops by "category" was unnecessary and more unnecessary that we added this category to the generated op constant names. Anyway, we can fix this in a follow-up. Your changes are necessary fixes because the previous ops were wrong.
| "name": "gen_ai.chat", | ||
| "description": "A chat interaction with a generative AI model" | ||
| }, | ||
| { |
There was a problem hiding this comment.
I think this is fine to be honest, given we fix incorrect ops. Now, the other question is, whether the exported constant names make much sense given they now all start with GEN_AI_GEN_AI_* 😅 IMHO grouping span ops by "category" was unnecessary and more unnecessary that we added this category to the generated op constant names. Anyway, we can fix this in a follow-up. Your changes are necessary fixes because the previous ops were wrong.
yeah i agree, similar thing happening with a lot of the browser ops starting with |
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
actually sorry, let's hold off from merging until we clarified whether the Rust package has any consumers that could break
There was a problem hiding this comment.
Relay doesn't depend on ops, for python we don't even generate them and the Sentry frontend neither uses any exported ops from the JS package. So we're good to break here. I'll track renaming the ops constants in general in #546
Can you still add a ! to the commit message to indicate that this is a breaking change?
msonnb
changed the title
msonnb
changed the title
gen_ai prefix to existing ops and add embeddings, generate_content, and rerank ops
…`generate_content`, and `rerank` ops Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Description
Adds
gen_ai.embeddings,gen_ai.generate_contentandgen_ai.rerankto gen ai ops and adds thegen_ai.prefix to existing ops to align with the docs and SDKs.PR Checklist
yarn testand verified that the tests pass.yarn generateto generate and format code and docs.If an attribute was added:
nextjs.function_id, notfunction_id)apply_scrubbing(i.e.manualorauto. Useneveronly for values that should never be scrubbed such as IDs)If an attribute was deprecated: