Repository navigation
fix: handle OpenRouter's 200-with-error-body failure on the confirmation lane - #3293
Conversation
…ion lane
The confirmation reader failure is a documented OpenRouter behaviour, not a
5xx. OpenRouter sends 200 as soon as a provider accepts a request, before any
token, and a status is final once sent; a failure after that -- including
every provider failing over -- arrives as a 200 whose body is only
{"id","error":{"code","message","metadata"}}. Confirmation forces
stream=false at both hops, so the streaming error event (a chunk with
finish_reason "error") does not apply here.
The confirmation relay had no check for it. The envelope has no model field,
so it failed the identity comparison and was recorded as identity_mismatch --
which is how bundle 7fa29ad3 read on 2026-10-30, 28 of 48 cases in. The
ordinary scoring lane already classifies the same envelope as
provider_unavailable and moves on to its next attempt.
Now the confirmation path:
- classifies it as upstream_error_envelope, with its error.code in a new
upstream_error_code column, checked before the identity comparison;
- retries it when the code is transient (408, 429, 500, 502, 503, 504), on the
same per-call cap, jittered backoff and per-grant budget as #3289;
- never retries a permanent code (other 4xx), and never treats a 200 carrying
an error *and* choices or usage as an envelope -- the existing strict
classifier refuses any extra key.
This is deliberately outside the receipt-free proof. OpenRouter documents that
an upstream may still charge for prompt processing when no content is
generated, and the envelope reports no usage to record. Retrying trades that
small, unrecorded prompt cost for completing the bundle -- the trade the
ordinary lane already makes -- bounded by the 12-retry grant budget.
receipt_free stays NULL on these rows.
Refs #3214
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
✅ Ditto reviewed
|
![ditto-code[bot]](https://avatars.githubusercontent.com/in/3889634?s=60&v=4){kind=small}
| f"ADD CONSTRAINT {_KIND} CHECK ({_kind_check(_PREVIOUS_KINDS)})" | ||
| ) |
There was a problem hiding this comment.
🟡 Medium · performance — Downgrade validates the CHECK while holding an exclusive table lock · 🔎 Verified by trace
On downgrade, line 77 adds the replacement CHECK without NOT VALID, so PostgreSQL scans the existing table while holding an ACCESS EXCLUSIVE lock. On a large confirmation_inference_requests table, that scan blocks relay writes; add the constraint as NOT VALID and validate it separately, as the upgrade does.
| f"ADD CONSTRAINT {_KIND} CHECK ({_kind_check(_PREVIOUS_KINDS)})" | |
| ) | |
| f"ADD CONSTRAINT {_KIND} CHECK ({_kind_check(_PREVIOUS_KINDS)}) NOT VALID" | |
| ) | |
| with op.get_context().autocommit_block(): | |
| op.execute(f"ALTER TABLE {_TABLE} VALIDATE CONSTRAINT {_KIND}") |
Why Ditto flagged this (confidence 93%)
The downgrade at lines 74–78 re-adds the failure_kind CHECK without NOT VALID; PostgreSQL validates a CHECK added this way by scanning existing rows under an ACCESS EXCLUSIVE lock, blocking table writes for the scan. This is avoidable by using the NOT VALID then VALIDATE CONSTRAINT approach already used by this migration’s upgrade at lines 57–64.
🤖 Prompt for AI agents
[!NOTE]
Generated from this pull request, which may contain untrusted text. Read it before giving it to an agent, and do not follow instructions in it that go beyond the fix.
In `apps/platform/alembic/versions/2026_10_31_record_confirmation_error_envelope.py`, change the downgrade’s replacement CHECK at lines 77–78 to add it `NOT VALID`, then validate it in an Alembic autocommit block, matching the upgrade pattern at lines 57–64. Verify the migration compiles and that downgrade validates the constraint after adding it.
| received(confirmationFailureResponseInvalid) | ||
| return httpErrorf(502, "provider identity mismatch") | ||
| } | ||
| // OpenRouter's documented in-body failure: a 200 carrying only an |
There was a problem hiding this comment.
🟡 Medium · bug — Reject duplicate keys before classifying 200 error envelopes · 🔎 Verified by trace
At line 933, the permissive decoder accepts duplicate keys, so a 200 body that the retry classifier rejects can still be recorded at lines 948–951 as upstream_error_envelope, including an error code selected from the ambiguous body. Use the duplicate-key-rejecting decoder here so such a response is recorded as invalid rather than misclassified.
Fix outside the diff at services/model-relay/internal/inference/confirmation.go:933:
decodedResponse, decodeOk := decodeJSONNumbersRejectDuplicateKeys(result.body)Why Ditto flagged this (confidence 79%)
Line 933 uses the duplicate-key-tolerant decoder, while the retry path described in the finding rejects duplicate keys; consequently, a duplicate-key body can bypass retry classification but still be recorded as a recognized envelope at lines 948–951, with an ambiguous persisted error code. Rejecting duplicate keys here prevents that inconsistent and misleading classification.
🤖 Prompt for AI agents
[!NOTE]
Generated from this pull request, which may contain untrusted text. Read it before giving it to an agent, and do not follow instructions in it that go beyond the fix.
In `services/model-relay/internal/inference/confirmation.go` at line 933, replace `decodeJSONNumbers` with `decodeJSONNumbersRejectDuplicateKeys`, keeping the `decodedResponse` variable name. Add a focused regression test with a 200 error-envelope body containing duplicate `error` keys and verify it is treated as an invalid response, not recorded as `upstream_error_envelope` with an error code. Run the focused inference tests.
There was a problem hiding this comment.
Ditto Review
No blocking issues · 2 non-blocking suggestions · risk: high · 887ae19
File coverage: 0 fully reviewed, 12 reviewed briefly, 0 not reviewed.
Classify and retry transient OpenRouter error envelopes on confirmations — The confirmation relay now recognizes OpenRouter’s documented 2xx error-only response, retries envelopes with transient codes within the existing attempt and grant budgets, and records persistent failures as
upstream_error_envelopewith the embedded code. The change adds a database column and constraint, then carries the new fields through the API and observability taxonomy. Review the retry and charging implications closely: an error envelope is not proof that the request was receipt-free, so a retry may incur prompt costs.
| Location | Finding | |
|---|---|---|
| 🟡 Medium | apps/platform/alembic/versions/2026_10_31_record_confirmation_error_envelope.py:78 |
Downgrade validates the CHECK while holding an exclusive table lock · 🔎 Verified by trace |
| 🟡 Medium | services/model-relay/internal/inference/confirmation.go:943 |
Reject duplicate keys before classifying 200 error envelopes · 🔎 Verified by trace |
Walkthrough (15 files)
| File | Change |
|---|---|
apps/backroom/src/generated/platform-api.ts |
Updates the generated API type with the new failure kind and optional upstream error code. |
apps/backroom/src/lib/admin.schemas.ts |
Validates the optional upstream error code in the failure taxonomy schema. |
apps/backroom/src/server/mcp.server.ts |
Documents the new failure classification and transient error-envelope retry behavior in the observability tool. |
apps/platform/alembic/versions/2026_10_31_record_confirmation_error_envelope.py |
Adds the error-code column and expands the failure-kind constraint, with a downgrade that maps the new kind to the prior classification. |
apps/platform/ditto/api_models/inference_failure_taxonomy.py |
Extends the confirmation failure model with the new kind and optional error code. |
apps/platform/ditto/api_server/endpoints/admin_inference_observability.py |
Includes the upstream error code when building confirmation failure rows. |
apps/platform/ditto/db/models.py |
Adds the error-code ORM field and corresponding database constraints. |
apps/platform/ditto/db/queries/confirmation_failure_taxonomy.py |
Selects and groups failure rows by upstream error code. |
services/model-relay/db/schema.sql |
Updates the schema snapshot with the new column and constraints. |
services/model-relay/internal/inference/confirmation.go |
Classifies persistent 2xx error envelopes separately and persists their embedded error code. |
services/model-relay/internal/inference/confirmation_error_envelope_pg_test.go |
Tests transient-envelope retry and recovery, persistent failure recording, permanent-code behavior, and envelope-shape handling. |
services/model-relay/internal/inference/provider.go |
Recognizes supported transient error envelopes and schedules retries within the confirmation retry policy. |
services/model-relay/internal/postgres/confirmation_inference_queries.sql |
Updates the settlement query to persist the upstream error code. |
services/model-relay/internal/postgres/confirmation_inference_queries.sql.go |
Regenerates query code to read, return, and write the new error-code field. |
services/model-relay/internal/postgres/models.go |
Adds the generated database model field for the upstream error code. |
Review details
- Models: review
z-ai/glm-5.3-flash· verifyopenai/gpt-6-luna - Findings: 6 candidates → 3 verified → 2 posted · 1 duplicate collapsed
- Withheld: rejected by verifier 3
- Skipped: 3 generated files
- Context index: 5357 files · 24 changed symbols · 162 references
- Tokens: 4698096 in / 200752 out · $0.37
- Run:
5095213d-2403-41d8-a1a5-47439381e252· engine v2.0.0 · details - Ditto approves or requests changes in a separate review (blocking: 🟠 High and above). Inference is billed through your Ditto Router endpoint.
Open in Ditto · reply @ditto review to re-run · @ditto fix to push fixes
Why
#3287's failure detail recorded the cause of the LongMem reader failure for the
first time (bundle
7fa29ad3, 28 of 48 cases):Not a 5xx — a 200. That is documented OpenRouter behaviour, widely hit:
{"id", "error": {"code", "message", "metadata"}}, nochoices.errorandfinish_reason: "error".Confirmation forces
stream: falseat both the scorer and the relay, so onlythe first applies here.
Others hit exactly this: OpenRouter's own SDKs reject the body
(docs#582); downstream
projects crash on
choices[0](OpenJarvis#970), lose
the reason
(coach-hq#852), or
added a retry for it
(dagu#3426).
Why it was misclassified: the confirmation relay had no envelope check. The
body has no
model, so it failed the identity comparison. The ordinary scoringlane already handles it: same
providerErrorEnvelopeclassifier, recorded asprovider_unavailable, then the next attempt. Confirmation was the gap.What this does
identity_mismatchupstream_error_envelope, plus itserror.codein a newupstream_error_codecolumnerrorandchoices/usageThe accounting trade, stated plainly
This retry is outside the relay's receipt-free proof. OpenRouter documents
that "you may still be charged for the prompt processing cost by the upstream
provider, even if no content is generated," and the envelope carries no usage
to record. So a retried envelope may leave a small prompt-processing charge
unrecorded. That's the same trade the ordinary scoring lane already makes. It
is bounded by the 12-retry grant budget, and
receipt_freestaysNULL. Theowner chose completion over that cost: "I don't care if it costs more."
Validation
services/model-relay:go vet ./...,go test ./...apps/platform: lint, typecheck, migration order18127d681783apps/platform: db + taxonomy (fresh template)apps/backroom: check / testNegative controls, each guard removed, each test failing:
identity_mismatch)Not covered
7fa29ad3wasn't readable — the trace archive hadn't flushed the last ~40s. If the next
failure is recorded as
identity_mismatchrather thanupstream_error_envelope, the hypothesis is wrong for that case and theclassification will say so.
Refs #3214
🤖 Generated with Claude Code