feat: add structured output - #49
Conversation
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesStructured output
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ApiClient
participant Provider
participant ModelAPI
Caller->>ApiClient: request_structured<T>()
ApiClient->>Provider: create_message_with_options(RequestOptions)
Provider->>ModelAPI: Send schema-constrained request
ModelAPI-->>Provider: Provider response
Provider-->>ApiClient: Extract structured JSON
ApiClient-->>Caller: Deserialize T
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/provider/gemini.rs (1)
49-51: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winEnforce the error-body limit while reading the response.
Line 194 calls
resp.bytes().awaitbefore truncation, so an oversized error response is still fully buffered and can exhaust memory. Stream at mostMAX_ERROR_BODY + 1bytes instead.🤖 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 `@src/provider/gemini.rs` around lines 49 - 51, Update the error-response reading logic around resp.bytes().await to stream and collect no more than MAX_ERROR_BODY + 1 bytes, rather than buffering the complete response before truncation. Preserve the existing truncation behavior while enforcing the limit during reading.
🧹 Nitpick comments (1)
src/provider/anthropic.rs (1)
224-294: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDelegate base methods to eliminate code duplication.
stream_messagesandcreate_messagenow share near-identical request-building and HTTP-handling logic with their newly introduced_with_optionsvariants. You can safely remove this duplication by delegating the base methods to the_with_optionsvariants with default options.♻️ Proposed refactor
fn stream_messages( &self, messages: Vec<Message>, system: Option<String>, tools: Option<Vec<ToolSchema>>, ) -> Pin<Box<dyn Stream<Item = Result<StreamEvent, ApiError>> + Send + 'static>> { - let model = self.model.lock().clone(); - let body = build_request_body( - &model, - &messages, - system.as_deref(), - tools.as_deref(), - true, - self.max_tokens, - None, - ); - let url = self.messages_url(); - let api_key = self.api_key.clone(); - let http = self.http.clone(); - - Box::pin(async_stream::try_stream! { - let resp = Self::post_messages(&http, &url, &api_key, &body).await?; - let mut sse = SseReader::from_response(resp); - let mut emitter = StreamEmitter::default(); - - while let Some((event_type, data)) = sse.next_event().await? { - emitter.process_event(&event_type, data); - for ev in emitter.drain() { - yield ev; - } - } - - for ev in emitter.finish() { - yield ev; - } - }) + self.stream_messages_with_options( + messages, + system, + tools, + crate::structured::RequestOptions::new(), + ) } fn create_message( &self, messages: Vec<Message>, system: Option<String>, tools: Option<Vec<ToolSchema>>, ) -> Pin<Box<dyn Future<Output = Result<Value, ApiError>> + Send + '_>> { - let model = self.model.lock().clone(); - let body = build_request_body( - &model, - &messages, - system.as_deref(), - tools.as_deref(), - false, - self.max_tokens, - None, - ); - let url = self.messages_url(); - Box::pin(async move { - let resp = Self::post_messages(&self.http, &url, &self.api_key, &body).await?; - let resp = resp - .bytes() - .await - .map_err(|e| ApiError::http(e.to_string()))?; - if resp.len() > MAX_RESPONSE_BODY { - return Err(ApiError::http(format!( - "response body too large: {} bytes (max {})", - resp.len(), - MAX_RESPONSE_BODY - ))); - } - serde_json::from_slice::<Value>(&resp).map_err(|e| ApiError::http(e.to_string())) - }) + self.create_message_with_options( + messages, + system, + tools, + crate::structured::RequestOptions::new(), + ) }🤖 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 `@src/provider/anthropic.rs` around lines 224 - 294, Update stream_messages and create_message to delegate to their corresponding _with_options variants using the default options, removing the duplicated request-building and HTTP-handling logic while preserving their existing behavior and return 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 `@src/api.rs`:
- Around line 197-225: Update the default implementations of
stream_messages_with_options and create_message_with_options to preserve
delegation only for empty RequestOptions; when options.response_format or any
other option is set, return an explicit unsupported-capability ApiError instead
of discarding the options. Keep provider overrides compatible and retain the
existing behavior for empty options.
In `@src/provider/gemini.rs`:
- Around line 68-71: Update the API-key transport documentation on the Gemini
configuration and related request symbols to state that authentication uses the
x-goog-api-key header, not a key query parameter. Keep the documentation
consistent with the request construction and the existing prohibition on key=
URLs across all noted comment locations.
- Around line 511-517: Update the response format handling in the Gemini request
construction to emit the shared schema under generationConfig.responseJsonSchema
instead of responseSchema, preserving the existing rf.schema value. Update the
matching assertion below to expect responseJsonSchema as well.
In `@src/provider/openai.rs`:
- Around line 590-602: Update OpenAI request serialization in to_json so
structured requests with response_format omit the tools key entirely rather than
emitting it as null. Build or remove tools conditionally while preserving tools
for non-structured requests, and update the relevant test expectation to assert
the key is absent.
In `@src/structured.rs`:
- Around line 309-325: The JSON extraction logic around the delimiter scan must
reset malformed candidates when closing delimiters do not match their
corresponding opening delimiters. Update the state managed by the scanner in
structured extraction to track delimiter types (or immediately clear the
candidate on mismatch), allowing later valid JSON such as {"a":1} to be found,
and add a regression test for the `{oops] then {"a":1}` case.
---
Outside diff comments:
In `@src/provider/gemini.rs`:
- Around line 49-51: Update the error-response reading logic around
resp.bytes().await to stream and collect no more than MAX_ERROR_BODY + 1 bytes,
rather than buffering the complete response before truncation. Preserve the
existing truncation behavior while enforcing the limit during reading.
---
Nitpick comments:
In `@src/provider/anthropic.rs`:
- Around line 224-294: Update stream_messages and create_message to delegate to
their corresponding _with_options variants using the default options, removing
the duplicated request-building and HTTP-handling logic while preserving their
existing behavior and return 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 Plus
Run ID: 561a1fa4-c1ce-4039-bfb4-8381c5b1f8fe
📒 Files selected for processing (8)
CHANGELOG.mdsrc/api.rssrc/lib.rssrc/provider/anthropic.rssrc/provider/gemini.rssrc/provider/openai.rssrc/structured.rssrc/tool.rs
No description provided.