Skip to content

fix: four provider defects the ECS runtime cannot see past - #2524

Open
gold-silver-copper wants to merge 3 commits into
mainfrom
fix/provider-correctness
Open

gold-silver-copper wants to merge 3 commits into
mainfrom
fix/provider-correctness

Conversation

@gold-silver-copper

@gold-silver-copper gold-silver-copper commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Four provider defects the ECS runtime cannot see past

Each of these silently breaks a promise crates/rig-ecs makes to its hosts. All four were confirmed against 2355e0966 before any edit; the evidence is inline.

1 — a stream-borne overload is never retried

completion_error_from_body builds every in-band stream error through ProviderResponseError::without_status, leaving status: None and transient: None, so is_retryable() fell to transient.unwrap_or(false)false. The runtime's retry gate keys exactly on that flag (rig-ecs/src/systems/mod.rs:2275), so a mid-stream overloaded_error or 429 ended the run without spending a single ProviderRetried — while the identical condition on a non-streamed effect retried normally.

is_retryable now consults the provider's own machine code when there is no status and the transport offered no verdict. The code table lives beside retryable_status and covers the spellings the wires actually use (overloaded_error, rate_limit_error, RESOURCE_EXHAUSTED, UNAVAILABLE, server_error, …). A spent quota (insufficient_quota) is not transient, an unknown code still decides nothing, and both a transport verdict and a refusal still win.

Proof: provider_response::tests::a_body_borne_transient_code_is_retryable_without_a_status, and end to end rig-ecs/tests/run_provider_retry.rs::an_overload_reported_without_a_status_is_reissued. The latter fails on main at the retry assertion.

Two committed expectations encoded the old verdict and were updated deliberately, not re-pinned to new text: the ECS matrix's status table (tests/common/ecs_matrix/world.rs) asserted retryable == retryable_status(status) for a frame that carries no status, which is now the code's call; and openai::cassette::ecs_stream_faults::error_event_after_content_... observed two streams because its server_error frame is now re-issued — it spends ProviderRetries(0) like the truncation cell beside it, since it is about how the fault reaches the run, not about the retry.

2 — Claude 5 could not run at all

default_max_tokens_for_model prefix-matched only claude-*-4*; anything else got None and the prelude hard-errors RequestError("max_tokens must be set for Anthropic"). MaxTokens is an optional agent setting in rig-ecs and a RequestError is not retryable, so every rig-ecs run against Claude 5 without an explicit MaxTokens failed on turn one.

Claude Opus 5 and Sonnet 5 both publish 128K synchronous output; the table now says so. It also resolves through a gateway's vendor prefix (anthropic/claude-opus-5), because the limit belongs to the model, not the route.

Proof, in the shape the defect actually took: anthropic::cassette::max_tokens::a_native_ecs_run_reaches_claude_5_without_a_max_tokens_setting drives a rig-ecs run with no MaxTokens on the agent. With the Claude 5 rows removed it fails before any request leaves the process:

native ECS run must succeed: Provider(ErrorReport { kind: Request, retryable: false,
  message: "RequestError: `max_tokens` must be set for Anthropic" })

3 — a structured answer nobody asked for or checked

resolve_output read composes_native_output_with_tools — whether native output survives alongside tools — as if it answered whether the provider supports native structured output at all. Twelve providers drop output_schema; in Native mode rig-ecs validates nothing and settles on the text that arrives. The host asked for a schema, nothing on the wire carried it, and free text became RunResult.

ProviderCapabilities gains supports_native_output_schema. The OpenAI-compatible family derives it from the SUPPORTS_RESPONSE_FORMAT flag it already keeps, so the seven droppers there become truthful at a stroke; rig-vertexai and rig-gemini-grpc declare it false and now warn instead of dropping silently (gRPC also warns for documents, tool_choice and additional_params).

The routing half of this fix is not in this PR, and the reason is measured. I implemented it — resolve_output routing a schema-bearing run to its output tool whenever the provider cannot carry the schema — and the recorded parity corpus rejected it: the three deepseek_output_prompted_* cells change the request they send (a final_result tool declaration replacing the prompted preamble), so their committed cassettes 404 on a body mismatch. Re-recording them needs a DeepSeek key, and the same routing would move the corresponding cells for the other six OpenAI-compatible droppers whose keys are equally unavailable. Routing is a wire change; it belongs in a PR that can re-record the wire. policy::resolve_output keeps its old signature and the limitation is stated at its doc comment: a host on a dropping provider should set OutputKind::Tool itself.

Option (b) lands instead, and it is the half that needs no wire change. The prompt offers two fixes and says they are not exclusive: (a) route around the provider, (b) validate what came back. materialise's settle arm applied nothing — (OutputKind::Tool, None) | (Auto | Native | Prompted, _) settled on whatever text arrived, while Tool mode beside it reprompted on a schema miss. It now applies the same policy::text_satisfies_schema check whenever a schema is wanted and no output tool validated the answer: the run spends one OutputRetries and reprompts with the schema, and settles on its last word once that budget is out. No request shape changes, so no cassette moves.

Regressions (crates/rig-ecs/tests/run_output_tool_config.rs), against a handler whose descriptor declares supports_native_output_schema: false:

  • a_native_run_does_not_settle_on_an_answer_that_misses_the_schema — prose, then JSON; the run reprompts and settles on the JSON. Without the fix: left: "forty two, roughly" / right: "{\"answer\":42}".
  • a_native_run_settles_once_the_schema_reprompt_is_spent — a model that never answers the schema settles on its prose with exactly one retry spent, so the reprompt cannot loop. Without the fix: left: Some(0) / right: Some(1).

Each newly-honest provider now has a test that the drop is reported, reading what a host's tracing subscriber would see: rig-vertexai (output_schema, documents) and rig-gemini-grpc (those two plus tool choice and additional params). The capability derivation is pinned both ways in providers/xai/completion/tests.rs — xai carries response_format, so it declares native schema support true even though it cannot compose one with tools, and deepseek drops it and declares false. That contrast is the defect in one assertion: the two questions were never the same.

The field is serialized only when false, so a provider that carries the schema has no golden, log header or replay identity move. The droppers do: 107 deepseek_*.effects.json fixtures now record supports_native_output_schema: false in their handler descriptors, with the matching native policy identities in test-support/rig-test-support/src/ecs_goldens/identities.json. Anthropic's 692 tests pass with no golden regenerated.

4 — a recursive schema aborted the process

Gemini's $ref inliner recursed forever on a self-referential schema. Not an error — a stack overflow, so the host dies and no Failure is ever written. Confirmed on main: fatal runtime error: stack overflow, aborting / SIGABRT.

resolve_refs now carries the chain of definitions being expanded and refuses a cycle by naming the path that closes it (Node -> Node, A -> B -> A). A definition reused on two branches is still inlined.

On the "delete ~400 lines by sending parametersJsonSchema" claim — true premise, wrong conclusion, and I measured both. Gemini does accept a recursive $defs/$ref schema under parametersJsonSchema; that is recorded in tests/cassettes/gemini/recursive_schema/. But the deletion is not available and the swap does not pay:

  • the typed Schema conversion is still consumed by GenerationConfig::response_schema and by rig-gemini-grpc, whose gRPC FunctionDeclaration has no parametersJsonSchema field, so the converter cannot be deleted without moving those too;
  • I implemented the swap and measured its cost: the Gemini suite goes 339 passed / 315 failed on request-body mismatch, i.e. 315 cassettes to re-record, for a wire change that fixes nothing the cycle guard does not already fix.

So the guard lands (crash fixed on every path, zero wire change) and the recording stands as evidence for the follow-up. The prompt's own instruction covers this: "If it does not, do not force it."

Cassettes

Every fix ships one, recorded against the live wire, inspected by hand, and replayed with no API key set to prove it stands alone.

fix cassette what the wire shows
2 anthropic/max_tokens/claude_5_default_max_tokens {"max_tokens":128000,…,"model":"anthropic/claude-opus-5"} and a real answer
2 anthropic/max_tokens/claude_5_ecs_run the same default on the wire for a rig-ecs run that sets no MaxTokens
4 gemini/recursive_schema/gemini_accepts_parameters_json_schema the recursive schema sent verbatim; Gemini returns "child":{"name":"b"}
3 openai/structured_output/native_schema_still_reaches_the_wire the schema-carrying side is unchanged
1 anthropic/retry_classification/healthy_stream_still_completes a healthy stream, unaffected

Two honest gaps, stated rather than papered over.

Fix 1 has no recording of an overload frame. A provider emits one when it is busy and that cannot be arranged: the Anthropic workspace key is over its usage limit and answers HTTP 400 before any stream opens, OpenRouter relayed a healthy stream for every shape meant to provoke an upstream failure, and no committed fixture in the tree carries one. The recording pins the neighbouring real case; the frame itself is covered by a hand-written body in the unit test and end to end by the rig-ecs regression. No fabricated recording is presented as captured.

Fix 3 has no recording of a dropping provider. All seven OpenAI-compatible droppers (perplexity, together, moonshot, mira, deepseek, huggingface, hyperbolic) need keys that were not available. The recording pins the carrier side; the dropper side is the rig-ecs regression above.

Fix 2 was recorded through OpenRouter's Anthropic Messages endpoint rather than api.anthropic.com for the same quota reason — same client code path, and the vendor-prefixed id additionally pins the gateway-prefix handling.

Testing

  • cargo test --locked -p rig-core --lib — 1803 passed
  • cargo test --locked -p rig-ecs — 351 passed
  • cargo test -p rig --all-features --test anthropic — 692 passed, no golden regenerated
  • cargo xtask verify --check default-tests — PASS (1420s), the workspace suite
  • cargo xtask verify --check ecs-parity — PASS (2627s), the recorded parity corpus
  • cargo xtask verify --check source-guards, --check fmt — PASS
  • cargo clippy --locked --all-features --all-targets -- -D warnings — clean
  • cargo fmt --all --check — clean
  • All four new cassettes replay with every provider key unset
  • Each fix's regression verified to fail on main first (retry assertion, structured-answer mismatch, SIGABRT)

Changelog

  • (rig-core) [breaking] ProviderCapabilities gains supports_native_output_schema; construct with ..Default::default() or the with_* builders rather than a full struct literal
  • (rig-core) a provider error arriving with no HTTP status is retried when its machine code says the condition is transient, so a mid-stream overload or rate limit is no longer permanent
  • (rig-core) Claude Opus 5 and Sonnet 5 get their published 128K max_tokens default, and the table resolves a gateway's vendor-prefixed model id
  • (rig-core) a recursive JSON schema is refused with an error naming the cycle instead of overflowing the stack and aborting the process
  • (rig-vertexai, rig-gemini-grpc) warn when a request's output_schema, documents, tool_choice or additional_params cannot be carried, and declare that the schema is not carried
  • (rig-ecs) a run that asked for a schema is no longer settled on an answer that does not satisfy it when no output tool was due: the run reprompts once with the schema, then settles

Migration

ProviderCapabilities is #[non_exhaustive]-shaped in practice: build it with ProviderCapabilities::default().with_native_output_schema(false) rather than a struct literal. A provider that does not put CompletionRequest::output_schema on the wire should declare false; the default stays true.

policy::resolve_output is unchanged. A rig-ecs host on a provider that declares supports_native_output_schema == false and wants its schema enforced should ask for OutputKind::Tool.

A provider error delivered inside a stream carries no HTTP status, so
`is_retryable` fell through to `transient.unwrap_or(false)` and reported
every one of them as permanent. rig-ecs retries only when
`ErrorReport::retryable` is set, so a mid-stream overload or throttle
ended the run without spending any of the budget the host configured.
The classification now reads the provider's own machine code, which is
all such a frame says about itself.

Anthropic's `max_tokens` table knew only the 4-series, so every Claude 5
request was refused before it left the process — a hard error, not
retryable, from a runtime that supplies no default of its own. The table
now covers Claude 5 and sees through a gateway's vendor prefix.

`resolve_output` decided native structured output from
`composes_native_output_with_tools`, which answers a different question,
and `Native` settles a run on whatever text arrives. Against the twelve
providers that drop `output_schema` the host asked for a schema, nothing
carried it, nothing checked the answer, and free text was reported as the
structured result. Providers now declare whether they carry the schema at
all, the runtime routes a schema-bearing run to its output tool when they
do not, and the two crates that dropped it silently say so.

Gemini's `$ref` inliner recursed forever on a schema that refers to
itself, overflowing the stack and aborting the process — in an ECS host
every other run dies with it and no failure is ever written. It now
refuses the request and names the cycle.

Each fix has a cassette recorded against the live wire; where one could
not be induced, the test says so and says what was recorded instead.
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