From fb7ff2488aa3e1b82a1e723b66ea5186fb0bb49d Mon Sep 17 00:00:00 2001 From: Yuanhao Li Date: Thu, 22 Oct 2026 00:28:24 +0200 Subject: [PATCH 1/3] fix(provider): Bedrock, Gemini, Vertex drop empty messages before merging An assistant turn that errored with no content (on Gemini also one with only thinking, which is not replayed) went out as content: [] / parts: [], which both APIs reject. merge_same_role now drops a message whose array is empty before merging, so the user turns around it go back as one. Closes #369. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG --- CHANGELOG.md | 1 + CLAUDE.md | 2 +- src/provider/bedrock.rs | 17 +++++++++++++++++ src/provider/google.rs | 23 +++++++++++++++++++++++ src/provider/mod.rs | 36 ++++++++++++++++++++++++++++++++++++ 5 files changed, 78 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e421f761..b7ce6e50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ adheres to [Semantic Versioning](https://semver.org/). ## Unreleased +- **Bedrock, Gemini and Vertex no longer send an empty message** (#369). An assistant turn that errored with no content — or, on Gemini, with only thinking, which is not replayed — went out as `content: []` / `parts: []`, which both APIs reject; the Anthropic provider already skipped it. Empty messages are now dropped before consecutive same-role messages are merged (#368), so the user turns around a dropped one go back as one. - **Bedrock, Gemini and Vertex: a parallel tool call's results go back in one message** (#223). Each tool result was sent as its own user-role message, and a steering message, stop marker or compaction summary after them added another, while Bedrock's Converse requires roles to alternate and Gemini requires a turn's function responses in one content, as many as its function calls. So the request after any turn with two or more tool calls (the default parallel strategy) could be rejected. The three request builders now merge consecutive same-role messages, keeping every block in order: the results in call order, then whatever followed; on Gemini and Vertex a merged turn's function responses come first, then any images and text (images among the responses are a reported cause of Gemini's response-count 400). Pinned by request-body tests; not run against a live endpoint. ## 0.25.12 (2026-10-21) diff --git a/CLAUDE.md b/CLAUDE.md index 4039766c..e8fdcd0f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -61,7 +61,7 @@ The Responses and Azure providers share one request builder + read loop (`respon Anthropic SSE (`anthropic.rs`): block `index` → position in `content` via a map, never padding, so a block type it does not surface (server tool blocks, `fallback`, anything new) leaves no placeholder and later blocks keep their deltas; unknown block/delta types `warn_once` per type by name, unparseable start/delta events are logged. `redacted_thinking` (`data` arrives whole at `content_block_start`, no delta type) → `Content::thinking_redacted(ApiProtocol::AnthropicMessages, data)` → replayed as `{"type":"redacted_thinking","data"}` in place (#197). -Bedrock is the one non-SSE provider: ConverseStream answers with binary `application/vnd.amazon.eventstream` frames, decoded by the private `provider/eventstream.rs` (bytes buffered across chunks; prelude + message CRC-32 verified with an in-house table, no dependency; all ten header types; checksum/malformed → `Other`, a body ending mid-frame → `Network`; pinned by aws-sdk-go's published test vectors). `bedrock.rs` dispatches on the `:message-type` / `:event-type` / `:exception-type` headers — payloads are the event structs themselves (no wrapper key), shapes per the AWS Bedrock Runtime API reference. Blocks accumulate per `contentBlockIndex`; a tool call is pushed with the `__partial_json` marker and only `contentBlockStop` finalizes it (`finalize_tool_arguments`), so an unclosed block is never runnable. Exceptions classify via their documented HTTP status (`throttlingException`/`serviceUnavailableException` → `RateLimited`; other 5xx stay `Api`). No `messageStop` (incl. empty body) or a transport error → `Network`, never an empty `Ok` — except once `messageStop` **and** `metadata` arrived: a later drop/truncation keeps the response (no re-billed retry). `messageStop` without `metadata` → zero usage + `warn!(usage_missing = true)` (same zero as other providers). Unsurfaced content (image/citation/toolResult, server_tool_use, unknown union members via `#[serde(flatten)] other`, unknown event types) is dropped with a once-per-block warning; an unknown `contentBlockStart` member creates no block, so the first delta types it. Reasoning replay: signed → `reasoningText`, `redactedContent` kept in `Content::Thinking::redacted` (base64, bytes concatenated across deltas; `redacted_protocol = BedrockConverseStream`) → `redactedContent`, only if it came from Bedrock (another API's redacted data is skipped, logged at debug); unsigned → `reasoningText {text}` with no signature key, except to a model id containing `claude` (signature-verifying), where it is skipped at debug (never `signature: ""`); the Anthropic provider skips Bedrock's redacted blocks. Signature deltas are appended. Tested only with mock frames (`tests/bedrock_stream_test.rs`). Consecutive same-role messages are merged before sending (`provider::merge_same_role(messages, "content")`; Gemini and Vertex do the same with `"parts"`, then `google::function_responses_first` puts each user content's `functionResponse` parts before its images and text), so a parallel tool call's results go back as one user message and roles alternate (#223). Auth (`resolve_auth`, first match wins): an `authorization` key in `ModelConfig.headers` (any case) → sent alone; non-empty `api_key` → `access:secret[:token]` = SigV4 (`ASIA…` without a token refused), no `:` = Bedrock API key (`Bearer`) after `checked_bearer` refuses whitespace/control chars, an `AKIA`/`ASIA` start, non-base64 chars and a bare 40-char secret (unknown prefix — not `bedrock-api-key-`/`ABSK` — only warns once); empty `api_key` → env read per request (type `Env` = `std::env::var`-shaped, faked in tests), `AWS_BEARER_TOKEN_BEDROCK` (same checks) then `AWS_ACCESS_KEY_ID`/`AWS_SECRET_ACCESS_KEY`(/`AWS_SESSION_TOKEN`); half a pair / NotUnicode / none → `Auth` naming the variable, before sending. `resolve_api_key_or_warn(Option<&ModelConfig>)` (Agent, SubAgentTool, LlmCompaction) keys on the **protocol**: any `BedrockConverseStream` config gets an empty key (never `API_KEY`/`YOAGENT_API_KEY`, whatever the provider string), so the provider reads env itself; `resolve_api_key("bedrock")` (public) prefers `AWS_BEARER_TOKEN_BEDROCK`. `request_headers` builds the whole `HeaderMap`: every value via `header_value` (credential values must be ASCII; failure → `Auth` naming the header, never the value; user non-auth header → `Other`), `authorization`/`x-amz-security-token` `set_sensitive`; on the SigV4 path a user header named like a signed one is refused; a `send()` `is_builder()` error → `Other`, not `Network`. SigV4 is the private `provider/sigv4.rs` (sha2 + hmac, no AWS SDK; pinned to aws-c-auth's `aws-sig-v4-test-suite` vectors incl. the `*-normalized` path ones): signing name `bedrock`, region from the host (`bedrock-runtime[-fips]..`, suffix ∈ `AWS_DNS_SUFFIXES` = partition dnsSuffix/dualStackDnsSuffix from botocore + VPC forms) > `AWS_REGION` > `AWS_DEFAULT_REGION` > `Auth` error; signs `host`, `content-type`, `x-amz-date`, `x-amz-content-sha256`, `x-amz-security-token` over the exact body bytes sent (serialize once — never `.json()`); `amz_date_now` errors on a pre-1970 clock; a 403 "Signature expired"/"not yet current" gets a check-the-clock hint. `base_url` is `trim_end_matches('/')`-ed; `canonical_uri` normalizes (drops empty/`.` segments, resolves `..`, keeps a trailing `/`) and encodes each segment again, so the model id (`:` → `%3A` on the wire, as the SDKs send it) is `%253A` in the canonical URI. `tests/bedrock_auth_test.rs` (env-mutating, serialized by a tokio `Mutex`) recomputes each signature from what wiremock received and uses `expect(0)` for every refusal. Never run against a live endpoint. +Bedrock is the one non-SSE provider: ConverseStream answers with binary `application/vnd.amazon.eventstream` frames, decoded by the private `provider/eventstream.rs` (bytes buffered across chunks; prelude + message CRC-32 verified with an in-house table, no dependency; all ten header types; checksum/malformed → `Other`, a body ending mid-frame → `Network`; pinned by aws-sdk-go's published test vectors). `bedrock.rs` dispatches on the `:message-type` / `:event-type` / `:exception-type` headers — payloads are the event structs themselves (no wrapper key), shapes per the AWS Bedrock Runtime API reference. Blocks accumulate per `contentBlockIndex`; a tool call is pushed with the `__partial_json` marker and only `contentBlockStop` finalizes it (`finalize_tool_arguments`), so an unclosed block is never runnable. Exceptions classify via their documented HTTP status (`throttlingException`/`serviceUnavailableException` → `RateLimited`; other 5xx stay `Api`). No `messageStop` (incl. empty body) or a transport error → `Network`, never an empty `Ok` — except once `messageStop` **and** `metadata` arrived: a later drop/truncation keeps the response (no re-billed retry). `messageStop` without `metadata` → zero usage + `warn!(usage_missing = true)` (same zero as other providers). Unsurfaced content (image/citation/toolResult, server_tool_use, unknown union members via `#[serde(flatten)] other`, unknown event types) is dropped with a once-per-block warning; an unknown `contentBlockStart` member creates no block, so the first delta types it. Reasoning replay: signed → `reasoningText`, `redactedContent` kept in `Content::Thinking::redacted` (base64, bytes concatenated across deltas; `redacted_protocol = BedrockConverseStream`) → `redactedContent`, only if it came from Bedrock (another API's redacted data is skipped, logged at debug); unsigned → `reasoningText {text}` with no signature key, except to a model id containing `claude` (signature-verifying), where it is skipped at debug (never `signature: ""`); the Anthropic provider skips Bedrock's redacted blocks. Signature deltas are appended. Tested only with mock frames (`tests/bedrock_stream_test.rs`). Consecutive same-role messages are merged before sending (`provider::merge_same_role(messages, "content")`; Gemini and Vertex do the same with `"parts"`, then `google::function_responses_first` puts each user content's `functionResponse` parts before its images and text), so a parallel tool call's results go back as one user message and roles alternate (#223); a message whose array is empty (an errored assistant turn) is dropped first (#369). Auth (`resolve_auth`, first match wins): an `authorization` key in `ModelConfig.headers` (any case) → sent alone; non-empty `api_key` → `access:secret[:token]` = SigV4 (`ASIA…` without a token refused), no `:` = Bedrock API key (`Bearer`) after `checked_bearer` refuses whitespace/control chars, an `AKIA`/`ASIA` start, non-base64 chars and a bare 40-char secret (unknown prefix — not `bedrock-api-key-`/`ABSK` — only warns once); empty `api_key` → env read per request (type `Env` = `std::env::var`-shaped, faked in tests), `AWS_BEARER_TOKEN_BEDROCK` (same checks) then `AWS_ACCESS_KEY_ID`/`AWS_SECRET_ACCESS_KEY`(/`AWS_SESSION_TOKEN`); half a pair / NotUnicode / none → `Auth` naming the variable, before sending. `resolve_api_key_or_warn(Option<&ModelConfig>)` (Agent, SubAgentTool, LlmCompaction) keys on the **protocol**: any `BedrockConverseStream` config gets an empty key (never `API_KEY`/`YOAGENT_API_KEY`, whatever the provider string), so the provider reads env itself; `resolve_api_key("bedrock")` (public) prefers `AWS_BEARER_TOKEN_BEDROCK`. `request_headers` builds the whole `HeaderMap`: every value via `header_value` (credential values must be ASCII; failure → `Auth` naming the header, never the value; user non-auth header → `Other`), `authorization`/`x-amz-security-token` `set_sensitive`; on the SigV4 path a user header named like a signed one is refused; a `send()` `is_builder()` error → `Other`, not `Network`. SigV4 is the private `provider/sigv4.rs` (sha2 + hmac, no AWS SDK; pinned to aws-c-auth's `aws-sig-v4-test-suite` vectors incl. the `*-normalized` path ones): signing name `bedrock`, region from the host (`bedrock-runtime[-fips]..`, suffix ∈ `AWS_DNS_SUFFIXES` = partition dnsSuffix/dualStackDnsSuffix from botocore + VPC forms) > `AWS_REGION` > `AWS_DEFAULT_REGION` > `Auth` error; signs `host`, `content-type`, `x-amz-date`, `x-amz-content-sha256`, `x-amz-security-token` over the exact body bytes sent (serialize once — never `.json()`); `amz_date_now` errors on a pre-1970 clock; a 403 "Signature expired"/"not yet current" gets a check-the-clock hint. `base_url` is `trim_end_matches('/')`-ed; `canonical_uri` normalizes (drops empty/`.` segments, resolves `..`, keeps a trailing `/`) and encodes each segment again, so the model id (`:` → `%3A` on the wire, as the SDKs send it) is `%253A` in the canonical URI. `tests/bedrock_auth_test.rs` (env-mutating, serialized by a tokio `Mutex`) recomputes each signature from what wiremock received and uses `expect(0)` for every refusal. Never run against a live endpoint. `ModelConfig` + quirk structs handle per-provider differences: `OpenAiCompat` (reasoning format, max_tokens field name, `max_reasoning_effort` ceiling — the ceiling, and `encrypted_reasoning`, are also read by Responses/Azure), `AnthropicCompat` (adaptive vs budget thinking, bearer auth, `native_structured_output`; `for_claude_id` infers them from an id), `GoogleCompat` (`thinkingLevel` vs `thinkingBudget`). `ThinkingLevel::Off` sends no thinking/effort field on any provider (DeepSeek's explicit `thinking: disabled` aside), so always-thinking models run at their default. diff --git a/src/provider/bedrock.rs b/src/provider/bedrock.rs index 12e8bec7..81533abb 100644 --- a/src/provider/bedrock.rs +++ b/src/provider/bedrock.rs @@ -1612,6 +1612,23 @@ mod tests { use super::*; /// A parallel tool call's results go back as one user message, the + /// An assistant turn that errored with no content is not sent as an empty + /// message; the user turns around it go back as one (#369). + #[test] + fn an_empty_assistant_turn_is_not_sent() { + let mut config = StreamConfig::new("anthropic.claude-sonnet", "a:b"); + config.messages = vec![ + Message::user("first"), + Message::assistant(vec![], StopReason::Error, "m", "bedrock", Usage::default()), + Message::user("again"), + ]; + let body = build_bedrock_body(&config); + assert_eq!( + body["messages"], + serde_json::json!([{"role": "user", "content": [{"text": "first"}, {"text": "again"}]}]) + ); + } + /// steering message after them, so Converse's roles alternate (#223). #[test] fn parallel_tool_results_go_back_in_one_message() { diff --git a/src/provider/google.rs b/src/provider/google.rs index b2e6f393..a6bc95d0 100644 --- a/src/provider/google.rs +++ b/src/provider/google.rs @@ -834,6 +834,29 @@ mod tests { use crate::provider::{GoogleCompat, ModelConfig}; /// A parallel tool call's results go back as one user turn, with the + /// A model turn with only thinking (not replayed) or none at all is not + /// sent as an empty content (#369). + #[test] + fn an_empty_model_turn_is_not_sent() { + let mut config = StreamConfig::new("gemini-2.5-pro", "k"); + config.messages = vec![ + Message::user("first"), + Message::assistant( + vec![Content::thinking("thought")], + StopReason::Error, + "m", + "google", + Usage::default(), + ), + Message::user("again"), + ]; + let body = build_request_body(&config); + assert_eq!( + body["contents"], + serde_json::json!([{"role": "user", "parts": [{"text": "first"}, {"text": "again"}]}]) + ); + } + /// steering message after them, so roles alternate (#223). #[test] fn parallel_tool_results_go_back_in_one_turn() { diff --git a/src/provider/mod.rs b/src/provider/mod.rs index 515c5882..5ef8f102 100644 --- a/src/provider/mod.rs +++ b/src/provider/mod.rs @@ -71,12 +71,24 @@ pub(crate) fn tool_result_text(content: &[crate::types::Content]) -> String { /// message, a stop marker or a compaction summary right after them), so these /// builders produce one wire message per result. Merging keeps every block in /// order: the results first, in call order, then whatever followed them. +/// +/// A message whose `key` array is empty is dropped first: both APIs reject an +/// empty message, and an assistant turn that errored with no content (or, on +/// Gemini, only thinking, which is not replayed) would otherwise be one. Its +/// neighbours then merge, so the roles still alternate (#369). pub(crate) fn merge_same_role( messages: Vec, key: &str, ) -> Vec { let mut merged: Vec = Vec::with_capacity(messages.len()); for message in messages { + if message + .get(key) + .and_then(serde_json::Value::as_array) + .is_some_and(Vec::is_empty) + { + continue; + } if let Some(last) = merged.last_mut() { if last.get("role") == message.get("role") { let from = message.get(key).and_then(serde_json::Value::as_array); @@ -130,6 +142,30 @@ mod merge_same_role_tests { use super::merge_same_role; use serde_json::json; + /// An empty message is dropped, wherever it is, and its neighbours merge: + /// an assistant turn that errored with no content between two user turns + /// leaves one user message (#369). + #[test] + fn empty_messages_are_dropped_and_their_neighbours_merge() { + let merged = merge_same_role( + vec![ + json!({"role": "user", "parts": [{"text": "1"}]}), + json!({"role": "model", "parts": []}), + json!({"role": "user", "parts": [{"text": "2"}]}), + json!({"role": "model", "parts": [{"text": "3"}]}), + json!({"role": "model", "parts": []}), + ], + "parts", + ); + assert_eq!( + merged, + vec![ + json!({"role": "user", "parts": [{"text": "1"}, {"text": "2"}]}), + json!({"role": "model", "parts": [{"text": "3"}]}), + ] + ); + } + #[test] fn adjacent_same_role_messages_become_one_in_order() { let merged = merge_same_role( From e6c340092b9178288ff255812bdf6a9d9590c7e4 Mon Sep 17 00:00:00 2001 From: Yuanhao Li Date: Thu, 22 Oct 2026 00:30:04 +0200 Subject: [PATCH 2/3] docs: rejoin the split test doc comments; CHANGELOG covers empty user messages too Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG --- CHANGELOG.md | 2 +- src/provider/bedrock.rs | 2 +- src/provider/google.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7ce6e50..1307f132 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ adheres to [Semantic Versioning](https://semver.org/). ## Unreleased -- **Bedrock, Gemini and Vertex no longer send an empty message** (#369). An assistant turn that errored with no content — or, on Gemini, with only thinking, which is not replayed — went out as `content: []` / `parts: []`, which both APIs reject; the Anthropic provider already skipped it. Empty messages are now dropped before consecutive same-role messages are merged (#368), so the user turns around a dropped one go back as one. +- **Bedrock, Gemini and Vertex no longer send an empty message** (#369). An assistant turn that errored with no content — or, on Gemini, with only thinking, which is not replayed — went out as `content: []` / `parts: []`, which both APIs reject (as did a user message with only empty text); the Anthropic provider already skipped such an assistant turn. Empty messages of either role are now dropped before consecutive same-role messages are merged (#368), so the user turns around a dropped one go back as one. - **Bedrock, Gemini and Vertex: a parallel tool call's results go back in one message** (#223). Each tool result was sent as its own user-role message, and a steering message, stop marker or compaction summary after them added another, while Bedrock's Converse requires roles to alternate and Gemini requires a turn's function responses in one content, as many as its function calls. So the request after any turn with two or more tool calls (the default parallel strategy) could be rejected. The three request builders now merge consecutive same-role messages, keeping every block in order: the results in call order, then whatever followed; on Gemini and Vertex a merged turn's function responses come first, then any images and text (images among the responses are a reported cause of Gemini's response-count 400). Pinned by request-body tests; not run against a live endpoint. ## 0.25.12 (2026-10-21) diff --git a/src/provider/bedrock.rs b/src/provider/bedrock.rs index 81533abb..6b8f64ea 100644 --- a/src/provider/bedrock.rs +++ b/src/provider/bedrock.rs @@ -1611,7 +1611,6 @@ impl TokenUsage { mod tests { use super::*; - /// A parallel tool call's results go back as one user message, the /// An assistant turn that errored with no content is not sent as an empty /// message; the user turns around it go back as one (#369). #[test] @@ -1629,6 +1628,7 @@ mod tests { ); } + /// A parallel tool call's results go back as one user message, the /// steering message after them, so Converse's roles alternate (#223). #[test] fn parallel_tool_results_go_back_in_one_message() { diff --git a/src/provider/google.rs b/src/provider/google.rs index a6bc95d0..730e5381 100644 --- a/src/provider/google.rs +++ b/src/provider/google.rs @@ -833,7 +833,6 @@ mod tests { use super::*; use crate::provider::{GoogleCompat, ModelConfig}; - /// A parallel tool call's results go back as one user turn, with the /// A model turn with only thinking (not replayed) or none at all is not /// sent as an empty content (#369). #[test] @@ -857,6 +856,7 @@ mod tests { ); } + /// A parallel tool call's results go back as one user turn, with the /// steering message after them, so roles alternate (#223). #[test] fn parallel_tool_results_go_back_in_one_turn() { From 6f0268af559b385527b8c992b0b97860202e7add Mon Sep 17 00:00:00 2001 From: Yuanhao Li Date: Thu, 22 Oct 2026 00:38:36 +0200 Subject: [PATCH 3/3] =?UTF-8?q?test+docs:=20#369=20review=20round=20?= =?UTF-8?q?=E2=80=94=20the=20loop's=20real=20error=20shape,=20end=20to=20e?= =?UTF-8?q?nd?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Tests: Bedrock and Gemini builder tests also use the loop's error message (one empty text block); helper tests cover an empty message first and an empty user message; a Vertex builder test; a Bedrock run through the loop where a failed prompt is followed by another — the second request carries one user message, the history keeps the error. - Docs: drop covers errored, refused and cancelled turns and unreplayable thinking on all three providers, either role; a dropped first message can leave an assistant turn first. bedrock.md: Anthropic redacted blocks are logged at debug, not warned. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01T7iq5hpndSiHQcnAsywKuG --- CHANGELOG.md | 2 +- CLAUDE.md | 2 +- docs/providers/bedrock.md | 2 +- src/provider/bedrock.rs | 34 ++++++++++++++++----------- src/provider/google.rs | 40 ++++++++++++++++---------------- src/provider/google_vertex.rs | 23 +++++++++++++++++++ src/provider/mod.rs | 41 +++++++++++++++++++++++++++------ tests/bedrock_stream_test.rs | 43 +++++++++++++++++++++++++++++++++++ 8 files changed, 145 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1307f132..c5418516 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ adheres to [Semantic Versioning](https://semver.org/). ## Unreleased -- **Bedrock, Gemini and Vertex no longer send an empty message** (#369). An assistant turn that errored with no content — or, on Gemini, with only thinking, which is not replayed — went out as `content: []` / `parts: []`, which both APIs reject (as did a user message with only empty text); the Anthropic provider already skipped such an assistant turn. Empty messages of either role are now dropped before consecutive same-role messages are merged (#368), so the user turns around a dropped one go back as one. +- **Bedrock, Gemini and Vertex no longer send an empty message** (#369). An assistant turn that errored, was refused or was cancelled with no content (the loop's error message is one empty text block, which the builders filter) — or whose only content is thinking the provider does not replay (on Gemini and Vertex any thinking; on Bedrock unsigned reasoning to a Claude model, or another API's) — went out as `content: []` / `parts: []`, which both APIs reject (as did a user message with only empty text); the Anthropic provider already skipped such an assistant turn. So the prompt after a failed or cancelled run was rejected too. Empty messages of either role are now dropped before consecutive same-role messages are merged (#368), so the user turns around a dropped one go back as one. - **Bedrock, Gemini and Vertex: a parallel tool call's results go back in one message** (#223). Each tool result was sent as its own user-role message, and a steering message, stop marker or compaction summary after them added another, while Bedrock's Converse requires roles to alternate and Gemini requires a turn's function responses in one content, as many as its function calls. So the request after any turn with two or more tool calls (the default parallel strategy) could be rejected. The three request builders now merge consecutive same-role messages, keeping every block in order: the results in call order, then whatever followed; on Gemini and Vertex a merged turn's function responses come first, then any images and text (images among the responses are a reported cause of Gemini's response-count 400). Pinned by request-body tests; not run against a live endpoint. ## 0.25.12 (2026-10-21) diff --git a/CLAUDE.md b/CLAUDE.md index e8fdcd0f..9cf6bc60 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -61,7 +61,7 @@ The Responses and Azure providers share one request builder + read loop (`respon Anthropic SSE (`anthropic.rs`): block `index` → position in `content` via a map, never padding, so a block type it does not surface (server tool blocks, `fallback`, anything new) leaves no placeholder and later blocks keep their deltas; unknown block/delta types `warn_once` per type by name, unparseable start/delta events are logged. `redacted_thinking` (`data` arrives whole at `content_block_start`, no delta type) → `Content::thinking_redacted(ApiProtocol::AnthropicMessages, data)` → replayed as `{"type":"redacted_thinking","data"}` in place (#197). -Bedrock is the one non-SSE provider: ConverseStream answers with binary `application/vnd.amazon.eventstream` frames, decoded by the private `provider/eventstream.rs` (bytes buffered across chunks; prelude + message CRC-32 verified with an in-house table, no dependency; all ten header types; checksum/malformed → `Other`, a body ending mid-frame → `Network`; pinned by aws-sdk-go's published test vectors). `bedrock.rs` dispatches on the `:message-type` / `:event-type` / `:exception-type` headers — payloads are the event structs themselves (no wrapper key), shapes per the AWS Bedrock Runtime API reference. Blocks accumulate per `contentBlockIndex`; a tool call is pushed with the `__partial_json` marker and only `contentBlockStop` finalizes it (`finalize_tool_arguments`), so an unclosed block is never runnable. Exceptions classify via their documented HTTP status (`throttlingException`/`serviceUnavailableException` → `RateLimited`; other 5xx stay `Api`). No `messageStop` (incl. empty body) or a transport error → `Network`, never an empty `Ok` — except once `messageStop` **and** `metadata` arrived: a later drop/truncation keeps the response (no re-billed retry). `messageStop` without `metadata` → zero usage + `warn!(usage_missing = true)` (same zero as other providers). Unsurfaced content (image/citation/toolResult, server_tool_use, unknown union members via `#[serde(flatten)] other`, unknown event types) is dropped with a once-per-block warning; an unknown `contentBlockStart` member creates no block, so the first delta types it. Reasoning replay: signed → `reasoningText`, `redactedContent` kept in `Content::Thinking::redacted` (base64, bytes concatenated across deltas; `redacted_protocol = BedrockConverseStream`) → `redactedContent`, only if it came from Bedrock (another API's redacted data is skipped, logged at debug); unsigned → `reasoningText {text}` with no signature key, except to a model id containing `claude` (signature-verifying), where it is skipped at debug (never `signature: ""`); the Anthropic provider skips Bedrock's redacted blocks. Signature deltas are appended. Tested only with mock frames (`tests/bedrock_stream_test.rs`). Consecutive same-role messages are merged before sending (`provider::merge_same_role(messages, "content")`; Gemini and Vertex do the same with `"parts"`, then `google::function_responses_first` puts each user content's `functionResponse` parts before its images and text), so a parallel tool call's results go back as one user message and roles alternate (#223); a message whose array is empty (an errored assistant turn) is dropped first (#369). Auth (`resolve_auth`, first match wins): an `authorization` key in `ModelConfig.headers` (any case) → sent alone; non-empty `api_key` → `access:secret[:token]` = SigV4 (`ASIA…` without a token refused), no `:` = Bedrock API key (`Bearer`) after `checked_bearer` refuses whitespace/control chars, an `AKIA`/`ASIA` start, non-base64 chars and a bare 40-char secret (unknown prefix — not `bedrock-api-key-`/`ABSK` — only warns once); empty `api_key` → env read per request (type `Env` = `std::env::var`-shaped, faked in tests), `AWS_BEARER_TOKEN_BEDROCK` (same checks) then `AWS_ACCESS_KEY_ID`/`AWS_SECRET_ACCESS_KEY`(/`AWS_SESSION_TOKEN`); half a pair / NotUnicode / none → `Auth` naming the variable, before sending. `resolve_api_key_or_warn(Option<&ModelConfig>)` (Agent, SubAgentTool, LlmCompaction) keys on the **protocol**: any `BedrockConverseStream` config gets an empty key (never `API_KEY`/`YOAGENT_API_KEY`, whatever the provider string), so the provider reads env itself; `resolve_api_key("bedrock")` (public) prefers `AWS_BEARER_TOKEN_BEDROCK`. `request_headers` builds the whole `HeaderMap`: every value via `header_value` (credential values must be ASCII; failure → `Auth` naming the header, never the value; user non-auth header → `Other`), `authorization`/`x-amz-security-token` `set_sensitive`; on the SigV4 path a user header named like a signed one is refused; a `send()` `is_builder()` error → `Other`, not `Network`. SigV4 is the private `provider/sigv4.rs` (sha2 + hmac, no AWS SDK; pinned to aws-c-auth's `aws-sig-v4-test-suite` vectors incl. the `*-normalized` path ones): signing name `bedrock`, region from the host (`bedrock-runtime[-fips]..`, suffix ∈ `AWS_DNS_SUFFIXES` = partition dnsSuffix/dualStackDnsSuffix from botocore + VPC forms) > `AWS_REGION` > `AWS_DEFAULT_REGION` > `Auth` error; signs `host`, `content-type`, `x-amz-date`, `x-amz-content-sha256`, `x-amz-security-token` over the exact body bytes sent (serialize once — never `.json()`); `amz_date_now` errors on a pre-1970 clock; a 403 "Signature expired"/"not yet current" gets a check-the-clock hint. `base_url` is `trim_end_matches('/')`-ed; `canonical_uri` normalizes (drops empty/`.` segments, resolves `..`, keeps a trailing `/`) and encodes each segment again, so the model id (`:` → `%3A` on the wire, as the SDKs send it) is `%253A` in the canonical URI. `tests/bedrock_auth_test.rs` (env-mutating, serialized by a tokio `Mutex`) recomputes each signature from what wiremock received and uses `expect(0)` for every refusal. Never run against a live endpoint. +Bedrock is the one non-SSE provider: ConverseStream answers with binary `application/vnd.amazon.eventstream` frames, decoded by the private `provider/eventstream.rs` (bytes buffered across chunks; prelude + message CRC-32 verified with an in-house table, no dependency; all ten header types; checksum/malformed → `Other`, a body ending mid-frame → `Network`; pinned by aws-sdk-go's published test vectors). `bedrock.rs` dispatches on the `:message-type` / `:event-type` / `:exception-type` headers — payloads are the event structs themselves (no wrapper key), shapes per the AWS Bedrock Runtime API reference. Blocks accumulate per `contentBlockIndex`; a tool call is pushed with the `__partial_json` marker and only `contentBlockStop` finalizes it (`finalize_tool_arguments`), so an unclosed block is never runnable. Exceptions classify via their documented HTTP status (`throttlingException`/`serviceUnavailableException` → `RateLimited`; other 5xx stay `Api`). No `messageStop` (incl. empty body) or a transport error → `Network`, never an empty `Ok` — except once `messageStop` **and** `metadata` arrived: a later drop/truncation keeps the response (no re-billed retry). `messageStop` without `metadata` → zero usage + `warn!(usage_missing = true)` (same zero as other providers). Unsurfaced content (image/citation/toolResult, server_tool_use, unknown union members via `#[serde(flatten)] other`, unknown event types) is dropped with a once-per-block warning; an unknown `contentBlockStart` member creates no block, so the first delta types it. Reasoning replay: signed → `reasoningText`, `redactedContent` kept in `Content::Thinking::redacted` (base64, bytes concatenated across deltas; `redacted_protocol = BedrockConverseStream`) → `redactedContent`, only if it came from Bedrock (another API's redacted data is skipped, logged at debug); unsigned → `reasoningText {text}` with no signature key, except to a model id containing `claude` (signature-verifying), where it is skipped at debug (never `signature: ""`); the Anthropic provider skips Bedrock's redacted blocks. Signature deltas are appended. Tested only with mock frames (`tests/bedrock_stream_test.rs`). Consecutive same-role messages are merged before sending (`provider::merge_same_role(messages, "content")`; Gemini and Vertex do the same with `"parts"`, then `google::function_responses_first` puts each user content's `functionResponse` parts before its images and text), so a parallel tool call's results go back as one user message and roles alternate (#223); a message whose array is empty (an errored, refused or cancelled turn, or one with only unreplayable thinking; either role) is dropped first and its neighbours merge (#369). Auth (`resolve_auth`, first match wins): an `authorization` key in `ModelConfig.headers` (any case) → sent alone; non-empty `api_key` → `access:secret[:token]` = SigV4 (`ASIA…` without a token refused), no `:` = Bedrock API key (`Bearer`) after `checked_bearer` refuses whitespace/control chars, an `AKIA`/`ASIA` start, non-base64 chars and a bare 40-char secret (unknown prefix — not `bedrock-api-key-`/`ABSK` — only warns once); empty `api_key` → env read per request (type `Env` = `std::env::var`-shaped, faked in tests), `AWS_BEARER_TOKEN_BEDROCK` (same checks) then `AWS_ACCESS_KEY_ID`/`AWS_SECRET_ACCESS_KEY`(/`AWS_SESSION_TOKEN`); half a pair / NotUnicode / none → `Auth` naming the variable, before sending. `resolve_api_key_or_warn(Option<&ModelConfig>)` (Agent, SubAgentTool, LlmCompaction) keys on the **protocol**: any `BedrockConverseStream` config gets an empty key (never `API_KEY`/`YOAGENT_API_KEY`, whatever the provider string), so the provider reads env itself; `resolve_api_key("bedrock")` (public) prefers `AWS_BEARER_TOKEN_BEDROCK`. `request_headers` builds the whole `HeaderMap`: every value via `header_value` (credential values must be ASCII; failure → `Auth` naming the header, never the value; user non-auth header → `Other`), `authorization`/`x-amz-security-token` `set_sensitive`; on the SigV4 path a user header named like a signed one is refused; a `send()` `is_builder()` error → `Other`, not `Network`. SigV4 is the private `provider/sigv4.rs` (sha2 + hmac, no AWS SDK; pinned to aws-c-auth's `aws-sig-v4-test-suite` vectors incl. the `*-normalized` path ones): signing name `bedrock`, region from the host (`bedrock-runtime[-fips]..`, suffix ∈ `AWS_DNS_SUFFIXES` = partition dnsSuffix/dualStackDnsSuffix from botocore + VPC forms) > `AWS_REGION` > `AWS_DEFAULT_REGION` > `Auth` error; signs `host`, `content-type`, `x-amz-date`, `x-amz-content-sha256`, `x-amz-security-token` over the exact body bytes sent (serialize once — never `.json()`); `amz_date_now` errors on a pre-1970 clock; a 403 "Signature expired"/"not yet current" gets a check-the-clock hint. `base_url` is `trim_end_matches('/')`-ed; `canonical_uri` normalizes (drops empty/`.` segments, resolves `..`, keeps a trailing `/`) and encodes each segment again, so the model id (`:` → `%3A` on the wire, as the SDKs send it) is `%253A` in the canonical URI. `tests/bedrock_auth_test.rs` (env-mutating, serialized by a tokio `Mutex`) recomputes each signature from what wiremock received and uses `expect(0)` for every refusal. Never run against a live endpoint. `ModelConfig` + quirk structs handle per-provider differences: `OpenAiCompat` (reasoning format, max_tokens field name, `max_reasoning_effort` ceiling — the ceiling, and `encrypted_reasoning`, are also read by Responses/Azure), `AnthropicCompat` (adaptive vs budget thinking, bearer auth, `native_structured_output`; `for_claude_id` infers them from an id), `GoogleCompat` (`thinkingLevel` vs `thinkingBudget`). `ThinkingLevel::Off` sends no thinking/effort field on any provider (DeepSeek's explicit `thinking: disabled` aside), so always-thinking models run at their default. diff --git a/docs/providers/bedrock.md b/docs/providers/bedrock.md index 82675559..18121a7b 100644 --- a/docs/providers/bedrock.md +++ b/docs/providers/bedrock.md @@ -209,7 +209,7 @@ is kept in `Content::Thinking::redacted` (with `redacted_protocol` set to `BedrockConverseStream`) and sent back as `redactedContent`. Encrypted reasoning from another API is not: AWS does not document `redactedContent` as the same bytes as the Anthropic API's `redacted_thinking` data, so an Anthropic -redacted block is skipped here with a warning (and Bedrock's is skipped by the +redacted block is skipped here, logged at debug (and Bedrock's is skipped by the Anthropic provider). Reasoning without a signature is sent back as `reasoningText {text}` with no signature key (the signature is optional in `ReasoningTextBlock`, and Bedrock's diff --git a/src/provider/bedrock.rs b/src/provider/bedrock.rs index 6b8f64ea..e69057b4 100644 --- a/src/provider/bedrock.rs +++ b/src/provider/bedrock.rs @@ -1611,21 +1611,29 @@ impl TokenUsage { mod tests { use super::*; - /// An assistant turn that errored with no content is not sent as an empty - /// message; the user turns around it go back as one (#369). + /// An errored assistant turn — as the loop builds it, one empty text block, + /// or with no content at all — is not sent as an empty message; the user + /// turns around it go back as one (#369). #[test] fn an_empty_assistant_turn_is_not_sent() { - let mut config = StreamConfig::new("anthropic.claude-sonnet", "a:b"); - config.messages = vec![ - Message::user("first"), - Message::assistant(vec![], StopReason::Error, "m", "bedrock", Usage::default()), - Message::user("again"), - ]; - let body = build_bedrock_body(&config); - assert_eq!( - body["messages"], - serde_json::json!([{"role": "user", "content": [{"text": "first"}, {"text": "again"}]}]) - ); + for errored in [ + vec![Content::Text { + text: String::new(), + }], + vec![], + ] { + let mut config = StreamConfig::new("anthropic.claude-sonnet", "a:b"); + config.messages = vec![ + Message::user("first"), + Message::assistant(errored, StopReason::Error, "m", "bedrock", Usage::default()), + Message::user("again"), + ]; + let body = build_bedrock_body(&config); + assert_eq!( + body["messages"], + serde_json::json!([{"role": "user", "content": [{"text": "first"}, {"text": "again"}]}]) + ); + } } /// A parallel tool call's results go back as one user message, the diff --git a/src/provider/google.rs b/src/provider/google.rs index 730e5381..f8c9f8b7 100644 --- a/src/provider/google.rs +++ b/src/provider/google.rs @@ -833,27 +833,29 @@ mod tests { use super::*; use crate::provider::{GoogleCompat, ModelConfig}; - /// A model turn with only thinking (not replayed) or none at all is not - /// sent as an empty content (#369). + /// A model turn whose only content is thinking (not replayed), or the + /// loop's error message (one empty text block), is not sent as an empty + /// content (#369). #[test] fn an_empty_model_turn_is_not_sent() { - let mut config = StreamConfig::new("gemini-2.5-pro", "k"); - config.messages = vec![ - Message::user("first"), - Message::assistant( - vec![Content::thinking("thought")], - StopReason::Error, - "m", - "google", - Usage::default(), - ), - Message::user("again"), - ]; - let body = build_request_body(&config); - assert_eq!( - body["contents"], - serde_json::json!([{"role": "user", "parts": [{"text": "first"}, {"text": "again"}]}]) - ); + for turn in [ + vec![Content::thinking("thought")], + vec![Content::Text { + text: String::new(), + }], + ] { + let mut config = StreamConfig::new("gemini-2.5-pro", "k"); + config.messages = vec![ + Message::user("first"), + Message::assistant(turn, StopReason::Error, "m", "google", Usage::default()), + Message::user("again"), + ]; + let body = build_request_body(&config); + assert_eq!( + body["contents"], + serde_json::json!([{"role": "user", "parts": [{"text": "first"}, {"text": "again"}]}]) + ); + } } /// A parallel tool call's results go back as one user turn, with the diff --git a/src/provider/google_vertex.rs b/src/provider/google_vertex.rs index 6df89126..6fc45a3a 100644 --- a/src/provider/google_vertex.rs +++ b/src/provider/google_vertex.rs @@ -373,6 +373,29 @@ mod tests { ); } + /// A model turn with only thinking (not replayed) is not sent as an empty + /// content; the user turns around it go back as one (#369). + #[test] + fn an_empty_model_turn_is_not_sent() { + let mut c = config(ThinkingLevel::Off); + c.messages = vec![ + Message::user("first"), + Message::assistant( + vec![Content::thinking("thought")], + crate::types::StopReason::Error, + "m", + "vertex", + crate::types::Usage::default(), + ), + Message::user("again"), + ]; + let body = build_vertex_request_body(&c); + assert_eq!( + body["contents"], + serde_json::json!([{"role": "user", "parts": [{"text": "first"}, {"text": "again"}]}]) + ); + } + /// A parallel tool call's results go back as one user turn (#223). #[test] fn parallel_tool_results_go_back_in_one_turn() { diff --git a/src/provider/mod.rs b/src/provider/mod.rs index 5ef8f102..49a26e2f 100644 --- a/src/provider/mod.rs +++ b/src/provider/mod.rs @@ -72,10 +72,14 @@ pub(crate) fn tool_result_text(content: &[crate::types::Content]) -> String { /// builders produce one wire message per result. Merging keeps every block in /// order: the results first, in call order, then whatever followed them. /// -/// A message whose `key` array is empty is dropped first: both APIs reject an -/// empty message, and an assistant turn that errored with no content (or, on -/// Gemini, only thinking, which is not replayed) would otherwise be one. Its -/// neighbours then merge, so the roles still alternate (#369). +/// A message whose `key` array is empty is dropped first, of either role: +/// both APIs reject an empty message, and a turn that errored, was refused or +/// was cancelled with no content (the loop's error message is one empty text +/// block, which the builders filter), or whose only content is thinking the +/// provider does not replay, would otherwise be one. Its neighbours then +/// merge, so the roles still alternate (#369). A first message dropped this +/// way can leave an assistant turn first, which Bedrock rejects — as it +/// rejected the empty message before. pub(crate) fn merge_same_role( messages: Vec, key: &str, @@ -142,13 +146,14 @@ mod merge_same_role_tests { use super::merge_same_role; use serde_json::json; - /// An empty message is dropped, wherever it is, and its neighbours merge: - /// an assistant turn that errored with no content between two user turns - /// leaves one user message (#369). + /// An empty message is dropped, first, in the middle or last, and its + /// neighbours merge: an assistant turn that errored with no content + /// between two user turns leaves one user message (#369). #[test] fn empty_messages_are_dropped_and_their_neighbours_merge() { let merged = merge_same_role( vec![ + json!({"role": "user", "parts": []}), json!({"role": "user", "parts": [{"text": "1"}]}), json!({"role": "model", "parts": []}), json!({"role": "user", "parts": [{"text": "2"}]}), @@ -166,6 +171,28 @@ mod merge_same_role_tests { ); } + /// An empty user message is dropped too, and the model turns around it + /// then merge (#369). + #[test] + fn an_empty_user_message_is_dropped() { + let merged = merge_same_role( + vec![ + json!({"role": "user", "content": [{"text": "a"}]}), + json!({"role": "assistant", "content": [{"text": "x"}]}), + json!({"role": "user", "content": []}), + json!({"role": "assistant", "content": [{"text": "y"}]}), + ], + "content", + ); + assert_eq!( + merged, + vec![ + json!({"role": "user", "content": [{"text": "a"}]}), + json!({"role": "assistant", "content": [{"text": "x"}, {"text": "y"}]}), + ] + ); + } + #[test] fn adjacent_same_role_messages_become_one_in_order() { let merged = merge_same_role( diff --git a/tests/bedrock_stream_test.rs b/tests/bedrock_stream_test.rs index 1e3f8f65..f2d7d411 100644 --- a/tests/bedrock_stream_test.rs +++ b/tests/bedrock_stream_test.rs @@ -634,6 +634,49 @@ async fn reasoning_with_signature_is_accumulated_and_replayed() { ); } +/// A run that failed leaves an errored assistant message (one empty text +/// block) in the history; the next prompt does not send it as an empty +/// message, so the two user turns go back as one (#369). The history keeps it. +#[tokio::test] +async fn an_errored_turn_is_not_resent_as_an_empty_message() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(400).set_body_json(json!({"message": "Bad request"}))) + .up_to_n_times(1) + .with_priority(1) + .mount(&server) + .await; + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(200).set_body_raw(text_response("ok"), EVENTSTREAM)) + .with_priority(2) + .mount(&server) + .await; + let mut agent = Agent::from_provider(BedrockProvider, model_config(&server.uri())) + .with_api_key("test-bedrock-api-key"); + for prompt in ["one", "two"] { + let mut rx = agent.prompt(prompt).await; + while rx.recv().await.is_some() {} + agent.finish().await; + } + let errored = agent.messages().iter().any(|m| { + matches!( + m, + AgentMessage::Llm(Message::Assistant { + stop_reason: StopReason::Error, + .. + }) + ) + }); + assert!(errored, "the history keeps the errored turn"); + let requests = server.received_requests().await.unwrap(); + assert_eq!(requests.len(), 2); + let body: Value = serde_json::from_slice(&requests[1].body).unwrap(); + assert_eq!( + body["messages"], + json!([{"role": "user", "content": [{"text": "one"}, {"text": "two"}]}]) + ); +} + /// A parallel tool call through the real loop: both results go back in one /// user message, in call order, so Converse's roles alternate (#223). #[tokio::test]