perf: eliminate unnecessary json marshalling for anthropic requests to bridge #102
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
SasSwart
left a comment
SasSwart
left a comment
There was a problem hiding this comment.
I've reviewed the PR structurally, and I only have one comment about test coverage. The rest looks good.
At a higher level I'm a little concerned that we might not sufficiently document why we need to do this kind of content modification.
Its a seemingly invalid assumption of mine that because Bridge acts as a gateway, there should be no discrepancies between the payloads we receive and the ones we track or send on.
Yet, we need to introduce the stream attribute, and me need to coerce tool calls into a specific format and I've been unable to determine why.
| checkContent: nil, | ||
| }, | ||
| { | ||
| name: "mcp_tool_result with string content unchanged", |
There was a problem hiding this comment.
Is this test case (and others related to tool_result, mcp_tool_result) no longer relevant? We still have logic in convertStringContentRecursive that relates to these types of content.
SasSwart
changed the title
Merge activity
|
Signed-off-by: Danny Kopping <[email protected]>
SasSwart
force-pushed
the
marshal-once
branch
from
14690bd to
78c808d
Compare
2 participants
The actions of determining whether to use Anthropic's streaming APIs and ensuring a uniform message payload format used to unmarshal and remarshal the same payload twice. This PR ensures we unmarshal once, do both, and then remarshal once.