Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Caution Review failedThe pull request is closed. WalkthroughRefactors vendor detection to use a prefix-based VENDOR_MAPPING and updates transformVendor to set LLM_SYSTEM from AI_MODEL_PROVIDER; adds AI SDK integration tests with Polly recordings and many HAR fixtures; introduces shared test setup utilities; moves tests to Polly Mocha integration; updates devDependencies and removes an old dataset test file. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test as Mocha Test
participant Polly as Polly (record/replay)
participant TL as Traceloop SDK
participant AI as AI SDK
participant Prov as Provider API
participant Expo as InMemorySpanExporter
Test->>TL: initializeSharedTraceloop()
Test->>Polly: setupPolly(record/replay)
Test->>AI: generateText(model, prompt, telemetry:on)
AI->>TL: start ai.generateText span
AI->>Polly: HTTP POST (recorded)
Polly->>Prov: forward request
Prov-->>Polly: 200 response (completion + usage)
Polly-->>AI: return response
AI->>TL: transformVendor (match prefixes via VENDOR_MAPPING)
TL->>TL: set span attrs (LLM_SYSTEM, model, usage, content)
TL->>Expo: export finished span
Test->>Expo: collect spans and assert attributes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to de72c86 in 1 minute and 9 seconds. Click for details.
- Reviewed
71lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/src/lib/tracing/ai-sdk-transformations.ts:117
- Draft comment:
Note: Vendor prefix matching relies on insertion order. Ensure that overlapping prefixes (e.g. 'amazon-bedrock' vs 'bedrock') are ordered correctly. Also consider case normalization to avoid mismatches. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =30%<= threshold50%The comment is informative and suggests ensuring correct ordering and case normalization for vendor prefix matching. It doesn't provide a specific code suggestion or ask for a test to be written. It seems to be more of a reminder or suggestion rather than a specific actionable comment.
2. packages/traceloop-sdk/test/ai-sdk-transformations.test.ts:337
- Draft comment:
Updated test expectations use capitalized vendor names (e.g. 'Anthropic'); ensure consistency with vendor mapping. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_bTStCgpf0ydjOK0q
You can customize 
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/traceloop-sdk/src/lib/tracing/ai-sdk-transformations.ts (2)
113-126: Make prefix matching robust (trim input, case-insensitive, prefer more-specific prefixes first)Today we rely on object insertion order and case-sensitive startsWith. This is fragile (e.g., “Amazon-Bedrock.chat” or “ openai.chat” won’t match; future reordering could make “bedrock” shadow “amazon-bedrock”). Trim, lowercase for matching, and sort prefixes by length to always prefer the most specific.
Apply:
if (AI_MODEL_PROVIDER in attributes) { - const vendor = attributes[AI_MODEL_PROVIDER]; - - // Find matching vendor prefix in mapping - let mappedVendor = null; - if (vendor && vendor.length > 0) { - for (const prefix of Object.keys(VENDOR_MAPPING)) { - if (vendor.startsWith(prefix)) { - mappedVendor = VENDOR_MAPPING[prefix]; - break; - } - } - } - - attributes[SpanAttributes.LLM_SYSTEM] = mappedVendor || vendor; + const raw = attributes[AI_MODEL_PROVIDER]; + const vendor = + typeof raw === "string" ? raw.trim() : String(raw ?? ""); + const vendorLc = vendor.toLowerCase(); + + // Prefer most-specific prefixes first to avoid shadowing (e.g., amazon-bedrock vs bedrock) + const entries = Object.entries(VENDOR_MAPPING).sort( + (a, b) => b[0].length - a[0].length, + ); + + let mappedVendor: string | null = null; + if (vendor) { + for (const [prefix, system] of entries) { + if (vendorLc.startsWith(prefix)) { + mappedVendor = system; + break; + } + } + } + + attributes[SpanAttributes.LLM_SYSTEM] = mappedVendor ?? vendor; delete attributes[AI_MODEL_PROVIDER]; }
17-38: Prettier CI failure: fix trailing whitespace and formattingCI reports a Prettier check failure. The new mapping block has trailing spaces on a few lines; transformingVendor block also has extra blank lines that Prettier will normalize.
Action:
- Run: pnpm prettier --write .
- Commit the formatting changes.
Also applies to: 113-126
packages/traceloop-sdk/test/ai-sdk-transformations.test.ts (1)
369-378: Rename test to reflect mapping semantics and add a few more casesThe assertion now expects a standardized mapped value (“Anthropic”), but the description still says “transform other providers to their value.” Suggest renaming for clarity and adding a couple of representative cases to lock the new behavior.
Apply:
- it("should transform other providers to their value", () => { + it("should map anthropic provider to standardized value", () => { const attributes = { "ai.model.provider": "anthropic", }; transformVendor(attributes); assert.strictEqual(attributes[SpanAttributes.LLM_SYSTEM], "Anthropic"); assert.strictEqual(attributes["ai.model.provider"], undefined); }); + + it("should map both amazon-bedrock and bedrock to AWS", () => { + ["amazon-bedrock.chat", "bedrock.converse"].forEach((provider) => { + const attrs = { "ai.model.provider": provider }; + transformVendor(attrs); + assert.strictEqual(attrs[SpanAttributes.LLM_SYSTEM], "AWS"); + assert.strictEqual(attrs["ai.model.provider"], undefined); + }); + }); + + it("should map vertex and google-prefixed providers to Google", () => { + ["vertexai", "google.generativelanguage"].forEach((provider) => { + const attrs = { "ai.model.provider": provider }; + transformVendor(attrs); + assert.strictEqual(attrs[SpanAttributes.LLM_SYSTEM], "Google"); + assert.strictEqual(attrs["ai.model.provider"], undefined); + }); + }); + + it("should handle case-insensitive and trimmed provider strings", () => { + const attrs = { "ai.model.provider": " OpenAI.Chat " }; + transformVendor(attrs); + assert.strictEqual(attrs[SpanAttributes.LLM_SYSTEM], "OpenAI"); + assert.strictEqual(attrs["ai.model.provider"], undefined); + });Note: the case-insensitive/trim test depends on the suggested transformVendor refactor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/traceloop-sdk/src/lib/tracing/ai-sdk-transformations.ts(2 hunks)packages/traceloop-sdk/test/ai-sdk-transformations.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/traceloop-sdk/test/ai-sdk-transformations.test.ts (1)
packages/ai-semantic-conventions/src/SemanticAttributes.ts (1)
SpanAttributes(17-59)
packages/traceloop-sdk/src/lib/tracing/ai-sdk-transformations.ts (1)
packages/ai-semantic-conventions/src/SemanticAttributes.ts (1)
SpanAttributes(17-59)
🪛 GitHub Actions: CI
packages/traceloop-sdk/src/lib/tracing/ai-sdk-transformations.ts
[error] 1-1: Prettier formatting check failed (pnpm prettier --check .) due to code style issues. Run 'pnpm prettier --write' to fix.
🔇 Additional comments (1)
packages/traceloop-sdk/src/lib/tracing/ai-sdk-transformations.ts (1)
17-38: Minor whitespace & branding nits in ai-sdk-transformations.ts
- Remove trailing spaces on these lines and re-run Prettier:
- Line 21:
"anthropic": "Anthropic",- Line 32:
"bedrock": "AWS",- Confirmed via a repo-wide search that the following mappings already align with other instrumentation modules:
OpenAI, Anthropic, Cohere, Azure, AWS, Google, Ollama, TogetherAI, Replicate, HuggingFace, OpenRouter.- The newly added entries—MistralAI, Groq, Fireworks, DeepSeek, Perplexity—don’t appear elsewhere in the codebase, so please verify their exact branding (dashboards/queries/etc.) before merging to avoid downstream mismatches.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 3480e68 in 40 seconds. Click for details.
- Reviewed
64lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/src/lib/tracing/ai-sdk-transformations.ts:19
- Draft comment:
Style: Using unquoted keys (when possible) improves readability. Verify all keys (e.g. 'amazon-bedrock') remain correctly handled. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/traceloop-sdk/src/lib/tracing/ai-sdk-transformations.ts:116
- Draft comment:
Suggestion: Normalize the vendor string (e.g. using toLowerCase()) before prefix matching to guard against inconsistent capitalization. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_VnHlrzaGFybx2LjQ
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 7d1a5ce in 25 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/src/lib/tracing/ai-sdk-transformations.ts:116
- Draft comment:
Good update ensuring 'vendor' is a string. Consider trimming the vendor (e.g. vendor.trim()) to handle potential whitespace. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_rO3uEEsogr9t78XN
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 1a854f5 in 40 seconds. Click for details.
- Reviewed
857lines of code in6files - Skipped
0files when reviewing. - Skipped posting
6draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/package.json:90
- Draft comment:
Updated devDependencies: New SDKs (@ai-sdk/amazon-bedrock and @ai-sdk/google) and version bumps (OpenAI and ai) support broader vendor normalization. Confirm these version updates align with the intended vendor name fixes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm that the version updates align with the intended vendor name fixes. This is a request for confirmation, which violates the rule against asking the PR author to confirm their intention. Therefore, this comment should be removed.
2. packages/traceloop-sdk/recordings/Test-AI-SDK-Integration-with-Recording_156038438/should-capture-Google-Gemini-provider-spans-correctly-with-recording_2671365066/recording.har:1
- Draft comment:
The HAR recording for Google Gemini appears complete with expected HTTP request/response details. Vendor-specific URL and response match the normalization expectations. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, describing the state of the HAR recording without suggesting any changes or improvements. It doesn't ask for confirmation or suggest any specific action.
3. packages/traceloop-sdk/recordings/Test-AI-SDK-Integration-with-Recording_156038438/should-capture-OpenAI-provider-spans-correctly-with-recording_3593617962/recording.har:1
- Draft comment:
The HAR recording for OpenAI is detailed and reflects a valid response with proper header details and payload. This provides a good basis for verifying vendor name normalization. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, describing the HAR recording without suggesting any changes or improvements. It doesn't align with the rules for useful comments.
4. packages/traceloop-sdk/recordings/Test-AI-SDK-Integration-with-Recording_156038438/should-transform-Google-Gemini-provider-spans-correctly-with-recording_4162977752/recording.har:1
- Draft comment:
The transformation recording for Google Gemini is consistent with the expected output; it reflects the right model version and response structure. This aids tests verifying vendor-based span adjustments. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, providing no actionable feedback or suggestions for improvement. It doesn't align with the rules for useful comments.
5. packages/traceloop-sdk/recordings/Test-AI-SDK-Integration-with-Recording_156038438/should-transform-OpenAI-provider-spans-correctly-with-recording_3580644828/recording.har:1
- Draft comment:
The transformed OpenAI recording captures the expected response differences (e.g. token counts, output messages) and header details, supporting tests for span attribute consistency. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment seems to be purely informative, describing what the transformed OpenAI recording does. It doesn't provide any actionable feedback or suggestions for improvement.
6. packages/traceloop-sdk/test/ai-sdk-integration.test.ts:91
- Draft comment:
Test cases properly invoke generateText for both OpenAI and Google vendors and assert span attributes (vendor, model, prompt, response, and token usage). The filtering of sensitive headers in beforeEach is a good security practice. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it describes what the test cases are doing and praises a security practice without suggesting any changes or improvements. It doesn't ask for any specific action or provide a code suggestion.
Workflow ID: wflow_Xaw1R8HUjkVaYWs1
You can customize
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/traceloop-sdk/test/ai-sdk-integration.test.ts (5)
38-56: Polly matching config is sensible; consider conditional timeout/loggingIgnoring query params avoids leaking API keys in URLs. Minor: when RECORD_MODE=NEW, 10s can be tight on CI; optionally increase timeout when recording and keep logs off in CI to reduce noise.
Apply:
describe("Test AI SDK Integration with Recording", function() { - this.timeout(10000); // Increase timeout to 10 seconds for API calls + this.timeout(process.env.RECORD_MODE === "NEW" ? 30000 : 10000); setupPolly({ @@ - logging: true, + logging: process.env.CI ? false : true,
76-84: Header sanitization is good; widen the scrub list to avoid secrets slipping inYou already filter Authorization/API-Key headers. Consider a broader regex-based filter to catch tokens/secrets that might appear under other names (e.g., Proxy-Authorization, X-Amz-Security-Token, Cookie/Set-Cookie).
- server.any().on("beforePersist", (_req, recording) => { - recording.request.headers = recording.request.headers.filter( - ({ name }: { name: string }) => - !["authorization", "x-api-key", "x-goog-api-key"].includes(name.toLowerCase()) - ); - }); + server.any().on("beforePersist", (_req, recording) => { + const shouldDrop = (name: string) => { + const n = name.toLowerCase(); + return ( + n === "authorization" || + n === "proxy-authorization" || + n === "x-api-key" || + n === "x-goog-api-key" || + n === "x-amz-security-token" || + n === "cookie" || + n === "set-cookie" || + /api[-_]?key|secret|token|session/i.test(n) + ); + }; + recording.request.headers = recording.request.headers.filter( + ({ name }: { name: string }) => !shouldDrop(name) + ); + });
108-118: Finder allows doGenerate but assertion enforces generate—make them consistentYou search for either ai.generateText.generate or ai.generateText.doGenerate, then assert strictly equals generate. This can cause a false failure if the found span is doGenerate.
- const generateTextSpan = spans.find( - (span) => span.name === "ai.generateText.generate" || span.name === "ai.generateText.doGenerate", - ); + const generateTextSpan = spans.find( + (span) => ["ai.generateText.generate", "ai.generateText.doGenerate"].includes(span.name), + ); @@ - assert.strictEqual(generateTextSpan.name, "ai.generateText.generate"); + assert.ok( + ["ai.generateText.generate", "ai.generateText.doGenerate"].includes(generateTextSpan.name), + `Unexpected span name: ${generateTextSpan.name}` + );
182-199: Nice disambiguation by workflow; align span-name assertion with finderFiltering by traceloop.workflow.name is robust. Apply the same “allow both names” assertion here to avoid brittle failures.
- const generateTextSpan = spans.find( - (span) => (span.name === "ai.generateText.generate" || span.name === "ai.generateText.doGenerate") - && span.attributes["traceloop.workflow.name"] === "test_google_workflow", - ); + const generateTextSpan = spans.find( + (span) => + ["ai.generateText.generate", "ai.generateText.doGenerate"].includes(span.name) && + span.attributes["traceloop.workflow.name"] === "test_google_workflow", + ); @@ - assert.strictEqual(generateTextSpan.name, "ai.generateText.generate"); + assert.ok( + ["ai.generateText.generate", "ai.generateText.doGenerate"].includes(generateTextSpan.name), + `Unexpected span name: ${generateTextSpan.name}` + );
163-165: Redundant exporter reset—afterEach already resetsThe manual reset at the start of the Gemini test is unnecessary because afterEach resets the exporter. Safe to remove to keep tests lean.
- // Clear any leftover spans from previous tests - memoryExporter.reset(); -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
packages/traceloop-sdk/package.json(2 hunks)packages/traceloop-sdk/recordings/Test-AI-SDK-Integration-with-Recording_156038438/should-capture-Google-Gemini-provider-spans-correctly-with-recording_2671365066/recording.har(1 hunks)packages/traceloop-sdk/recordings/Test-AI-SDK-Integration-with-Recording_156038438/should-capture-OpenAI-provider-spans-correctly-with-recording_3593617962/recording.har(1 hunks)packages/traceloop-sdk/recordings/Test-AI-SDK-Integration-with-Recording_156038438/should-transform-Google-Gemini-provider-spans-correctly-with-recording_4162977752/recording.har(1 hunks)packages/traceloop-sdk/recordings/Test-AI-SDK-Integration-with-Recording_156038438/should-transform-OpenAI-provider-spans-correctly-with-recording_3580644828/recording.har(1 hunks)packages/traceloop-sdk/test/ai-sdk-integration.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- packages/traceloop-sdk/recordings/Test-AI-SDK-Integration-with-Recording_156038438/should-transform-Google-Gemini-provider-spans-correctly-with-recording_4162977752/recording.har
- packages/traceloop-sdk/recordings/Test-AI-SDK-Integration-with-Recording_156038438/should-capture-Google-Gemini-provider-spans-correctly-with-recording_2671365066/recording.har
- packages/traceloop-sdk/recordings/Test-AI-SDK-Integration-with-Recording_156038438/should-transform-OpenAI-provider-spans-correctly-with-recording_3580644828/recording.har
- packages/traceloop-sdk/recordings/Test-AI-SDK-Integration-with-Recording_156038438/should-capture-OpenAI-provider-spans-correctly-with-recording_3593617962/recording.har
🧰 Additional context used
🪛 GitHub Actions: CI
packages/traceloop-sdk/package.json
[error] 1-1: specifiers in the lockfile don't match specifiers in package.json: 2 dependencies were added: @ai-sdk/amazon-bedrock@^3.0.10, @ai-sdk/google@^2.0.8; 2 dependencies are mismatched: - @ai-sdk/openai (lockfile: ^1.3.23, manifest: ^2.0.19) - ai (lockfile: ^4.3.19, manifest: ^5.0.22)
🔇 Additional comments (8)
packages/traceloop-sdk/package.json (3)
93-95: All @ai-sdk imports are aligned with the bumped versionsI ran the provided scan across the repository and confirmed that every import of
@ai-sdk/openai,@ai-sdk/google, and@ai-sdk/amazon-bedrockmatches the newly specified versions inpackages/traceloop-sdk/package.json. No legacy or mismatched@ai-sdk/*imports remain.
111-111: generateText API in v5 still supports bothpromptandmessages, andexperimental_telemetryremains unchangedI’ve confirmed in the AI SDK v5 reference that
generateText’s signature includes both apromptstring and amessagesarray option, so any tests using either are valid. Additionally, theexperimental_telemetryoption still uses theisEnabledflag (e.g.experimental_telemetry: { isEnabled: true }) as shown in the v5 docs, so no test changes are required for that setting. (sdk.vercel.ai)
93-95: Action Required: Commit updated pnpm-lock.yaml to unblock CIThe lockfile update ran successfully, but the automated commit failed due to missing Git user identity. Please:
- Configure your Git user in this repository (or globally):
git config user.email "you@example.com" git config user.name "Your Name"- Re-run the lockfile sync steps:
pnpm install --lockfile-only pnpm -w dedupe -t highest- Stage and commit the updated lockfile:
git add pnpm-lock.yaml git commit -m "chore: sync lockfile with ai-sdk deps (@ai-sdk/*, ai v5)" git push- Verify that CI now passes with the updated lockfile.
packages/traceloop-sdk/test/ai-sdk-integration.test.ts (5)
34-37: Good choice registering both Node and fetch adapters + FS persisterThis ensures both adapter paths are covered and recordings are file-backed, which is ideal for CI stability.
86-89: Nice: forceFlush + exporter reset in afterEach prevents cross-test contamination
150-159: Good coverage of usage countersAsserting prompt/completion/total tokens helps catch regressions in instrumentation and provider adapters.
196-205: Vendor mapping assertion matches PR intentChecking gen_ai.system === "Google" validates the new prefix-based transform. Similar checks for additional providers can be added later if you extend recordings.
91-101: Could you share the exact versions and API surface so we can confirm?Specifically, please provide one of the following for
ai@5and@ai-sdk/openai@2.x:
- A link to the package’s documentation or GitHub repo for those versions
- The relevant excerpt from your
package.jsonshowing those versions- A short TypeScript snippet or
.d.tsextract showing thegenerateTextsignatureWith that we can verify whether
generateTextaccepts amessagesarray or if you need to useprompt/streamTextinstead.
| it("should capture Google Gemini provider spans correctly with recording", async () => { | ||
| // Clear any leftover spans from previous tests | ||
| memoryExporter.reset(); | ||
|
|
||
| const result = await traceloop.withWorkflow( | ||
| { name: "test_google_workflow" }, | ||
| async () => { | ||
| return await generateText({ | ||
| messages: [{ role: "user", content: "What is 2+2? Give a brief answer." }], | ||
| model: vercel_google("gemini-1.5-flash"), | ||
| experimental_telemetry: { isEnabled: true }, | ||
| }); | ||
| } | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Same note on generateText input shape for Gemini
As above, confirm messages is valid with @ai-sdk/google@2.x and ai@5; switch to prompt if needed.
🌐 Web query:
For @ai-sdk/google@2 with ai@5, is `generateText({ model: google("gemini-1.5-flash"), messages: [...] })` supported, or should we use `prompt`?
💡 Result:
Short answer: Use prompt. The @ai-sdk/google v2 provider expects a text/plain prompt for generateText in ai@5 — it does not accept the chat-style messages array. See provider docs/examples for using prompt with google("gemini-1.5-flash") [1][2].
References
- @ai-sdk/google README / usage examples (generateText with prompt) — https://github.com/ai-sdk/google#usage
- ai (v5) docs — provider-specific guidance: using prompt vs messages — https://vercel.com/docs/ai/sdk/providers/google
Update generateText call to use prompt for Google Gemini provider
The Google v2 provider with ai@5 does not support a chat-style messages array. You need to pass a prompt string instead:
- File:
packages/traceloop-sdk/test/ai-sdk-integration.test.ts - Lines: 162–175
Suggested diff:
- return await generateText({
- messages: [{ role: "user", content: "What is 2+2? Give a brief answer." }],
- model: vercel_google("gemini-1.5-flash"),
- experimental_telemetry: { isEnabled: true },
- });
+ return await generateText({
+ prompt: "What is 2+2? Give a brief answer.",
+ model: vercel_google("gemini-1.5-flash"),
+ experimental_telemetry: { isEnabled: true },
+ });This aligns with the @ai-sdk/google v2 usage examples and ai@5 provider requirements.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("should capture Google Gemini provider spans correctly with recording", async () => { | |
| // Clear any leftover spans from previous tests | |
| memoryExporter.reset(); | |
| const result = await traceloop.withWorkflow( | |
| { name: "test_google_workflow" }, | |
| async () => { | |
| return await generateText({ | |
| messages: [{ role: "user", content: "What is 2+2? Give a brief answer." }], | |
| model: vercel_google("gemini-1.5-flash"), | |
| experimental_telemetry: { isEnabled: true }, | |
| }); | |
| } | |
| ); | |
| return await generateText({ | |
| prompt: "What is 2+2? Give a brief answer.", | |
| model: vercel_google("gemini-1.5-flash"), | |
| experimental_telemetry: { isEnabled: true }, | |
| }); |
🤖 Prompt for AI Agents
In packages/traceloop-sdk/test/ai-sdk-integration.test.ts around lines 162–175,
the test calls generateText with a chat-style messages array which the Google
Gemini v2 ai@5 provider does not support; change the generateText invocation to
pass a single prompt string (e.g. "What is 2+2? Give a brief answer.") instead
of messages, keep the same model and experimental_telemetry settings, and remove
the messages array so the request conforms to the @ai-sdk/google v2/ai@5 usage.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed d24c4b5 in 48 seconds. Click for details.
- Reviewed
143lines of code in1files - Skipped
1files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/recordings/Test-SDK-Decorators_847855269/should-fix-Vercel-AI-spans-to-match-OpenLLMetry-format_2061519753/recording.har:23
- Draft comment:
Request headers: Updated 'headersSize' from 167 to 172. Verify these hardcoded values remain in sync with actual header changes. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/traceloop-sdk/recordings/Test-SDK-Decorators_847855269/should-fix-Vercel-AI-spans-to-match-OpenLLMetry-format_2061519753/recording.har:32
- Draft comment:
Response content: Body size and JSON payload updated (including new 'annotations' field and token count changes). Confirm these modifications reflect the new API response schema. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. packages/traceloop-sdk/recordings/Test-SDK-Decorators_847855269/should-fix-Vercel-AI-spans-to-match-OpenLLMetry-format_2061519753/recording.har:80
- Draft comment:
Response headers: Multiple header values (e.g. cookie, cf-ray, date, rate limits, openai-project) have been updated. Ensure these changes don’t introduce test flakiness with dynamic values. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_PL0R5nbs3u6JTpKi
You can customize
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/traceloop-sdk/recordings/Test-SDK-Decorators_847855269/should-fix-Vercel-AI-spans-to-match-OpenLLMetry-format_2061519753/recording.har (1)
41-51: Redact cookies and Set-Cookie from the recording (leaks identifiers and adds churn).The
_cfuvidcookie and theSet-Cookieheader are persisted in the fixture. While not secrets per se, they are user/session identifiers and shouldn’t live in the repo. They also change frequently and force needless fixture updates.Apply this minimal redaction in-place:
- "value": "6OCymRr_pq59AuM8_jpvZCL0CjxOxn4DzISXFGK5d34-1756047991023-0.0.1.1-604800000" + "value": "<redacted>" ... - "value": "_cfuvid=6OCymRr_pq59AuM8_jpvZCL0CjxOxn4DzISXFGK5d34-1756047991023-0.0.1.1-604800000; path=/; domain=.api.openai.com; HttpOnly; Secure; SameSite=None" + "value": "<redacted>"Optionally, also sanitize at record-time to avoid future churn:
// test/setupPolly.ts (example – adjust to your setup) import { Polly } from '@pollyjs/core'; export function installPollySanitizers(polly: Polly) { polly.server.any().on('beforePersist', (_req, recording) => { // Strip cookies array if (recording.response.cookies) recording.response.cookies = []; // Redact cookie-like headers const redactHeader = (name: string) => { const h = recording.response.headers?.find(h => h.name.toLowerCase() === name); if (h) h.value = '<redacted>'; }; ['set-cookie', 'cookie'].forEach(redactHeader); }); }I can open a follow-up PR to wire this into your test bootstrap if helpful.
Also applies to: 106-108
🧹 Nitpick comments (5)
packages/traceloop-sdk/recordings/Test-SDK-Decorators_847855269/should-fix-Vercel-AI-spans-to-match-OpenLLMetry-format_2061519753/recording.har (5)
67-67: Stabilize volatile correlation/timing headers to reduce fixture churn.Headers like
cf-ray,date,openai-processing-ms,x-envoy-upstream-service-time, andx-request-idvary on every run and bloat diffs.Either ignore these for matching or normalize values:
- "value": "9743c10608d5a6f4-ATL" + "value": "<cf-ray>" ... - "value": "Sun, 24 Aug 2025 15:06:31 GMT" + "value": "<date>" ... - "value": "214" + "value": "0" ... - "value": "248" + "value": "0" ... - "value": "req_9688be6e03ea4e04b8ecb671b43cb141" + "value": "<request-id>"Or configure Polly once:
// test/setupPolly.ts (example) polly.server.any().on('beforePersist', (_req, rec) => { const redact = (names: string[]) => { names.forEach(n => { const h = rec.response.headers?.find(h => h.name.toLowerCase() === n); if (h) h.value = '<redacted>'; }); }; redact(['x-request-id', 'cf-ray', 'date', 'openai-processing-ms', 'x-envoy-upstream-service-time']); });Also applies to: 83-83, 91-92, 121-124, 151-151
127-147: Rate-limit headers are inherently volatile; prefer ignoring them for matching.
x-ratelimit-*values shift regularly and are not relevant to the test’s intent.You can ignore them during matching instead of editing fixtures:
// test/setupPolly.ts (example) polly.server.any().on('beforePersist', (_req, rec) => { rec.response.headers = rec.response.headers?.filter( h => !/^x-ratelimit-/i.test(h.name) ); });
23-23: Deflake HAR sizes and timings (set to “unknown”/constant).Fields like
headersSize,bodySize,content.size,startedDateTime,time, andtimings.waitare noise for assertions and cause frequent re-records.If you keep them, consider normalizing:
- "headersSize": 172, + "headersSize": -1, ... - "bodySize": 822, + "bodySize": -1, ... - "size": 822, + "size": -1, ... - "headersSize": 998, + "headersSize": -1, ... - "startedDateTime": "2025-08-24T15:06:29.867Z", + "startedDateTime": "1970-01-01T00:00:00.000Z", - "time": 978, + "time": 0, ... - "wait": 978 + "wait": 0These values are valid in HAR 1.2 (
-1means “unknown”). If your tooling prefers preserving them, alternatively drop them in abeforePersistsanitizer.Also applies to: 35-35, 38-38, 154-154, 160-161, 169-169
39-39: Sanitize dynamic fields inside the JSON response body string.The embedded JSON includes volatile fields (
id,created). Normalizing them avoids future churn and reduces internal IDs in the repo.Minimal edit to the string payload:
- "text": "{\n \"id\": \"chatcmpl-C86ac6A14JHgaULSyyiWvTnUOKF2R\",\n \"object\": \"chat.completion\",\n \"created\": 1756047990, + "text": "{\n \"id\": \"chatcmpl-REDACTED\",\n \"object\": \"chat.completion\",\n \"created\": 0,If editing the string is brittle, sanitize at record-time by parsing JSON and overwriting fields before persisting:
// test/setupPolly.ts (example) polly.server.any().on('beforePersist', (_req, rec) => { const c = rec.response.content; if (c?.mimeType === 'application/json' && typeof c.text === 'string') { try { const obj = JSON.parse(c.text); if (obj.id) obj.id = 'chatcmpl-REDACTED'; if (obj.created) obj.created = 0; c.text = JSON.stringify(obj, null, 2); } catch { /* ignore parse errors */ } } });
29-29: Optional: move away from legacy model ids in fixtures.The request uses
gpt-3.5-turbowhich is EOL-ish and can confuse future readers. Since the vendor mapping isn’t model-dependent, consider using a contemporary small model id in the fixture (or a generic placeholder) for longevity.Example:
- "text": "{\"model\":\"gpt-3.5-turbo\", ... + "text": "{\"model\":\"gpt-4o-mini\", ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
packages/traceloop-sdk/recordings/Test-SDK-Decorators_847855269/should-fix-Vercel-AI-spans-to-match-OpenLLMetry-format_2061519753/recording.har(7 hunks)
🔇 Additional comments (2)
packages/traceloop-sdk/recordings/Test-SDK-Decorators_847855269/should-fix-Vercel-AI-spans-to-match-OpenLLMetry-format_2061519753/recording.har (2)
39-39: Good capture of new OpenAI response fields.The fixture reflects the newer payload shape (e.g.,
annotations,service_tier). This helps keep transformations resilient to schema drift. No action needed here.
39-39: No Volatile Fields Detected in Tests or RecordingsI ran the provided ripgrep checks across both the test suite and recording fixtures and found no occurrences of:
"id": "chatcmpl-…"or"created": <number>x-request-idorcf-ray"set-cookie","cookies": [,"openai-project", or"Authorization"This confirms that no dynamic or sensitive fields are being snapshotted. Your span transformations and vendor‐normalization tests are not inadvertently depending on volatile response content. Everything looks safe to merge.
avivhalfon
force-pushed
the
ah/fix-vendors-verecel-sdk
branch
from
d24c4b5 to
b7991e4
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed b7991e4 in 52 seconds. Click for details.
- Reviewed
179lines of code in1files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/test/ai-sdk-integration.test.ts:38
- Draft comment:
Formatting improvement: the function declaration now uses 'function ()' (with a space) for consistency and improved readability. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/traceloop-sdk/test/ai-sdk-integration.test.ts:80
- Draft comment:
The arrow function formatting for filtering headers has been reformatted into a multi-line style which enhances clarity. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. packages/traceloop-sdk/test/ai-sdk-integration.test.ts:139
- Draft comment:
Assertion formatting has been streamlined (e.g. assert.ok inline formatting) to improve readability. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. packages/traceloop-sdk/test/ai-sdk-integration.test.ts:120
- Draft comment:
For consistency and better debugging, consider adding an error message to the assert checking for generateTextSpan in the OpenAI test, similar to the Google test. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
5. packages/traceloop-sdk/test/ai-sdk-integration.test.ts:126
- Draft comment:
The tests currently cover vendor normalization for OpenAI and Google. Consider extending the tests to include additional provider names mentioned in the release notes for comprehensive coverage. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
Workflow ID: wflow_5xXWUCgXCUdoclPc
You can customize
There was a problem hiding this comment.
Skipped PR review on 1af8362 because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.
nirga
force-pushed
the
ah/fix-vendors-verecel-sdk
branch
from
1af8362 to
1d433f9
Compare
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 1d433f9 in 3 minutes and 5 seconds. Click for details.
- Reviewed
12925lines of code in50files - Skipped
1files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/src/lib/tracing/ai-sdk-transformations.ts:115
- Draft comment:
Ensure vendor mapping handles overlapping prefixes. Currently the loop returns the first match; if a vendor string (e.g., 'amazon-bedrock' vs 'bedrock') could match multiple keys, consider clarifying the ordering or using a more precise match. Also, logging a warning when no prefix matches may help diagnose unsupported vendor names. - Reason this comment was not posted:
Comment was on unchanged code.
2. packages/traceloop-sdk/src/lib/tracing/ai-sdk-transformations.ts:63
- Draft comment:
In transformPromptMessages, the catch block silently swallows JSON parsing errors. Consider logging a debug message to alert developers when prompt data isn’t in valid JSON format. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. packages/traceloop-sdk/test/datasets-comprehensive.test.ts:57
- Draft comment:
Using a constant dataset name ('test-dataset-comprehensive-example') may cause collisions in concurrent test runs. Consider appending a unique identifier (e.g. a timestamp) for uniqueness. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This appears to be a test file using Polly.js for recording/replaying HTTP requests. The fixed dataset name is intentional - when in replay mode, we want the exact same requests to be made to match the recorded responses. Adding a timestamp would break the replay functionality since the requests would never match. The previous code had a timestamp but it was deliberately removed as part of this change. Could there still be collision issues in record mode? The code does use timestamps for error test datasets later in the file. No - in record mode, the code is already set up to use a different name via the process.env.RECORD_MODE check that was removed. The error test datasets are different because they are temporary and meant to be deleted, not recorded for replay. The comment should be deleted. The fixed dataset name is intentional for the Polly.js record/replay functionality to work correctly.
4. packages/traceloop-sdk/recordings/Dataset-API-Comprehensive-Tests_1618738334/Row-Management_2874258994/should-partial-update-row-data_3739884320/recording.har:276
- Draft comment:
There appears to be an inconsistent key casing in the PUT request postData. In other parts of the HAR the key is "values" (all lowercase), but here it is "Values" with an uppercase V. Consider using consistent casing for clarity. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a test recording file that captures HTTP traffic. The casing inconsistency could indicate a real bug in the code making the API request. However, since the request succeeded (status 200), the API appears to handle both casings fine. The comment is suggesting a style improvement rather than fixing a functional issue. Test recordings are typically auto-generated and may not need the same level of style consistency as actual code. The comment points out a real inconsistency, but I may be overvaluing style consistency in test recording files. These files are meant to capture real traffic rather than serve as example code. While style consistency is generally good, modifying auto-generated test recordings for pure style reasons could make them diverge from the actual traffic they're meant to capture. The comment should be deleted as it suggests changing an auto-generated test recording file for purely stylistic reasons.
5. packages/traceloop-sdk/test/decorators.test.ts:648
- Draft comment:
I noticed a potential typo in the test description on line 648: "should fix Vercel AI spans to match OpenLLMetry format". If this is intended to reference "OpenTelemetry", please update the spelling accordingly. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_NaMCH22qb21JKCZr
You can customize
nirga
force-pushed
the
ah/fix-vendors-verecel-sdk
branch
from
1d433f9 to
b262f8d
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed b262f8d in 1 minute and 36 seconds. Click for details.
- Reviewed
12705lines of code in50files - Skipped
1files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/test/test-setup.ts:23
- Draft comment:
The shared test setup is clear and concise. Using an InMemorySpanExporter with an initialization guard (isInitialized) prevents duplicate initialization. Consider adding a brief comment noting that if tests ever run concurrently, the mutable module-level flag may require extra care. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/traceloop-sdk/test/decorators.test.ts:640
- Draft comment:
There appears to be a typographical error in the test description on line 640: "should fix Vercel AI spans to match OpenLLMetry format". Is "OpenLLMetry" intended, or should it be "OpenTelemetry"? - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_iVxc5tOvkFM2c7Mz
You can customize
nirga
force-pushed
the
ah/fix-vendors-verecel-sdk
branch
from
b262f8d to
1902c3c
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 1902c3c in 1 minute and 47 seconds. Click for details.
- Reviewed
12702lines of code in50files - Skipped
1files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/test/test-setup.ts:1
- Draft comment:
The test setup file is clear and concise. It creates a shared in-memory span exporter and uses an initialization guard with an isInitialized flag. This pattern is good for preventing duplicate initialization across tests. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply praises the existing code without offering any specific guidance or questions.
2. packages/traceloop-sdk/test/test-setup.ts:23
- Draft comment:
Consider whether additional safety is needed (e.g. thread-safety or race condition checks) if tests run concurrently, though for typical Mocha runs this is acceptable. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =20%<= threshold50%The comment suggests considering additional safety measures for concurrent tests, which is a general suggestion and not specific to the code changes. It doesn't provide a specific code suggestion or point out a specific issue in the code.
3. packages/traceloop-sdk/test/test-setup.ts:28
- Draft comment:
The traceloop.initialize call includes disableBatch:true which is appropriate for testing. Optionally, add logging for debugging initialization if needed. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is purely informative, explaining the use of a parameter in a function call. It does not provide a specific suggestion or ask for a test to be written. It also suggests adding logging optionally, which is not a specific request. This comment should be removed as it violates the rules.
4. packages/traceloop-sdk/test/test-setup.ts:37
- Draft comment:
The getSharedExporter function straightforwardly returns the shared exporter, which meets the requirements for sharing span data across tests. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states what the function does, which is not necessary in a code review context.
5. packages/traceloop-sdk/test/decorators.test.ts:640
- Draft comment:
Typographical error: 'OpenLLMetry' in the test description looks like it might be a misspelling of 'OpenTelemetry'. Please correct if this is not intentional. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_PWBgA0qTP6VneuUA
You can customize
Co-authored-by: Nir Gazit <nirga@users.noreply.github.com>
2 participants
Important
Standardized AI provider detection and updated tests for consistent vendor names across multiple providers.
ai-sdk-transformations.tsto show consistent vendor names across providers like OpenAI, Anthropic, Google, etc.transformVendor()to map vendor prefixes to standardized names.ai-sdk-integration.test.tsfor OpenAI and Google Gemini spans, prompts/responses, and token usage.decorators.test.tsto expect standardized vendor names.package.jsonand added provider SDK packages.This description was created by
for 1902c3c. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Tests
Chores