Repository navigation
fix(model): omit request model attribute instead of setting 'unknown' - #25379
kashish00208 wants to merge 2 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| [GEN_AI_OPERATION_NAME]: operationName, | ||
| [SENTRY_ORIGIN]: WORKERS_AI_ORIGIN, | ||
| [GEN_AI_REQUEST_MODEL]: typeof model === 'string' ? model : 'unknown', | ||
| [GEN_AI_REQUEST_MODEL]: typeof model === 'string', |
There was a problem hiding this comment.
Workers AI records boolean as model
High Severity
GEN_AI_REQUEST_MODEL and the span-name modelName are assigned typeof model === 'string' (a boolean) instead of the model id. Every Workers AI span therefore reports true/false as the request model, and static span names become values like chat true rather than the real model.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f459c34. Configure here.
| operationName: string, | ||
| ): void { | ||
| // Store embeddings input on a separate attribute | ||
| if (operationName === 'embeddings' && 'input' in params) { |
There was a problem hiding this comment.
Fix PR has no covering tests
Medium Severity
This is a fix PR but the diff has no unit, integration, or E2E tests for omitting gen_ai.request.model or for the new span-name behavior. Existing tests still expect 'unknown' (and 'chat unknown' / 'embeddings unknown' in static mode) and were not updated, so the regression this PR claims to fix is not covered. Flagged because the review rules require a fix PR to include a test that fails without the change and passes with it.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit f459c34. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
There are 6 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f339a32. Configure here.
| return originalResult.then(result => { | ||
| addResponseAttributes(span, result, options.recordOutputs, operationName); | ||
| return originalResult.then((result) => { | ||
| addResponseAttributes(span, result, options.recordOutputs); |
There was a problem hiding this comment.
Decide spans lose recorded outputs
High Severity
The non-streaming OpenAI path now calls addResponseAttributes without operationName. Decision answers are only recorded when that argument is 'decide', so gen_ai.output_messages is no longer set on decisions.create spans.
Reviewed by Cursor Bugbot for commit f339a32. Configure here.
| span: Span, | ||
| params: Record<string, unknown>, | ||
| operationName: string, | ||
| ): void { |
There was a problem hiding this comment.
Decide inputs overwritten as chat messages
High Severity
The early return that skipped addRequestAttributes for decide operations was removed. Decision input is already recorded in evaluation format at span start; this path now re-reads params.input as chat messages and overwrites gen_ai.input_messages.
Reviewed by Cursor Bugbot for commit f339a32. Configure here.
| const spanConfig = { | ||
| name: | ||
| (typeof model === 'string' && model !== 'unknown') || !(client && hasSpanStreamingEnabled(client)) | ||
| (typeof model === 'string') || !(client && hasSpanStreamingEnabled(client)) |
There was a problem hiding this comment.
Missing model interpolates as undefined
Medium Severity
When no model is resolved, the name still interpolates model whenever span streaming is off (and in the Google GenAI integration, even when it is on). Template strings then produce names like chat undefined instead of chat or chat unknown. Empty-string models also become chat with a trailing space.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit f339a32. Configure here.
| ): Record<string, SpanAttributeValue | undefined> { | ||
| const system = langSmithMetadata?.ls_provider; | ||
| const modelName = invocationParams?.model ?? langSmithMetadata?.ls_model_name ?? 'unknown'; | ||
| const modelName = invocationParams?.model ?? langSmithMetadata?.ls_model_name; |
There was a problem hiding this comment.
Chat models still default to unknown
Medium Severity
extractChatModelRequestAttributes still falls back to 'unknown' for the model, so LangChain chat spans keep setting gen_ai.request.model to 'unknown' and naming spans {operation} unknown. The LLM helper was updated; the chat-model path, which is the common one, was not.
Reviewed by Cursor Bugbot for commit f339a32. Configure here.
|
Hi @kashish00208, thanks for the PR. We are still debating whether to do this in v11 or wait until the next major. Will get back to you once we decided. |
Thanks for the update! I appreciate you taking the time to consider this. |
|
👋 @mydea, @andreiborza — Please review this PR when you get a chance! |
|
Hi @kashish00208, we decided to keep the current behavior until our next major 12.0.0. I added a milestone to that ticket. We will close this PR until then, but thank you again for the contribution. Feel free to subscribe to the original issue and reopen your PR then. |
Summary
Fixes #25353
Fixes an issue where AI spans defaulted
gen_ai.request.modelto'unknown'and generated span names like<operation> unknownwhen no model was provided.Changes
Across all AI integrations (
openai,anthropic-ai,google-genai,langchain,workers-ai):'unknown'fallback return values from model extraction helpers.gen_ai.request.modelattribute when no model can be resolved.<operation>instead of<operation> unknownwhenmodelis undefined (consistent with and without span streaming).Affected Integrations
packages/server-utils/src/ai/openai/&integrations/openai.tspackages/server-utils/src/ai/anthropic-ai/&integrations/anthropic.tspackages/server-utils/src/ai/google-genai/&integrations/google-genai.tspackages/server-utils/src/ai/langchain/packages/server-utils/src/ai/workers-ai/Test Plan
gen_ai.request.modelis not set whenmodelis absent.