Open
Conversation
70bb699 to
87e76fb
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves strict-mode handling of Open Responses provider payloads by normalizing/backfilling omitted (schema-required) fields so that otherwise-successful Responses API outputs can round-trip through the strict schema (including in SSE streaming), rather than failing sanitization.
Changes:
- Added JSON
Valuenormalizers to backfill missing fields forResponsesResponseand for Responses SSE streaming events. - Updated strict sanitizers to normalize raw JSON before coercing into typed strict structs (both non-streaming and streaming paths).
- Added tests covering backfill behavior for non-streaming and streaming Responses flows.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/strict/schemas/responses.rs |
Adds normalization helpers to backfill missing strict-schema fields in response payloads and streaming event snapshots. |
src/strict/handlers.rs |
Uses the normalizers in strict response sanitization (JSON->normalize->typed coercion) and adds regression tests for backfilling. |
Comments suppressed due to low confidence (1)
src/strict/handlers.rs:1822
- In streaming sanitization, provider SSE error objects like
{"error": ...}are valid JSON, soserde_json::from_str::<Value>succeeds, butserde_json::from_value::<ResponsesStreamingEvent>then fails and the stream is terminated. This regresses the previous behavior that detected these error objects and forwarded them viatry_format_sse_error(...). Consider reintroducing thetry_format_sse_errorfallback when the typedResponsesStreamingEventcoercion fails (or check forerrorbefore attempting to coerce).
normalize_responses_streaming_event_value(&mut raw_event);
match serde_json::from_value::<ResponsesStreamingEvent>(raw_event) {
Ok(mut event) => {
// Rewrite model on response-level events
if let Some(ref mut response) = event.response {
response.model = original_model.clone();
}
// Re-serialize
match serde_json::to_string(&event) {
Ok(sanitized_json) => {
sanitized_lines.push(format!("data: {}", sanitized_json));
}
Err(e) => {
error!(error = %e, "Failed to serialize responses chunk, terminating stream");
return Err(std::io::Error::other(
"Failed to serialize chunk",
));
}
}
}
Err(e) => {
error!(
error = %e,
data_sample = ?data_part.chars().take(200).collect::<String>(),
"Failed to parse responses SSE data line from provider, terminating stream"
);
return Err(std::io::Error::other(
"Malformed SSE data from provider",
));
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+39
to
+51
| ensure_field(object, "id", || { | ||
| Value::String("resp_placeholder".to_string()) | ||
| }); | ||
| ensure_field(object, "object", || Value::String("response".to_string())); | ||
| ensure_field(object, "created_at", || Value::from(0)); | ||
| ensure_field(object, "completed_at", || Value::Null); | ||
| ensure_field(object, "status", || Value::String("completed".to_string())); | ||
| ensure_field(object, "incomplete_details", || Value::Null); | ||
| ensure_field(object, "model", || Value::String(String::new())); | ||
| ensure_field(object, "previous_response_id", || Value::Null); | ||
| ensure_field(object, "instructions", || Value::Null); | ||
| ensure_field(object, "output", || Value::Array(Vec::new())); | ||
| ensure_field(object, "error", || Value::Null); |
src/strict/schemas/responses.rs
Outdated
Comment on lines
+121
to
+128
| if missing_completed_at | ||
| && matches!( | ||
| event_type.as_str(), | ||
| "response.created" | "response.in_progress" | "response.failed" | ||
| ) | ||
| { | ||
| response_object.insert("completed_at".to_string(), Value::Null); | ||
| } |
87e76fb to
bddafa2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In strict mode, success responses are sanitized by deserializing provider payloads into typed OpenAI schemas and then re-serializing them.
Some downstream providers return valid generations but omit schema-required, non-critical bookkeeping fields. In those cases, strict mode rejects the payload during sanitization even though the
generation itself succeeded and already incurred downstream cost. That turns a usable provider response into a user-visible error.
This failure mode is not limited to
/v1/responses; the same pattern can affect strict chat completions and legacy completions as well.Solution
Add sanitizer-layer normalization before typed deserialization for strict success responses.
This backfills OpenAI-compatible placeholder defaults for omitted non-critical fields so successful generations can still round-trip through the strict schema. The actual generated output is
preserved, and
usageremainsnullwhen the provider did not send it.This now applies to:
/v1/chat/completions/v1/chat/completions/v1/completions/v1/completions/v1/responses/v1/responsesresponse snapshotsScope and Safety
The normalization is intentionally scoped to the sanitizer layer rather than implemented as serde defaults on the schema structs.
Those structs are used as strict schemas in multiple deserialize paths. Struct-level defaults would silently relax all of them, including internal loads and other strict-schema callsites. We only
want this leniency when shaping third-party provider payloads into the public strict-mode response.
Normalization is also gated so it only applies when the payload already resembles the expected endpoint response:
choiceschoicesoutputSSE error objects such as
{"error": ...}are still handled through the existing streaming error path and are not coerced into successful responses.Why not serde defaults?
This is intentionally not implemented with struct-level serde defaults.
ResponsesResponse,ChatCompletionResponse,ChatCompletionChunk,CompletionResponse, andCompletionChunkare used as strict schema types. Adding serde defaults there would silently relaxevery deserialize path for those types, not just provider response sanitization.
Streaming also needs event-aware defaults. In particular, some
/v1/responsessnapshot fields, especiallystatus, depend on the SSE event type such asresponse.createdvsresponse.completed,which is not a good fit for plain struct-level defaults.