Skip to content

refactor!: remove backwards-compat shims (aliases, legacy preamble, extractor usage API, persisted-format tolerance, dead model ids) - #2420

Open
gold-silver-copper wants to merge 4 commits into
mainfrom
remove-backcompat-shims
Open

refactor!: remove backwards-compat shims (aliases, legacy preamble, extractor usage API, persisted-format tolerance, dead model ids)#2420
gold-silver-copper wants to merge 4 commits into
mainfrom
remove-backcompat-shims

Conversation

@gold-silver-copper

Copy link
Copy Markdown
Contributor

A sweep removing every remaining backwards-compatibility shim, per the project's no-backcompat policy. Every break is documented in MIGRATING.md ("Backwards-compat shims removed" under 0.41 → next).

Removed

Rename aliases (pure old-name shims):

  • rig_candle::ModelFamily → use ConversationProtocol; rig_candle::LlamaModel → use CandleModel (internal code mass-renamed to the canonical names)
  • rig::tool_macro / rig_agent::tool_macro re-exports of rig_tool
  • rig_agent::tool::server::ToolRegistrySnapshot alias of ToolCatalog
  • rig_bedrock::embedding's historical constant names (AMAZON_TITAN_EMBED_TEXT_V1 etc.) aliasing the canonical completion constants
  • the hook event struct is now named CompletionCallEvent itself (was hook::CompletionCall + a renaming re-export), ending the collision with the run-record CompletionCall

CompletionRequest::preamble — the legacy field "preserved for backwards compatibility". The builder has funneled the preamble into a leading Message::System since 0.33 and always emitted preamble: None; every provider read of the field was a dead parallel channel for hand-built requests. Providers now derive system instructions solely from leading system messages (they already did). CompletionRequestBuilder::preamble(..) survives as pure build-time sugar. Provider telemetry system_instructions, fed only by the dead field, now reports none.

Extractor APIextract_with_usage/extract_with_chat_history_with_usage deleted; extract/extract_with_chat_history now return ExtractionResponse<T> ({ data, usage }). ~120 call sites migrated.

Extractor legacy-transport policyignore_unhandled_invalid_tool_calls (invalid tool calls silently discarded in extractor runs) removed; unresolved invalid calls now fail the attempt and the extractor's retry loop retries. AgentRun::ignore_invalid_tool_call, which existed only for that policy, is removed.

Retired provider model constants — all 13 #[deprecated] model ids (Cohere ×6, Mistral ×5, DeepSeek ×2) plus the undead MOONSHOT_CHAT (moonshot-v1-128k, doc-marked "(legacy)") are deleted rather than left warning; the providers no longer serve these models.

Persisted-format tolerances:

  • CompletionCall.usage is a required object — the pre-monoid "usage": null encoding no longer loads (run records, stream items, suspended AgentRun state). Pinning tests flipped to assert rejection.
  • json_utils::string_or_vec re-tightened: null is a loud error again. The one field where null is a real wire shape (OpenAI assistant content on tool-calls-only messages) uses the new string_null_or_vec, so provider decoding is unchanged.

Considered and deliberately kept

  • AdditionalParams {}/null → absent canonicalization — this is the type's core absence doctrine (non-empty-map invariant), not a bolt-on compat path; removing it would break the design, including serialization-skip symmetry.
  • PromptResponse::new(output, usage) / TypedPromptResponse::new — flagged by a test comment as "backward compatibility", but new + with_* is the intended builder shape; only the stale test labels were rewritten.
  • The rig-agent → rig-run/rig-core re-export layer — not a compat alias set but the facade's live API: rig-agent's own signatures return rig-run types (PromptResponse, CompletionCall, AgentRun), so the re-exports are load-bearing.
  • Copilot bootstrap_token_fingerprint: Option<String> — user on-disk auth state, fail-safe (None forces refresh), cheap.
  • All #[serde(alias)] sites — verified to handle currently-live provider wire dialects (OpenAI developer role, gateway role: "model", Mistral dual spelling, Jina/TEI rerank keys, …), not rig compat.
  • keys_lost_in_round_trip — opt-in migration tooling, correctly outside the load path.

Verification

  • cargo check/clippy --workspace --all-features --all-targets: clean
  • cargo check --target wasm32-unknown-unknown for rig-core/rig-agent/rig-run: clean
  • full test suite (cargo nextest run --workspace --all-features): 6239/6239 passed (213 skipped)
  • cassette scenario literals untouched (replays remain valid; the preamble → leading-system-message change is wire-identical for recorded requests)

…acro, ToolRegistrySnapshot, CompletionCall event, bedrock historical constant names)
System instructions are carried solely by leading Message::System entries in
chat_history (what the builder has emitted since 0.33). Providers no longer
read the dead field; hand-built requests must put the system prompt in
chat_history. Telemetry system_instructions, which was fed only by the dead
field, now reports none.
…trict persisted-format serde

- extract/extract_with_chat_history now return ExtractionResponse<T>
  (data + usage); the _with_usage variants are deleted (~120 call sites
  migrated).
- The extractor no longer silently discards unhandled invalid tool calls
  (legacy extractor-transport semantics); unresolved invalid calls fail
  the attempt and the retry loop retries. AgentRun::ignore_invalid_tool_call
  is removed.
- CompletionCall.usage is a required object: the pre-monoid "usage": null
  encoding no longer deserializes (run records, stream items, suspended
  runs); pinning tests flipped to assert rejection.
- json_utils::string_or_vec no longer accepts null; the one field where
  null is a real wire shape (OpenAI tool-calls-only assistant content)
  uses the new string_null_or_vec.
- Stale "backward compatibility" test labels rewritten; MIGRATING.md
  documents every break under 0.41 -> next.
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