fix: harden OpenAI-compatible parsing with SSE fallback

[jamiepine]

Summary

• add an OpenAI-compatible fallback path that retries with stream: true and parses SSE when the primary non-stream response cannot be decoded as JSON (the worker failure path seen with OpenRouter/Moonshot)
• force accept-encoding: identity on these calls to reduce response-body decode failures from compressed/chunked provider responses
• implement SpacebotModel::stream with a usable streamed wrapper and add coverage for SSE reconstruction (including tool-call deltas) and streaming usage reporting

Testing

• ./scripts/preflight.sh
• ./scripts/gate-pr.sh

[coderabbitai]

Walkthrough

Implemented streaming support for OpenAI-compatible providers in SpacebotModel::stream by constructing live RawStreamingChoice events. Extended RawStreamingResponse with optional usage field. Added SSE parsing utilities and streaming fallback mechanism with enhanced error handling and usage propagation throughout.

Changes

Cohort / File(s)	Summary
Streaming Core Implementation src/llm/model.rs	Implemented SpacebotModel::stream to produce StreamingCompletionResponse from CompletionRequest, constructing live RawStreamingChoice events with MessageId, Message contents, ToolCall details, Reasoning, and FinalResponse carrying usage.
SSE Parsing & Reconstruction src/llm/model.rs	Added OpenAiStreamingToolCall struct and parse_openai_chat_sse_response function to aggregate text, reasoning, tool calls, and usage from SSE data lines into unified OpenAI format; includes helper utilities like with_streaming_enabled, looks_like_sse_response, and format_api_error_from_response_text.
Fallback & Integration Pattern src/llm/model.rs	Introduced call_openai_chat_with_stream_fallback orchestration method and call_openai_compatible_with_optional_auth helper to support streaming fallback on primary request failure; replaced direct JSON-call patterns with streaming-aware builder injecting headers (authorization, account-id, user-agent, extra headers) and optional bearer auth across multiple provider paths.
Response Enhancement & Tests src/llm/model.rs	Extended RawStreamingResponse with optional usage field and updated GetTokenUsage trait; added four test cases covering tool_call reconstruction, SSE data requirements, raw streaming usage reporting, and OpenRouter metadata error formatting.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• fix: harden OpenAI-compatible parsing for OpenRouter responses #304: Strengthens OpenAI-compatible response parsing around tool_call/function_call handling and provider-specific labeling within the same model.rs file.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: hardening OpenAI-compatible parsing with an SSE fallback mechanism, which is the primary focus of the changeset.
Description check	✅ Passed	The description is directly related to the changeset, detailing the SSE fallback path, header handling, SpacebotModel::stream implementation, and test coverage.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/openai-sse-fallback

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/llm/model.rs`:
- Around line 1006-1010: The code builds a primary_failure_reason by embedding
the raw response body (using truncate_body(&primary_text)) and later logs it at
warn level; change this so sensitive payloads are not included in logs: update
the format! that sets primary_failure_reason in src/llm/model.rs to avoid
inserting the response text (instead include non-sensitive metadata like
response length, a fixed redaction marker, or a short hash/checksum), and update
the warn call that logs primary_failure_reason to only emit that sanitized
metadata. Locate the string construction and the warn! that references
primary_failure_reason and replace the embedded body with a
redacted/metadata-only value.
- Around line 1447-1449: The SSE chunk parsing currently swallows JSON parse
errors where the code does serde_json::from_str::<serde_json::Value>(data) and
continues; change this to fail-fast by returning or propagating an Err (or
logging the error with context and breaking the stream) instead of silently
continuing. Similarly, find the reconstructed tool-argument parsing site that
currently falls back to {} on failure (the block that parses the reconstructed
tool args and uses {} on parse error) and replace the silent fallback with
proper error handling: propagate the parse error or return a descriptive Err/log
entry including the raw payload and tool name so a single corrupt chunk cannot
silently mutate arguments.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 636b3743-8654-4a69-aee2-64fb0c0cbadb

📥 Commits

Reviewing files that changed from the base of the PR and between 23929f9 and cf7cfdf.

📒 Files selected for processing (1)

• src/llm/model.rs

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Avoid logging response-body snippets at warn level.

Line 1007 embeds response text into primary_failure_reason, and Line 1015 logs that field at warn. This can leak model/user payload data into centralized logs.

🔧 Proposed fix

-                Some(format!(
-                    "response body was not valid JSON and did not look like SSE. Body: {}",
-                    truncate_body(&primary_text)
-                ))
+                Some("response body was not valid JSON and did not look like SSE".to_string())

Also applies to: 1015-1018

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/llm/model.rs` around lines 1006 - 1010, The code builds a
primary_failure_reason by embedding the raw response body (using
truncate_body(&primary_text)) and later logs it at warn level; change this so
sensitive payloads are not included in logs: update the format! that sets
primary_failure_reason in src/llm/model.rs to avoid inserting the response text
(instead include non-sensitive metadata like response length, a fixed redaction
marker, or a short hash/checksum), and update the warn call that logs
primary_failure_reason to only emit that sanitized metadata. Locate the string
construction and the warn! that references primary_failure_reason and replace
the embedded body with a redacted/metadata-only value.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Fail fast on malformed SSE JSON/tool-argument chunks instead of silently continuing.

Line 1447 currently drops invalid SSE data: payloads, and Line 1606 tolerates malformed reconstructed tool arguments by degrading to {}. A single corrupt chunk can silently mutate a tool call into incorrect arguments.

🔧 Proposed fix

-        let Ok(event_body) = serde_json::from_str::<serde_json::Value>(data) else {
-            continue;
-        };
+        let event_body = serde_json::from_str::<serde_json::Value>(data).map_err(|error| {
+            CompletionError::ProviderError(format!(
+                "{provider_label} streaming response contained invalid JSON event: {error}. Event: {}",
+                truncate_body(data)
+            ))
+        })?;
...
-        let arguments =
-            parse_openai_tool_arguments(&serde_json::Value::String(tool_call.arguments));
+        let arguments = if tool_call.arguments.trim().is_empty() {
+            serde_json::json!({})
+        } else {
+            serde_json::from_str::<serde_json::Value>(&tool_call.arguments).map_err(|error| {
+                CompletionError::ProviderError(format!(
+                    "{provider_label} streaming tool_call arguments were invalid JSON: {error}"
+                ))
+            })?
+        };

As per coding guidelines: "Never silently discard errors. No let _ = on Results. Handle them, log them, or propagate them."

Also applies to: 1605-1606

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/llm/model.rs` around lines 1447 - 1449, The SSE chunk parsing currently
swallows JSON parse errors where the code does
serde_json::from_str::<serde_json::Value>(data) and continues; change this to
fail-fast by returning or propagating an Err (or logging the error with context
and breaking the stream) instead of silently continuing. Similarly, find the
reconstructed tool-argument parsing site that currently falls back to {} on
failure (the block that parses the reconstructed tool args and uses {} on parse
error) and replace the silent fallback with proper error handling: propagate the
parse error or return a descriptive Err/log entry including the raw payload and
tool name so a single corrupt chunk cannot silently mutate arguments.