0xPlaygrounds / 0xPlaygrounds/rig

Responses decoder loses token usage when serde_json/arbitrary_precision is enabled (flattened top_p)

Aperta
#2,493 1 commento 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Rust
Stelle
8.6k
Fork
959
Merge medio
4h 32m
PR unite (30g)
117

Descrizione

## Problem

With `serde_json/arbitrary_precision` enabled anywhere in the dependency graph,
Rig's OpenAI Responses decoder rejects every `response.*` SSE frame that echoes
a numeric `top_p`. On the streaming path Rig 0.41.0 logs and **skips** such a
frame:

```text
DEBUG Couldn't deserialize SSE data as StreamingCompletionChunk:
Error("data did not match any variant of untagged enum StreamingCompletionChunk")
```

Because the terminal `response.completed` frame is skipped too, the stream ends
with `Usage::new()` (the zero sentinel) even though the provider did report
usage, including cached input tokens. Callers that record usage (context
accounting, cache-hit reporting) silently see "provider reported no usage".

The inner error, obtained by decoding the frame manually, is:

```text
ResponseChunk error: Error("invalid type: map, expected f64", line: 1, column: 1263)
```

Observed against MiniMax's OpenAI-compatible Responses endpoint
(`https://api.minimaxi.com/v1/responses`, model `MiniMax-M3`), whose completed
frame looks like this (abridged):

```json
{"type":"response.completed","sequence_number":8,"response":{"id":"resp_1","object":"response","created_at":1789235850,"model":"MiniMax-M3","status":"completed","output":[{"type":"message","id":"msg_1","status":"completed","role":"assistant","content":[{"type":"output_text","text":"ok","annotations":null}]}],"usage":{"input_tokens":7173,"input_tokens_details":{"cached_tokens":7168},"output_tokens":2,"total_tokens":7175},"top_p":0.95,"truncation":"disabled"}}
```

## Cause

`responses_api::CompletionResponseWire` flattens the provider metadata:

```rust
#[serde(flatten)]
additional_parameters: AdditionalParameters,
```

and `AdditionalParameters` models `top_p: Option`. `#[serde(flatten)]`
routes the payload through Serde's content buffer. Under
`serde_json/arbitrary_precision` a buffered JSON number is represented as an
internal map, so the `f64` field fails with `invalid type: map, expected f64`
and the whole `CompletionResponse` fails with it.

In our build `arbitrary_precision` is enabled by `starlark` (through a pinned
Codex helper crate), so we cannot turn it off without dropping that
integration. Note that enabling the feature is not exotic: any dependency that
needs lossless JSON numbers turns it on for the entire binary.

On `main`/0.42.0 the same frame is classified as a *known* event that failed to
decode and is surfaced as a stream error, so the turn fails instead of merely
losing its usage. Both outcomes break MiniMax-style Responses endpoints.

## Reproduction

```rust
// Cargo.toml: serde_json = { version = "1", features = ["arbitrary_precision"] }
let frame = r#"{"type":"response.completed","sequence_number":1,"response":
{"id":"resp_1","object":"response","created_at":1,"status":"completed",
"model":"m","output":[],"tools":[],"top_p":0.95}}"#;

// fails with: data did not match any variant of untagged enum StreamingCompletionChunk
let _ = serde_json::from_str::(frame);
// inner error: invalid type: map, expected f64
let _ = serde_json::from_str::(frame);
```

Removing `top_p` from the payload, or setting it to `null`, makes both decodes
succeed. A non-flattened numeric field (`created_at`) is unaffected.

## Suggested fix

Decode the flattened numeric metadata tolerantly rather than failing the whole
response on it. Options:

- Give the flattened numeric fields (currently `AdditionalParameters::top_p`) a
deserializer that accepts the `arbitrary_precision` number representation and
falls back to `None` for shapes it cannot read.
- Or stop flattening `AdditionalParameters` for response decoding, so its
fields are decoded by the ordinary `serde_json` number path.
- Independently, consider not discarding a whole frame because one optional
metadata field disagrees with its Rust type; keeping the raw value would let
usage survive.

We work around it today by normalizing the provider's `data:` payloads in our
HTTP client before Rig decodes them, but this belongs upstream. Happy to open a
PR if you have a preferred approach.

Related to #2483, which reports object-shaped `top_p` from the same provider
hitting the same field (that one fails regardless of `arbitrary_precision`).

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.