normalize tool_search_tool_* on the Responses path - #5891
Merged
akshaydeo merged 6 commits intoAug 6, 2026
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change normalizes dated and variant-specific tool-search types, adds Anthropic conversion coverage, updates Bedrock capability and document expectations, uses trace snapshots for injection, and documents ChangesTool search support
Bedrock document conversion expectations
Tracing snapshot delivery
Cursor indexing documentation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ResponsesTool
participant normalizeResponsesToolType
participant AnthropicConversion
ResponsesTool->>normalizeResponsesToolType: Decode tool type
normalizeResponsesToolType->>ResponsesTool: Return canonical tool-search type
ResponsesTool->>AnthropicConversion: Pass normalized tool
AnthropicConversion->>ResponsesTool: Preserve dated Anthropic variant
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This was referenced Aug 6, 2026
Merged
Contributor
Author
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@core/schemas/responses.go`:
- Around line 2439-2473: Restrict the normalization case in
normalizeResponsesToolType to recognize only tool_search_tool_regex and
tool_search_tool_bm25 forms, including their supported dated or undated
variants, while leaving unrelated values such as tool_search_preview unchanged
for unknown-tool handling. Add a regression test covering the prefix-sharing
unknown type and preserve the existing canonical ResponsesToolTypeToolSearch
result for recognized variants.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a7403d1-e45f-465e-941d-3e6385061271
📒 Files selected for processing (8)
.cursorignorecore/internal/llmtests/provider_feature_support_test.gocore/providers/anthropic/toolsearchrequest_test.gocore/providers/bedrock/bedrock_test.gocore/schemas/responses.gocore/schemas/responsestoolunmarshal_test.gocore/schemas/serialization_test.goframework/tracing/tracer_test.go
Merged
17 tasks
akshaydeo
force-pushed
the
08-05-deepseek_thinking_fixes
branch
from
August 6, 2026 09:11
53d6ff5 to
d088d10
Compare
akshaydeo
force-pushed
the
08-06-normalize_tool_search_tool__on_the_responses_path
branch
from
August 6, 2026 09:11
f6c605d to
8c154bc
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
This was referenced Aug 6, 2026
akshaydeo
force-pushed
the
08-05-deepseek_thinking_fixes
branch
from
August 6, 2026 17:18
d088d10 to
76ea20e
Compare
akshaydeo
force-pushed
the
08-06-normalize_tool_search_tool__on_the_responses_path
branch
from
August 6, 2026 17:18
8c154bc to
656f6a1
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@core/schemas/responses.go`:
- Around line 2757-2761: Update convertBifrostToolToAnthropic and
toolSearchVariantName so explicit tool-search names are validated against only
the exact supported values tool_search_tool_regex and tool_search_tool_bm25;
reject unsupported names rather than preserving them or mapping them to BM25,
while retaining inference only when t.Name is nil.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cf4a9dd9-bb6c-4080-abfb-bee545860e7a
📒 Files selected for processing (8)
.cursorignorecore/internal/llmtests/provider_feature_support_test.gocore/providers/anthropic/toolsearchrequest_test.gocore/providers/bedrock/bedrock_test.gocore/schemas/responses.gocore/schemas/responsestoolunmarshal_test.gocore/schemas/serialization_test.goframework/tracing/tracer_test.go
🚧 Files skipped from review as they are similar to previous changes (7)
- .cursorignore
- framework/tracing/tracer_test.go
- core/schemas/serialization_test.go
- core/providers/bedrock/bedrock_test.go
- core/internal/llmtests/provider_feature_support_test.go
- core/providers/anthropic/toolsearchrequest_test.go
- core/schemas/responsestoolunmarshal_test.go
akshaydeo
force-pushed
the
08-06-normalize_tool_search_tool__on_the_responses_path
branch
from
August 6, 2026 18:01
656f6a1 to
f15a03d
Compare
akshaydeo
force-pushed
the
08-05-deepseek_thinking_fixes
branch
from
August 6, 2026 18:01
76ea20e to
45a458b
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
akshaydeo
force-pushed
the
08-06-normalize_tool_search_tool__on_the_responses_path
branch
from
August 6, 2026 19:38
f15a03d to
8eabaaf
Compare
akshaydeo
force-pushed
the
08-05-deepseek_thinking_fixes
branch
from
August 6, 2026 19:38
45a458b to
84c3bdd
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
akshaydeo
force-pushed
the
08-06-normalize_tool_search_tool__on_the_responses_path
branch
from
August 6, 2026 19:57
8eabaaf to
7d0f894
Compare
akshaydeo
force-pushed
the
08-05-deepseek_thinking_fixes
branch
from
August 6, 2026 19:57
84c3bdd to
d3d2281
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
Author
Merge activity
|
akshaydeo
changed the base branch from
08-05-deepseek_thinking_fixes
to
graphite-base/5891
August 6, 2026 20:02
akshaydeo
dismissed
coderabbitai[bot]’s stale review
August 6, 2026 20:03
The base branch was changed.
akshaydeo
deleted the
08-06-normalize_tool_search_tool__on_the_responses_path
branch
August 6, 2026 20:03
akshaydeo
added a commit
that referenced
this pull request
Aug 7, 2026
## Summary Fixes a bug (#5279) where Anthropic's server-side tool-search meta-tool (`tool_search_tool_regex_20251119` / `tool_search_tool_bm25_20251119`) was silently downcast to a plain custom client tool. Because `normalizeResponsesToolType` had no case for the dated `tool_search_tool_*` spellings, they passed through unchanged, missed every switch comparing against `ResponsesToolTypeToolSearch`, and were emitted without a `type` field — causing Anthropic to treat them as client tools and never run the server-side search. Additionally, Bedrock's Converse API cannot run server-side tool search at all (AWS restricts it to `InvokeModel`/`InvokeModelWithResponseStream`), so `ToolSearch` is now correctly gated per provider rather than assumed universally supported. ## Changes - **`normalizeResponsesToolType`** now maps all `tool_search_tool_*` spellings (dated and undated, regex and bm25) to the canonical `ResponsesToolTypeToolSearch`, matching the existing pattern for other dated tool types. - **`toolSearchVariantName`** is introduced to recover the regex-vs-bm25 variant from the raw type string. The two variants are not interchangeable (regex expects Python `re.search()` patterns; bm25 expects natural language), so the variant is backfilled onto `Name` during unmarshaling when the caller declared the tool by type alone (as Anthropic's own Go/C# SDK examples do). - **`ResponsesTool.UnmarshalJSON`** is refactored from a `map[string]interface{}` decode + re-marshal round-trip to a single `gjson.GetManyBytes` pass. This eliminates redundant allocations and the intermediate marshal/unmarshal of structured fields (`cache_control`, `allowed_callers`, `input_examples`). Malformed JSON that the map decode previously rejected for free is now explicitly validated via `gjson.ValidBytes`. Wrong-typed scalar fields (e.g. `"defer_loading": "true"`) continue to be silently skipped rather than rejected, preserving existing tolerance. - **Provider feature map** updated: `ToolSearch` is now asserted `false` for Bedrock (Converse cannot run it) and `true` for all other providers (Anthropic, Vertex, Azure). - **Bedrock document placeholder tests** updated to reflect that a document-only Converse message gets a required placeholder text block injected ahead of it. - **Tracer test** fixed to use `trace.SnapshotForExport()` instead of a bare struct copy, which would duplicate a `sync.Mutex` and share the underlying spans slice. ## Type of change - [x] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/schemas/... ./core/providers/anthropic/... ./core/providers/bedrock/... ./core/internal/llmtests/... ./framework/tracing/... ``` - Confirm `TestResponsesToolUnmarshalNormalizesToolSearchType` passes for all dated/undated/no-name variants. - Confirm `TestConvertBifrostToolToAnthropicDatedToolSearch` passes and that the emitted tool carries a `type` and no `input_schema`. - Confirm `TestValidateResponsesToolsForProviderDatedToolSearch` drops the dated regex variant for Bedrock and keeps it for Anthropic and Vertex. - Confirm `TestProviderFeatureMapCompleteness` passes with Bedrock asserting `ToolSearch = false`. ## Breaking changes - [ ] Yes - [x] No ## Related issues Closes #5279 ## Security considerations None. Changes are confined to request normalization and JSON decoding; no auth, secrets, or PII are involved. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
atharvamhaske
pushed a commit
to atharvamhaske/bifrost
that referenced
this pull request
Aug 13, 2026
## Summary Fixes a bug (maximhq#5279) where Anthropic's server-side tool-search meta-tool (`tool_search_tool_regex_20251119` / `tool_search_tool_bm25_20251119`) was silently downcast to a plain custom client tool. Because `normalizeResponsesToolType` had no case for the dated `tool_search_tool_*` spellings, they passed through unchanged, missed every switch comparing against `ResponsesToolTypeToolSearch`, and were emitted without a `type` field — causing Anthropic to treat them as client tools and never run the server-side search. Additionally, Bedrock's Converse API cannot run server-side tool search at all (AWS restricts it to `InvokeModel`/`InvokeModelWithResponseStream`), so `ToolSearch` is now correctly gated per provider rather than assumed universally supported. ## Changes - **`normalizeResponsesToolType`** now maps all `tool_search_tool_*` spellings (dated and undated, regex and bm25) to the canonical `ResponsesToolTypeToolSearch`, matching the existing pattern for other dated tool types. - **`toolSearchVariantName`** is introduced to recover the regex-vs-bm25 variant from the raw type string. The two variants are not interchangeable (regex expects Python `re.search()` patterns; bm25 expects natural language), so the variant is backfilled onto `Name` during unmarshaling when the caller declared the tool by type alone (as Anthropic's own Go/C# SDK examples do). - **`ResponsesTool.UnmarshalJSON`** is refactored from a `map[string]interface{}` decode + re-marshal round-trip to a single `gjson.GetManyBytes` pass. This eliminates redundant allocations and the intermediate marshal/unmarshal of structured fields (`cache_control`, `allowed_callers`, `input_examples`). Malformed JSON that the map decode previously rejected for free is now explicitly validated via `gjson.ValidBytes`. Wrong-typed scalar fields (e.g. `"defer_loading": "true"`) continue to be silently skipped rather than rejected, preserving existing tolerance. - **Provider feature map** updated: `ToolSearch` is now asserted `false` for Bedrock (Converse cannot run it) and `true` for all other providers (Anthropic, Vertex, Azure). - **Bedrock document placeholder tests** updated to reflect that a document-only Converse message gets a required placeholder text block injected ahead of it. - **Tracer test** fixed to use `trace.SnapshotForExport()` instead of a bare struct copy, which would duplicate a `sync.Mutex` and share the underlying spans slice. ## Type of change - [x] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/schemas/... ./core/providers/anthropic/... ./core/providers/bedrock/... ./core/internal/llmtests/... ./framework/tracing/... ``` - Confirm `TestResponsesToolUnmarshalNormalizesToolSearchType` passes for all dated/undated/no-name variants. - Confirm `TestConvertBifrostToolToAnthropicDatedToolSearch` passes and that the emitted tool carries a `type` and no `input_schema`. - Confirm `TestValidateResponsesToolsForProviderDatedToolSearch` drops the dated regex variant for Bedrock and keeps it for Anthropic and Vertex. - Confirm `TestProviderFeatureMapCompleteness` passes with Bedrock asserting `ToolSearch = false`. ## Breaking changes - [ ] Yes - [x] No ## Related issues Closes maximhq#5279 ## Security considerations None. Changes are confined to request normalization and JSON decoding; no auth, secrets, or PII are involved. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
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
1 participant
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
Fixes a bug (#5279) where Anthropic's server-side tool-search meta-tool (
tool_search_tool_regex_20251119/tool_search_tool_bm25_20251119) was silently downcast to a plain custom client tool. BecausenormalizeResponsesToolTypehad no case for the datedtool_search_tool_*spellings, they passed through unchanged, missed every switch comparing againstResponsesToolTypeToolSearch, and were emitted without atypefield — causing Anthropic to treat them as client tools and never run the server-side search. Additionally, Bedrock's Converse API cannot run server-side tool search at all (AWS restricts it toInvokeModel/InvokeModelWithResponseStream), soToolSearchis now correctly gated per provider rather than assumed universally supported.Changes
normalizeResponsesToolTypenow maps alltool_search_tool_*spellings (dated and undated, regex and bm25) to the canonicalResponsesToolTypeToolSearch, matching the existing pattern for other dated tool types.toolSearchVariantNameis introduced to recover the regex-vs-bm25 variant from the raw type string. The two variants are not interchangeable (regex expects Pythonre.search()patterns; bm25 expects natural language), so the variant is backfilled ontoNameduring unmarshaling when the caller declared the tool by type alone (as Anthropic's own Go/C# SDK examples do).ResponsesTool.UnmarshalJSONis refactored from amap[string]interface{}decode + re-marshal round-trip to a singlegjson.GetManyBytespass. This eliminates redundant allocations and the intermediate marshal/unmarshal of structured fields (cache_control,allowed_callers,input_examples). Malformed JSON that the map decode previously rejected for free is now explicitly validated viagjson.ValidBytes. Wrong-typed scalar fields (e.g."defer_loading": "true") continue to be silently skipped rather than rejected, preserving existing tolerance.ToolSearchis now assertedfalsefor Bedrock (Converse cannot run it) andtruefor all other providers (Anthropic, Vertex, Azure).trace.SnapshotForExport()instead of a bare struct copy, which would duplicate async.Mutexand share the underlying spans slice.Type of change
Affected areas
How to test
go test ./core/schemas/... ./core/providers/anthropic/... ./core/providers/bedrock/... ./core/internal/llmtests/... ./framework/tracing/...TestResponsesToolUnmarshalNormalizesToolSearchTypepasses for all dated/undated/no-name variants.TestConvertBifrostToolToAnthropicDatedToolSearchpasses and that the emitted tool carries atypeand noinput_schema.TestValidateResponsesToolsForProviderDatedToolSearchdrops the dated regex variant for Bedrock and keeps it for Anthropic and Vertex.TestProviderFeatureMapCompletenesspasses with Bedrock assertingToolSearch = false.Breaking changes
Related issues
Closes #5279
Security considerations
None. Changes are confined to request normalization and JSON decoding; no auth, secrets, or PII are involved.
Checklist
docs/contributing/README.mdand followed the guidelines