fix: four provider defects the ECS runtime cannot see past - #2524
Open
gold-silver-copper wants to merge 3 commits into
Open
gold-silver-copper wants to merge 3 commits into
gold-silver-copper wants to merge 3 commits into
Conversation
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.
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
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.
Four provider defects the ECS runtime cannot see past
Each of these silently breaks a promise
crates/rig-ecsmakes to its hosts. All four were confirmed against2355e0966before any edit; the evidence is inline.1 — a stream-borne overload is never retried
completion_error_from_bodybuilds every in-band stream error throughProviderResponseError::without_status, leavingstatus: Noneandtransient: None, sois_retryable()fell totransient.unwrap_or(false)→ false. The runtime's retry gate keys exactly on that flag (rig-ecs/src/systems/mod.rs:2275), so a mid-streamoverloaded_erroror 429 ended the run without spending a singleProviderRetried— while the identical condition on a non-streamed effect retried normally.is_retryablenow consults the provider's own machine code when there is no status and the transport offered no verdict. The code table lives besideretryable_statusand 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 endrig-ecs/tests/run_provider_retry.rs::an_overload_reported_without_a_status_is_reissued. The latter fails onmainat 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) assertedretryable == retryable_status(status)for a frame that carries no status, which is now the code's call; andopenai::cassette::ecs_stream_faults::error_event_after_content_...observed two streams because itsserver_errorframe is now re-issued — it spendsProviderRetries(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_modelprefix-matched onlyclaude-*-4*; anything else gotNoneand the prelude hard-errorsRequestError("max_tokens must be set for Anthropic").MaxTokensis an optional agent setting in rig-ecs and aRequestErroris not retryable, so every rig-ecs run against Claude 5 without an explicitMaxTokensfailed 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_settingdrives a rig-ecs run with noMaxTokenson the agent. With the Claude 5 rows removed it fails before any request leaves the process:3 — a structured answer nobody asked for or checked
resolve_outputreadcomposes_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 dropoutput_schema; inNativemode 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 becameRunResult.ProviderCapabilitiesgainssupports_native_output_schema. The OpenAI-compatible family derives it from theSUPPORTS_RESPONSE_FORMATflag it already keeps, so the seven droppers there become truthful at a stroke;rig-vertexaiandrig-gemini-grpcdeclare it false and now warn instead of dropping silently (gRPC also warns fordocuments,tool_choiceandadditional_params).The routing half of this fix is not in this PR, and the reason is measured. I implemented it —
resolve_outputrouting a schema-bearing run to its output tool whenever the provider cannot carry the schema — and the recorded parity corpus rejected it: the threedeepseek_output_prompted_*cells change the request they send (afinal_resulttool 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_outputkeeps its old signature and the limitation is stated at its doc comment: a host on a dropping provider should setOutputKind::Toolitself.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 samepolicy::text_satisfies_schemacheck whenever a schema is wanted and no output tool validated the answer: the run spends oneOutputRetriesand 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 declaressupports_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
tracingsubscriber would see:rig-vertexai(output_schema,documents) andrig-gemini-grpc(those two plus tool choice and additional params). The capability derivation is pinned both ways inproviders/xai/completion/tests.rs— xai carriesresponse_format, so it declares native schema supporttrueeven though it cannot compose one with tools, and deepseek drops it and declaresfalse. 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: 107deepseek_*.effects.jsonfixtures now recordsupports_native_output_schema: falsein their handler descriptors, with the matching native policy identities intest-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
$refinliner recursed forever on a self-referential schema. Not an error — a stack overflow, so the host dies and noFailureis ever written. Confirmed onmain:fatal runtime error: stack overflow, aborting/SIGABRT.resolve_refsnow 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/$refschema underparametersJsonSchema; that is recorded intests/cassettes/gemini/recursive_schema/. But the deletion is not available and the swap does not pay:Schemaconversion is still consumed byGenerationConfig::response_schemaand byrig-gemini-grpc, whose gRPCFunctionDeclarationhas noparametersJsonSchemafield, so the converter cannot be deleted without moving those too;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.
anthropic/max_tokens/claude_5_default_max_tokens{"max_tokens":128000,…,"model":"anthropic/claude-opus-5"}and a real answeranthropic/max_tokens/claude_5_ecs_runMaxTokensgemini/recursive_schema/gemini_accepts_parameters_json_schema"child":{"name":"b"}openai/structured_output/native_schema_still_reaches_the_wireanthropic/retry_classification/healthy_stream_still_completesTwo 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.comfor 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 passedcargo test --locked -p rig-ecs— 351 passedcargo test -p rig --all-features --test anthropic— 692 passed, no golden regeneratedcargo xtask verify --check default-tests— PASS (1420s), the workspace suitecargo xtask verify --check ecs-parity— PASS (2627s), the recorded parity corpuscargo xtask verify --check source-guards,--check fmt— PASScargo clippy --locked --all-features --all-targets -- -D warnings— cleancargo fmt --all --check— cleanmainfirst (retry assertion, structured-answer mismatch,SIGABRT)Changelog
ProviderCapabilitiesgainssupports_native_output_schema; construct with..Default::default()or thewith_*builders rather than a full struct literalmax_tokensdefault, and the table resolves a gateway's vendor-prefixed model idoutput_schema,documents,tool_choiceoradditional_paramscannot be carried, and declare that the schema is not carriedMigration
ProviderCapabilitiesis#[non_exhaustive]-shaped in practice: build it withProviderCapabilities::default().with_native_output_schema(false)rather than a struct literal. A provider that does not putCompletionRequest::output_schemaon the wire should declarefalse; the default staystrue.policy::resolve_outputis unchanged. A rig-ecs host on a provider that declaressupports_native_output_schema == falseand wants its schema enforced should ask forOutputKind::Tool.