Skip to content

refactor!: data-oriented rearchitecture — providers as config, sans-IO agent protocol, de-genericized runtime and stores - #2228

Open
gold-silver-copper wants to merge 87 commits into
mainfrom
audit/generic-bounds-rearchitecture
Open

refactor!: data-oriented rearchitecture — providers as config, sans-IO agent protocol, de-genericized runtime and stores#2228
gold-silver-copper wants to merge 87 commits into
mainfrom
audit/generic-bounds-rearchitecture

Conversation

@gold-silver-copper

@gold-silver-copper gold-silver-copper commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Converts Rig from trait-and-generic polymorphism to a data-oriented runtime. Providers are serializable configuration plus free functions, agents hold one closed provider configuration, hooks are callback records over owned events, tools use portable records, and vector stores expose concrete inherent APIs.

Architecture

Providers are data

Each provider exposes a serde Config, a ProviderDescriptor, and free capability functions such as complete, open_stream, embed, transcribe, or rerank. Transport state lives in Runtime, so provider configuration and agent run state can be serialized without hiding clients or callbacks inside them.

ProviderConfig is deliberately exhaustive. Adding a provider is a breaking change by design: every fulfilment path must make an explicit capability decision. Bedrock and Gemini gRPC retain stable enum/config shapes across feature combinations; only fulfilment is feature-gated.

The deterministic Mock completion and embedding variants are also always-present, production-visible enum and serde variants for testing and host simulation, so exhaustive downstream matches must handle them. Mock completion scripts can deliberately remain pending until cancellation; hosts that deserialize provider configuration across an untrusted boundary must validate or allowlist variants and fields before fulfilment.

One non-generic agent protocol

Agent, AgentBuilder, requests, extractors, sessions, and streams are no longer generic over model/client types. AgentRun owns the serializable state machine; prepare_request is pure request construction; AgentSession and AgentStream are two drivers over the same protocol.

The lifecycle is divided into explicit ownership boundaries:

  • model attempts are provisional until accepted;
  • an AcceptedModelTurn is the normalized post-resolution record shared by blocking and streaming drivers;
  • ModelTurnFinished is a steering boundary for accepted model output, not a claim that the model-plus-tools turn has settled;
  • tool-call interception, body execution, result middleware, execution observation, and durable result publication are separate phases;
  • durable tool results follow assistant source order even when bodies complete concurrently or a host submits results in another order.

Each provider operation owns its real prompt, fresh attempt identity, and effective Tool-mode output name/schema plus validation-name sets. Public hand-driven unary and streaming callers preserve that exact attempt contract across provider I/O with the opaque PreparedModelAttempt receipt carried by PreparedRequest; failed, cancelled, or abandoned attempts roll back without leaking provisional state.

Post-repair turn verdicts are resume-durable. AgentRun represents “verdict required” and “ready to advance” as distinct states. A checkpoint before the verdict resurfaces it exactly once; a checkpoint after Continue advances to tools without duplicating it.

Execution provenance is explicit

ToolInvocationDisposition records whether a call was locally Executed, ExternallyExecuted, or NotExecuted { reason }. The value travels with the invocation/result identity and cannot be inferred from a rewritten payload or result classification.

ToolExecutionCommitted is therefore emitted only for locally or explicitly externally executed calls. Hook skips, invalid-call recovery, peer suppression, and unknown tools still produce durable model-visible results without falsely reporting execution. Unknown tools are rejected during preflight, open no execute_tool span, and do not advance last_span_id. Rewrites can change presentation, never provenance.

Concrete public surfaces

  • CompletionResponse and streaming finals carry normalized finish, provider, model, and usage data.
  • Provider clients remain fluent, monomorphic connection builders that convert to serializable provider configuration.
  • Hooks are ordered HookEntry records with shared folds for blocking/streaming parity.
  • Tools converge on PortableTool and ToolExecutor records.
  • Memory is host-owned and vector stores expose concrete top_n/top_n_ids operations.
  • System instructions have one canonical message representation and reserved instruction keys are rejected at the generic parameter boundary.

MIGRATING.md contains the full migration guide and the changelogs describe the breaking surface.

Review map

At the current pushed head, the complete merge-base diff is 802 files, +85,117 / -70,565 (net +14,552). Most gross churn is deleted generic runners/clients, provider function-module adoption, and mechanical test/example migration. The main semantic review boundaries are:

  1. crates/rig-agent/src/provider.rs — provider registry, capabilities, and dispatch.
  2. crates/rig-agent/src/agent/attempt.rs and agent/run/ — transactional model attempts, accepted turns, identity, and durable state transitions.
  3. crates/rig-agent/src/session.rs and stream.rs — blocking/streaming drivers and lifecycle parity.
  4. crates/rig-agent/src/executor.rs — tool preflight/execution/result gates and immutable execution disposition.
  5. crates/rig-agent/src/hooks.rs — record-based hooks and ordered composition.
  6. crates/rig-core/src/providers/*/functions.rs — provider request/response fulfilment over HttpRuntime.

Previously documented gaps are closed: VoyageAI connection/debug handling is redacted and shared, instruction-shaped reserved keys are rejected, and EmbedderConfig includes Llamafile, Mistral, OpenRouter, and Together. FastEmbed remains intentionally outside the serializable config enum because its loaded local model is runtime state.

Cassette evidence

The merge-base diff changes five cassette files, with 7 insertions and 7 deletions total:

  • four OpenRouter requests add the now-explicit max_tokens field; recorded responses are unchanged;
  • one Gemini parallel-tool request changes only tool-result order from host submission order to the assistant's source order, matching the new durable-order invariant; its recorded response is unchanged.

All replay and cassette safety tests pass. No credentials, account identifiers, headers, cookies, or unrelated response churn were introduced.

Verification

All of the following pass on the final local code diff:

cargo fmt --all --check
cargo clippy --workspace --all-targets --all-features -- -D warnings
cargo test --workspace --all-features
cargo test --workspace --all-features --doc
RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --all-features
cargo check --target wasm32-unknown-unknown -p rig-core -p rig-agent -p rig -p rig-candle
git diff --check

The workspace test run includes provider cassette suites, core integration tests, compile-fail tests, and doctests. Targeted regressions additionally cover repaired-turn parity and stop/retry behavior, resume checkpoints on both sides of the accepted-turn verdict, public unary/streaming prepared-attempt binding, skipped versus executed tool provenance, unknown-tool span ownership, result-rewrite immutability, duplicate provider IDs, mixed parallel execution, and source-order durable result commitment.

All 12 current GitHub checks pass at the pushed head.

Three companion documents for making rig-core and rig-agent fully
non-generic and data-oriented (enums + exhaustive match, plain structs,
free functions; no dyn, no type erasure, no behavior-bounding generics):

- audit/generic-bounds.md — tiered inventory of every generic parameter,
  trait bound, trait object, and associated type (use with the corrections
  in the design doc's §2).
- audit/generic-bounds-rearchitecture.md — the earlier model-slot-only
  proposal (ProviderConfig enum in the facade), superseded by the full
  design but kept for its dispatch-mechanism rationale.
- audit/data-oriented-rearchitecture.md — the full design: resolves all
  four behavior slots in Agent (model, hooks, tools, memory) plus
  streaming, with a verified disposition for the whole trait surface, an
  8-phase migration plan, cost accounting, and a 5-entry exceptions
  ledger. Survived three adversarial review rounds (~120 file:line
  anchors re-verified against this tree).
…dd rig-bevy runtime and keep de-genericized store crates

Per maintainer direction: the original P7 deletion is retired. The classic
rig-agent runtime survives with its ergonomic surface (hooks, memory, tool
server) intact, migrated onto the new data-oriented substrate; a simple
bevy_ecs runtime crate (rig-bevy, new P9) becomes a first-class deliverable;
the 13 vector-store crates are kept but lose all generic trait machinery.
Phase table (P6-P9) and cost accounting updated accordingly.
…ners on the two prior docs

- data-oriented-rearchitecture.md: CompletionResponse.provider is String
  (a &'static str field cannot Deserialize); StreamFinal carries provider/
  model so the stream→response conversion fills the normalized fields; the
  GetTokenUsage row records its full compiler-verified dependent set
  (telemetry record_token_usage, the OpenAICompatibleProvider bounds, the
  internal compat streaming layer, test mocks); the §14 P1 row records the
  shared-conversion provider-stamping pattern and the working edit order.
- generic-bounds.md / generic-bounds-rearchitecture.md: status banners
  stating what was corrected, what was rejected (the Custom fn-pointer
  arm), and that Revision 2 retired the delete-the-shell endgame.
Phase P1 of the data-oriented rearchitecture
(audit/data-oriented-rearchitecture.md §14, Revision 2.1).

The provider-typed response generics are gone from the payload layer:

- CompletionResponse<T> -> concrete CompletionResponse with normalized
  metadata: finish_reason (new FinishReason enum — closes #2090, #1886),
  provider, model, message_id. raw_response is removed; provider-typed
  wire data stays reachable through each provider's own conversion layer.
- The provider-typed streaming final becomes the normalized StreamFinal
  (kind discriminant for the untagged enum, usage as a field, finish
  reason, message_id, provider, model). The GetTokenUsage trait is
  deleted; wire usage types convert via From/Into<Usage>.
- RawStreamingChoice, StreamingResult, StreamingCompletionResponse,
  StreamedAssistantContent, MultiTurnStreamItem, DriveStream/DriveItem and
  the StreamedTurnAssembler lose their type parameters; the entire
  rig-agent run module is now generic-free.
- CompletionModel loses type Response / type StreamingResponse (the trait
  itself survives until P7); TurnSource loses type Raw entirely;
  StreamingPrompt/StreamingChat lose their response parameter.
- Every finish/stop vocabulary is mapped per provider (25 in-core + the
  four companion crates); the OpenAI-compatible shared path stamps
  Ext::PROVIDER_NAME post-conversion.
- Provider-specific streaming-final aliases (deepseek, groq, mistral,
  openrouter, gemini, copilot, candle placeholder) are removed.

Verification: cargo check --workspace --all-targets clean; clippy clean;
rig-core 1008 tests, rig-agent 500 tests, facade cassette suites 1214
tests all passing with cassette YAMLs byte-identical (no request-building
code changed). Full narrative in audit/migration-log.md.
…request (P2)

Phase P2 of the data-oriented rearchitecture (design doc §5.3, §6.2, §7.1).

- AgentConfig: the model-free serde agent definition (no behavior slots).
- ToolCatalog + PreparedRequest + prepare_request(): request preparation as
  a pure, sans-IO free function any driver can call between
  AgentRun::next_step and its provider fulfilment; provider capability
  passed as a plain flag.
- The classic driver is rewired THROUGH the pure function:
  PreparedCompletionRequest is now non-generic (holds the built
  CompletionRequest; the driver sends via model.completion/stream), and
  CompletionRequest::messages_for_telemetry keeps input telemetry
  identical.
- Hook decision vocabulary gains serde; composition semantics become pure
  helpers: fold_completion_actions / fold_observation_actions /
  fold_invalid_resolutions plus the chained-event accumulators
  ToolCallResolution / ToolResultResolution (terminate-with-salvage), with
  parity tests.

Proof of faithfulness: the full cassette suite (1214 tests,
--test-threads=1) passes with cassettes untouched — the pure path emits
byte-identical requests. rig-agent 505 and rig-core 1008 unit tests green;
workspace check and clippy clean.
…s, HttpRuntime (P3)

Phase P3 of the data-oriented rearchitecture (design doc §5.2, §12).

- providers/descriptor: ProviderDescriptor (capability sheet as const
  data) and ApiKeyLocation (Env/Inline/None credential references).
- http_runtime: HttpRuntime, the concrete HTTP executor replacing the H
  type parameter; transport variation is an enum (reqwest + test-utils
  recording arm), never a generic.
- providers/openai/functions: serde Config, DESCRIPTOR, pure
  build_request_body/build_request and parse_response, async
  complete/open_stream over HttpRuntime. The pure functions delegate to
  the same typed conversion as the generic path during the transition,
  guaranteeing byte-identical request bodies.

Proof: completions_api_pure_functions_replay_recorded_request replays a
classic-path cassette recording through functions::complete — the
cassette server only serves on request match, making the green test the
byte-identity proof. Deferred to P5 (logged): the sans-IO SseParser push
parser, which ModelStream's no-boxed-stream shape actually requires.

Verification: clippy clean; rig-core 1011 tests; facade suites 1215
passed (--test-threads=1), cassettes untouched.
…ers (P4)

Phase P4 of the data-oriented rearchitecture (design doc §12).

Every in-core provider (25) and the three non-HTTP companions now carry
the openai-pilot face: serde Config, const ProviderDescriptor capability
sheet, pure build_request_body/build_request/parse_response, and async
complete/open_stream over HttpRuntime.

- The 16 OpenAI-compatible providers ride shared pub(crate) helpers
  (compatible_request_body/request/parse_response/open_stream +
  stream_profile_for<Ext>) — descriptor values read from their Ext consts.
- Standalone providers (anthropic, cohere, xai, chatgpt, gemini, ollama,
  copilot) had their SSE machinery genuinely extracted into free functions
  with the trait impls rewired through them — single source of truth, so
  the cassette suites certify byte-identity.
- Companions: bedrock/gemini-grpc Configs describe client construction
  (client_from_config builders over the async credential chain / tonic
  channel); candle's Config + ModelArtifacts preserve its no-filesystem
  invariant; trait impls rewired through extracted functions.
- ProviderDescriptor gains const with_* builders (non_exhaustive blocks
  companion struct literals); openai max_embedding_documents corrected to
  the code's actual 1024.

Documented deferrals in audit/migration-log.md: gemini Interactions API
face, copilot /responses + OAuth (interactive auth is not plain data),
anthropic caching knobs, bedrock naming unification.

Verification: workspace check 0 errors; clippy clean; rig-core 1079 tests
(+68 provider-function tests); companions green; facade cassette suite
1215 passed with cassettes untouched.
…ession, AgentStream, extract, ToolRouter, rig-mcp (P5)
…oviderConfig via the ToProviderConfig bridge (P6)
…ors; unify bedrock provider name (P7 progress)
… doubles to rig-memory, TurnSource enum, normalize flattened additional_params (P7 progress)
@gold-silver-copper
gold-silver-copper force-pushed the audit/generic-bounds-rearchitecture branch from be2f714 to 4b4dffa Compare July 29, 2026 16:43
@gold-silver-copper
gold-silver-copper force-pushed the audit/generic-bounds-rearchitecture branch from 4b4dffa to c8d8a26 Compare July 29, 2026 16:45
…ranscribe/image/audio/rerank free functions, pre-embedded stores, no shared store traits (P8)
…ded Runtime clients; candle README on the sans-IO path
@gold-silver-copper gold-silver-copper changed the title docs(audit): data-oriented rearchitecture design for rig-core/rig-agent refactor!: data-oriented rearchitecture — providers as config, sans-IO agent protocol, de-genericized runtime and stores Jul 29, 2026
…fidelity + secret redaction, doc coherence (quality review)
…er, mechanism-only removal (maintainer direction)
…ent, event/serde/telemetry parity, list_models, vertexai face (single-architecture R1)
…rds — delete ToolServer/ToolSet/ToolContext/ErasedTool/rmcp module and the IntoToolOutput Any sieve (single-architecture R2)
@gold-silver-copper
gold-silver-copper force-pushed the audit/generic-bounds-rearchitecture branch from 1b662ba to 43b703b Compare July 29, 2026 22:22
…o host calls — concrete Hooks records, enum memory policies, owned AppendOutcome (single-architecture R3)
…ds, runner as the fluent API, extract_with_options replaces Extractor (single-architecture R4)
…m are the only drivers (single-architecture R5)

`AgentRunner` and `agent/prompt_request/` (17 488 lines) are gone, and with
them the second agent engine (`drive_agent`/`TurnSource`/`DriveItem`),
`StreamingPromptRequest`, `MultiTurnStreamItem`, `StreamingResult`, and
`StreamingError`.

R1's `SessionAgent` and the classic `Agent` were structurally identical, so
they merge into ONE `Agent` — plain data (`AgentConfig` + `ProviderConfig` +
`Arc<Runtime>` + `ToolCatalog` + `Option<ToolExecutor>` + `Hooks`) with
inherent methods over `AgentSession::drive` / `AgentStream::drive`, the
classic loops expressed as data. `agent_api::SessionAgent` is a deprecated
alias.

The fluent per-request surface survives as `agent::SessionRunner` with every
`AgentRunner` setter name intact, so `.runner(..)` call sites do not move.
Streaming becomes `agent.stream_run(p)` / `agent.runner(p)….stream_run()`
yielding `AgentStreamItem` with `PromptError` as the error type;
`stream_prompt`/`stream_chat` now return the host-driven `AgentStream`.

`PromptResponse`/`CompletionCall` and the shared history/tool-result helpers
move to `agent/response.rs`, the GenAI span shapes to `agent/telemetry.rs`
(public paths unchanged).

Audited against the deleted driver, which restored seven parity behaviors the
session layer had drifted from: chat-span content telemetry and preamble
patching, per-call streaming usage, tool argument/result telemetry recorded
once post-hook, the blocking `follows_from` span chain, structured
`ToolResult` classification on the tool-result hook, one `CompletionCall`
item per model call, and rejection of post-final provider content. Also
fixes a `Send`-generality break that made `rig-agent`'s Discord integration
uncompilable.

203 deleted tests: 191 ported, 3 already covered, 9 dropped with written
justification (see audit/migration-log.md). rig-agent lib tests 179 → 372;
every cassette suite replays byte-identically and cassettes are untouched.
`HookEntry::new` takes an async callback returning `WasmBoxedFuture`. Most
hooks inspect the event and answer immediately, so 60 of the 90 call sites in
this repo paid for an await they never used:

    HookEntry::new("context", |event| {
        let decision = match event { .. };
        Box::pin(async move { decision })
    })

`HookEntry::sync` takes `Fn(HookEvent) -> HookDecision` and does the boxing
itself, so those read as what they are — a function from event to decision,
with no `let decision`, no `Box::pin`, and one less indent level.

This is a constructor, not a redesign: it delegates straight to `new`, carries
the same `WasmCompatSend`/`WasmCompatSync` bounds, and leaves the `HookEvent`
and `HookDecision` vocabulary, the fold semantics, registration ordering, and
the `observing_deltas` opt-in untouched. `new` stays for hooks that genuinely
await — a retrieval hook embedding its query, for instance.
Two mechanical sweeps, both behaviour-preserving.

**120 `CompletionRequest` literals** move onto `CompletionRequest::builder`.
These were previously blocked: they carry a preamble, and the guide claimed
the builder's leading-system-message form was not interchangeable with the
legacy field. With that equivalence now established and pinned by a test, they
are safe.

The measurement that made it safe: of the 129 preamble-carrying sites, **zero**
also place a `Message::System` in the history. Every one supplies system
instructions through a single channel — the case where all ten providers agree,
including Gemini's Interactions API, whose `.or_else` divergence only shows up
when both channels are used at once.

The proof is the recorded suites, not the reasoning: 730 cassette tests across
nine providers replay byte-identically with fixtures untouched.

**60 hook callbacks** move to `HookEntry::sync`, dropping the
`let decision = ..; Box::pin(async move { decision })` wrapper. That in turn
made six closures redundant (`move |event| decide(event)` -> `decide`), which
clippy caught.

Roughly 34 remaining `CompletionRequest` literals are left alone: they are
already compact enough that a builder chain is a wash rather than an
improvement. Sites with no functional-update base, and one inside a
commented-out block, are also untouched.

`CompletionRequest::preamble` still exists — deleting it is a separate change
of about 350 edit sites, and it was not needed for this.
`CompletionSpanBuilder` had one incremental setter, `system_instructions(..)`,
fed from the scalar `request.preamble`. That made it a free function wearing a
builder's clothes, and it created a way for telemetry and the wire body to
disagree about what the system instructions were.

It is replaced by a concrete function:

    completion_span(provider, request_model, operation, &CompletionRequest)

which builds the span immediately, so the provider/model borrows never escape
and the type needs no lifetime or type parameters.

`system_instructions_json` now takes the request and reads its canonical
`Message::System` entries in order, serializing each as its own
`TelemetryPart::Text`. Deliberately not `messages_for_telemetry()`, which
clones and normalizes unrelated prompt and document content this attribute has
no business carrying. It returns `None` when there are no system messages or
when content recording is off.

The scalar form survives as `configured_system_instructions_json`, narrowly
scoped to the run-level `invoke_agent` span, where no turn request exists yet
and the agent's configured preamble is genuinely the only thing available.

Per-call `chat`/`chat_streaming` spans now derive from the prepared request
instead of an `effective_preamble: Option<&str>`. The scalar path could miss
history system messages, output-mode preamble augmentation, and per-turn
overrides — a silent telemetry gap no cassette would catch, because span
attributes are not part of the recorded bytes. That change made
`SessionSpanParams::record_telemetry_content` and an entire `effective_preamble`
computation dead; both are removed.
`CompletionRequest` modelled system instructions two ways: a legacy
`preamble: Option<String>` field, and `Message::System` entries in
`chat_history`. Which one a caller got depended purely on the constructor, and
100 of the test-suite's request literals were pinned to the field because of
it. There is now one representation: ordered system messages.

`with_history` becomes message-only — `with_history(history, prompt)`. The
positional preamble argument is not kept: retaining it would preserve the
duplication as a second low-level scalar channel on a type that no longer has
the field. Preamble-bearing callers move to the fluent builder, which is where
scalar-to-message adaptation belongs:

    CompletionRequest::builder(prompt).preamble(pre).messages(history).build()

`CompletionRequestBuilder::preamble`/`without_preamble` and the agent
configuration and request-patch preamble APIs are untouched. They are adapters
at the construction boundary; the builder's private `Option<String>` is builder
state, not duplicated request data, and it is what makes `.preamble(..)`
last-writer-wins and `.without_preamble()` precise.

Every provider already handled system messages arriving through the history —
that is what the earlier convergence audit established — so the provider
changes are branch deletions, not rewrites. Bedrock, Vertex AI, Gemini gRPC,
and Candle are covered too; they sit outside the nine-provider cassette claim,
so their conversions were read rather than assumed.

**This fixes a latent bug by construction.** Gemini's Interactions API used
`.or_else`: a scalar preamble won and every system message already in the
history was *discarded*, where every other provider appended both. A caller
supplying both channels silently lost data. With one representation there is
nothing left to prefer, and the two-channel state is now unrepresentable.
`every_system_message_reaches_the_instruction_in_order` pins the formerly lossy
case, asserting the wire instruction is `"preamble\n\nhistory system"`.

The old equivalence test had nothing left to compare and is replaced by
`canonical_system_messages_reach_the_wire_in_order`, which pins that
`.preamble(..)` is sugar for a leading system message and that multiple system
messages keep their relative order on the wire.

Wire placement is untouched. `SystemInstructionsPlacement` and its three
variants remain per-provider configuration: ChatGPT's `AllInstructions`
(its backend rejects the `system` role) and Copilot's `InputSystemMessages`
(the escape hatch for backends that ignore top-level `instructions`) both
replay byte-identically with the field gone — 63 and 67 tests respectively,
which is the only proof the non-default placements survive.

Net diff is roughly neutral rather than negative: the telemetry replacement and
the builder migrations spend about what the field deletions save. That is the
right trade for removing a second input shape.
`completion_span` reads the request's canonical system messages, but
ChatGPT's configured `default_instructions` were merged into
`request.instructions` *after* Responses conversion and never became a
`Message::System`. So `gen_ai.system_instructions` omitted an instruction that
was actually sent.

No cassette catches this: span attributes are not part of the recorded bytes.
It was introduced by the telemetry refactor, which moved the general case onto
canonical messages and left the one provider with a configured default still
merging post-conversion.

The default is now folded into the request as a leading system message before
both telemetry and conversion, and the post-conversion merge is deleted, so the
two paths are structurally incapable of disagreeing. `AllInstructions` joins
system messages with `"\n\n"` — the same separator the old merge used — so the
resulting `instructions` string is unchanged.

One case needs explicit handling. The old `merge_instructions("", None)`
emitted `Some("")`, and a canonical system message cannot express that: the
Responses lift drops empty text, which would yield `None` and change the wire
body. Three ChatGPT cassettes caught it. An empty default is therefore skipped
for materialization and its `Some("")` restored after conversion — byte
preservation only, and no telemetry is lost because there is no content to
report.

`merge_instructions` is gone; its three semantic tests are retargeted onto
`build_codex_responses_request` so they exercise the live path rather than a
helper. All 63 ChatGPT cassettes replay with fixtures unchanged.
`AdditionalParameters::system_instruction` let a raw `additional_params` value
supply system instructions alongside canonical `Message::System` entries — a
second way to say the same thing, on the same provider whose `.or_else`
precedence rule between those two sources was the data-loss bug fixed earlier
in this branch.

The field is removed and system instructions now come only from canonical
messages. The wire request and response fields of the same name
(`CreateInteractionRequest`, `Interaction`) model the real API and are kept.

Scope check before narrowing: a grep across every provider for
instruction-shaped keys read out of `additional_params` found no other case, so
removing this one leaves nothing currently exposed. A general reserved-key
guard — rejecting `system` / `instructions` / `system_instruction` when they
would compete with canonical data — still does not exist, and is recorded as
known-missing in the reviewer's guide rather than implied.
The cassette suites are this PR's byte-fidelity proof, but they do not reach
everywhere. Cohere's suite is ignored entirely — it had zero replay proof — and
Bedrock, Vertex AI, Gemini gRPC and Candle sit outside the nine-provider claim.
Their system-instruction handling rested on code reading alone.

One focused conversion test each, needing no credentials:

- Cohere renders system instructions as `system`-role messages (no dedicated
  field), so the test asserts they survive conversion in order.
- Bedrock, Vertex AI and Gemini gRPC each use a dedicated system field, so the
  tests assert ordered placement there **and** that the text does not also leak
  into the message array — the failure mode that would double-send.
- Candle renders a prompt string, so the test asserts system messages survive
  in order rather than being dropped with the deleted preamble field.

Gemini gRPC's is the sharpest of the five: `rig_message_to_grpc_content`
deliberately errors on a system message, so the split-out must happen before
conversion or the request fails outright.
This PR cannot be reviewed line by line, and pretending otherwise spends a
reviewer's attention on mechanical sweeps instead of the parts that carry risk.

The guide names the five files worth reading closely, marks the rest as
compiler- or script-driven sweeps, and states plainly what substitutes for
line-by-line review: ~730 recorded provider exchanges replaying byte-identically
through an entirely new dispatch path. It also tabulates where that evidence
does *not* reach — Cohere, the four companion crates, and span attributes — and
which tests cover each gap instead.

Two accepted risks are written down as decisions rather than left to be
discovered: `ProviderConfig` being deliberately not `#[non_exhaustive]` (and
what that costs out-of-tree providers), and `extra_headers` printing verbatim
in `Debug`. Both are also surfaced in the PR description.

It closes with the three ways this repo can hand you a false green, all of
which were hit during development: a pass-count summary cannot see a failure,
`cargo check --all-targets` skips doctests, and `cargo check --examples` exits
0 on `no targets matched`.
Doc-only. Every `.rs` hunk is a comment line; cassettes untouched.

`AgentSession`'s module doc promised "the whole session except its `Runtime`
handle is serializable between events". It is not: `AgentSession` derives no
`Serialize`, and `resume` rebuilds `Pending` from `run.pending_invalid_tool_call()`
alone. Tool-call and tool-result gate state — arguments already rewritten, calls
already skipped, results already supplied — lives in the session, not the run,
and is dropped. The gates re-surface and the host decides again, so this is
idempotent rather than corrupting, but it is not the checkpoint the doc sold. It
now says what persists, names both gaps (in-flight gate decisions, in-flight
model calls), and warns that a host whose decisions carry side effects should
drain a gate before serializing. `resume` gained the matching list, plus the
fact that it resets `tools` and `policy` to defaults — undocumented until now,
and a quiet way to resume a run with no tools.

The `preamble` field was deleted in this PR, but three places still described
it. `CompletionRequestBuilder::build` claimed the field "stays `None`";
`::preamble` contrasted itself against it; and MIGRATING.md's 0.42 example
called `CompletionRequest::with_history(Some(preamble), history, prompt)`, a
three-argument signature that no longer exists. That last one would not compile
if anyone typed it. The surrounding paragraph justified the two request forms
being interchangeable via the same dead field and cited a pinning test,
`system_instruction_forms_produce_identical_request_bodies`, that no longer
exists either; it now cites tests that do.

Two more of the same kind, found while in there. MIGRATING.md documented
Gemini Interactions' `.or_else` — preamble wins, history system messages are
discarded — as a live caveat readers must design around. 9c77742 removed that
channel; it is now recorded as a bug the collapse to one representation closed
by construction. And the comment at that site had grown two overlapping blocks
saying so twice, now merged.

Verified with `RUSTDOCFLAGS="-D warnings" cargo doc`, so the intra-doc links
added here resolve.
`-d` and `2026-07-29T00:00:00` are both empty, both tracked, and both entered
the branch in 305302e (R4). The names are a shell accident — a `-d` flag and
its date argument becoming filenames via a stray redirect. They were the only
zero-byte tracked files in the repo.
The six files under `audit/` were working notes — the rearchitecture design,
the per-phase migration log, two superseded predecessor designs, the
single-architecture plan, and the reviewer's guide. Together they added 4,625
lines of prose to a diff that is already hard to hold, and none of them
describe how to *use* the library: `MIGRATING.md` and `CHANGELOG.md` own that,
and they are unaffected.

Nothing outside `audit/` referenced them, so no link breaks. The PR
description carried the only pointers and has been rewritten to stand on its
own. The content remains in this branch's history for anyone who wants it
(`git show 4c2bc13:audit/migration-log.md`).
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