refactor!: assistant content is tagged and provider extras are a named field - #2277
Merged
Conversation
…d field
AssistantContent gains #[serde(tag = "type", rename_all = "lowercase")],
the same scheme UserContent always had — rig was the only framework of
seven surveyed (itself included, on the user side) whose content parts
were not discriminated. The tag is required: there is no fallback to the
tagless shape, which was never in a release, and a test pins the
rejection so removing the requirement is a decision, not an accident.
The five content-block flattens (Text, Image, Audio, Video, Document)
become named additional_params fields with deny_unknown_fields. Two
defect classes die at the root: a stray key can no longer be silently
captured and replayed to providers (the round-2 recipe bug), and an
absent field round-trips as None instead of the flatten's Some({})
artifact — so is_empty_assistant_turn drops the has_no_annotations shim
that papered over restored runs classifying differently from live ones,
and the round-trip test now asserts None outright.
The one wire type that borrowed the rig-level Text — the OpenAI
Responses output_text arm — gets its own OutputText wire struct,
preserving annotations and future sibling keys verbatim for value-equal
replay. Wire passthrough belongs to wire types; the domain type no
longer carries it.
Zero cassettes change: providers serialize their own wire types into
requests, and the full recorded suite passing under the new tags is the
proof, not an assumption.
0.41 shipped the untagged AssistantContent, so the claim that the bare
shape 'was never in a release' was false in four places — MIGRATING, the
changelog, the enum doc, and a test comment. All four now say what is
true: 0.41-persisted assistant content does not load, and MIGRATING
carries the tag-insertion recipe per variant. The same honesty for the
other two released-data casualties: 0.41 user-content blocks with
flattened extras (anthropic document title/citations) fail under
deny_unknown_fields — re-nest them under additional_params, recipe
included — and a 0.41-serialized stream item with flattened text extras
decodes as StreamedAssistantContent::Unknown and is dropped from
assembly rather than erroring, which is now documented instead of
implied loud. The deny guarantee is scoped to the five block structs,
with ToolCall/Reasoning's deliberate tolerance named.
OutputText.extras adopts the file's own artifact-free idiom
(flatten + default + skip_serializing_if = Map::is_empty), so a decoded
bare block equals a request-assembled one — the Some({}) inequality this
PR deletes elsewhere no longer sneaks back in through the wire type. The
streaming parts merge drops empty-object metadata alongside null,
making the normalization boundary explicit for out-of-tree providers.
The round-trip test preamble stops describing the deleted flatten
behavior, and the resume-identity test compares messages directly now
that the serialized-form detour has nothing to normalize away.
…he docs diagnose instead of misdirect
message::non_empty_params is the one home for the 'null and {} mean no
extras' rule: the five content-block deserializers route through it
(deserialize_with), so a mechanically migrated "additional_params": {}
— the shape MIGRATING's own re-nesting recipe invites — classifies
exactly like an absent field, and Some(null)'s round-trip flip is gone.
The streaming accumulation guard and anthropic's document-params builder
call the same predicate instead of hand-rolling their thirds of it, and
the round-trip test pins both empty spellings decoding to None.
MIGRATING quotes the error 0.41 data actually produces — missing field
type, the internally-tagged wording — instead of an untagged-enum
message that greps to nothing. The re-nesting recipe covers tool-result
nested blocks (ToolResultContent reuses the strict structs), and the
streaming section says the Unknown fall-through is an ongoing property
of stray sibling keys, not a 0.41-only hazard. The one silent spot gets
loud: the assembler logs a warning when an excluded Unknown payload
carries a text key, since that exclusion loses transcript content.
…e promises match the code
params_carry_data is the read-side spelling of the one rule (None, null,
and {} all mean no extras); the classifier judges annotation through it,
so an uncanonicalized in-memory Some({}) — constructible out-of-tree,
the fields are public — classifies exactly like None. The serialize side
closes too: skip_serializing_if routes through the same rule, so empty
params never reach the wire and live/restored classification agrees for
every value, not just the canonical one. Both directions are pinned in
rig-core next to the contract, and the rig-agent round-trip test covers
the in-memory case.
The assistant migration recipe gains the re-nesting step the tag alone
does not cover (0.41 flattened anthropic citations onto assistant text
as top-level siblings). The excluded-text warning fires at both
exclusion points — the streaming normalizer and the agent assembler —
with the condition narrowed to text-with-no-type (provider-native
unmodeled items always carry their wire tag, so the diagnosis is no
longer hard-coded onto them), and MIGRATING scopes the promise to rig's
assembly points instead of implying universality. The {}-TextStart
ordering change is disclosed in the changelog.
The poll_next warn is deleted: everything reaching that arm is a live
wire frame a provider adapter chose not to model — adapters warn about
those themselves, and gemini interactions frames discriminate on
event_type, so the 'always carries a type tag' premise was false there —
while a persisted item that failed the strict Text decode is created by
consumer-side serde and never re-enters the live stream. That also
un-duplicates the byte-identical warn the agent path emitted twice per
item. The agent assembler, the one rig component that ingests replayed
events, keeps the warning, and MIGRATING/CHANGELOG scope the promise to
it instead of claiming coverage at points that cannot see the hazard.
ToolOutput::as_text judges annotation through params_carry_data like
every other reader, so a publicly constructed Some({}) no longer makes
the read side call a block annotated while the write side serializes it
bare. And the changelog entry gets its missing sentence break.
gold-silver-copper
force-pushed
the
refactor/tagged-assistant-content
branch
from
August 10, 2026 17:13
f9ae7b5 to
26ac17c
Compare
…all speak params_carry_data
- The Unknown-arm warn now fires for tagged {"type":"text"} items too (the
exact shape MIGRATING teaches), extracted as unknown_payload_loses_text and
pinned by tests; its wording no longer misattributes tagless unmodeled frames.
- assistant_text_items_from_choice judges annotation by params_carry_data, not
is_some(), restoring live/restored round-trip agreement for Some({}).
- MIGRATING covers flattened extras on assistant image blocks (openrouter) and
the widened warn condition.
- non_empty_params doc states the real reader contract (params_carry_data)
instead of promising plain is_none().
…cts or errors - The lost-content heuristic (now unknown_payload_loses_assistant_content) also fires for rig's own toolcall/reasoning/image tags — a replayed tagged assistant block is not a stream-item shape, and a silently dropped tool call desyncs the turn. - additional_params must decode as an object or null: a mis-migrated [] or bare string is a loud decode error (deny_unknown_fields doctrine), not a phantom annotation no extractor can read. MIGRATING says so. - The two anthropic citation guards route through message::non_empty instead of hand-rolling empty-means-None. - The agent round-trip test pins classification only, citing rig-core's empty_params_canonicalize_to_none_in_both_serde_directions for the serde mechanics.
…wn keys tolerate instead of reject The four-helper convention family (non_empty_params / params_carry_data / params_carry_no_data / params_as_none_when_empty) is replaced by message::AdditionalParams — a newtype that is a non-empty JSON object by construction. Some always carries data (plain is_none/is_some everywhere), non-objects are unrepresentable in memory so serialization can never emit what deserialization rejects, and the wire shape is unchanged (transparent). Grep-proofs: the helper family has zero references; every remaining additional_params.is_some()/is_none() is on the newtype or a ToolCall/request-level Option<Value> outside the invariant. deny_unknown_fields drops from the five content blocks (audit: the only remaining uses are rig-candle protocol/config structs, the legitimate kind). The doctrine, stated on AdditionalParams: known field wrong shape -> loud; unknown key -> ignored; unknown tag -> loud. Zero of nine surveyed frameworks forbid unknown fields on durable conversation types; vercel shipped that and logged four our-validator-rejected-what-we-wrote regrets. Fallout: stray-keyed and tagged text stream items now decode as stream text (text assembled, stray keys dropped) instead of vanishing into Unknown. Migration loudness moves to the opt-in boundary: message::keys_lost_in_round_trip is the recipe MIGRATING now pins. The assembler counts excluded replayed blocks (tagged toolcall/reasoning/ image, judged by the AssistantContent decoder itself, not a mirrored tag list) and warns once per turn; excluded_assistant_content exposes the count. OpenAI Responses output_text extras (annotations with data) now survive into history under additional_params["openai_responses"] and replay only through the Responses serializer — cassettes pass unchanged, proving the wire bytes didn't move. Enum-wide additional_params() accessors (or-pattern collapse, non-carrying variants named) and exhaustive destructures at the touched conversion boundaries round out the discipline.
…ed extras cannot shadow the wire
- keys_lost_in_round_trip no longer reports the blessed
"additional_params": {} spelling as a loss — a missing key whose original
the loader canonicalizes to absence (empty object; null was already
skipped) is not a loss, so a correctly migrated history passes its own
verification.
- OutputText::from_message_text filters the reserved text/type keys before
the serde flatten (a duplicate JSON key would let persisted history data
shadow the block's real text or tag on the wire — probe-verified
last-wins hazard) and warns instead of silently dropping a non-object
openai_responses value. Annotation-only empty text blocks now survive
request assembly at both sites: only a bare empty block is skipped.
- The per-turn lost-content warning moves into Drop, so it survives every
termination path — finish, stream errors, hook cancellation, abandonment —
not just the happy path (finish now mem::takes its fields).
- unknown_payload_loses_assistant_content probes via &Value as Deserializer:
no per-item deep clone on the Unknown path.
- The null/{}/object canonicalization policy has one home: both serde
routes and the test fixture delegate to try_from_value, and the direct
Deserialize impl's error text no longer promises "(or null)" while
rejecting null. AdditionalParams::merge delegates top level and nested
maps to one merge_maps routine.
…ks one gate Contract completions, not scenario patches — with the red-green proof the matrix has teeth: decode_outcome_matrix_is_total_and_no_shape_is_silent failed on exactly the MalformedParamsText cell (counted 0, mandated 1) before the predicate fix, and on nothing else. The matrix is enum-driven (wildcard-free expected() so a new shape class cannot compile without a mandated outcome; coverage and vacuity asserts so it cannot pass vacuously), and it surfaced one more truth while being written: a provider-native frame carrying a string text key is assembled — the documented tolerance tradeoff — now its own named row. The replay gate is one primitive: AdditionalParams::wire_extras / into_wire_extras (capture unconditional at ingest, replay only the key a wire owns, never a hard error at serialization time). Both Responses empty-text guards, from_message_text (now by value — no per-call extras clone), and openrouter's response-image reader all go through it; the guards also encode deliverability — the id-less AssistantInput form cannot carry extras, so an annotated empty block replays only when the id-carrying form is available, and a foreign-annotated one produces no item at all (test pins both, plus the wire bytes of the delivered extras). Anthropic's document params are top-level keys, not wire-namespaced, and deliberately stay on get. The per-turn lost-content warning lives on a one-field ExclusionCount guard's Drop — every termination path, once, zero stays silent — and the assembler no longer implements Drop, so finish moves its fields freely again. MIGRATING's verification snippet is now the compiled doc example on keys_lost_in_round_trip (doctest), so the recipe and the behavior cannot drift. fixture_additional_params delegates to try_from_value. The zero-caller enum accessors are deleted: one honest call-site fit was found, below the keep bar of two. Grep-proofs: no additional_params.is_none() in responses_api (both guards go through wire_extras); 4 non-test wire_extras/into_wire_extras callers across 2 providers; no impl Drop for StreamedTurnAssembler; the finish mem::take contortion is gone (two remaining takes are pre-existing reasoning/delta drains); no fn additional_params(&self) on the content enums.
gold-silver-copper
force-pushed
the
refactor/tagged-assistant-content
branch
from
August 10, 2026 23:49
6939469 to
ce1b81c
Compare
… the rule has one home The round-9 review's four confirmed corridors, closed as two contract completions: - assistant_text_replay_message is the one home for the Responses assistant-text replay rule — deliverability gate, empty-block skip, both message forms — shared by both request conversions (they were duplicated token-for-token 2400 lines apart). Loudness moves into it: a malformed value under the wire's key warns even when the block is skipped (previously unreachable for empty blocks), and own-wire extras stranded on the id-less form warn as they drop (previously silent). from_message_text is now a silent gate+filter with one production caller. - The empty-block rule extends to the Anthropic serializer: an empty text block with no anthropic-deliverable content (foreign-annotated blocks — the shape Responses ingest newly mints — included) produces no Content at all instead of an empty text block the API 400s on; a message left with no blocks fails loudly and locally at the existing non-empty check. Test pins block-level skip and whole-message survival of siblings. The streaming capture gap is a named limitation, not an overclaim: the extras-key doc and CHANGELOG now say capture happens on the blocking response path and the streaming adapter does not yet route annotation events into params (follow-up work; nothing is silently dropped at replay because nothing was captured). The logprobs-replay candidate stays open as an accepted edge pending live-API confirmation.
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
1 participant
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.
Follow-up to #2273/#2276, executing the root fix for the serde bug class both hit:
AssistantContentwas#[serde(untagged)]with provider extras carried by a#[serde(flatten)]— a design 0 of 6 surveyed reference frameworks share (all six tag their content parts; none flatten vendor extras into the part's own key namespace), and one rig was internally inconsistent about, sinceUserContenthas always been tagged.No legacy or backwards-compatibility shims — maintainer direction. The tagged form is the only form. This breaks released data: 0.41 serialized assistant content untagged (and wrote block extras flattened), so 0.41-persisted histories/runs do not load — MIGRATING carries per-variant tag-insertion and extras-re-nesting recipes instead of a decode fallback. The no-fallback decision is pinned by test.
What changed
AssistantContentis#[serde(tag = "type", rename_all = "lowercase")], matchingUserContent. Serialized shapes:{"type": "text", "text": "hello"} {"type": "toolcall", "id": "call_1", "function": {...}} {"type": "reasoning", ...} {"type": "image", ...}Text,Image,Audio,Video,Document) become namedadditional_paramsfields withdeny_unknown_fields. Two defect classes die at the root:"type":"text"key can no longer be flatten-captured and replayed to providers (the cleanup: post-Vec-migration precision and the pre-Vec serde accommodations go #2276 recipe bug) — now it's either the tag or a decode error;Noneinstead of the flatten'sSome({})artifact, sois_empty_assistant_turndrops itshas_no_annotationsshim and live vs restored runs classify identically with no special-casing (round-trip test now assertsNone).Text— OpenAI Responsesoutput_text— gets its ownOutputTextwire struct preservingannotations/sibling keys verbatim for value-equal replay. Wire passthrough belongs to wire types.Cassette churn: zero, proven not assumed
Providers serialize their own wire types into requests, not rig's domain types; the one borrowed-type case (Responses
output_text) was decoupled in this PR. The full recorded suite passes under the new tags: modified cassettes 0, added 0.Verification
--all-targets --all-features -D warningscleancargo nextest run --all-features --no-fail-fast --test-threads=1: 3557/3557 (231 skipped) — count unchanged from base: the three affected tests were updated in place (tag-required pin replaces the stray-key-capture pin 1:1)cargo nextest run -p rig-derive: 43/43; doc tests andrig-vertexaiclean#[serde(flattenincompletion/message.rs→ 0;untaggedincompletion/message.rs→ 0