feat: Add support for @strands-agents/sdk#2144
Conversation
| function normalizeSerializable(value: unknown): unknown { | ||
| if (!isObject(value)) { | ||
| return value; | ||
| } | ||
| const toJSON = value.toJSON; | ||
| if (typeof toJSON === "function") { | ||
| try { | ||
| return toJSON.call(value); | ||
| } catch { | ||
| return value; | ||
| } | ||
| } | ||
| return value; | ||
| } |
There was a problem hiding this comment.
this just feels like slop tbh. Do we need all this normalization/guarding? We do that anyway before serializing the span to be sent to braintrust.
There was a problem hiding this comment.
yeah unnecessary, removed
There was a problem hiding this comment.
btw, this is pure slop. Strands has barely any downloads and I can't justify putting in too much work 💩
| | typeof strandsAgentSDKChannels.swarmStream; | ||
|
|
||
| export class StrandsAgentSDKPlugin extends BasePlugin { | ||
| private readonly activeChildParents = new WeakMap<object, Span[]>(); |
There was a problem hiding this comment.
We are treating this as a stack w/ pushChildParent and peekChildParent, but that goes against having this as a WeakMap. We should make sure we aren't using it as a stack, otherwise we'll definitely end up with incorrect parent child relationships.
| : ""; | ||
| } | ||
|
|
||
| function stringifyUnknown(value: unknown): string { |
There was a problem hiding this comment.
do we need to stringifyUnknown? Shouldn't braintrust span serialization take care of this?
There was a problem hiding this comment.
also unnecessary, removed
The only thing that I left is the error serialization because Error objects serialize to {} with JSON.stringify()
Fixes #2115