Normalize completion responses at the provider boundary by gold-silver-copper · Pull Request #2256 · 0xPlaygrounds/rig

gold-silver-copper · 2026-08-04T10:45:03Z

Summary

This normalizes completion responses at the provider boundary so that
CompletionModel describes only what a model does, not what shape its
provider happens to answer in.

CompletionModel loses Response, StreamingResponse, Client, and make.
CompletionResponse and StreamingCompletionResponse are concrete.
The normalized response carries what callers actually reached into
raw_response for: finish_reason, provider, the provider-reported
model, and message_id.
Provider-native payloads stay reachable, and stay typed, through each model's
inherent raw_completion / raw_stream.
Model construction moves to the required CompletionClient::completion_model.
ProviderCapabilities replaces composes_native_output_with_tools with plain
data a runtime can snapshot.

Breaking API changes are intentional. This is the prerequisite for #2252 — the
associated types are exactly what prevents Box<dyn CompletionModel>, so they
have to go before an erased ModelHandle is expressible. It does not modify
#2252 or introduce any runtime model-selection machinery.

This is an independent implementation of the same prerequisite as #2254, not a
revision of it.

Core API shape

pub trait CompletionModel: Clone + WasmCompatSend + WasmCompatSync {
    fn completion(&self, request: CompletionRequest)
        -> impl Future<Output = Result<CompletionResponse, CompletionError>> + WasmCompatSend;

    fn stream(&self, request: CompletionRequest)
        -> impl Future<Output = Result<StreamingCompletionResponse, CompletionError>> + WasmCompatSend;

    fn completion_request(&self, prompt: impl Into<Message>) -> CompletionRequestBuilder<Self> { .. }

    fn capabilities(&self) -> ProviderCapabilities { ProviderCapabilities::default() }
}

pub trait CompletionClient {
    type CompletionModel: CompletionModel;
    fn completion_model(&self, model: impl Into<String>) -> Self::CompletionModel;
}

CompletionModel no longer drags in a client type, so a model with no client at
all is now expressible — covered by a test in client/completion.rs.

Three details that are load-bearing

The Stop → ToolCalls reconciliation lives in exactly one place.
Several OpenAI-compatible gateways report a bare stop on a turn that carried
tool calls. FinishReason::reconcile_with_output is the single implementation,
and it is applied by CompletionResponse::with_finish_reason /
with_optional_finish_reason on the unary path and by normalize_stream — using
the tool calls actually observed — on the streaming path. The Option setter
exists precisely so a provider holding an Option<FinishReason> never has to
choose between ergonomics and correctness by assigning the field directly. Only
a natural Stop is upgraded; Length, ContentFilter, and Other describe
terminations that remain true regardless of content.

A conversion over a shared wire type takes the provider name as an input.
NormalizeCompletionResponse::normalize(self, provider) is implemented by the
OpenAI chat-completions payload that ~16 providers share. Hardcoding "openai"
there would mislabel every one of them, and a "placeholder the caller overwrites
afterwards" would be correct only by convention. Threading the name through the
conversion makes it impossible to forget.

This is a trait rather than TryFrom<(&str, T)> for a concrete reason found
during review: a tuple is not a local type, so impl TryFrom<(&str, TheirType)> for CompletionResponse is rejected by the orphan rule in every crate except
rig-core. The tuple form would have made external provider extensions
impossible to write. Implementing the trait on a provider's own response type is
allowed anywhere, which is verified by the four companion crates plus a probe.

provider is known when a stream opens, not when it terminates.
StreamingCompletionResponse::stream takes the descriptor name as its first
argument, so a stream that errors or is cancelled before its terminal record
still names its provider. Every other missing value has a documented sentinel
(Usage::new(), None); this one should not degrade to an empty string.

Native-response escape hatches

Both raw_completion and raw_stream are implemented for every
completion-capable provider model:

	Provider
rig-core	Anthropic (+ the anthropic-compatible family), OpenAI Chat Completions (`GenericCompletionModel<Ext, H>`, shared by Azure, DeepSeek, Doubleword, Groq, Hugging Face, Hyperbolic, Llamafile, MiniMax, Mira, Mistral, Moonshot, OpenRouter, Perplexity, Together, Xiaomi MiMo, Z.AI), OpenAI Responses, ChatGPT, Copilot (both routes), Gemini REST, Gemini Interactions, Cohere, Ollama, xAI
companion crates	Bedrock, Candle, Gemini gRPC, Vertex AI

raw_completion returns the provider's own unary type. raw_stream returns
RawStreamingResult<Native>, whose terminal item is the provider's own type —
the normalized path maps it once with normalize_stream. The normalized method
calls the raw one, so there is exactly one network request either way, and both
share the request builder, transport, parser, telemetry and error-preservation
path.

Finish-reason normalization

Provider family	Native reason	Normalized
OpenAI-compatible chat / Copilot chat	`stop`; `length` \| `max_tokens`; `tool_calls` \| `function_call`; `content_filter`	`Stop`; `Length`; `ToolCalls`; `ContentFilter`
OpenAI Responses / ChatGPT / Copilot Responses / xAI	`completed`; `incomplete` + `max_output_tokens`; `incomplete` + `content_filter`; `failed` \| `cancelled`	`Stop`; `Length`; `ContentFilter`; `Other(status)`
Anthropic	`end_turn` \| `stop_sequence`; `max_tokens`; `tool_use`; `refusal`	`Stop`; `Length`; `ToolCalls`; `ContentFilter`
Cohere	`COMPLETE` \| `STOP_SEQUENCE`; `MAX_TOKENS`; `TOOL_CALL`; `ERROR`	`Stop`; `Length`; `ToolCalls`; `Other("ERROR")`
Gemini REST / gRPC / Vertex	`STOP`; `MAX_TOKENS`; `SAFETY` \| `BLOCKLIST` \| `PROHIBITED_CONTENT` \| `SPII`	`Stop`; `Length`; `ContentFilter`
Gemini Interactions	`COMPLETED`; `REQUIRES_ACTION`; `BUDGET_EXCEEDED`	`Stop`; `ToolCalls`; `Length`
Ollama	`stop`; `length`	`Stop`; `Length`
Bedrock	`end_turn` \| `stop_sequence`; `max_tokens`; `tool_use`; `content_filtered` \| `guardrail_intervened`	`Stop`; `Length`; `ToolCalls`; `ContentFilter`
Candle (local)	EOS token sampled; max tokens	`Stop`; `Length`

Unknown values are preserved verbatim in Other(String), in the provider's own
spelling — Gemini's RECITATION stays RECITATION, Ollama's operational
load/unload stay themselves. Nothing is smoothed into Stop, and no variant
is stringified through Debug. None means the provider genuinely reported no
reason (an in-flight Responses status, Gemini's FINISH_REASON_UNSPECIFIED).

Where reconciliation applies, a Stop on a tool-calling turn becomes
ToolCalls — consistently on both the unary and streaming paths.

Notes on the design

Construction hook. Rust's coherence rules rule out one blanket
CompletionClient impl per provider family (they would all overlap on
Client<Ext, H>), and a public bound such as From<(Client<Ext, H>, String)>
would push a synthetic conversion into every provider model's public API. The
blanket impl instead uses a pub(crate) trait ConstructCompletionModel, which
constrains only rig's own generic client. Provider crates outside rig-core
implement CompletionClient directly and never see it.

Surviving associated types. OpenAICompatibleProvider::{Response, StreamingUsage} and CompatibleStreamProfile::{Usage, Detail, FinalResponse}
remain, because they name genuinely provider-native wire types at the provider's
own boundary. They do not appear in CompletionModel, the agent, or any
ordinary streaming type.

One known limitation. ChatGPT's /responses answers with an SSE body even
for a non-streaming request, and its terminal event sometimes carries an empty
output; the assistant content then exists only in the preceding events, which
CompletionModel::completion reassembles. So for that provider there is no
public single-request way to obtain both the raw wire response and the
normalized one — raw_stream is the full-fidelity escape hatch. This is
documented on ResponsesCompletionModel::raw_completion, and it cost one
assertion in codex_behaviors::store_false_and_prompt_cache_fields_roundtrip
(the namesake store/prompt_cache_key assertions are preserved; the assistant
text assertion it also carried is covered by codex_sessions).

Independent review

A four-lens adversarial review ran over the complete diff (correctness,
raw/normalized parity, streaming behavior, API leakage), with every finding
re-checked by a separate agent instructed to refute it. 38 findings were raised;
14 were refuted and 24 confirmed. All 24 are fixed in this branch. The ones worth
knowing about:

The provider-name conversion originally used TryFrom<(&str, T)>, which the
orphan rule makes unimplementable outside rig-core — it would have silently
blocked external provider extensions. Now a trait (see above).
normalize_stream counted tool-call deltas toward the Stop → ToolCalls
upgrade while the unary path counts only completed calls, so a stream whose
tool call failed to assemble reported ToolCalls over a choice with none. A
verifier reproduced this with a real SSE payload. Both paths now count only
completed calls.
send_compatible_streaming_request is public and stamped a hardcoded
"openai" on every stream — the same placeholder hazard this change removes
elsewhere. It now takes the descriptor name.
Gemini populated message_id on the unary path but not the streaming one.
Both do now.

Correction (superseding an earlier version of this description). An
earlier revision of this text claimed that request conversion drops
Message::Assistant's id, and used that to justify preserving
response-scoped ids in message_id. That claim was wrong. It was based on
checking only the chat-completions converter (id: None); the Responses API
converter does the opposite and serializes that id onto the wire as an
assistant output-message id
(providers/openai/responses_api/mod.rs:583-612). A follow-up review caught
it. See "Known issues under active fix" below.

Two findings were deliberately not changed, because they are pre-existing at
the merge base and fixing them here would be unrelated behavior changes:
bedrock's MaxTokens early yield, and Z.AI reporting "zai" on its OpenAI route
versus "z.ai" on its Anthropic route (both already fed
gen_ai.provider.name).

Known issues under active fix

A follow-up review found three P1s in this branch. All three are verified
against the code, and fixes are in progress on this branch — do not merge until
they land.

Response-scoped ids are replayed as assistant-message ids. OpenAI Chat's
chatcmpl-… reaches CompletionResponse::message_id
(openai/completion/mod.rs:1220), is promoted into Message::Assistant { id }
(rig-agent/src/agent/run/mod.rs:540), and is serialized by the Responses
converter as an assistant output-message id
(openai/responses_api/mod.rs:583-612). This gets worse under refactor(agent): support hook-driven runtime model routing #2252, where
switching provider between turns is the point. Fix: split response_id from
message_id, so only genuine assistant-message ids are replayable.
The construction hook is pub(crate). The blanket
impl CompletionClient for Client<Ext, H> requires
ConstructCompletionModel, which downstream crates cannot name; and
impl CompletionClient for Client<TheirExt, H> is E0117 (verified with a
probe). So an extension riding rig's generic client is locked out — the same
class of break this PR fixed for the normalization trait. Fix: make it public
and documented, with downstream compile coverage.
Truncated streams still synthesize successful terminal records. The
terminated_with_error / stream_failed flags cover explicit errors but not
truncation, so anthropic, cohere, Gemini REST and Gemini gRPC all yield a
terminal record at clean EOF; gRPC fabricates one outright
(final_resp.or(last_resp).unwrap_or_default()). Anthropic's malformed-SSE
branch also yields an error without setting the flag or breaking, so one
stream can emit both an error and a success record — contradicting an
invariant stated above. Fix: guard each stream on the provider's genuine
terminal event, with a regression test per provider.

Verification

cargo fmt --all -- --check
cargo clippy --workspace --all-targets --all-features — clean
cargo test --workspace --all-features -- --test-threads=1
RUSTDOCFLAGS='-D warnings' cargo doc -p rig-core -p rig-agent -p rig --no-deps
cargo check --target wasm32-unknown-unknown -p rig-core -p rig-agent -p rig
cargo check --workspace --all-targets --all-features — zero errors, zero warnings

git diff <merge-base>..HEAD -- tests/cassettes is empty. Request
construction did not change, so no fixture was re-recorded and no request
matcher moved.

… boundary Remove the response and construction associated types from `CompletionModel` so the trait describes only what a model *does*, and make the ordinary completion and streaming types concrete. `CompletionResponse` loses `raw_response` and gains the metadata callers actually reached into it for: a normalized `finish_reason`, the stable `provider` descriptor name, the provider-reported `model`, and `message_id`. Provider-native payloads stay reachable through each model's inherent `raw_completion`/`raw_stream`, which share one request, transport, parser, telemetry path, and error path with their normalized counterparts. Three details are load-bearing: - The `Stop` -> `ToolCalls` reconciliation lives in exactly one place (`FinishReason::reconcile_with_output`) and is applied by both the unary setters and `normalize_stream`. Several OpenAI-compatible gateways report a bare `stop` on a tool-calling turn, so a caller branching on `ToolCalls` would otherwise miss the call on the unary path while the streaming path caught it. - Conversions over a wire type shared by many providers take the descriptor name as an input rather than hardcoding one, so a shared shape cannot mislabel the provider that used it. - `StreamingCompletionResponse` records its provider when the stream is opened, so a stream that errors or truncates before its terminal record still reports one. Construction moves to the required `CompletionClient::completion_model`; `ProviderCapabilities` replaces `composes_native_output_with_tools` with plain data a runtime can snapshot. Request construction is unchanged — no cassette fixture differs.

…dary Assertions that read normalized metadata now read the fields that replaced `raw_response` (`model`, `finish_reason`, `provider`, `message_id`). Raw-vs-normalized parity tests keep their intent by taking the provider's own response from the model's inherent `raw_completion` and normalizing that same value, rather than issuing a second request — each cassette holds exactly one interaction and fails on both unmatched extra requests and unconsumed ones. Tests that read genuinely provider-specific stream payloads (reasoning metadata, local generation counters) move to `raw_stream`, which is what that escape hatch is for. No cassette fixture changed.

/responses answers with an SSE body even for a non-streaming request, so the value raw_completion returns is reassembled from the terminal event — and that event sometimes carries an empty output, with the content living only in the preceding events. Point callers who need full provider fidelity at raw_stream rather than letting them discover this from an empty field.

…dary An adversarial review of the full diff raised 38 findings; 24 survived refutation. The load-bearing ones: Normalization was stated as `TryFrom<(&str, Response)>`, which the orphan rule makes unimplementable outside rig-core — a tuple is not a local type, so no other crate could satisfy the bound. That silently blocked external provider extensions, which are explicitly in scope. It is now a `NormalizeCompletionResponse` trait, implementable on any provider's own response type anywhere. `normalize_stream` counted tool-call *deltas* toward the Stop -> ToolCalls upgrade while the unary path counts only completed calls, so a stream whose tool call never assembled reported `ToolCalls` over a choice containing none — the exact divergence the single-implementation rule is supposed to prevent. Both paths now count only completed calls. `send_compatible_streaming_request` is public and stamped a hardcoded "openai" onto every stream it produced, reintroducing in a public helper the same placeholder hazard this change removes elsewhere. It takes the descriptor name now. Vertex AI built `FinishReason::Other` by `Debug`-formatting the SDK enum, which turned MALFORMED_FUNCTION_CALL into MALFORMEDFUNCTIONCALL. It uses the SDK's own `name()`. Also reverted two behavior changes that had crept in and do not belong to a normalization change: `message_id` was being fed response-scoped ids (`chatcmpl-`, Gemini `responseId`) on five paths even though the agent replays that field into assistant history, and Cohere's usage silently moved from `billed_units` to `tokens`. Both are back to the pre-existing semantics. Three confirmed findings are deliberately untouched because they are identical at the merge base: anthropic's SSE parse-failure guard, bedrock's MaxTokens early yield, and Z.AI's two descriptor names.

Both were cases of acting on a finding's mechanism without first checking whether it described a reachable harm. The test suite caught both. `message_id` was stripped from five providers on the claim that a response-scoped id would corrupt agent history, since the agent writes it into `Message::Assistant { id }`. That data path is real, but every provider's request conversion drops the id before it reaches the wire (see providers/openai/completion/mod.rs, `id: None`), so nothing is replayed. Stripping it discarded identifiers this change is meant to preserve. They are restored, and the field now documents what it actually holds — providers differ on whether it names the message or the response — and that it is not echoed back. Cohere's usage source had been changed to prefer `tokens`; reverting to `billed_units` kept every existing number stable but left rig reporting zero usage — the documented "no metrics" sentinel — for a response carrying `tokens` and no `billed_units`. Neither was right. `billed_units` stays primary so no caller's numbers move, with `tokens` as a fallback only where rig previously reported nothing. Gemini's Interactions API keeps its continuation handle reachable through `raw_completion`, which is where the pre-refactor tests read it from (`raw_response.id`); it is a `previous_interaction_id` handle, not an assistant message, so the escape hatch is its proper home.

gold-silver-copper added 5 commits August 4, 2026 02:22

gold-silver-copper mentioned this pull request Aug 4, 2026

Normalize completion responses at the provider boundary and erase the model type at agent construction #2257

Merged

gold-silver-copper closed this Aug 10, 2026

Navigation Menu

Uh oh!

Normalize completion responses at the provider boundary - #2256

Normalize completion responses at the provider boundary#2256
gold-silver-copper wants to merge 5 commits into
mainfrom
refactor/normalize-completion-boundary

gold-silver-copper commented Aug 4, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

gold-silver-copper commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Core API shape

Three details that are load-bearing

Native-response escape hatches

Finish-reason normalization

Notes on the design

Independent review

Known issues under active fix

Verification

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

gold-silver-copper commented Aug 4, 2026 •

edited

Loading