Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe changes introduce AI SDK version tracking by reading the version from the AI SDK package and injecting it as a span attribute during transformation. The tracer is also initialized with the SDK version. Additionally, test import paths are corrected, and comprehensive AI SDK v5 compatibility tests are added. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to d76fb6e in 1 minute and 46 seconds. Click for details.
- Reviewed
714lines of code in6files - Skipped
0files when reviewing. - Skipped posting
4draft 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/ai-sdk-v5-compatibility.test.ts:500
- Draft comment:
The integration test is currently marked with it.skip. Consider adding a comment explaining why it’s skipped, or enabling it when the external dependencies (e.g. API keys) are available. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This comment is asking the PR author to either add a comment explaining why the test is skipped, or to enable it. This falls into the category of asking the author to explain or confirm their intention. The rules specifically state "Do NOT ask the PR author to confirm their intention, to explain, to double-check things, to ensure the behavior is intended". The comment is also somewhat speculative - it assumes that the test should either have a comment or be enabled, but the author may have intentionally skipped it temporarily during development. Additionally, whether a test is skipped is a decision the author made, and asking them to explain it is not a clear code change requirement. The comment could be seen as helpful for code maintainability - having documentation for why tests are skipped is generally good practice. It's also a relatively minor ask that could improve code quality. The author may have simply forgotten to add a comment or remove the skip. While documenting skipped tests is good practice, this comment is asking the author to explain their decision or confirm their intention, which violates the rules. The comment doesn't point to a definite code issue - it's more of a suggestion about documentation. If the test is skipped, there's likely a reason, and asking the author to document it falls under "asking to explain". This comment should be deleted because it asks the PR author to explain why the test is skipped or to confirm their intention to enable it, which violates the rule against asking authors to confirm intentions or explain their decisions. It's not pointing to a clear code defect.
2. packages/traceloop-sdk/test/ai-sdk/ai-sdk-v5-compatibility.test.ts:115
- Draft comment:
Several tests compare transformed tool parameters by checking the JSON.stringify output. This approach may be brittle if key order changes. Consider comparing parsed objects (using deepStrictEqual) to ensure robustness. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This comment is suggesting a refactor to make tests more robust against key ordering issues in JSON. However, this is a code quality suggestion, not a bug fix. The current approach usingJSON.stringifycomparison is actually intentional - the code appears to be testing that the transformation produces a specific JSON string format, not just that the data structure is correct. If the system stores or transmits these as JSON strings, then string comparison is the right approach. The comment is speculative ("may be brittle") and suggests a refactor without clear evidence that the current approach is wrong. According to the rules, I should only keep comments with STRONG EVIDENCE of correctness, and speculative refactoring suggestions should be deleted unless they are clearly actionable and necessary. The comment could be valid if JSON key ordering actually causes test failures in practice. Also, if the attributes are stored as strings (which they appear to be based on the code), then string comparison might be the correct approach to ensure the exact string format is preserved. Looking at the code more carefully, the attributes appear to be stored as strings (the transformation converts objects to JSON strings). Testing the exact string output is appropriate here. The comment is speculative about brittleness without evidence of actual problems, and the suggested change might actually make the tests less accurate if string format matters. This comment should be deleted. It's a speculative code quality suggestion without strong evidence that the current approach is wrong. The tests are validating that JSON strings are produced correctly, which makes string comparison appropriate. The comment doesn't identify a clear bug or required change.
3. packages/traceloop-sdk/test/ai-sdk/ai-sdk-v5-compatibility.test.ts:240
- Draft comment:
The test file is quite large and covers many aspects (version detection, tool schema transformation, tool call transformation, integration). Consider splitting these tests into multiple files or using describe blocks more granularly for maintainability. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This comment is about code quality and maintainability, which can be valuable. However, looking at the rules: 1) The file is already organized with describe blocks, 2) This is a subjective opinion about file size/organization, not a clear bug, 3) The comment uses "Consider" which is a soft suggestion, not a required change, 4) The file structure seems reasonable with clear separation of concerns already in place, 5) The rules state "Do NOT comment unless there is clearly a code change required" - this is more of a suggestion than a requirement, 6) The rules also say "Comments that suggest code quality refactors are good! But only if they are actionable and clear" - but this comment is vague about what specifically should be split and how. The comment could be considered actionable since it suggests splitting the file. The file is indeed quite long at 585 lines. Some teams might have conventions about maximum test file sizes. The comment is about maintainability which is a legitimate concern for long-term code health. While the comment is about maintainability, the file is already well-organized with clear describe blocks. The comment is too vague - it doesn't specify what should be split or why the current organization is problematic. It's a subjective opinion rather than pointing to a clear issue. The rules emphasize not making comments unless there's clearly a code change required, and this is more of a "nice to have" suggestion. The file structure appears logical: version detection, tool schema transformation, tool call transformation, and integration tests - these are related concerns that make sense together in a compatibility test file. This comment should be deleted. It's a subjective suggestion about file organization without clear actionable guidance. The file is already well-organized with appropriate describe blocks, and the comment doesn't point to a specific problem that needs fixing. It violates the rule that comments should only be made when there's clearly a code change required.
4. packages/traceloop-sdk/src/lib/tracing/tracing.ts:10
- Draft comment:
Typo: The key name "ASSOCATION_PROPERTIES_KEY" seems to be misspelled. Consider correcting it to "ASSOCIATION_PROPERTIES_KEY". - 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_rCezJ06qAXeUqL3J
You can customize 
doronkopit5
reviewed
Jan 18, 2026
Comment on lines
+726
to
+733
| const getAiSdkVersion = (): string | undefined => { | ||
| try { | ||
| return require("ai/package.json").version; | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| }; | ||
|
|
Member
There was a problem hiding this comment.
does this work? from the picture you added it doesnt
Contributor
Author
There was a problem hiding this comment.
@doronkopit5 It does work - it is just not added to the scope.
I added it to the spans_attributes as: ai.sdk.version
doronkopit5
approved these changes
Jan 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: TLP-1439
Important
Enhance Traceloop SDK with AI SDK v5 compatibility, version tracking, and updated scope version for node-server-sdk, including new tests.
getAiSdkVersion()inai-sdk-transformations.tsto retrieve AI SDK version and adds it to span attributes.getTracer()intracing.tsto includeTRACER_VERSION.ai-sdk-v5-compatibility.test.tsto test AI SDK v5 compatibility, including tool schema and tool call transformations.ai-sdk/ai-sdk-agent-integration.test.ts,ai-sdk/ai-sdk-integration.test.ts, andai-sdk/ai-sdk-transformations.test.tsfor better organization.ai.sdk.versionfield to span attributes intransformAiSdkSpanAttributes()inai-sdk-transformations.ts.This description was created by
for d76fb6e. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.