fix: web fetch fixes - #4945
Merged
Merged
Conversation
|
|
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds typed web-fetch result support across Anthropic and Bifrost, including new schemas, conversion helpers, streaming/message wiring, deep-copy updates, a new ChangesWeb Fetch Typed Result Support
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
Collaborator
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
core/providers/anthropic/passthroughstream_test.go (1)
429-493: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding an error-variant round-trip test.
TestAnthropicWebFetchResultRoundTriponly exercises the success path (web_fetch_resultwith a document). The error branch (ResultType: "web_fetch_tool_result_error",ErrorCodeset, noDocument) is a distinct code path in bothconvertAnthropicWebFetchResultToBifrostandconvertBifrostWebFetchCallToAnthropicBlocksand isn't covered.Want me to draft a second test case covering the
web_fetch_tool_result_errorround trip?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/anthropic/passthroughstream_test.go` around lines 429 - 493, `TestAnthropicWebFetchResultRoundTrip` only covers the success path, so add a second round-trip case for the error variant of the web fetch result. Update the test to construct an `AnthropicContentBlock`/Bifrost payload with `ResultType` set to `web_fetch_tool_result_error`, an `ErrorCode`, and no `Document`, then verify both `convertAnthropicWebFetchResultToBifrost` and `convertBifrostWebFetchCallToAnthropicBlocks` preserve that error state. Use the existing helpers like `convertAnthropicWebFetchResultToBifrost`, `convertBifrostWebFetchCallToAnthropicBlocks`, and `getAnthropicContentObject` to assert the rebuilt block still represents the error variant.core/providers/anthropic/responses.go (1)
5880-5896: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: prefer
schemas.Ptr()over&toolUseID.Based on learnings, this repository prefers
bifrost.Ptr()/schemas.Ptr()over the address operator even when&is syntactically valid.✏️ Proposed tweak
- msg.ResponsesToolMessage.CallID = &toolUseID + msg.ResponsesToolMessage.CallID = schemas.Ptr(toolUseID)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/anthropic/responses.go` around lines 5880 - 5896, Prefer the repository’s pointer helper instead of taking the address directly in attachAnthropicWebFetchResult. Update the CallID assignment on ResponsesToolMessage to use schemas.Ptr() for toolUseID rather than &toolUseID, keeping the existing conversion flow and symbol names unchanged.Source: Learnings
core/schemas/responses.go (1)
1367-1369: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAvoid promoted-field ambiguity in
ResponsesToolMessageThe anonymousResponsesAdvisorCall,ResponsesWebFetchCall, andResponsesCodeExecutionCallembeddings all publishResultTypeandErrorCode, soResponsesToolMessage.ResultType/.ErrorCodestays ambiguous. Keep those accesses fully qualified or switch these variants to named fields.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/schemas/responses.go` around lines 1367 - 1369, The anonymous embeddings in ResponsesToolMessage are causing promoted-field ambiguity for ResultType and ErrorCode. Update ResponsesToolMessage so the ResponsesAdvisorCall, ResponsesWebFetchCall, and ResponsesCodeExecutionCall variants are no longer accessed through ambiguous promoted fields; either make those fields explicitly qualified at each use site or replace the anonymous embeddings with named fields and adjust all references accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@core/providers/anthropic/passthroughstream_test.go`:
- Around line 429-493: `TestAnthropicWebFetchResultRoundTrip` only covers the
success path, so add a second round-trip case for the error variant of the web
fetch result. Update the test to construct an `AnthropicContentBlock`/Bifrost
payload with `ResultType` set to `web_fetch_tool_result_error`, an `ErrorCode`,
and no `Document`, then verify both `convertAnthropicWebFetchResultToBifrost`
and `convertBifrostWebFetchCallToAnthropicBlocks` preserve that error state. Use
the existing helpers like `convertAnthropicWebFetchResultToBifrost`,
`convertBifrostWebFetchCallToAnthropicBlocks`, and `getAnthropicContentObject`
to assert the rebuilt block still represents the error variant.
In `@core/providers/anthropic/responses.go`:
- Around line 5880-5896: Prefer the repository’s pointer helper instead of
taking the address directly in attachAnthropicWebFetchResult. Update the CallID
assignment on ResponsesToolMessage to use schemas.Ptr() for toolUseID rather
than &toolUseID, keeping the existing conversion flow and symbol names
unchanged.
In `@core/schemas/responses.go`:
- Around line 1367-1369: The anonymous embeddings in ResponsesToolMessage are
causing promoted-field ambiguity for ResultType and ErrorCode. Update
ResponsesToolMessage so the ResponsesAdvisorCall, ResponsesWebFetchCall, and
ResponsesCodeExecutionCall variants are no longer accessed through ambiguous
promoted fields; either make those fields explicitly qualified at each use site
or replace the anonymous embeddings with named fields and adjust all references
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e3a48ce-8184-4503-bc59-c5304043d22e
📒 Files selected for processing (8)
core/providers/anthropic/passthroughstream_test.gocore/providers/anthropic/requestbuilder_test.gocore/providers/anthropic/responses.gocore/providers/anthropic/types.gocore/providers/anthropic/utils.gocore/schemas/responses.gocore/schemas/utils.goframework/streaming/responses.go
Contributor
Confidence Score: 5/5Safe to merge; the web-fetch round-trip logic is well-tested and all previously flagged issues have been addressed The three previously flagged issues (text-block drop, missing passthrough phantom-index fallback, and streaming Caller aliasing) are all resolved and covered by new tests. The remaining findings are narrow edge cases: the UseCache type-upgrade condition misses the 20260209 family for intermediate model versions, and a malformed item with a nil ResponsesToolMessage on the streaming path would produce a content_block_start with no block. Neither affects the primary web-fetch round-trip path. core/providers/anthropic/responses.go — UseCache type-upgrade condition and the nil-ResponsesToolMessage guard on the streaming output_item.added path Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant AC as Anthropic API
participant FWD as ToBifrostResponsesStream
participant NF as Neutral Format (ResponsesMessage)
participant REV as ToAnthropicResponsesStreamResponse
AC->>FWD: content_block_start (server_tool_use, web_fetch)
FWD->>NF: output_item.added web_fetch_call (in_progress)
AC->>FWD: content_block_start (web_fetch_tool_result)
Note over FWD: accumulate result block
AC->>FWD: content_block_stop (web_fetch_tool_result)
FWD->>NF: output_item.done web_fetch_call + ResponsesWebFetchCall payload
NF->>REV: web_fetch_call item with typed result
REV->>AC: content_block_stop (server_tool_use)
REV->>AC: content_block_start (web_fetch_tool_result)
REV->>AC: content_block_stop (web_fetch_tool_result)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant AC as Anthropic API
participant FWD as ToBifrostResponsesStream
participant NF as Neutral Format (ResponsesMessage)
participant REV as ToAnthropicResponsesStreamResponse
AC->>FWD: content_block_start (server_tool_use, web_fetch)
FWD->>NF: output_item.added web_fetch_call (in_progress)
AC->>FWD: content_block_start (web_fetch_tool_result)
Note over FWD: accumulate result block
AC->>FWD: content_block_stop (web_fetch_tool_result)
FWD->>NF: output_item.done web_fetch_call + ResponsesWebFetchCall payload
NF->>REV: web_fetch_call item with typed result
REV->>AC: content_block_stop (server_tool_use)
REV->>AC: content_block_start (web_fetch_tool_result)
REV->>AC: content_block_stop (web_fetch_tool_result)
Reviews (3): Last reviewed commit: "fix: web fetch fixes" | Re-trigger Greptile |
TejasGhatte
force-pushed
the
07-06-fix_web_fetch_fixes
branch
from
July 6, 2026 10:49
70f1f9b to
8a5ec7a
Compare
18 tasks
Collaborator
Merge activity
|
Pratham-Mishra04
changed the base branch from
07-06-test_additions_to_harness_tests
to
graphite-base/4945
July 6, 2026 11:29
Pratham-Mishra04
dismissed
coderabbitai[bot]’s stale review
July 6, 2026 11:32
The base branch was changed.
Pratham-Mishra04
force-pushed
the
07-06-fix_web_fetch_fixes
branch
from
July 6, 2026 11:32
8a5ec7a to
30c1b9a
Compare
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
3 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.
Summary
The Anthropic
web_fetch_tool_resultblock was previously discarded after conversion — only the request URL was preserved. This PR carries the full typed result payload (fetched document content, metadata, error codes) through the Bifrost neutral format so that Anthropic-compatible reverse conversion can faithfully reconstruct theserver_tool_use+web_fetch_tool_resultblock pair on both the streaming and non-streaming paths.Changes
ResponsesWebFetchCall,ResponsesWebFetchDocument, andResponsesWebFetchSourceschema types to represent theweb_fetch_tool_resultpayload in the neutral format. Embedded*ResponsesWebFetchCallinResponsesToolMessagealongside the existing advisor and code-interpreter payloads.convertAnthropicWebFetchResultToBifrostto map anAnthropicContentBlockweb fetch result into the new neutral types, andconvertBifrostWebFetchCallToAnthropicBlocksto rebuild theserver_tool_use+web_fetch_tool_resultblock pair from a neutralResponsesMessage. Both the streaming and non-streaming reverse converters now call these helpers instead of duplicating inline logic.ToBifrostResponsesStreamnow attaches the typed result payload to the emittedweb_fetch_callitem instead of dropping it.ToAnthropicResponsesStreamResponsenow emits theweb_fetch_tool_resultcontent_block_start/content_block_stoppair atoutput_item.donewhen the payload is present, removing the previous passthrough-only index-bump workaround for web fetch.convertAnthropicContentBlocksToResponsesMessagesnow callsattachAnthropicWebFetchResultonweb_fetch_tool_resultblocks instead of silently skipping them, attaching the result to the matchingweb_fetch_callmessage.ConvertBifrostMessagesToAnthropicMessagesnow delegates web fetch block construction toconvertBifrostWebFetchCallToAnthropicBlocks, removing the duplicated inline assembly.AnthropicToolTypeWebFetch20260318and itsresponse_inclusionfield onAnthropicToolWebFetch.convertBifrostToolToAnthropicselects this type whenResponseInclusionis set.UseCacheandResponseInclusionare now round-tripped throughconvertAnthropicToolToBifrost.core/schemas/utils.goandframework/streaming/responses.goupdated to coverResponsesWebFetchCalland its nestedDocument/Source/Citationsfields, and to copyCalleronResponsesToolMessage.TestAnthropicConverterOnly_IndicesContiguouscomments to reflect thatweb_fetchno longer collapses a result block on the all-normalized path. AddedTestAnthropicWebFetchResultRoundTripto verify the full Anthropic → Bifrost → Anthropic round-trip for a web fetch result with a nested document.Type of change
Affected areas
How to test
The new
TestAnthropicWebFetchResultRoundTriptest exercises the full round-trip: anAnthropicContentBlockweb fetch result is converted to the neutralResponsesWebFetchCall, assembled into aResponsesMessage, and then rebuilt back intoserver_tool_use+web_fetch_tool_resultblocks with the document content intact.Breaking changes
Related issues
Security considerations
No new auth, secrets, PII, or sandboxing surface introduced. Fetched document content that was previously discarded is now stored in memory within the request lifetime and forwarded to the caller — consistent with how other tool result payloads are handled.
Checklist
docs/contributing/README.mdand followed the guidelines