Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Gemini adapter to introduce bounded incremental SSE decoding, enforce strict semantic validation on tool signatures and content blocks, and add comprehensive conformance tests. The feedback highlights a high-severity performance bottleneck in the SSE parsing loop that can be optimized by checking delimiters only on newlines, a medium-severity recommendation to pre-allocate vector capacity in collect_unary_response when the content length is known, and a medium-severity warning about potential test flakiness caused by sharing a process-global environment variable across parallel tests.
| for (offset, &byte) in chunk.iter().enumerate() { | ||
| self.buffer.push(byte); | ||
| if let Some(delimiter_len) = terminal_delimiter_len(&self.buffer) { | ||
| let frame_len = self.buffer.len() - delimiter_len; | ||
| if frame_len > self.max_event_bytes { | ||
| return Err(self.fail(format!( | ||
| "Gemini SSE event exceeded {} bytes", | ||
| self.max_event_bytes | ||
| ))); | ||
| } | ||
| let mut frame = std::mem::take(&mut self.buffer); | ||
| frame.truncate(frame_len); | ||
| if self.done { | ||
| return Err(self.fail("Gemini SSE frame arrived after [DONE]")); | ||
| } | ||
| let item = match parse_frame(&frame) { | ||
| Ok(Some(Item::Done)) => { | ||
| self.done = true; | ||
| Some(Item::Done) | ||
| } | ||
| Ok(item) => item, | ||
| Err(error) => return Err(self.fail(error)), | ||
| }; | ||
| return Ok((offset + 1, item)); | ||
| } else if retained_candidate_len(&self.buffer) > self.max_event_bytes { | ||
| return Err(self.fail(format!( | ||
| "Gemini SSE event exceeded {} bytes", | ||
| self.max_event_bytes | ||
| ))); | ||
| } | ||
| } |
There was a problem hiding this comment.
[HIGH] Performance bottleneck in incremental SSE parsing loop
Problem: For every single byte in the input chunk, push_one pushes the byte to self.buffer and then performs multiple ends_with checks via terminal_delimiter_len and retained_candidate_len. This results in
Rationale: Org Style Guide §1 (Performance - call out obvious anti-patterns).
Suggestion: Only perform the terminal_delimiter_len check when byte == b'\n' (since all terminal delimiters end with \n), and only check retained_candidate_len when self.buffer.len() > self.max_event_bytes.
for (offset, &byte) in chunk.iter().enumerate() {
self.buffer.push(byte);
if byte == b'\n' {
if let Some(delimiter_len) = terminal_delimiter_len(&self.buffer) {
let frame_len = self.buffer.len() - delimiter_len;
if frame_len > self.max_event_bytes {
return Err(self.fail(format!(
"Gemini SSE event exceeded {} bytes",
self.max_event_bytes
)));
}
let mut frame = std::mem::take(&mut self.buffer);
frame.truncate(frame_len);
if self.done {
return Err(self.fail("Gemini SSE frame arrived after [DONE]"));
}
let item = match parse_frame(&frame) {
Ok(Some(Item::Done)) => {
self.done = true;
Some(Item::Done)
}
Ok(item) => item,
Err(error) => return Err(self.fail(error)),
};
return Ok((offset + 1, item));
}
}
if self.buffer.len() > self.max_event_bytes {
if retained_candidate_len(&self.buffer) > self.max_event_bytes {
return Err(self.fail(format!(
"Gemini SSE event exceeded {} bytes",
self.max_event_bytes
)));
}
}
}References
- Key Principles - Performance: Measure-first; still call out obvious anti-patterns without benchmarks. (link)
| let mut stream = response.bytes_stream(); | ||
| let mut body = Vec::new(); |
There was a problem hiding this comment.
[MEDIUM] Optimize memory allocation in collect_unary_response
Problem: body is initialized as an empty Vec and grows dynamically as chunks are read. This causes multiple heap reallocations and memory copying during stream collection.
Rationale: Org Style Guide §1 (Performance - call out obvious anti-patterns).
Suggestion: Pre-allocate the vector with the capacity of content_length if it is known and within the limit.
let mut stream = response.bytes_stream();
let mut body = match response.content_length() {
Some(len) if len <= max_bytes as u64 => Vec::with_capacity(len as usize),
_ => Vec::new(),
};References
- Key Principles - Performance: Measure-first; still call out obvious anti-patterns without benchmarks. (link)
| fn gemini_config_with_retry(base_url: String, max_retries: u32) -> Config { | ||
| let mut config = Config::default(); | ||
| let provider = config.providers.get_mut("gemini").unwrap(); | ||
| provider.base_url = base_url; | ||
| provider.auth = AuthMode::ApiKey; | ||
| provider.api_key_env = Some("SHUNT_GEMINI_CONFORMANCE_KEY".to_string()); | ||
| provider.retry = RetryConfig { | ||
| max_retries, | ||
| initial_backoff_ms: 1, | ||
| max_backoff_ms: 1, | ||
| ..RetryConfig::default() | ||
| }; | ||
| config.server.default_provider = "gemini".to_string(); | ||
| config.routes = vec![RouteConfig { | ||
| model: "claude-via-gemini".to_string(), | ||
| provider: "gemini".to_string(), | ||
| upstream_model: Some("gemini-2.5-pro".to_string()), | ||
| effort: None, | ||
| service_tier: None, | ||
| }]; | ||
| config | ||
| } | ||
|
|
||
| fn gemini_config(base_url: String) -> Config { | ||
| gemini_config_with_retry(base_url, 0) | ||
| } | ||
|
|
||
| async fn start_gateway(base_url: String) -> Gateway { | ||
| start_gateway_with_config(gemini_config(base_url)).await | ||
| } | ||
|
|
||
| async fn start_gateway_with_config(mut config: Config) -> Gateway { | ||
| std::env::set_var("SHUNT_GEMINI_CONFORMANCE_KEY", "fixture-key"); |
There was a problem hiding this comment.
[MEDIUM] Prevent test flakiness from process-global environment variables
Problem: The tests in tests/gemini_conformance.rs share a single process-global environment variable SHUNT_GEMINI_CONFORMANCE_KEY. Since tests run in parallel, this can cause cross-test interference and flakiness.
Rationale: General Rules - Process-global environment variables in tests should use unique suffixes and be cleaned up using a drop guard.
Suggestion: Use a unique suffix for the environment variable name per test, and manage its lifecycle with a drop guard.
References
- When testing code that relies on process-global environment variables, prevent test flakiness and cross-test interference by using unique suffixes (such as
std::process::id()) for environment variable names, and clean them up using a drop guard at the end of the test instead of resetting them on entry.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| fn retry_safety_for_auth(auth: AuthMode) -> crate::retry::RetrySafety { | ||
| if auth == AuthMode::AntigravityOauth { | ||
| crate::retry::RetrySafety::Idempotent |
There was a problem hiding this comment.
When an Antigravity OAuth generation returns a retryable 429, 502, 503, 504, or 529 status, this Idempotent classification allows the generation POST to be sent again even though the upstream may already have accepted it. That can duplicate billable work and contradicts the documented guarantee that returned Gemini statuses are not retried to the same upstream. Antigravity generation requests need the same non-idempotent retry protection as the other Gemini paths.
Knowledge Base Used: Protocol and model translation
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/adapters/gemini/mod.rs
Line: 550
Comment:
**Antigravity requests can replay**
When an Antigravity OAuth generation returns a retryable 429, 502, 503, 504, or 529 status, this `Idempotent` classification allows the generation POST to be sent again even though the upstream may already have accepted it. That can duplicate billable work and contradicts the documented guarantee that returned Gemini statuses are not retried to the same upstream. Antigravity generation requests need the same non-idempotent retry protection as the other Gemini paths.
**Knowledge Base Used:** [Protocol and model translation](https://app.greptile.com/passionfactory/-/custom-context/knowledge-base/pleaseai/shunt/-/docs/protocol-translation.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 355b79c15e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if self.done { | ||
| return Err(self.fail("Gemini SSE frame arrived after [DONE]")); |
There was a problem hiding this comment.
[DONE]
When an upstream terminates with [DONE] followed by an extra blank separator (for example, data: [DONE]\n\n\n\n), the second delimiter produces an empty frame, but this check rejects it solely because self.done is set. The adapter then discards the deferred message_stop and emits an error for an otherwise successful stream. Parse the frame first and continue accepting empty/whitespace-only frames after [DONE], while still rejecting semantic data.
AGENTS.md reference: AGENTS.md:L40-L41
Useful? React with 👍 / 👎.
| let signature = decode_tool_use_signature(id)?; | ||
| if model.starts_with(GEMINI_3_MODEL_PREFIX) | ||
| && function_call_index == 0 | ||
| && signature.is_none() | ||
| { | ||
| return Err(bad_request( | ||
| "Gemini 3 tool history requires an authentic thought signature", |
There was a problem hiding this comment.
For a Gemini 3 request whose history contains a tool call produced by another provider or an older shunt version, the first ID is normally an unsigned toolu_*, so this new branch rejects the whole request before dispatch. This removes the imported-history behavior still required by docs/plans/gemini-provider/TICKETS.md:73, which says to supply Google's documented placeholder on the first unsigned call; either retain that fallback or update the engineering spec as part of this behavior change.
AGENTS.md reference: src/AGENTS.md:L34-L37
Useful? React with 👍 / 👎.
| | `authentication_error` on a mapped model | Expired/absent provider credential — re-run `codex login`, or export `OPENAI_API_KEY`. shunt surfaces the backend's real `detail` message. | | ||
| | <!-- shunt-contract: gemini-code-assist strict-terminal malformed-fails non-idempotent-preheader tool-result-roundtrip no-writeback ai-studio-web-excluded --> Gemini fails instead of completing when Code Assist closes the stream | Gemini success responses require both an explicit provider finish and clean transport closure. EOF or `[DONE]` without that finish, invalid UTF-8, malformed JSON or unsupported fields, oversized data, multiple candidates, data after terminal, or truncated responses fail explicitly. Streaming and unary modes apply the same terminal and provider-error semantics, never converting embedded provider errors into synthetic success. | | ||
| | Gemini tool history rejected before dispatch | Client-visible `tool_use` IDs preserve authentic Code Assist call pairings and `thoughtSignature` on Gemini 3. Return the matching ID in `tool_result`. Foreign, malformed, duplicate, or orphaned metadata, as well as metadata missing required signatures, are rejected without repair. Valid results become `functionResponse` in the next request. shunt does not persist or write back this metadata. | | ||
| | Gemini transient status not retried to the same upstream | Gemini generations are non-idempotent. Only transient connection or timeout failures proven to have occurred before response headers are retried with the same selected token, project, and payload. Returned statuses and all body-time failures surface immediately without retrying the same upstream. | |
There was a problem hiding this comment.
This new blanket statement is false for auth = "antigravity_oauth": retry_safety_for_auth marks that path Idempotent, so with the default retry policy its returned 429/502/503/504/529 statuses are retried against the same upstream. Operators troubleshooting duplicate attempts or rate-limit behavior will be misled; either make Antigravity non-idempotent too or explicitly document the exception.
AGENTS.md reference: AGENTS.md:L47-L53
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/model/gemini_request.rs">
<violation number="1" location="src/model/gemini_request.rs:188">
P2: When an invalid `tool_result` supplies a large unknown `tool_use_id`, this path interpolates the client-controlled string into `AdapterError` without the 96 KiB cap applied to `tool_use.id`. Bound result IDs before matching and in the orphan-error path to prevent large duplicate allocations for malformed requests.</violation>
<violation number="2" location="src/model/gemini_request.rs:323">
P2: Gemini 3 histories with unsigned imported IDs such as `toolu_*` now fail before dispatch. Add the documented dummy `thoughtSignature` for the first unsigned call instead of rejecting migrated history.</violation>
</file>
<file name="src/adapters/gemini/sse.rs">
<violation number="1" location="src/adapters/gemini/sse.rs:64">
P2: After `[DONE]`, this branch rejects even an empty SSE frame. Because `forward` defers `message_stop` until EOF, an extra separator converts a successful stream into an error; parse empty frames before rejecting semantic data.</violation>
<violation number="2" location="src/adapters/gemini/sse.rs:111">
P2: When Gemini emits the valid SSE CR-only line terminator, an event ends with `\r\r`, but `terminal_delimiter_len` never recognizes it. The decoder then reports an unterminated frame instead of translating the response; accept the CR-only delimiter.</violation>
</file>
<file name="tests/gemini_translate.rs">
<violation number="1" location="tests/gemini_translate.rs:571">
P2: This 'bounded' block does not actually exercise the 32 MiB retained-byte ceiling. Each {"citationMetadata": {}} part returns 0 retained bytes (CheckedPart::Metadata, 0), so 4096 of them accumulate ~0 bytes and the following chunk is not rejected for exceeding the byte bound. Assert the ceiling with parts that contribute retained bytes (e.g. text parts totalling near 32 MiB) so the memory-bound claim is really tested.</violation>
</file>
<file name="src/model/gemini.rs">
<violation number="1" location="src/model/gemini.rs:530">
P2: When a Gemini part contains `text` and `thoughtSignature`, this branch returns before the orphan-signature check, silently dropping the metadata. Move the existing signature guard before the text branch so malformed signature metadata is rejected.</violation>
</file>
<file name="src/adapters/gemini/mod.rs">
<violation number="1" location="src/adapters/gemini/mod.rs:550">
P1: When Antigravity OAuth returns a retryable status, this `Idempotent` classification can replay the generation POST. Return `NonIdempotentPost` to prevent duplicate upstream work.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as Client (Claude Code)
participant Gateway as shunt Gateway
participant Adapter as Gemini Adapter
participant Decoder as Bounded SSE Decoder
participant Machine as GeminiSseMachine
participant Validator as Request Validator
participant Retry as Retry Logic
participant Upstream as Gemini/Code Assist Upstream
Note over Client,Upstream: Gemini Streaming Request Flow
Client->>Gateway: POST /v1/messages (stream: true)
Gateway->>Adapter: translate and forward request
Adapter->>Adapter: Build request with validated tool history
Note over Adapter: Validates tool_use IDs match,<br/>preserves thoughtSignature for Gemini 3
Adapter->>Upstream: POST :streamGenerateContent?alt=sse
Upstream-->>Adapter: 200 + SSE stream
Adapter->>Decoder: Feed raw bytes
Note over Decoder: Bounded frame buffer (8 MiB/event)<br/>Handles CRLF, multiline data
Decoder-->>Adapter: Parsed JSON frame or error
alt Valid frame
Decoder->>Machine: process_chunk_checked()
Note over Machine: Validates semantics,<br/>32 MiB state bound,<br/>terminal state tracking
alt Provider error embedded
Machine->>Adapter: error payload
else Semantic error
Machine->>Adapter: protocol error
else Normal content
Machine->>Adapter: Anthropic SSE events
end
Adapter-->>Client: Forward SSE events
else [DONE] frame
Decoder->>Machine: terminal marker
Machine-->>Adapter: Finalize stream
else Malformed frame (UTF-8/JSON/size)
Decoder-->>Adapter: protocol error (disposed)
Adapter-->>Client: error event
end
Note over Decoder,Machine: EOF handling
alt Clean EOF with SuccessPending
Machine->>Machine: transport_close_checked()
Machine-->>Adapter: message_stop events
else Cut stream (no terminal state)
Machine-->>Adapter: explicit failure
end
Adapter-->>Client: SSE error / complete events
Note over Client,Upstream: Tool Result Round-trip Flow (NEW semantics)
Client->>Gateway: POST /v1/messages (with tool_result history)
Gateway->>Validator: validate tool history
Note over Validator: Track outstanding tool_use batches
alt Valid tool_result sequence
Validator->>Validator: Map tool_use_id to functionResponse
Note over Validator: Requires exact match count,<br/>immediate follow with no intervening user turn,<br/>authentic thoughtSignature for Gemini 3
else Orphan/foreign/duplicate tool_result
Validator-->>Gateway: 400 Bad Request
Gateway-->>Client: Rejection (no dispatch)
end
Validator-->>Adapter: Validated request payload
Adapter->>Upstream: POST with functionResponse
alt Non-streaming request
Upstream-->>Adapter: JSON response (bounded 32 MiB)
else Streaming request
Upstream-->>Adapter: SSE stream
end
alt Success
Adapter-->>Client: Tool use events with call_gemini_v1_ IDs
else Provider error
Adapter-->>Client: Original error (never synthetic success)
end
Note over Client,Adapter: Subsequent request preserves thoughtSignature
Client->>Gateway: tool_result with matching call_gemini_v1_ ID
Gateway-->>Adapter: functionResponse in next request
Note over Gateway,Upstream: Retry Policy (non-idempotent protection)
Gateway->>Retry: Send with retry (safety-gated)
alt Failure BEFORE response headers (connection/timeout)
Retry->>Upstream: Retry same payload
Note over Retry: Only pre-header failures retried
else Returned HTTP status (429/502/503/504/529)
Retry->>Adapter: Fail immediately
Adapter-->>Gateway: Map error status
Gateway-->>Client: Error (no retry to same upstream)
else Body-time failure (mid-stream)
Retry->>Adapter: Fail immediately
Adapter-->>Client: error SSE event
Note over Adapter: Stream terminated after partial output
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| fn retry_safety_for_auth(auth: AuthMode) -> crate::retry::RetrySafety { | ||
| if auth == AuthMode::AntigravityOauth { | ||
| crate::retry::RetrySafety::Idempotent |
There was a problem hiding this comment.
P1: When Antigravity OAuth returns a retryable status, this Idempotent classification can replay the generation POST. Return NonIdempotentPost to prevent duplicate upstream work.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/adapters/gemini/mod.rs, line 550:
<comment>When Antigravity OAuth returns a retryable status, this `Idempotent` classification can replay the generation POST. Return `NonIdempotentPost` to prevent duplicate upstream work.</comment>
<file context>
@@ -398,10 +545,30 @@ async fn forward(
+fn retry_safety_for_auth(auth: AuthMode) -> crate::retry::RetrySafety {
+ if auth == AuthMode::AntigravityOauth {
+ crate::retry::RetrySafety::Idempotent
+ } else {
+ crate::retry::RetrySafety::NonIdempotentPost
</file context>
| .iter() | ||
| .filter(|block| block.get("type").and_then(Value::as_str) == Some("tool_result")) | ||
| { | ||
| let tool_use_id = block |
There was a problem hiding this comment.
P2: When an invalid tool_result supplies a large unknown tool_use_id, this path interpolates the client-controlled string into AdapterError without the 96 KiB cap applied to tool_use.id. Bound result IDs before matching and in the orphan-error path to prevent large duplicate allocations for malformed requests.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/model/gemini_request.rs, line 188:
<comment>When an invalid `tool_result` supplies a large unknown `tool_use_id`, this path interpolates the client-controlled string into `AdapterError` without the 96 KiB cap applied to `tool_use.id`. Bound result IDs before matching and in the orphan-error path to prevent large duplicate allocations for malformed requests.</comment>
<file context>
@@ -112,17 +112,128 @@ fn translate_messages(request: &Value, model: &str) -> Result<Vec<Value>, Adapte
+ .iter()
+ .filter(|block| block.get("type").and_then(Value::as_str) == Some("tool_result"))
+ {
+ let tool_use_id = block
+ .get("tool_use_id")
+ .and_then(Value::as_str)
</file context>
| } | ||
|
|
||
| fn terminal_delimiter_len(buffer: &[u8]) -> Option<usize> { | ||
| [b"\r\n\r\n".as_slice(), b"\n\r\n", b"\r\n\n", b"\n\n"] |
There was a problem hiding this comment.
P2: When Gemini emits the valid SSE CR-only line terminator, an event ends with \r\r, but terminal_delimiter_len never recognizes it. The decoder then reports an unterminated frame instead of translating the response; accept the CR-only delimiter.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/adapters/gemini/sse.rs, line 111:
<comment>When Gemini emits the valid SSE CR-only line terminator, an event ends with `\r\r`, but `terminal_delimiter_len` never recognizes it. The decoder then reports an unterminated frame instead of translating the response; accept the CR-only delimiter.</comment>
<file context>
@@ -0,0 +1,333 @@
+}
+
+fn terminal_delimiter_len(buffer: &[u8]) -> Option<usize> {
+ [b"\r\n\r\n".as_slice(), b"\n\r\n", b"\r\n\n", b"\n\n"]
+ .into_iter()
+ .find(|delimiter| buffer.ends_with(delimiter))
</file context>
| .process_chunk_checked(&semantic_fixture()) | ||
| .is_err()); | ||
|
|
||
| let metadata_parts = vec![json!({"citationMetadata": {}}); 4_096]; |
There was a problem hiding this comment.
P2: This 'bounded' block does not actually exercise the 32 MiB retained-byte ceiling. Each {"citationMetadata": {}} part returns 0 retained bytes (CheckedPart::Metadata, 0), so 4096 of them accumulate ~0 bytes and the following chunk is not rejected for exceeding the byte bound. Assert the ceiling with parts that contribute retained bytes (e.g. text parts totalling near 32 MiB) so the memory-bound claim is really tested.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/gemini_translate.rs, line 571:
<comment>This 'bounded' block does not actually exercise the 32 MiB retained-byte ceiling. Each {"citationMetadata": {}} part returns 0 retained bytes (CheckedPart::Metadata, 0), so 4096 of them accumulate ~0 bytes and the following chunk is not rejected for exceeding the byte bound. Assert the ceiling with parts that contribute retained bytes (e.g. text parts totalling near 32 MiB) so the memory-bound claim is really tested.</comment>
<file context>
@@ -400,3 +420,479 @@ fn test_gemini_sse_machine_non_streaming_accumulation() {
+ .process_chunk_checked(&semantic_fixture())
+ .is_err());
+
+ let metadata_parts = vec![json!({"citationMetadata": {}}); 4_096];
+ let mut bounded = GeminiSseMachine::new("gemini-2.5-pro");
+ bounded
</file context>
| cost, | ||
| )); | ||
| } | ||
| if let Some(text) = part.get("text").and_then(Value::as_str) { |
There was a problem hiding this comment.
P2: When a Gemini part contains text and thoughtSignature, this branch returns before the orphan-signature check, silently dropping the metadata. Move the existing signature guard before the text branch so malformed signature metadata is rejected.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/model/gemini.rs, line 530:
<comment>When a Gemini part contains `text` and `thoughtSignature`, this branch returns before the orphan-signature check, silently dropping the metadata. Move the existing signature guard before the text branch so malformed signature metadata is rejected.</comment>
<file context>
@@ -66,117 +153,432 @@ impl GeminiSseMachine {
+ cost,
+ ));
+ }
+ if let Some(text) = part.get("text").and_then(Value::as_str) {
+ if text.is_empty() {
+ return Ok((CheckedPart::Metadata, 0));
</file context>
| if let Some(text) = part.get("text").and_then(Value::as_str) { | |
| if part.contains_key("thoughtSignature") { | |
| return Err(GeminiSemanticError::protocol( | |
| "Gemini thoughtSignature is not attached to a functionCall", | |
| )); | |
| } | |
| if let Some(text) = part.get("text").and_then(Value::as_str) { |
| } | ||
| let mut frame = std::mem::take(&mut self.buffer); | ||
| frame.truncate(frame_len); | ||
| if self.done { |
There was a problem hiding this comment.
P2: After [DONE], this branch rejects even an empty SSE frame. Because forward defers message_stop until EOF, an extra separator converts a successful stream into an error; parse empty frames before rejecting semantic data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/adapters/gemini/sse.rs, line 64:
<comment>After `[DONE]`, this branch rejects even an empty SSE frame. Because `forward` defers `message_stop` until EOF, an extra separator converts a successful stream into an error; parse empty frames before rejecting semantic data.</comment>
<file context>
@@ -0,0 +1,333 @@
+ }
+ let mut frame = std::mem::take(&mut self.buffer);
+ frame.truncate(frame_len);
+ if self.done {
+ return Err(self.fail("Gemini SSE frame arrived after [DONE]"));
+ }
</file context>
| let signature = decode_tool_use_signature(id)?; | ||
| if model.starts_with(GEMINI_3_MODEL_PREFIX) | ||
| && function_call_index == 0 | ||
| && signature.is_none() | ||
| { | ||
| return Err(bad_request( | ||
| "Gemini 3 tool history requires an authentic thought signature", | ||
| )); | ||
| } |
There was a problem hiding this comment.
P2: Gemini 3 histories with unsigned imported IDs such as toolu_* now fail before dispatch. Add the documented dummy thoughtSignature for the first unsigned call instead of rejecting migrated history.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/model/gemini_request.rs, line 323:
<comment>Gemini 3 histories with unsigned imported IDs such as `toolu_*` now fail before dispatch. Add the documented dummy `thoughtSignature` for the first unsigned call instead of rejecting migrated history.</comment>
<file context>
@@ -179,53 +290,72 @@ fn translate_messages(request: &Value, model: &str) -> Result<Vec<Value>, Adapte
+ "duplicate Gemini tool_use id is ambiguous",
+ ));
+ }
+ let signature = decode_tool_use_signature(id)?;
+ if model.starts_with(GEMINI_3_MODEL_PREFIX)
+ && function_call_index == 0
</file context>
| let signature = decode_tool_use_signature(id)?; | |
| if model.starts_with(GEMINI_3_MODEL_PREFIX) | |
| && function_call_index == 0 | |
| && signature.is_none() | |
| { | |
| return Err(bad_request( | |
| "Gemini 3 tool history requires an authentic thought signature", | |
| )); | |
| } | |
| let signature = decode_tool_use_signature(id)?.or_else(|| { | |
| (model.starts_with(GEMINI_3_MODEL_PREFIX) && function_call_index == 0).then(|| { | |
| "context_engineering_is_the_way to go".to_string() | |
| }) | |
| }); |
1 participant
Summary
Hardens the Gemini adapter across response translation, SSE stream framing, tool-result pairing, and memory bounding.
Problem
[DONE]frames, and embedded provider errors were not strictly validated, risking desynchronized stream states.Solution
GeminiSseMachine(TerminalState,CheckedChunk,CheckedPart) with a 32 MiB memory bound and strict terminal enforcement.BoundedSseFrameBuffer) that preserves multiline data, handles CRLF boundaries, caps event bytes, and disposes state upon errors.thoughtSignatures across turns.[DONE]or cut streams.tests/gemini_translate.rs(28 unit tests) andtests/gemini_conformance.rs(23 integration tests).en,ja,ko,zh-cn).Verification
cargo fmt --all --checkcargo clippy --all-targets -- -D warningscargo test --test gemini_translatecargo test --test gemini_conformancecargo test --lib model::gemini::Summary by cubic
Hardens the Gemini adapter so truncated or malformed streams fail explicitly instead of being synthesized as successful responses, and tool history is validated before dispatch.
Streaming and terminal handling
Tool-call pairing
tool_useIDs must match authentic Code Assist pairing; foreign, malformed, duplicate, or orphaned metadata is rejected before dispatch.thoughtSignatures are preserved across turns and required on tool history; valid results becomefunctionResponsein the next request.Written for commit 355b79c. Summary will update on new commits.