feat: bedrock system tools - #3435
Merged
Merged
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds Bedrock Nova system-tool support: new Bedrock types for citations and system tools, streaming-state tracking and event branching for nova_code_interpreter and nova_grounding, Bedrock↔Bifrost tool conversion and message buffering, Anthropic Bedrock WebSearch flag, a Nova2 model helper, and integration tests. ChangesNova system tools integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
🧪 Test Suite AvailableThis PR can be tested by a repository admin. |
TejasGhatte
marked this pull request as ready for review
May 12, 2026 14:03
|
tejas ghatte seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Contributor
Confidence Score: 3/5The core round-trip conversion and non-streaming paths look correct after the changes, but the streaming path for nova_grounding still emits a protocol-incorrect event type for tool-use input deltas, which was flagged in a previous review pass and remains unaddressed. The nova_grounding toolUse delta handler falls through to the else branch (line 882-889) and emits function_call_arguments.delta on a web_search_call output item — wrong per the Responses API contract, and a spec-compliant streaming consumer may treat it as an error. This was flagged in a prior review cycle and is still present. The round-trip issues (silent drop of code_interpreter_call, unmatched nova_grounding toolUse) noted in the same cycle appear fixed. The overall feature is logically sound and the non-streaming path looks correct. core/providers/bedrock/responses.go — the ToolUse delta branch around line 872 needs a state.NovaGroundingIndices[outputIndex] guard to suppress the spurious function_call_arguments.delta event for nova_grounding. Important Files Changed
Reviews (8): Last reviewed commit: "feat: bedrock system tools" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: 3
🧹 Nitpick comments (2)
core/providers/anthropic/types.go (1)
181-186: ⚡ Quick winUpdate the Bedrock support note to match the new
WebSearch: trueflag.The comment still says Bedrock does not support WebSearch, but Line 186 enables it. Please align the note to avoid future confusion/regressions.
Proposed comment-only fix
- // Notably NOT supported per docs: MCP, Skills, FilesAPI, WebFetch, - // WebSearch, CodeExecution, FastMode, TaskBudgets, AdvisorTool, + // Notably NOT supported per docs: MCP, Skills, FilesAPI, WebFetch, + // CodeExecution, FastMode, TaskBudgets, AdvisorTool, // InferenceGeo, RedactThinking, AdvancedToolUse (full), PromptCachingScope. + // WebSearch is enabled here for Bedrock Nova grounding tool support.🤖 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/types.go` around lines 181 - 186, The comment describing Bedrock support is out of sync with the flag in schemas.Bedrock where WebSearch is set to true; update the explanatory comment above schemas.Bedrock to remove WebSearch from the "Notably NOT supported" list (or explicitly state it is supported) so the prose matches the WebSearch: true entry and avoid future confusion between the comment and the schemas.Bedrock configuration.core/providers/bedrock/responses.go (1)
812-825: ⚡ Quick winPartial JSON delta parsing will usually fail.
The code attempts to parse each streaming delta chunk as complete JSON to extract the
codefield. However, streaming deltas are typically partial JSON fragments (e.g.,{"code": "priwithout closing), sojson.Unmarshalwill fail on most chunks.This means
codeDeltawill be empty for most delta events, and only the final chunk (if it happens to be valid JSON) will produce a non-empty delta. Consider accumulating the full JSON inToolArgumentBuffersand only parsing when complete, or emitting raw deltas without attempting to extract thecodefield from partial chunks.🤖 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/bedrock/responses.go` around lines 812 - 825, The current logic tries to json.Unmarshal partial streaming chunks from toolUseDelta.Input to extract code (inside the state.CodeInterpreterIndices handling), which fails for fragments; instead either append the raw chunk to the existing ToolArgumentBuffers[outputIndex] and only attempt json.Unmarshal when the accumulated buffer parses successfully, or skip parsing entirely and emit the raw delta string (toolUseDelta.Input) as the Delta in schemas.BifrostResponsesStreamResponse so every fragment is forwarded; update the branch that constructs schemas.BifrostResponsesStreamResponse for CodeInterpreterCallCodeDelta to use the buffered/accumulated value or the raw chunk rather than attempting to unmarshal every partial chunk.
🤖 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/providers/bedrock/responses.go`:
- Around line 777-800: The annotationIndex is currently hardcoded to 0 for each
citation in the delta handling block, causing collisions when multiple citations
are emitted for the same output/content; add a counter in
BedrockResponsesStreamState (e.g., a map keyed by outputIndex+contentIndex or a
nested map like AnnotationIndexByOutputContent) to track the next annotation
index per output/content, use that counter instead of the hardcoded
annotationIndex, increment and persist the counter after emitting a citation
(update any places that create responses like the chunk.Delta.Citation handling
and ensure existing uses of ContentIndexToOutputIndex remain unchanged).
In `@tests/integrations/python/tests/test_bedrock.py`:
- Around line 2969-2973: Replace the broad except Exception catch with a
specific except botocore.exceptions.ClientError and only call pytest.skip when
the caught ClientError's service error code (accessed via
e.response['Error']['Code']) matches explicit Bedrock validation/unsupported
codes (e.g., 'ValidationException', 'UnknownOperation', 'NotFound' or whatever
service codes your provider returns); otherwise re-raise the exception. Update
the four similar Nova test blocks (the except blocks shown) to use
botocore.exceptions.ClientError, inspect e.response['Error']['Code'] for allowed
skip codes, and call pytest.skip with the original error when matched,
re-raising for any other errors.
- Around line 3131-3139: The assertion currently allows passing when only a
"toolUse" block exists; change the check to require either non-empty synthesized
text or an explicit execution result by verifying presence of "toolResult" in
content_blocks. Update the variables/logic around has_tool_use/full_text to
instead compute has_tool_result = any("toolResult" in b for b in content_blocks)
and assert full_text or has_tool_result (and apply the same change to the
similar assertion later around the other block at 3141-3145), so the test fails
when execution/result synthesis is missing.
---
Nitpick comments:
In `@core/providers/anthropic/types.go`:
- Around line 181-186: The comment describing Bedrock support is out of sync
with the flag in schemas.Bedrock where WebSearch is set to true; update the
explanatory comment above schemas.Bedrock to remove WebSearch from the "Notably
NOT supported" list (or explicitly state it is supported) so the prose matches
the WebSearch: true entry and avoid future confusion between the comment and the
schemas.Bedrock configuration.
In `@core/providers/bedrock/responses.go`:
- Around line 812-825: The current logic tries to json.Unmarshal partial
streaming chunks from toolUseDelta.Input to extract code (inside the
state.CodeInterpreterIndices handling), which fails for fragments; instead
either append the raw chunk to the existing ToolArgumentBuffers[outputIndex] and
only attempt json.Unmarshal when the accumulated buffer parses successfully, or
skip parsing entirely and emit the raw delta string (toolUseDelta.Input) as the
Delta in schemas.BifrostResponsesStreamResponse so every fragment is forwarded;
update the branch that constructs schemas.BifrostResponsesStreamResponse for
CodeInterpreterCallCodeDelta to use the buffered/accumulated value or the raw
chunk rather than attempting to unmarshal every partial chunk.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 73524ccd-c6bd-41af-8314-b93ac12eb421
📒 Files selected for processing (4)
core/providers/anthropic/types.gocore/providers/bedrock/responses.gocore/providers/bedrock/types.gotests/integrations/python/tests/test_bedrock.py
TejasGhatte
force-pushed
the
05-12-feat_bedrock_system_tools
branch
from
May 12, 2026 19:34
4b2ac3f to
3b40f1e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/providers/bedrock/responses.go (1)
791-817:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftCode delta extraction from JSON fragments will fail and emit empty values.
During streaming, each
toolUseDelta.Inputcontains an incremental JSON fragment (e.g.,{"co,de": ",print(), not a complete JSON object. The current code attempts to unmarshal each fragment individually as complete JSON, which fails silently (_ = json.Unmarshal), leavingcodeChunk.Codeempty for most/all deltas.The final
code.doneevent works correctly because it parses the accumulated buffer viaemitCodeInterpreterDoneEvents, but intermediatecode_deltaevents will emit empty strings.Emit the raw delta like regular function calls do (line 811:
&toolUseDelta.Input), allowing clients to accumulate fragments. The finalcode.doneevent provides the parsed code once the full buffer is available.Proposed fix
if state.CodeInterpreterIndices[outputIndex] { - // Extract the code field from the accumulated JSON delta - var codeChunk struct { - Code string `json:"code"` - } - _ = json.Unmarshal([]byte(toolUseDelta.Input), &codeChunk) - codeDelta := codeChunk.Code + // Emit raw delta — JSON fragments accumulate across chunks. + // Clients reconstruct code from deltas; code.done event provides final parsed code. + codeDelta := toolUseDelta.Input response = &schemas.BifrostResponsesStreamResponse{🤖 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/bedrock/responses.go` around lines 791 - 817, The code currently attempts to json.Unmarshal each incremental JSON fragment in toolUseDelta.Input into codeChunk.Code (inside the state.CodeInterpreterIndices[outputIndex] branch) which fails for partial fragments; change this branch to emit the raw fragment just like the else branch: set response.Type to ResponsesStreamResponseTypeCodeInterpreterCallCodeDelta, SequenceNumber/OutputIndex/ContentIndex as done, and set Delta to &toolUseDelta.Input (removing the silent json.Unmarshal and codeChunk usage) so clients can accumulate fragments; preserve the itemID assignment and the existing return shape.
🤖 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 `@tests/integrations/python/tests/test_bedrock.py`:
- Line 3074: The assertion uses an unnecessary f-string for a literal message
(assert full_text, f"Expected non-empty streamed text from nova_grounding, got
empty"); remove the leading `f` so the assertion message is a normal
string—i.e., change the message to "Expected non-empty streamed text from
nova_grounding, got empty" where the `full_text` assertion is performed.
---
Outside diff comments:
In `@core/providers/bedrock/responses.go`:
- Around line 791-817: The code currently attempts to json.Unmarshal each
incremental JSON fragment in toolUseDelta.Input into codeChunk.Code (inside the
state.CodeInterpreterIndices[outputIndex] branch) which fails for partial
fragments; change this branch to emit the raw fragment just like the else
branch: set response.Type to
ResponsesStreamResponseTypeCodeInterpreterCallCodeDelta,
SequenceNumber/OutputIndex/ContentIndex as done, and set Delta to
&toolUseDelta.Input (removing the silent json.Unmarshal and codeChunk usage) so
clients can accumulate fragments; preserve the itemID assignment and the
existing return shape.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 243fbca8-d91c-4b7c-99cc-e94b2646264e
📒 Files selected for processing (4)
core/providers/anthropic/types.gocore/providers/bedrock/responses.gocore/providers/bedrock/types.gotests/integrations/python/tests/test_bedrock.py
🚧 Files skipped from review as they are similar to previous changes (2)
- core/providers/bedrock/types.go
- core/providers/anthropic/types.go
TejasGhatte
force-pushed
the
05-12-feat_bedrock_system_tools
branch
from
May 15, 2026 05:55
3b40f1e to
5f860b0
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
core/providers/bedrock/responses.go (2)
2484-2556:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate inferred Nova system tools on the target model.
This helper now appends
nova_grounding/nova_code_interpreterwhenever those message types appear in history, butensureResponsesToolConfigForConversation()runs even whenbifrostReq.Modelis not a Nova model. Replaying aweb_search_callorcode_interpreter_callhistory into another Bedrock family will synthesize an invalidsystemToolconfig. Pass the target model into this inference path and only emitSystemToolentries for Nova models.🤖 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/bedrock/responses.go` around lines 2484 - 2556, The code unconditionally sets hasNovaGrounding/hasNovaCodeInterpreter from message history and later appends Nova system tools even when the target model isn't Nova; update ensureResponsesToolConfigForConversation() (or the calling path that builds tool inference) to accept the target model (bifrostReq.Model) and gate the final appends: only append BedrockSystemTool entries for BedrockSystemToolNovaGrounding and BedrockSystemToolNovaCodeInterpreter when the provided model is a Nova model (use the existing model-check helper if available or check Nova model name/prefix). Keep the message-scanning logic for hasNovaGrounding/hasNovaCodeInterpreter but add a conditional around the two blocks that append system tools so non-Nova models never receive those SystemTool entries.
857-893:⚠️ Potential issue | 🟠 Major | ⚡ Quick winParse code deltas from the accumulated JSON buffer.
toolUseDelta.Inputis just the latest JSON fragment, butcode_interpreter_call.code_deltais derived from unmarshalling that fragment in isolation. Bedrock splits tool JSON arbitrarily, so most chunks won't decode and this path will emit empty or corrupted code deltas. Usestate.ToolArgumentBuffers[outputIndex]as the parse source and track the last emitted code length so you only send the newly completed suffix.🤖 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/bedrock/responses.go` around lines 857 - 893, The current handler unmarshals only the latest fragment (toolUseDelta.Input) which can be partial; instead parse from the accumulated buffer state.ToolArgumentBuffers[outputIndex] and keep a per-output "last emitted length" marker (e.g. add or use state.ToolArgumentEmittedLengths[outputIndex]) so you only attempt to unmarshal complete JSON and emit the new suffix beyond the last emitted length; for code interpreter calls (when state.CodeInterpreterIndices[outputIndex] is true) repeatedly try to unmarshal the buffer into the codeChunk struct, advance the emitted-length marker to the successfully parsed code's end, set codeDelta to the newly completed suffix, and construct the ResponsesStreamResponse with that delta (fall back to not emitting if no new complete JSON is present); preserve existing behavior for function call argument deltas when not enough JSON is available.
🧹 Nitpick comments (2)
tests/integrations/python/tests/test_bedrock.py (2)
2984-2995: ⚡ Quick winAssert citation signal in non-streaming grounding responses.
This currently validates only text/keywords, so it can pass even if citation mapping regresses. Add a citation assertion (Bedrock
citationsContentor mapped annotation equivalent) for the grounding path.Suggested assertion hardening
full_text = " ".join(b["text"] for b in content_blocks if b.get("text", "").strip()) assert full_text, ( f"Expected non-empty text in grounding response, got: {content_blocks}" ) + + # Grounding should surface at least one citation signal. + has_citation_signal = any( + b.get("citationsContent") or b.get("annotations") for b in content_blocks + ) + assert has_citation_signal, ( + f"Expected citation metadata in grounding response, got: {content_blocks}" + )As per coding guidelines:
always check the stack if there is one for the current PR ... see all changes in the light of the whole stack.🤖 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 `@tests/integrations/python/tests/test_bedrock.py` around lines 2984 - 2995, The test currently only checks full_text built from content_blocks and must also assert that grounding responses include citation metadata; update the grounding assertions around full_text/content_blocks (the nova_grounding path) to verify Bedrock's citation field (e.g., check response["citationsContent"] or mapped annotation equivalents within content_blocks) exists and is non-empty and/or contains expected source identifiers, failing the test if no citation mapping is present; reference the variables full_text and content_blocks and add the new assertion directly after the existing text assertions so missing citations cause test failure.
3052-3075: ⚡ Quick winTrack and validate citation deltas in streaming grounding test.
The stream assertion currently checks only text and stop. Given this stack adds citation-delta handling, add a check for citation deltas so this path is actually covered.
Suggested assertion hardening
event_types = [] text_parts = [] + citation_deltas = [] start_time = time.time() timeout = 60 # grounding may take longer due to web fetch for event in stream: @@ if "contentBlockDelta" in event: delta = event["contentBlockDelta"].get("delta", {}) if "text" in delta and delta["text"]: text_parts.append(delta["text"]) + if delta.get("citation"): + citation_deltas.append(delta["citation"]) @@ full_text = "".join(text_parts) assert full_text, f"Expected non-empty streamed text from nova_grounding, got empty" + assert citation_deltas, "Expected at least one citation delta from nova_grounding stream"As per coding guidelines:
always check the stack if there is one for the current PR ... see all changes in the light of the whole stack.🤖 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 `@tests/integrations/python/tests/test_bedrock.py` around lines 3052 - 3075, The test loop currently only tracks "messageStart"/"messageStop" and text deltas; add detection and tracking for citation deltas so the new citation-delta streaming path is exercised: inside the for event in stream loop (where event_types and text_parts are used) add a branch to detect citation deltas (e.g., if "citationDelta" in event or if contentBlockDelta.delta contains a citation field), append a marker (e.g., "citationDelta") to event_types and collect any citation payloads into a new list (e.g., citation_parts), then update the assertions to assert that "citationDelta" is in event_types and that citation_parts contains expected non-empty items (or at least one citation), while keeping the existing text and messageStop assertions using the same variables full_text and event_types.
🤖 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/providers/bedrock/responses.go`:
- Around line 552-607: ToBedrockConverseStreamResponse() currently only maps the
older generic events and will drop the new Nova events (code_interpreter_call.*,
web_search_call.*, and citation-annotation); update that function to reverse-map
the new Bifrost stream types produced in this diff (e.g.,
ResponsesStreamResponseTypeCodeInterpreterCallInProgress,
ResponsesStreamResponseTypeWebSearchCallInProgress,
ResponsesStreamResponseTypeWebSearchCallSearching,
ResponsesStreamResponseTypeOutputItemAdded and citation-related events) back
into Bedrock-style streaming Converse responses. Implement handlers that: detect
these stream response Types and reconstruct the corresponding Bedrock
tool-start/tool-delta/tool-end JSON payloads (for code interpreter use the
CodeInterpreter indices/state like state.CodeInterpreterIndices to buffer and
emit JSON chunks; for Nova grounding use state.NovaGroundingIndices and
state.NovaGroundingCitations to recreate web_search call start/delta/completion
and attach citation annotations). Ensure ItemID/Item mapping (toolUseID) and
sequence/content indices are preserved when emitting the Bedrock messages so
Bifrost→Bedrock bridges don't drop tool starts, deltas, completions, or citation
updates.
---
Outside diff comments:
In `@core/providers/bedrock/responses.go`:
- Around line 2484-2556: The code unconditionally sets
hasNovaGrounding/hasNovaCodeInterpreter from message history and later appends
Nova system tools even when the target model isn't Nova; update
ensureResponsesToolConfigForConversation() (or the calling path that builds tool
inference) to accept the target model (bifrostReq.Model) and gate the final
appends: only append BedrockSystemTool entries for
BedrockSystemToolNovaGrounding and BedrockSystemToolNovaCodeInterpreter when the
provided model is a Nova model (use the existing model-check helper if available
or check Nova model name/prefix). Keep the message-scanning logic for
hasNovaGrounding/hasNovaCodeInterpreter but add a conditional around the two
blocks that append system tools so non-Nova models never receive those
SystemTool entries.
- Around line 857-893: The current handler unmarshals only the latest fragment
(toolUseDelta.Input) which can be partial; instead parse from the accumulated
buffer state.ToolArgumentBuffers[outputIndex] and keep a per-output "last
emitted length" marker (e.g. add or use
state.ToolArgumentEmittedLengths[outputIndex]) so you only attempt to unmarshal
complete JSON and emit the new suffix beyond the last emitted length; for code
interpreter calls (when state.CodeInterpreterIndices[outputIndex] is true)
repeatedly try to unmarshal the buffer into the codeChunk struct, advance the
emitted-length marker to the successfully parsed code's end, set codeDelta to
the newly completed suffix, and construct the ResponsesStreamResponse with that
delta (fall back to not emitting if no new complete JSON is present); preserve
existing behavior for function call argument deltas when not enough JSON is
available.
---
Nitpick comments:
In `@tests/integrations/python/tests/test_bedrock.py`:
- Around line 2984-2995: The test currently only checks full_text built from
content_blocks and must also assert that grounding responses include citation
metadata; update the grounding assertions around full_text/content_blocks (the
nova_grounding path) to verify Bedrock's citation field (e.g., check
response["citationsContent"] or mapped annotation equivalents within
content_blocks) exists and is non-empty and/or contains expected source
identifiers, failing the test if no citation mapping is present; reference the
variables full_text and content_blocks and add the new assertion directly after
the existing text assertions so missing citations cause test failure.
- Around line 3052-3075: The test loop currently only tracks
"messageStart"/"messageStop" and text deltas; add detection and tracking for
citation deltas so the new citation-delta streaming path is exercised: inside
the for event in stream loop (where event_types and text_parts are used) add a
branch to detect citation deltas (e.g., if "citationDelta" in event or if
contentBlockDelta.delta contains a citation field), append a marker (e.g.,
"citationDelta") to event_types and collect any citation payloads into a new
list (e.g., citation_parts), then update the assertions to assert that
"citationDelta" is in event_types and that citation_parts contains expected
non-empty items (or at least one citation), while keeping the existing text and
messageStop assertions using the same variables full_text and event_types.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7d7df591-d1ca-470e-aaee-c93f116fa27b
📒 Files selected for processing (4)
core/providers/anthropic/types.gocore/providers/bedrock/responses.gocore/providers/bedrock/types.gotests/integrations/python/tests/test_bedrock.py
🚧 Files skipped from review as they are similar to previous changes (1)
- core/providers/anthropic/types.go
TejasGhatte
force-pushed
the
05-12-feat_bedrock_system_tools
branch
from
May 15, 2026 06:45
5f860b0 to
f126abe
Compare
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 `@tests/integrations/python/tests/test_bedrock.py`:
- Around line 3049-3050: The tests currently assume response_stream contains
"stream" only; change the lookup to fall back to "eventStream" so SDK shape
differences don't cause false failures — replace usages like stream =
response_stream.get("stream") with stream = response_stream.get("stream") or
response_stream.get("eventStream") in the relevant tests (the ones using
response_stream in tests 51 and 53, similar to test_04_converse_streaming), and
keep the existing assert that stream is not None.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f5d898c-8d45-4491-b68f-fd66eddd15e6
📒 Files selected for processing (5)
core/providers/anthropic/types.gocore/providers/bedrock/responses.gocore/providers/bedrock/types.gocore/schemas/utils.gotests/integrations/python/tests/test_bedrock.py
🚧 Files skipped from review as they are similar to previous changes (3)
- core/providers/anthropic/types.go
- core/providers/bedrock/types.go
- core/providers/bedrock/responses.go
TejasGhatte
force-pushed
the
05-12-feat_bedrock_system_tools
branch
from
May 15, 2026 10:00
f126abe to
25c38fe
Compare
TejasGhatte
force-pushed
the
05-12-feat_bedrock_system_tools
branch
from
May 15, 2026 10:01
25c38fe to
cd7ceaa
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/providers/anthropic/types.go (1)
182-188:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBedrock capability comment is now stale and contradicts the flags
Lines 182–184 say Bedrock does not support
WebSearch/CodeExecution, but Lines 186–187 now enable both. Please update the comment (or split by model-specific support) so future maintenance doesn’t regress this intentionally enabled behavior.Suggested comment update
- // Notably NOT supported per docs: MCP, Skills, FilesAPI, WebFetch, - // WebSearch, CodeExecution, FastMode, TaskBudgets, AdvisorTool, + // Notably NOT supported per docs: MCP, Skills, FilesAPI, WebFetch, + // FastMode, TaskBudgets, AdvisorTool, // InferenceGeo, RedactThinking, AdvancedToolUse (full), PromptCachingScope. + // WebSearch and CodeExecution are enabled here for Bedrock Nova system tools.🤖 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/types.go` around lines 182 - 188, The comment above the schemas.Bedrock capability map is stale and contradicts the actual flags (WebSearch and CodeExecution are set true); update the comment to either remove the blanket "NOT supported" list or split it by model/provider so it reflects current enabled capabilities for schemas.Bedrock (and mention WebSearch and CodeExecution explicitly), or add a clarifying note that some features are enabled per-provider while others remain unsupported; ensure the comment and the schemas.Bedrock map stay consistent so future changes to WebSearch, CodeExecution, or other flags (e.g., ComputerUse, Bash, Memory) don’t conflict.
🧹 Nitpick comments (1)
tests/integrations/python/tests/test_bedrock.py (1)
2984-3001: ⚡ Quick winAdd explicit non-streaming citation evidence assertions for
nova_grounding.This test currently validates grounded text only. It should also assert citation evidence so the non-streaming citation mapping path in this stack can’t regress silently.
Suggested hardening
full_text = " ".join(b["text"] for b in content_blocks if b.get("text", "").strip()) assert full_text, ( f"Expected non-empty text in grounding response, got: {content_blocks}" ) + citation_urls = [] + for block in content_blocks: + citations_content = block.get("citationsContent", {}) + for citation in citations_content.get("citations", []): + web = citation.get("location", {}).get("web", {}) + if web.get("url"): + citation_urls.append(web["url"]) + assert citation_urls, ( + f"Expected grounding citations in non-streaming response, got: {content_blocks}" + )As per coding guidelines
**: always check the stack if there is one and review changes in light of the whole stack.🤖 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 `@tests/integrations/python/tests/test_bedrock.py` around lines 2984 - 3001, The test currently only checks grounded text; add explicit assertions that the non-streaming citation evidence path is present by verifying citation data is returned: iterate content_blocks and assert at least one block has a non-empty citation/evidence field (e.g., check block.get("citation") or block.get("evidence") is truthy) and assert the top-level response contains a non-empty citations mapping (e.g., response.get("citations") or response.get("groundingEvidence") is a dict/list with entries); keep these checks adjacent to the existing full_text and stop_reason assertions so failures point to nova_grounding citation regressions.
🤖 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/providers/bedrock/responses.go`:
- Around line 859-867: The buffer currently concatenates raw toolUseDelta.Input
JSON blobs into state.ToolArgumentBuffers[outputIndex], which breaks later JSON
parsing; instead, when state.CodeInterpreterIndices[outputIndex] is true,
extract the "snippet" field from toolUseDelta.Input (using
providerUtils.GetJSONField or equivalent) and append that snippet text to
state.ToolArgumentBuffers[outputIndex] (not the full JSON), preserving ordering
and whitespace; update the branch around codeDelta and any other spots that
append for code interpreter deltas to consistently accumulate only the snippet
pieces so later parsing of the complete code body succeeds.
- Around line 3350-3356: The serialized tool input for the Nova code interpreter
is using the wrong field name ("code") and must match what
convertSingleBedrockMessageToBifrostMessages expects ("snippet"); update the
BedrockContentBlock creation where ToolUse.Input is built (the block that sets
BedrockToolUse.ToolUseID, Name == BedrockSystemToolNovaCodeInterpreter) to
marshal map[string]string{"snippet": code} (and likewise any other place that
marshals this tool input), so ConvertBifrostMessagesToBedrockMessages ⇄
convertSingleBedrockMessageToBifrostMessages use the same "snippet" field.
- Around line 2280-2295: The loop over bifrostReq.Params.Tools currently only
emits web_search/code_interpreter as Bedrock system tools when isNova2 is true,
causing those tool entries to be dropped for other models; add an explicit check
inside that loop: if tool.Type is ResponsesToolTypeWebSearch or
ResponsesToolTypeCodeInterpreter and isNova2 is false, return an error (or
otherwise surface an unsupported-model error) instead of letting the tool fall
through—this should be done before the existing isNova2 block so callers see an
informative error for unsupported-model Nova system tools rather than silent
dropping; reference the bifrostReq.Params.Tools iteration, the isNova2 variable,
ResponsesToolTypeWebSearch, ResponsesToolTypeCodeInterpreter,
BedrockSystemToolNovaGrounding, BedrockSystemToolNovaCodeInterpreter and the
ResponsesToolFunction path when making the change.
In `@tests/integrations/python/tests/test_bedrock.py`:
- Around line 3073-3081: The current handling in the contentBlockDelta branch
uses an elif so citation extraction is skipped whenever text is present; update
the logic in the block that inspects event["contentBlockDelta"] / delta so
text_parts.append(delta["text"]) and citation_urls.append(web["url"]) are
evaluated independently (replace the elif with a separate if for the "citation"
path and keep the existing text check), using the same variables referenced
(contentBlockDelta, delta, text_parts, citation_urls) so citations produced
alongside text are captured.
---
Outside diff comments:
In `@core/providers/anthropic/types.go`:
- Around line 182-188: The comment above the schemas.Bedrock capability map is
stale and contradicts the actual flags (WebSearch and CodeExecution are set
true); update the comment to either remove the blanket "NOT supported" list or
split it by model/provider so it reflects current enabled capabilities for
schemas.Bedrock (and mention WebSearch and CodeExecution explicitly), or add a
clarifying note that some features are enabled per-provider while others remain
unsupported; ensure the comment and the schemas.Bedrock map stay consistent so
future changes to WebSearch, CodeExecution, or other flags (e.g., ComputerUse,
Bash, Memory) don’t conflict.
---
Nitpick comments:
In `@tests/integrations/python/tests/test_bedrock.py`:
- Around line 2984-3001: The test currently only checks grounded text; add
explicit assertions that the non-streaming citation evidence path is present by
verifying citation data is returned: iterate content_blocks and assert at least
one block has a non-empty citation/evidence field (e.g., check
block.get("citation") or block.get("evidence") is truthy) and assert the
top-level response contains a non-empty citations mapping (e.g.,
response.get("citations") or response.get("groundingEvidence") is a dict/list
with entries); keep these checks adjacent to the existing full_text and
stop_reason assertions so failures point to nova_grounding citation regressions.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d954cba3-1d35-4978-8706-eb49522141c1
📒 Files selected for processing (5)
core/providers/anthropic/types.gocore/providers/bedrock/responses.gocore/providers/bedrock/types.gocore/schemas/utils.gotests/integrations/python/tests/test_bedrock.py
🚧 Files skipped from review as they are similar to previous changes (2)
- core/schemas/utils.go
- core/providers/bedrock/types.go
TejasGhatte
force-pushed
the
05-12-feat_bedrock_system_tools
branch
from
May 15, 2026 10:44
cd7ceaa to
5c00b07
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/integrations/python/tests/test_bedrock.py (1)
3071-3071: ⚡ Quick winRemove per-event streaming log noise at Line 3071.
print(event)inside the stream loop can flood CI logs and make long runs slower/flakier. Keep only summarized assertions/logs.Suggested minimal patch
- print(event)🤖 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 `@tests/integrations/python/tests/test_bedrock.py` at line 3071, The test contains a noisy per-event print call ("print(event)") inside the streaming loop that floods CI logs; remove the "print(event)" call and either drop it entirely or replace it with a single summarized log or assertion after the stream completes (e.g., check event count or key fields) so the test only emits concise, meaningful output; locate the streaming loop in tests/integrations/python/tests/test_bedrock.py where "print(event)" appears and update the loop to avoid per-event printing.
🤖 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 `@tests/integrations/python/tests/test_bedrock.py`:
- Line 3071: The test contains a noisy per-event print call ("print(event)")
inside the streaming loop that floods CI logs; remove the "print(event)" call
and either drop it entirely or replace it with a single summarized log or
assertion after the stream completes (e.g., check event count or key fields) so
the test only emits concise, meaningful output; locate the streaming loop in
tests/integrations/python/tests/test_bedrock.py where "print(event)" appears and
update the loop to avoid per-event printing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 44faaf44-1cc3-4361-bc89-38db4334b404
📒 Files selected for processing (5)
core/providers/anthropic/types.gocore/providers/bedrock/responses.gocore/providers/bedrock/types.gocore/schemas/utils.gotests/integrations/python/tests/test_bedrock.py
🚧 Files skipped from review as they are similar to previous changes (4)
- core/schemas/utils.go
- core/providers/anthropic/types.go
- core/providers/bedrock/types.go
- core/providers/bedrock/responses.go
TejasGhatte
force-pushed
the
05-12-feat_bedrock_system_tools
branch
from
May 15, 2026 13:52
5c00b07 to
7afb2ad
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/integrations/python/tests/test_bedrock.py (2)
2979-2995: ⚡ Quick winAssert citation evidence in non-streaming grounding responses.
This test currently proves text generation but not the non-streaming citation contract, so citation reconstruction regressions can still pass.
Suggested assertion hardening
content_blocks = msg.get("content", []) assert isinstance(content_blocks, list) and len(content_blocks) > 0, ( f"Expected non-empty content blocks, got: {content_blocks}" ) + has_citations_content = any("citationsContent" in b for b in content_blocks) + assert has_citations_content, ( + f"Expected at least one citationsContent block from nova_grounding, got: {content_blocks}" + ) # nova_grounding returns multiple content blocks: empty text, toolUse,🤖 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 `@tests/integrations/python/tests/test_bedrock.py` around lines 2979 - 2995, The test currently only asserts generated text (content_blocks and full_text) but doesn't validate the non-streaming citation contract; update the assertions to also verify that grounding responses include citation evidence by checking each content block in content_blocks for expected citation-related fields or markers (e.g., presence of a "citation" key, "toolResult" entries with source/attribution metadata, or explicit source URLs/ids) and assert at least one citation is present and non-empty alongside the existing full_text checks so that regressions in citation reconstruction fail the test (use the variables content_blocks, full_text, and msg to locate and modify the assertions).
2997-3001: ⚡ Quick winTighten stop-reason assertions to validate the stop-reason fix.
Allowing
"max_tokens"here weakens regression coverage for the stack’s server-managed-tool stop-reason behavior. These prompts are short and should end with"end_turn".Suggested assertion update
- assert stop_reason in ("end_turn", "max_tokens"), ( - f"Unexpected stopReason: {stop_reason}" - ) + assert stop_reason == "end_turn", f"Unexpected stopReason: {stop_reason}"Also applies to: 3178-3181
🤖 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 `@tests/integrations/python/tests/test_bedrock.py` around lines 2997 - 3001, The test currently allows stop_reason to be either "end_turn" or "max_tokens" by checking stop_reason = response.get("stopReason", "") and asserting it in ("end_turn", "max_tokens"); tighten this to assert only "end_turn" (i.e., replace the tuple with "end_turn") to validate the server-managed-tool stop-reason behavior, and make the same change for the other identical assertion instance that also reads response.get("stopReason", "") later in the file so both checks only accept "end_turn".
🤖 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 `@tests/integrations/python/tests/test_bedrock.py`:
- Around line 2979-2995: The test currently only asserts generated text
(content_blocks and full_text) but doesn't validate the non-streaming citation
contract; update the assertions to also verify that grounding responses include
citation evidence by checking each content block in content_blocks for expected
citation-related fields or markers (e.g., presence of a "citation" key,
"toolResult" entries with source/attribution metadata, or explicit source
URLs/ids) and assert at least one citation is present and non-empty alongside
the existing full_text checks so that regressions in citation reconstruction
fail the test (use the variables content_blocks, full_text, and msg to locate
and modify the assertions).
- Around line 2997-3001: The test currently allows stop_reason to be either
"end_turn" or "max_tokens" by checking stop_reason = response.get("stopReason",
"") and asserting it in ("end_turn", "max_tokens"); tighten this to assert only
"end_turn" (i.e., replace the tuple with "end_turn") to validate the
server-managed-tool stop-reason behavior, and make the same change for the other
identical assertion instance that also reads response.get("stopReason", "")
later in the file so both checks only accept "end_turn".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5806e955-ae52-4f2e-a8d7-988d79726e1f
📒 Files selected for processing (5)
core/providers/anthropic/types.gocore/providers/bedrock/responses.gocore/providers/bedrock/types.gocore/schemas/utils.gotests/integrations/python/tests/test_bedrock.py
✅ Files skipped from review due to trivial changes (1)
- core/schemas/utils.go
🚧 Files skipped from review as they are similar to previous changes (2)
- core/providers/anthropic/types.go
- core/providers/bedrock/responses.go
TejasGhatte
force-pushed
the
05-12-feat_bedrock_system_tools
branch
from
May 15, 2026 14:13
7afb2ad to
941e75c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@tests/integrations/python/tests/test_bedrock.py`:
- Around line 3066-3068: The tests currently only assert that messageStop was
received (variables got_message_stop / messageStop) but do not verify the stop
reason; update both streaming Nova tests to assert messageStop.stopReason equals
the expected self-resolving stop (e.g., "stop" or the specific enum/string used
in the codebase) wherever messageStop is set inside the streaming loop,
replacing the loose existence check with a strict check of
messageStop.stopReason; apply the same change to the other occurrences
referenced (around the other blocks using got_message_stop/messageStop at the
noted locations).
- Around line 3246-3248: The test collects parsed text into text_parts but never
asserts on it, so add assertions to ensure the final streamed assistant answer
is present: after the stream finishes assert that text_parts is not empty (or
equals the expected final message) and that got_message_stop is True; also
assert that code_snippets contains the expected parsed snippet(s) when
exercising the code-delta path. Update the test blocks that initialize
text_parts, code_snippets and got_message_stop (the sections around the existing
variables text_parts, code_snippets, got_message_stop) to include these
assertions so the test fails if the assistant drops the post-execution text
block; apply the same assertions in the other occurrence noted (the second test
block covering the 3302-3308 region).
- Around line 2958-2961: The test currently only asserts the answer text for the
non-streaming response and misses verifying the new citationsContent round-trip;
update the assertions in the test_bedrock integration test to also assert that
the non-streaming response includes the expected citationsContent (e.g., check
the response object/variable used for the non-streaming call contains a
non-empty/expected citationsContent field and that its contents match the
streaming version or expected citation structure). Apply the same additional
assertions in the second block that corresponds to lines 2979-2995 so both
non-streaming cases validate citationsContent.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 60ee72d4-38af-4241-993d-f59b24cb9cc3
📒 Files selected for processing (5)
core/providers/anthropic/types.gocore/providers/bedrock/responses.gocore/providers/bedrock/types.gocore/schemas/utils.gotests/integrations/python/tests/test_bedrock.py
🚧 Files skipped from review as they are similar to previous changes (2)
- core/providers/anthropic/types.go
- core/providers/bedrock/responses.go
akshaydeo
approved these changes
May 15, 2026
Contributor
Merge activity
|
2 tasks
akshaydeo
pushed a commit
that referenced
this pull request
May 15, 2026
## Summary Adds support for Amazon Nova system tools (`nova_grounding` and `nova_code_interpreter`) through the Bedrock Converse and Converse-Stream paths. These are AWS-managed tools that the model invokes and executes automatically within a single turn — no client-side tool loop is required. The changes wire up bidirectional translation between Bedrock's native system tool format and Bifrost's neutral schema (`web_search` and `code_interpreter`), covering both non-streaming and streaming response handling. ## Changes - **`nova_grounding` support**: Enables `WebSearch: true` for the Bedrock provider feature map. Translates `nova_grounding` ↔ `web_search` in tool config conversion. Handles `citation` deltas in the streaming path, emitting them as `url_citation` annotations. Maps `citationsContent` blocks in non-streaming responses to `url_citation` annotations on text content blocks. - **`nova_code_interpreter` support**: Translates `nova_code_interpreter` ↔ `code_interpreter` in tool config conversion. Introduces `CodeInterpreterIndices` tracking in `BedrockResponsesStreamState` to distinguish code interpreter tool calls from regular function calls during streaming. Emits `code_interpreter_call.in_progress`, `code_interpreter_call.code_delta`, `code_interpreter_call.code.done`, and `code_interpreter_call.completed` stream events instead of the function-call event sequence. In non-streaming responses, merges the `toolUse` (code) and paired `nova_code_interpreter_result` `toolResult` blocks into a single `code_interpreter_call` output item with execution logs. - **Stop reason fix**: Server-managed tools resolve their own `toolResult` in the same turn. The stop reason derivation now pre-scans for matched `toolUse`/`toolResult` pairs so that `hasToolUse` stays false for self-resolving tools, preserving the original `end_turn` stop reason rather than incorrectly emitting `tool_use`. - **New Bedrock types**: Added `BedrockSystemTool`, `BedrockSystemToolType`, `BedrockCitation`, `BedrockCitationLocation`, `BedrockWebCitationLocation`, `BedrockCitationsContent`, and a `Type` field on `BedrockToolResult` to support `nova_code_interpreter_result` identification. - **Integration tests**: Added `TestNovaSystemTools` with four test cases (50–53) covering `nova_grounding` and `nova_code_interpreter` in both non-streaming and streaming modes via the Bedrock Converse API. ## Type of change - [ ] Bug fix - [x] Feature - [ ] 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/providers/bedrock/... go test ./core/providers/anthropic/... ``` For integration tests against a live Bedrock endpoint with Nova model access: ```sh cd tests/integrations/python pip install -r requirements.txt pytest tests/test_bedrock.py::TestNovaSystemTools -v ``` Test cases require AWS credentials with access to `us.amazon.nova-2-lite-v1:0` and the `nova_grounding` / `nova_code_interpreter` system tools enabled in your account. Tests will be skipped automatically if the tools are unavailable or the API key is not configured. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations Nova system tools are fully AWS-managed and execute in AWS infrastructure. No user-supplied code or credentials are passed through Bifrost beyond the standard Bedrock API call. Citation URLs returned by `nova_grounding` are surfaced as annotations and are not fetched or processed by Bifrost. ## Checklist - [ ] 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) - [ ] I verified the CI pipeline passes locally if applicable
akshaydeo
pushed a commit
that referenced
this pull request
May 20, 2026
## Summary Adds support for Amazon Nova system tools (`nova_grounding` and `nova_code_interpreter`) through the Bedrock Converse and Converse-Stream paths. These are AWS-managed tools that the model invokes and executes automatically within a single turn — no client-side tool loop is required. The changes wire up bidirectional translation between Bedrock's native system tool format and Bifrost's neutral schema (`web_search` and `code_interpreter`), covering both non-streaming and streaming response handling. ## Changes - **`nova_grounding` support**: Enables `WebSearch: true` for the Bedrock provider feature map. Translates `nova_grounding` ↔ `web_search` in tool config conversion. Handles `citation` deltas in the streaming path, emitting them as `url_citation` annotations. Maps `citationsContent` blocks in non-streaming responses to `url_citation` annotations on text content blocks. - **`nova_code_interpreter` support**: Translates `nova_code_interpreter` ↔ `code_interpreter` in tool config conversion. Introduces `CodeInterpreterIndices` tracking in `BedrockResponsesStreamState` to distinguish code interpreter tool calls from regular function calls during streaming. Emits `code_interpreter_call.in_progress`, `code_interpreter_call.code_delta`, `code_interpreter_call.code.done`, and `code_interpreter_call.completed` stream events instead of the function-call event sequence. In non-streaming responses, merges the `toolUse` (code) and paired `nova_code_interpreter_result` `toolResult` blocks into a single `code_interpreter_call` output item with execution logs. - **Stop reason fix**: Server-managed tools resolve their own `toolResult` in the same turn. The stop reason derivation now pre-scans for matched `toolUse`/`toolResult` pairs so that `hasToolUse` stays false for self-resolving tools, preserving the original `end_turn` stop reason rather than incorrectly emitting `tool_use`. - **New Bedrock types**: Added `BedrockSystemTool`, `BedrockSystemToolType`, `BedrockCitation`, `BedrockCitationLocation`, `BedrockWebCitationLocation`, `BedrockCitationsContent`, and a `Type` field on `BedrockToolResult` to support `nova_code_interpreter_result` identification. - **Integration tests**: Added `TestNovaSystemTools` with four test cases (50–53) covering `nova_grounding` and `nova_code_interpreter` in both non-streaming and streaming modes via the Bedrock Converse API. ## Type of change - [ ] Bug fix - [x] Feature - [ ] 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/providers/bedrock/... go test ./core/providers/anthropic/... ``` For integration tests against a live Bedrock endpoint with Nova model access: ```sh cd tests/integrations/python pip install -r requirements.txt pytest tests/test_bedrock.py::TestNovaSystemTools -v ``` Test cases require AWS credentials with access to `us.amazon.nova-2-lite-v1:0` and the `nova_grounding` / `nova_code_interpreter` system tools enabled in your account. Tests will be skipped automatically if the tools are unavailable or the API key is not configured. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations Nova system tools are fully AWS-managed and execute in AWS infrastructure. No user-supplied code or credentials are passed through Bifrost beyond the standard Bedrock API call. Citation URLs returned by `nova_grounding` are surfaced as annotations and are not fetched or processed by Bifrost. ## Checklist - [ ] 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) - [ ] I verified the CI pipeline passes locally if applicable
akshaydeo
added a commit
that referenced
this pull request
May 20, 2026
## Summary This PR cuts the `v1.5.11` / `v1.3.11` release across core, framework, and all plugins, and introduces a new Claude skill (`release-checklist`) for pre-release migration safety auditing. ## Changes - **`release-checklist` skill** — Adds `.claude/skills/release-checklist/SKILL.md`, a read-only pre-release audit tool that scans Go-defined database migrations changed in a release for high-scale deadlock/lock-contention risks and boot-time-blocking operations. It produces a structured `PASS`/`WARN`/`FAIL` report with a concrete remediation plan per finding. The skill is designed to grow via an extensible Checks Registry. - **Version bumps** — `core` → `1.5.11`, `framework` → `1.3.11`, `transports` → `1.5.3`, `plugins/governance` → `1.5.11`, `plugins/logging` → `1.5.11`, `plugins/semanticcache` → `1.5.11`, `plugins/otel` → `1.2.11`, `plugins/maxim` → `1.6.11`, `plugins/prompts` → `1.0.11`, and remaining plugins bumped accordingly. - **Changelogs populated** — All per-package changelogs updated with the full set of features and fixes shipping in this release. Key highlights in this release: - Temporary access tokens for scoped, time-limited API access - MCP per-user OAuth flow refactor - Bedrock Mantle inference engine support - Azure Realtime provider with enriched session tracking - Direct access control (DAC) and virtual key rotation - Cluster-aware log metadata and per-node usage aggregation - Feature flag framework - Config-hash-based file value override of DB on restart - Semantic cache plugin rewrite - Numerous streaming stability, Bedrock, Anthropic, and Gemini fixes - AWS SDK and dependency security updates ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Verify version files reflect the new release cat core/version # expect 1.5.11 cat framework/version # expect 1.3.11 cat transports/version # expect 1.5.3 # Core/Transports go test ./... ``` To exercise the new `release-checklist` skill, invoke it via Claude with: ``` /release-checklist origin/dev...HEAD ``` Expected output: a structured report with `PASS`/`WARN`/`FAIL` per check and a Remediation Plan table for any findings. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues #3603, #3565, #3489, #3334, #3335, #3435, #3554, #3590, #3444, #3198, #3581, #3610, #3599, #3567, #3382, #3461 and others listed in the changelogs. ## Security considerations - AWS SDK and dependency security updates are included (#3461). - `FullyRedacted()` for proxy passwords and `MarshalForStorage()` for `ProxyConfig` prevent partial secret leakage in API responses (#3445). - The `release-checklist` skill is strictly read-only and never modifies files. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
2 tasks
Merged
This was referenced May 22, 2026
Closed
18 tasks
9 tasks
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
Adds support for Amazon Nova system tools (
nova_groundingandnova_code_interpreter) through the Bedrock Converse and Converse-Stream paths. These are AWS-managed tools that the model invokes and executes automatically within a single turn — no client-side tool loop is required. The changes wire up bidirectional translation between Bedrock's native system tool format and Bifrost's neutral schema (web_searchandcode_interpreter), covering both non-streaming and streaming response handling.Changes
nova_groundingsupport: EnablesWebSearch: truefor the Bedrock provider feature map. Translatesnova_grounding↔web_searchin tool config conversion. Handlescitationdeltas in the streaming path, emitting them asurl_citationannotations. MapscitationsContentblocks in non-streaming responses tourl_citationannotations on text content blocks.nova_code_interpretersupport: Translatesnova_code_interpreter↔code_interpreterin tool config conversion. IntroducesCodeInterpreterIndicestracking inBedrockResponsesStreamStateto distinguish code interpreter tool calls from regular function calls during streaming. Emitscode_interpreter_call.in_progress,code_interpreter_call.code_delta,code_interpreter_call.code.done, andcode_interpreter_call.completedstream events instead of the function-call event sequence. In non-streaming responses, merges thetoolUse(code) and pairednova_code_interpreter_resulttoolResultblocks into a singlecode_interpreter_calloutput item with execution logs.Stop reason fix: Server-managed tools resolve their own
toolResultin the same turn. The stop reason derivation now pre-scans for matchedtoolUse/toolResultpairs so thathasToolUsestays false for self-resolving tools, preserving the originalend_turnstop reason rather than incorrectly emittingtool_use.New Bedrock types: Added
BedrockSystemTool,BedrockSystemToolType,BedrockCitation,BedrockCitationLocation,BedrockWebCitationLocation,BedrockCitationsContent, and aTypefield onBedrockToolResultto supportnova_code_interpreter_resultidentification.Integration tests: Added
TestNovaSystemToolswith four test cases (50–53) coveringnova_groundingandnova_code_interpreterin both non-streaming and streaming modes via the Bedrock Converse API.Type of change
Affected areas
How to test
For integration tests against a live Bedrock endpoint with Nova model access:
cd tests/integrations/python pip install -r requirements.txt pytest tests/test_bedrock.py::TestNovaSystemTools -vTest cases require AWS credentials with access to
us.amazon.nova-2-lite-v1:0and thenova_grounding/nova_code_interpretersystem tools enabled in your account. Tests will be skipped automatically if the tools are unavailable or the API key is not configured.Breaking changes
Related issues
Security considerations
Nova system tools are fully AWS-managed and execute in AWS infrastructure. No user-supplied code or credentials are passed through Bifrost beyond the standard Bedrock API call. Citation URLs returned by
nova_groundingare surfaced as annotations and are not fetched or processed by Bifrost.Checklist
docs/contributing/README.mdand followed the guidelines