Skip to content

feat!: finish the data layer — blessed ModelTurn conversion, counter/newtype ids, owned dispatch, serde across the protocol surface - #2419

Merged
gold-silver-copper merged 6 commits into
mainfrom
bevy-prep/finish-data-layer
Aug 24, 2026
Merged

feat!: finish the data layer — blessed ModelTurn conversion, counter/newtype ids, owned dispatch, serde across the protocol surface#2419
gold-silver-copper merged 6 commits into
mainfrom
bevy-prep/finish-data-layer

Conversation

@gold-silver-copper

Copy link
Copy Markdown
Contributor

Interstitial PR between #2405 and the rig-bevy plugin (bevy-prep series, one-protocol-two-drivers). An audit of main against the ECS-driver requirements found the protocol extraction complete but left data-layer gaps that would force a second driver to duplicate runner logic or fight the borrow checker at every task-spawn site. This PR closes all of them so the plugin PR is pure assembly. Ordered commits, each green.

1. ModelTurn::from_response — the conformance anchor

The only CompletionResponse → ModelTurn conversion was ~15 inlined lines in rig-agent's runner, with two inputs hand-assembly gets wrong: the tool-name sets must come from the prepared request (never re-derived from the spec), and the finish reason must go through the normalized finish_reason() accessor. A second driver copying those lines can silently drift — which defeats "one protocol, two drivers, conformance by construction".

rig_run::ModelTurn::from_response(resp, &PreparedRequest) (plus from_response_parts for drivers carrying the name sets by value) is now the one blessed conversion; the runner is rewritten over it. Also drops the stale "Arc clone" comment (CompletionResponse::raw is a plain serde_json::Value).

2. InternalCallId counter id + ConversationId newtype (breaking)

  • internal_call_id was a 21-char nanoid String cloned at every boundary. It is now a Copy + Hash + Ord, serde-transparent NonZeroU64 counter id (the existing counter_id! macro, same shape as RunId), threaded through rig-core's streaming assembler and stream items, rig-run's pending-call/policy types, and rig-agent's hook events (which drop their &'a str borrow for a by-value id).
  • Persisted runs re-emit saved ids after resume, so the macro gains advance_past (an AtomicU64::fetch_max) and PendingToolCall's deserialize advances the mint counter past any persisted id — a resumed process cannot mint an id its consumers already saw (regression-tested).
  • ConversationId is a string-backed, serde-transparent newtype in rig_core::id; the ConversationMemory trait, rig-memory adapters, and the conversation(..) setters (now impl Into<ConversationId>, so string call sites compile unchanged) use it.
  • Serialized stream items and persisted runs carry internal_call_id as a number now — documented in MIGRATING; run state has no cross-version stability contract.

3. Owned-future entry points

ToolCatalog::execute_owned(self, name, args, context) -> (ToolResult, ToolContext) and ConversationMemoryExt::{load_owned, append_owned, clear_owned} (blanket impl over Arc<Self> + owned ConversationId) give hosts that spawn tasks Send + 'static futures without per-site clone-into-async move ceremony. The mutated ToolContext returns by value, preserving execute's publish contract (the runner's dispatch observes context metadata the same way). The borrowed methods stay the implementation surface.

4. Serde across the step/policy types

AgentRunStep, PreparedRequest, RequestPatch, InvalidToolCallContext, RetryRequest, InvalidToolCallAction, StreamedTurnEvent derive Serialize + Deserialize; ModelTurnOutcome gains Clone (+ serde). AgentRun could re-derive its step, but a component caching an in-flight step/prepared request could not round-trip through a saved world. Round-trip tests for each.

5. StreamedTurnAssembler: Clone + Serialize + Deserialize

A mid-stream streamed turn was the one piece of run state that could not persist. The assembler and its internals now derive Clone + serde; the ExclusionCount drop-guard serializes as a transparent count (loudness contract survives a resume) and Clone copies it so each lineage warns for its own drops. Dropping an assembler mid-turn (an ECS despawn) stays silent unless replayed assistant content was actually excluded. A midway serialize/resume test proves the restored assembler produces the identical turn.

(ModelRef: Deserialize, flagged by the audit, turned out to already exist — no change.)

Verification

  • cargo nextest run --workspace --all-features: 6239 passed, 0 failed; git diff --stat main -- tests/cassettes empty (commit 1 is byte-neutral on requests — full provider suites replay clean).
  • clippy/fmt/doc (-D warnings) clean; cargo check --target wasm32-unknown-unknown green for rig-core (incl. --all-features), rig-run, rig-agent, rig.
  • rig-run keeps zero async/tokio/futures deps; dependency-graph guard green.
  • New tests: from_response field-for-field equality, counter-id/ConversationId round-trips + advance_past, persisted-id counter advance on deserialize, owned-future Send + 'static pins with behavior parity, step/policy serde round-trips, assembler midway resume.
  • grep ModelTurn::new in rig-agent: only test-fixture construction remains.

…n conversion

The only CompletionResponse -> ModelTurn conversion lived as ~15 inlined
lines in rig-agent's runner; a second driver copying them can silently
drift (the tool-name sets must come from the prepared request, and the
finish reason must go through the normalized accessor). Move the
assembly into rig-run as ModelTurn::from_response(resp, &PreparedRequest)
plus from_response_parts for drivers that carry the name sets by value,
and rewrite the runner to use it. Also drops the stale 'Arc clone'
comment: CompletionResponse::raw is a plain serde_json::Value.
internal_call_id becomes a Copy + Hash + Ord + serde-transparent
NonZeroU64 counter id (via the existing counter_id! macro) everywhere it
flows: rig-core's streaming assembler and StreamedAssistantContent /
StreamedUserContent items, rig-run's PendingToolCall / TurnState /
policy context / StreamedTurnAssembler, rig-agent's hook events, rewrite
frames and stream feed. It was a 21-char nanoid String cloned at every
boundary; an ECS driver keys per-frame correlation maps by it.

Persisted runs re-emit saved ids after resume, so the counter_id! macro
gains advance_past (an AtomicU64 fetch_max) and PendingToolCall's
deserialize advances the mint counter past any persisted id — a resumed
process cannot mint an id its consumers already saw.

ConversationId is a string-backed Hash + Eq + serde(transparent) newtype
in rig-core id.rs; ConversationMemory (and the rig-memory adapters),
AgentConfig, the builder/runner/prompt-request conversation setters and
InMemoryConversationMemory now use it instead of bare strings.

Serialized stream items and persisted runs carry internal_call_id as a
number now (documented break; run-state format has no cross-version
stability contract).
ToolCatalog::execute (async fn over &self + &mut ToolContext) and the
ConversationMemory methods (&'a self + &'a ConversationId) return
borrowed futures, so an executor-agnostic host spawning tasks had to do
the clone-into-async-move dance at every spawn site. Add:

- ToolCatalog::execute_owned(self, name, args, context) -> impl Future
  + Send + 'static yielding (ToolResult, ToolContext) — the mutated
  context comes back by value, preserving execute's publish contract.
- ConversationMemoryExt::{load_owned, append_owned, clear_owned} over
  Arc<Self> + owned ConversationId, blanket-implemented for every
  ConversationMemory including trait objects.

Both are wrappers over the borrowed methods, which remain the
implementation surface. Send + 'static pinned by tests.
Derive Serialize + Deserialize on AgentRunStep, PreparedRequest,
RequestPatch, InvalidToolCallContext, RetryRequest,
InvalidToolCallAction and StreamedTurnEvent, and add Clone (+ serde) to
ModelTurnOutcome. AgentRun could already re-derive its step from
persisted state, but a component caching an in-flight step, prepared
request or patch could not round-trip through a saved world — now the
whole protocol surface is data. Round-trip tests for each.
A mid-stream *streamed* turn was the one piece of run state that could
not be persisted, breaking the save/load parity AgentRun promises.
Derive Clone + serde through the assembler and its internals
(ToolCallDeltaState, ReasoningPart(State), PendingInvalid,
ExclusionCount as a transparent count). Clone copies the exclusion
count — each lineage warns for its own drops — and deserialize restores
it, keeping the loudness contract across a resume. Dropping an
assembler mid-turn (an ECS despawn) stays silent unless content was
actually excluded. Midway serialize/resume test proves the resumed
assembler produces the identical turn.
@gold-silver-copper
gold-silver-copper added this pull request to the merge queue Aug 24, 2026
Merged via the queue into main with commit 475cfdd Aug 24, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant