diff --git a/devlog/_plan/260920_meaning_preservation_batch/030_lane_c2.md b/devlog/_plan/260920_meaning_preservation_batch/030_lane_c2.md new file mode 100644 index 00000000000..c07447cf53f --- /dev/null +++ b/devlog/_plan/260920_meaning_preservation_batch/030_lane_c2.md @@ -0,0 +1,220 @@ +# Lane C2 — telemetry projections on the landed vocabulary + +Status: OPEN. Branch `codex/260920-lane-c2-telemetry-projections`, cut from `dev` at +`043aa435ff8f86095f55cbe08f74d45b9858da59`, which is where lane C's stage, cause and resend +vocabulary landed ([020_lane_c.md](020_lane_c.md)). One branch, ordered commits, one pull request +to `dev`. + +## What this lane fixes + +The completion condition for bundle 14 is that the UI, the durable log and Prometheus agree on +logical and physical counts and on terminal classification. They did not, and the disagreement was +not subtle: + +- Three surfaces classified a terminal three different ways. The durable row carried + `terminalStatus` and `closeReason`; the exporter kept its own private `classifyResult`; the + dashboard read the numeric HTTP status and nothing else. A turn cut short by + `max_output_tokens` is durably `status: 200, terminalStatus: "incomplete"`, which the exporter + reported as `incomplete` and the dashboard rendered as a green 200. The metric said incident and + the operator saw success, for the same request. +- The dashboard showed no physical send count at all. `sendCount` and `spend` were never rendered, + so an attempt that sent three times appeared as one row with nothing to say otherwise. +- The dashboard's recovery-kind union had drifted to nine of the durable thirteen, so `key-401`, + `oauth-account-429`, `opaque-blob-rejection` and `reasoning-effort-downgrade` all reached the + operator as "Unknown recovery reason" — four real causes rendered as the absence of one. + +The fix is one classifier in `src/usage/request-outcome.ts` that the exporter imports, the +management payload already carries, and the dashboard calls. Agreement is structural rather than a +rule someone maintains. The data was never missing: `requestLogDto` spreads the whole durable +entry, so the page only had to declare the fields and stop reinventing the precedence. + +The exporter's result label set **is** the shared vocabulary rather than a copy of it. Restating +those four strings is what let the two drift while both looked correct. + +### The dashboard may only reach a contract leaf + +Adversarial review caught this before the first push and it is worth recording, because the +mistake is invisible from the backend side. The dashboard is a separate TypeScript project with +`erasableSyntaxOnly`, and a **type-only** import still pulls the imported file's entire import +graph into that project. Importing the recovery roster from `src/usage/log.ts` therefore dragged +`node:fs`, `node:crypto` and the config barrel into the browser build, where a parameter property +in `src/config/atomic-write.ts` does not compile. "It is only a type import" is not a defence. + +So the names a browser legitimately needs now live in `src/usage/telemetry-contract.ts`, which has +no imports at all and must keep none. `src/usage/log.ts` re-exports them so every existing importer +keeps its path, and `src/lib/request-failure-model.ts` stopped depending on the ledger module as a +side effect. Three cases hold the boundary: the page must not name `src/usage/log`, the contract +must have no imports, and the outcome module must reach nothing but the contract. + +The same review caught the send total disagreeing in the other direction. An earlier draft reported +`max(sends, reserved)`, on the reasoning that a budget charge with no attempt row behind it is +still a send that left. That is true, and it still made the dashboard say four where the exporter, +summing the same attempts the recorder summed, said three. Two defensible formulas are two answers; +the surfaces now read the recorded totals and recompute nothing, and a case asserts the exporter's +`opencodex_physical_sends_total` equals what the dashboard shows. + +## Dispositions, updated + +### Who is credited, and why only one + +Attribution follows what was actually taken, not what was read. Only #2366's work is carried here, +so only its author carries a `Co-authored-by` trailer, and that trailer sits in a branch commit so +it survives the squash. The other three were analysed in depth and their designs informed the +deferral reasons below, but no line of their work is in this branch; crediting them would claim a +landing that did not happen and would make the contributor graph say something false. + +A note on the gate, because getting this right took two attempts. `pr-carry-attribution.cjs` looks +for a carry verb and reads the pull request numbers in the eighty characters after it. A first +draft of the description read "#2366's rehydration half is carried and ... ; #3748 is blocked", +which put #3748 inside that window and asked for a trailer naming an author whose work is +deliberately absent. Rewording split the sentences — but it also moved #2366 out of every window, +so the check went green by having nothing left to check. A gate that passes because the trigger was +removed is not evidence. The provenance sentence now names #2366 after the verb, in the commit +itself, so the check resolves the author and matches the trailer instead of skipping. + +### #2366 (chilung-cgu) — partially carried + +**Carried:** the rehydration and UI-projection half. The durable terminal facts now reach an +operator instead of stopping at the API boundary. + +**Not carried, and why:** `FailureSide` and the seven-member `FailureStage` are a second +attribution vocabulary beside the one that just landed, and defining two is the exact class that +blocked 2.60.0. The PR also widens `transportPhase` and `terminalSource` from their existing closed +unions to arbitrary strings, which would let bounded upstream-controlled text into the durable row; +those validators stay. Copying the request-relative timeline into an attempt at finalization is +simply wrong — request-relative elapsed values do not become attempt-relative by being copied. + +**Next step, specified:** persist `failureStage?: RequestFailureStage` and +`failureCause?: RequestFailureCause` on `PersistedUsageAttempt`, projected onto the entry, with +`resendPermission` computed at read time and never persisted. That is the smallest durable record +that makes a failure attributable, and it is the prerequisite for #3748 below. It is not in this +branch because it is new classification logic on the finalization path, and a branch whose only +verification is static review plus hosted CI should not add a new derivation and the surface that +consumes it in the same change. + +### #3748 (yansigit) — still deferred, reason updated + +The earlier reason was the parallel SQLite store. That still holds, but the blocking reason today is +narrower and more useful: **the recorder does not yet record why a request finally failed.** +`causeForRecoveryKind` answers why a *recovery* was attempted, which is a different question — a +request that failed without any recovery, or that recovered and then failed for another reason, +has no cause to group by. A derived failure ledger therefore cannot compute a grouping key today +without reading `errorCode` or `upstreamError`, which are open strings. + +The design is otherwise settled and should be built once the field above exists: group failed rows +scanned through the existing `scanUsageLedgerCooperatively` by a versioned fingerprint over closed +vocabularies only — cause, status class, inbound protocol, terminal status, close reason, transport +phase, terminal source — with fixed tuple positions so a missing field cannot collide structurally. +No provider, no model, no account label, no free-text signature. First-seen, last-seen and count +fall out of the scan; no second timestamp list is retained. + +Two parts of the original are not derivable from request history at all: the mutable +`monitoring/dispatched/fixed/ignored` remediation status and its free-text notes. Those are +operator state, not event history, and need their own owner rather than being presented as a +derived ledger. + +### #3983 (yansigit) — still deferred as an emission path, reason updated + +The earlier reason was "a second emission path". The updated reason is stronger: the path is not +ephemeral. `emitDebugLine` writes the in-process ring **and** stderr, and stderr is redirected to +the service log under both launchd and systemd, so an installed service gets a durable per-event +record with its own retention, sequencing, request identity and masking — beside the ledger and +sourced from something other than it. + +Two further facts: four of its eighteen files no longer apply, including +`run-turn-execution.ts` where carrying it literally would regress the current send-budget +accounting; and its per-content HMAC is a process-global random key, so equality of every prompt, +tool name and error message is correlatable for the process lifetime. + +**The useful half, specified:** a bounded normalized summary on the attempt — adapter events, +actually relayed events, semantic bytes, side-effect events, terminal events — counted where the +event is delivered rather than where it is read. That keeps the signals worth having (missing +terminal, adapter-to-relay loss, empty output, partial output size) and inherits the ledger's +normalization, masking and retention instead of inventing its own. + +### #5063 (Vocllum) — still deferred, reason updated + +The earlier reason was "a separate product slice with GUI surface". The updated reason is a +correctness one found while reviewing it against current `dev`: + +- Retention captures the file size, copies a retained suffix to a temp file and renames. A row + appended by **another process** between the size snapshot and the rename is silently dropped. + The PR's own "concurrent re-entrancy" test performs two sequential calls and says it cannot test + true concurrency. +- Both the temp-file fsync and the parent-directory fsync failures are swallowed, and replacement + proceeds anyway. There is no revision, inode or digest comparison immediately before the rename. +- It hand-rolls the copy rather than using `src/config/atomic-write.ts`. +- It does not clear the 2,000-entry request-log ring, so rows deleted from disk stay visible + through `/api/logs` until eviction or restart. +- `src/server/index.ts` is 892 lines against a 893-line cap, so the startup wiring cannot land as + written without offsetting a line elsewhere. + +Retention on the canonical ledger is the right architecture and its derived-index invalidation is +correct in shape. It needs a ledger-owner serialization and revision contract first. + +### The dashboard roster defect — fixed here + +Recorded in [020_lane_c.md](020_lane_c.md) as deferred because it needed strings across ten +catalogs and a screenshot. It is fixed in this branch because the agreement condition required +touching this page anyway. The union is now derived from the durable roster and a `satisfies` +clause makes the next added kind a typecheck failure rather than a silent fallback. + +**This trips the `missing_ui_screenshot` gate.** That gate fires on changed paths under `gui/`, +not on words in the description, and this lane may not build or run the GUI, so it cannot produce +the screenshot itself. A maintainer comment or the `gui-screenshot-waived` label is the documented +resolution; the visible change is four attempt rows that said "Unknown recovery reason" now naming +their cause, plus an outcome row and a send count in the detail dialog. + +## Retention and masking, stated in one place + +The instruction was to unify rather than add a fifth policy. The honest form of that today is to +record what already exists and bind projections to it, because every bound below is already +enforced and none of them needs a new owner: + +| Surface | Bound | +| --- | --- | +| Usage ledger rows (scanner) | 1 MiB per row, 1 MiB read chunks, 64 KiB boundary digest | +| Startup request-log hydration | 64 MiB read ceiling | +| Management usage snapshot | 64 MiB, 500,000 rows | +| Usage metadata strings | sanitized, capped at 64 characters | +| Upstream error text | `redactSecretString`, capped at 500 characters | +| Conversation correlation | rejects over 4,096 characters, persists a 32-hex digest | +| Debug ring | 2,000 lines, 16 KiB per line, 64 subscribers | +| Request-log ring | 2,000 entries plus app-owned-memory eviction | +| Routing history | rebuildable projection; pages capped, records over 1 MiB omitted | +| Email masking | masked unless `privacy.maskEmails: false` | + +The policy this lane adopts: **masking happens at capture and projection boundaries, retention +deletes canonical rows, and every derived surface inherits both.** A projection does not get its +own TTL, its own row cap or its own redaction pass. The failure projection specified above obeys +this by construction — it holds only aggregates and a scanner checkpoint, and discards them when +the source is replaced. + +## Issues + +#4191 and #5180 stay open and are not closed here. What narrowed: the dashboard now reports the +terminal classification and the send count the durable row always carried, so an operator can tell +an incomplete turn from a successful one without reading the ledger. What remains unchanged: the +WebSocket-to-SSE fallback for #4191, and the shared cooldown and `Retry-After` handling for #5180. + +## Verification + +Static source review plus exact-head hosted CI. + +NOT RUN on this branch, by instruction: `bun run test`, any individual `bun test` file, +`bun run typecheck`, `bun run build:gui`, `bun run lint:gui`, `bun install`, +`bun run structure:check`, `bun run privacy:scan`, and any live `ocx` execution. None of these may +be recorded as passing. + +Checked statically on this branch: + +- all eleven new label keys are present in all ten catalogs, and the catalog edits are purely + additive (+11 lines, 0 removed, per file); +- the ten catalogs are explicitly exempt from the file-size ratchet, for the reason the exemption + list gives: they grow by one line per UI string across every locale at once; +- no ratchet-capped file is touched by this branch; +- `scripts/test-layout/layout.json` and `tests/fixtures/test-layout-expected.json` agree key for + key, and the new test's regex seed resolves to the same domain it is registered to, which is the + oracle that failed lane C on its first push; +- no test restates a source constant: the outcome vocabulary, the recovery roster and the label + keys are all read from the modules that declare them. diff --git a/gui/src/i18n/de.ts b/gui/src/i18n/de.ts index 1f75761045e..28c3e235794 100644 --- a/gui/src/i18n/de.ts +++ b/gui/src/i18n/de.ts @@ -873,6 +873,17 @@ export const de: Record = { "logs.detail.attempt.recovery.image413": "Bildnutzlast zu groß (413)", "logs.detail.attempt.recovery.emptyCompletion": "Wiederholung nach leerer Antwort", "logs.detail.attempt.recovery.consoleGoUpload": "Console-Upload erneut versucht", + "logs.detail.attempt.recovery.key401": "API-Schlüssel erneut authentifiziert", + "logs.detail.attempt.recovery.oauthAccount429": "Konto rate-limitiert (429)", + "logs.detail.attempt.recovery.opaqueBlobRejection": "Veralteter verschlüsselter Zustand verworfen", + "logs.detail.attempt.recovery.reasoningEffortDowngrade": "Reasoning-Aufwand reduziert", + "logs.detail.outcome.label": "Ergebnis", + "logs.detail.outcome.completed": "Abgeschlossen", + "logs.detail.outcome.failed": "Fehlgeschlagen", + "logs.detail.outcome.incomplete": "Unvollständig", + "logs.detail.outcome.aborted": "Abgebrochen", + "logs.detail.sends.label": "Upstream-Sendungen", + "logs.detail.sends.unresolved": "ungeklärt", "logs.detail.attempt.recovery.unknown": "Unbekannter Wiederherstellungsgrund", "logs.detail.reason.usage_missing": "Nutzung wurde nicht gemeldet.", "logs.detail.reason.usage_unsupported": "Dieser Anbieter meldet keine Nutzung.", diff --git a/gui/src/i18n/en.ts b/gui/src/i18n/en.ts index 4146debb96d..1583d8c6817 100644 --- a/gui/src/i18n/en.ts +++ b/gui/src/i18n/en.ts @@ -922,6 +922,17 @@ export const en = { "logs.detail.attempt.recovery.image413": "Image payload too large (413)", "logs.detail.attempt.recovery.emptyCompletion": "Empty completion retry", "logs.detail.attempt.recovery.consoleGoUpload": "Console upload retry", + "logs.detail.attempt.recovery.key401": "API key re-authentication", + "logs.detail.attempt.recovery.oauthAccount429": "Account rate-limited (429)", + "logs.detail.attempt.recovery.opaqueBlobRejection": "Stale encrypted state dropped", + "logs.detail.attempt.recovery.reasoningEffortDowngrade": "Reasoning effort downgraded", + "logs.detail.outcome.label": "Outcome", + "logs.detail.outcome.completed": "Completed", + "logs.detail.outcome.failed": "Failed", + "logs.detail.outcome.incomplete": "Incomplete", + "logs.detail.outcome.aborted": "Aborted", + "logs.detail.sends.label": "Upstream sends", + "logs.detail.sends.unresolved": "unresolved", "logs.detail.attempt.recovery.unknown": "Unknown recovery reason", "logs.detail.reason.usage_missing": "Usage was not reported.", "logs.detail.reason.usage_unsupported": "This provider does not report usage.", diff --git a/gui/src/i18n/fr.ts b/gui/src/i18n/fr.ts index 1153c7329b2..01794e0dacd 100644 --- a/gui/src/i18n/fr.ts +++ b/gui/src/i18n/fr.ts @@ -898,6 +898,17 @@ export const fr: Record = { "logs.detail.attempt.recovery.connectionReset": "Réinitialisation de la connexion", "logs.detail.attempt.recovery.emptyCompletion": "Nouvelle tentative après une réponse vide", "logs.detail.attempt.recovery.consoleGoUpload": "Nouvelle tentative d’envoi Console", + "logs.detail.attempt.recovery.key401": "Ré-authentification de la clé API", + "logs.detail.attempt.recovery.oauthAccount429": "Compte limité (429)", + "logs.detail.attempt.recovery.opaqueBlobRejection": "État chiffré obsolète abandonné", + "logs.detail.attempt.recovery.reasoningEffortDowngrade": "Effort de raisonnement réduit", + "logs.detail.outcome.label": "Résultat", + "logs.detail.outcome.completed": "Terminé", + "logs.detail.outcome.failed": "Échec", + "logs.detail.outcome.incomplete": "Incomplet", + "logs.detail.outcome.aborted": "Interrompu", + "logs.detail.sends.label": "Envois en amont", + "logs.detail.sends.unresolved": "non résolus", "logs.detail.attempt.recovery.oauth401": "Réauthentification OAuth", "logs.detail.attempt.recovery.key429": "Clé soumise à une limitation de débit (429)", "logs.detail.attempt.recovery.rateLimit429": "Limitation de débit (429)", diff --git a/gui/src/i18n/ja.ts b/gui/src/i18n/ja.ts index 64a68f59419..ca56fadb2b1 100644 --- a/gui/src/i18n/ja.ts +++ b/gui/src/i18n/ja.ts @@ -834,6 +834,17 @@ export const ja: Record = { "logs.detail.attempt.recovery.image413": "画像ペイロードが大きすぎます (413)", "logs.detail.attempt.recovery.emptyCompletion": "空の完了を再試行", "logs.detail.attempt.recovery.consoleGoUpload": "Console アップロード再試行", + "logs.detail.attempt.recovery.key401": "API キーの再認証", + "logs.detail.attempt.recovery.oauthAccount429": "アカウントのレート制限 (429)", + "logs.detail.attempt.recovery.opaqueBlobRejection": "古い暗号化状態を破棄", + "logs.detail.attempt.recovery.reasoningEffortDowngrade": "推論強度を下げて再試行", + "logs.detail.outcome.label": "結果", + "logs.detail.outcome.completed": "完了", + "logs.detail.outcome.failed": "失敗", + "logs.detail.outcome.incomplete": "未完了", + "logs.detail.outcome.aborted": "中断", + "logs.detail.sends.label": "アップストリーム送信", + "logs.detail.sends.unresolved": "未確定", "logs.detail.attempt.recovery.unknown": "不明なリカバリ理由", "logs.detail.reason.usage_missing": "使用量が報告されませんでした。", "logs.detail.reason.usage_unsupported": "このプロバイダーは使用量を報告しません。", diff --git a/gui/src/i18n/ko.ts b/gui/src/i18n/ko.ts index d5a4a9392c5..b7436744aef 100644 --- a/gui/src/i18n/ko.ts +++ b/gui/src/i18n/ko.ts @@ -904,6 +904,17 @@ export const ko: Record = { "logs.detail.attempt.recovery.image413": "이미지 페이로드가 너무 큼 (413)", "logs.detail.attempt.recovery.emptyCompletion": "빈 응답 재시도", "logs.detail.attempt.recovery.consoleGoUpload": "Console 업로드 재시도", + "logs.detail.attempt.recovery.key401": "API 키 재인증", + "logs.detail.attempt.recovery.oauthAccount429": "계정 속도 제한 (429)", + "logs.detail.attempt.recovery.opaqueBlobRejection": "만료된 암호화 상태 제거", + "logs.detail.attempt.recovery.reasoningEffortDowngrade": "추론 강도 하향", + "logs.detail.outcome.label": "결과", + "logs.detail.outcome.completed": "완료", + "logs.detail.outcome.failed": "실패", + "logs.detail.outcome.incomplete": "미완료", + "logs.detail.outcome.aborted": "중단됨", + "logs.detail.sends.label": "업스트림 전송", + "logs.detail.sends.unresolved": "미확인", "logs.detail.attempt.recovery.unknown": "알 수 없는 복구 사유", "logs.detail.reason.usage_missing": "usage가 보고되지 않았습니다.", "logs.detail.reason.usage_unsupported": "이 프로바이더는 usage 보고를 지원하지 않습니다.", diff --git a/gui/src/i18n/ru.ts b/gui/src/i18n/ru.ts index 631637977eb..8478bb7027a 100644 --- a/gui/src/i18n/ru.ts +++ b/gui/src/i18n/ru.ts @@ -890,6 +890,17 @@ export const ru: Record = { "logs.detail.attempt.recovery.image413": "Слишком большой размер изображения (413)", "logs.detail.attempt.recovery.emptyCompletion": "Повтор пустого завершения", "logs.detail.attempt.recovery.consoleGoUpload": "Повтор загрузки Console", + "logs.detail.attempt.recovery.key401": "Повторная аутентификация API-ключа", + "logs.detail.attempt.recovery.oauthAccount429": "Ограничение частоты для аккаунта (429)", + "logs.detail.attempt.recovery.opaqueBlobRejection": "Устаревшее зашифрованное состояние отброшено", + "logs.detail.attempt.recovery.reasoningEffortDowngrade": "Уровень рассуждения понижен", + "logs.detail.outcome.label": "Итог", + "logs.detail.outcome.completed": "Завершено", + "logs.detail.outcome.failed": "Сбой", + "logs.detail.outcome.incomplete": "Не завершено", + "logs.detail.outcome.aborted": "Прервано", + "logs.detail.sends.label": "Отправки к провайдеру", + "logs.detail.sends.unresolved": "не подтверждено", "logs.detail.attempt.recovery.unknown": "Неизвестная причина восстановления", "logs.detail.reason.usage_missing": "Данные об использовании не были сообщены.", "logs.detail.reason.usage_unsupported": "Этот провайдер не сообщает данные об использовании.", diff --git a/gui/src/i18n/tr.ts b/gui/src/i18n/tr.ts index 1646ccb8251..7cef55db10e 100644 --- a/gui/src/i18n/tr.ts +++ b/gui/src/i18n/tr.ts @@ -909,6 +909,17 @@ export const tr: Record = { "logs.detail.attempt.recovery.image413": "Görsel boyutu çok büyük (413)", "logs.detail.attempt.recovery.emptyCompletion": "Boş tamamlama yeniden denemesi", "logs.detail.attempt.recovery.consoleGoUpload": "Console yüklemesi yeniden denendi", + "logs.detail.attempt.recovery.key401": "API anahtarı yeniden doğrulandı", + "logs.detail.attempt.recovery.oauthAccount429": "Hesap hız sınırına takıldı (429)", + "logs.detail.attempt.recovery.opaqueBlobRejection": "Eski şifreli durum bırakıldı", + "logs.detail.attempt.recovery.reasoningEffortDowngrade": "Akıl yürütme düzeyi düşürüldü", + "logs.detail.outcome.label": "Sonuç", + "logs.detail.outcome.completed": "Tamamlandı", + "logs.detail.outcome.failed": "Başarısız", + "logs.detail.outcome.incomplete": "Tamamlanmadı", + "logs.detail.outcome.aborted": "İptal edildi", + "logs.detail.sends.label": "Yukarı akış gönderimleri", + "logs.detail.sends.unresolved": "çözülmemiş", "logs.detail.attempt.recovery.unknown": "Bilinmeyen kurtarma nedeni", "logs.detail.reason.usage_missing": "Kullanım bildirilmedi.", "logs.detail.reason.usage_unsupported": "Bu sağlayıcı kullanım bildirmeyebilir.", diff --git a/gui/src/i18n/vi.ts b/gui/src/i18n/vi.ts index 3d726cc35e7..f8fc270edbf 100644 --- a/gui/src/i18n/vi.ts +++ b/gui/src/i18n/vi.ts @@ -896,6 +896,17 @@ export const vi: Record = { "logs.detail.attempt.recovery.image413": "Kích thước hình ảnh quá lớn (413)", "logs.detail.attempt.recovery.emptyCompletion": "Thử lại do hoàn thành rỗng (Empty completion retry)", "logs.detail.attempt.recovery.consoleGoUpload": "Thử lại tải lên Console", + "logs.detail.attempt.recovery.key401": "Xác thực lại khóa API", + "logs.detail.attempt.recovery.oauthAccount429": "Tài khoản bị giới hạn tần suất (429)", + "logs.detail.attempt.recovery.opaqueBlobRejection": "Đã bỏ trạng thái mã hóa cũ", + "logs.detail.attempt.recovery.reasoningEffortDowngrade": "Đã giảm mức suy luận", + "logs.detail.outcome.label": "Kết quả", + "logs.detail.outcome.completed": "Hoàn tất", + "logs.detail.outcome.failed": "Thất bại", + "logs.detail.outcome.incomplete": "Chưa hoàn tất", + "logs.detail.outcome.aborted": "Đã hủy", + "logs.detail.sends.label": "Số lần gửi lên nhà cung cấp", + "logs.detail.sends.unresolved": "chưa xác định", "logs.detail.attempt.recovery.unknown": "Lý do khôi phục không rõ", "logs.detail.reason.usage_missing": "Mức sử dụng không được báo cáo.", "logs.detail.reason.usage_unsupported": "Provider này không báo cáo mức sử dụng.", diff --git a/gui/src/i18n/zh-TW.ts b/gui/src/i18n/zh-TW.ts index cb4513ba7ef..bf67ea39360 100644 --- a/gui/src/i18n/zh-TW.ts +++ b/gui/src/i18n/zh-TW.ts @@ -2313,6 +2313,17 @@ export const zhTW: Record = { "logs.detail.attempt.recovery.image413": "圖片承載過大 (413)", "logs.detail.attempt.recovery.emptyCompletion": "空白完成重試", "logs.detail.attempt.recovery.consoleGoUpload": "Console 上傳重試", + "logs.detail.attempt.recovery.key401": "API 金鑰重新驗證", + "logs.detail.attempt.recovery.oauthAccount429": "帳號速率受限 (429)", + "logs.detail.attempt.recovery.opaqueBlobRejection": "已捨棄過期加密狀態", + "logs.detail.attempt.recovery.reasoningEffortDowngrade": "已降低推理強度", + "logs.detail.outcome.label": "結果", + "logs.detail.outcome.completed": "已完成", + "logs.detail.outcome.failed": "失敗", + "logs.detail.outcome.incomplete": "未完成", + "logs.detail.outcome.aborted": "已中止", + "logs.detail.sends.label": "上游傳送次數", + "logs.detail.sends.unresolved": "未結算", "logs.detail.attempt.recovery.unknown": "未知的復原原因", "logs.detail.estimate.provider_cost_overlay": "已使用供應商設定的價格覆蓋。", "logs.detail.estimate.priority_lower_bound": "無法取得已確認的 Priority 價格；目前顯示的估算是已知下限。", diff --git a/gui/src/i18n/zh.ts b/gui/src/i18n/zh.ts index aa5049f34ce..376acabd0fc 100644 --- a/gui/src/i18n/zh.ts +++ b/gui/src/i18n/zh.ts @@ -885,6 +885,17 @@ export const zh: Record = { "logs.detail.attempt.recovery.image413": "图片载荷过大 (413)", "logs.detail.attempt.recovery.emptyCompletion": "空完成重试", "logs.detail.attempt.recovery.consoleGoUpload": "Console 上传重试", + "logs.detail.attempt.recovery.key401": "API 密钥重新认证", + "logs.detail.attempt.recovery.oauthAccount429": "账号速率受限 (429)", + "logs.detail.attempt.recovery.opaqueBlobRejection": "已丢弃过期加密状态", + "logs.detail.attempt.recovery.reasoningEffortDowngrade": "已降低推理强度", + "logs.detail.outcome.label": "结果", + "logs.detail.outcome.completed": "已完成", + "logs.detail.outcome.failed": "失败", + "logs.detail.outcome.incomplete": "未完成", + "logs.detail.outcome.aborted": "已中止", + "logs.detail.sends.label": "上游发送次数", + "logs.detail.sends.unresolved": "未结算", "logs.detail.attempt.recovery.unknown": "未知的恢复原因", "logs.detail.reason.usage_missing": "未上报 usage。", "logs.detail.reason.usage_unsupported": "该提供方不支持上报 usage。", diff --git a/gui/src/pages/Logs.tsx b/gui/src/pages/Logs.tsx index d7fc3ab5c49..529e62cef74 100644 --- a/gui/src/pages/Logs.tsx +++ b/gui/src/pages/Logs.tsx @@ -28,6 +28,13 @@ import { validCachedRouteDecision, } from "./log-route-decision"; import { mergeLogDelta, parseLogPollResponse } from "./log-poll"; +import type { AttemptRecoveryKind, RequestSpendTotals } from "../../../src/usage/telemetry-contract"; +import { + classifyRequestOutcome, + requestPhysicalSends, + requestUnresolvedSends, + type RequestOutcomeClass, +} from "../../../src/usage/request-outcome"; function logsCacheKey(apiBase: string): string { return `ocx.logs.list.v1:${apiBase}`; @@ -101,21 +108,6 @@ interface LogDisplayMetrics { cost: CostResult; } -/** - * Recovery kinds recorded on a log attempt; rendered as localized labels in the logs - * detail dialog instead of raw wire values. - */ -type AttemptRecoveryKind = - | "transient-5xx" - | "connection-reset" - | "oauth-401" - | "key-429" - | "rate-limit-429" - | "anthropic-oauth-429" - | "image-413" - | "empty-completion" - | "console-go-upload-retry"; - interface LogAttempt { ordinal: number; provider: string; @@ -172,6 +164,15 @@ export interface LogEntry { durationMs: number; errorCode?: string; upstreamError?: string; + /** + * Semantic terminal facts. `/api/logs` has always carried these -- `requestLogDto` spreads the + * whole durable entry -- but this page declared neither, so it classified every request by its + * numeric HTTP status alone and reported an incomplete 200 as a plain success. + */ + terminalStatus?: string; + closeReason?: "terminal" | "client_cancel" | "non_stream" | "body_stall" | "body_overflow"; + /** Upstream spend for the whole logical request, aggregated across attempts and combo children. */ + spend?: RequestSpendTotals; usageStatus?: LogUsageStatus; usage?: UsageBreakdown; totalTokens?: number; @@ -298,17 +299,27 @@ const ESTIMATE_REASON_KEYS = { /** * i18n keys for every {@link AttemptRecoveryKind}, so the logs detail dialog renders a * localized label instead of the raw wire value (e.g. `rate-limit-429`). + * + * The union is now the durable roster rather than a copy of it. The copy had drifted to nine of + * thirteen members, so `key-401`, `oauth-account-429`, `opaque-blob-rejection` and + * `reasoning-effort-downgrade` all reached the operator as "Unknown recovery reason" -- four real + * causes rendered as an absence of information. `satisfies Record` is + * what now makes the next added kind a typecheck failure here instead of a silent blank. */ const RECOVERY_KIND_KEYS = { "transient-5xx": "logs.detail.attempt.recovery.transient5xx", "connection-reset": "logs.detail.attempt.recovery.connectionReset", "oauth-401": "logs.detail.attempt.recovery.oauth401", + "key-401": "logs.detail.attempt.recovery.key401", "key-429": "logs.detail.attempt.recovery.key429", "rate-limit-429": "logs.detail.attempt.recovery.rateLimit429", "anthropic-oauth-429": "logs.detail.attempt.recovery.anthropicOauth429", + "oauth-account-429": "logs.detail.attempt.recovery.oauthAccount429", "image-413": "logs.detail.attempt.recovery.image413", "empty-completion": "logs.detail.attempt.recovery.emptyCompletion", "console-go-upload-retry": "logs.detail.attempt.recovery.consoleGoUpload", + "opaque-blob-rejection": "logs.detail.attempt.recovery.opaqueBlobRejection", + "reasoning-effort-downgrade": "logs.detail.attempt.recovery.reasoningEffortDowngrade", } as const satisfies Record; /** Map a metric-unavailable reason to its i18n key. */ @@ -334,6 +345,25 @@ function verificationKey(status: MatchedPriceInfo["status"]): "logs.detail.verif return status === "verified" ? "logs.detail.verification.verified" : "logs.detail.verification.derived"; } +/** i18n key for each shared outcome class, total by construction. */ +const OUTCOME_KEYS = { + completed: "logs.detail.outcome.completed", + failed: "logs.detail.outcome.failed", + incomplete: "logs.detail.outcome.incomplete", + aborted: "logs.detail.outcome.aborted", +} as const satisfies Record; + +/** + * How this request ended, using the same classifier the Prometheus exporter uses. + * + * Calling the shared function rather than reimplementing the precedence is the point: the numeric + * status beside it can be 200 while the answer was never delivered, and reading the status first + * is exactly the disagreement this removes. + */ +function outcomeKey(entry: Pick) { + return OUTCOME_KEYS[classifyRequestOutcome(entry)]; +} + function statusColor(status: number): string { if (status >= 200 && status < 300) return "var(--green)"; if (status >= 400) return "var(--red)"; @@ -971,6 +1001,18 @@ function LogDetailDialog({

{t("logs.detail.section.basic")}

{t("logs.col.time")}{formatLogDateTime(detail.timestamp, localeTag, serverTimeZone)} + {t("logs.detail.outcome.label")} + {t(outcomeKey(detail))} + {detail.spend && ( + <> + {t("logs.detail.sends.label")} + + {requestPhysicalSends(detail.spend)} + {requestUnresolvedSends(detail.spend) > 0 + && ` (${t("logs.detail.sends.unresolved")}: ${requestUnresolvedSends(detail.spend)})`} + + + )} {t("logs.col.request")} {detail.requestId ?? "\u2014"} diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 74b27f8859c..a5b783faf81 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1207,6 +1207,7 @@ "request-log-conversation.test.ts": "usage", "request-log-estimate-cap.test.ts": "usage", "request-log.test.ts": "usage", + "request-outcome-agreement.test.ts": "usage", "request-pacing.test.ts": "usage", "reserve-auth-context.test.ts": "codex-integration", "reserve-availability.test.ts": "codex-integration", diff --git a/src/lib/request-failure-model.ts b/src/lib/request-failure-model.ts index 6066cdf616d..f12517f2a45 100644 --- a/src/lib/request-failure-model.ts +++ b/src/lib/request-failure-model.ts @@ -16,7 +16,7 @@ * usage or budget subsystems into a request path that did not already have them. */ import type { SendClass } from "./request-execution-budget"; -import type { AttemptRecoveryKind } from "../usage/log"; +import type { AttemptRecoveryKind } from "../usage/telemetry-contract"; /** * How far the exchange got, ordered by how much the DOWNSTREAM CLIENT observed. diff --git a/src/server/request-metrics.ts b/src/server/request-metrics.ts index 7ba966f5c45..f678b6565ba 100644 --- a/src/server/request-metrics.ts +++ b/src/server/request-metrics.ts @@ -1,9 +1,19 @@ import type { ResponsesTerminalStatus } from "../bridge"; import type { AttemptRecoveryKind } from "../usage/log"; import { type RequestFailureCause, causeForRecoveryKind } from "../lib/request-failure-model"; +import { + REQUEST_OUTCOME_CLASSES, + classifyRequestOutcome, + type RequestOutcomeClass, +} from "../usage/request-outcome"; export const REQUEST_METRICS_PROTOCOLS = Object.freeze(["responses", "chat", "messages", "unknown"] as const); -export const REQUEST_METRICS_RESULTS = Object.freeze(["completed", "failed", "incomplete", "aborted"] as const); +/** + * The exporter's result label set IS the shared outcome vocabulary, not a copy of it. Restating + * these four strings here is what let the exporter and the dashboard drift into disagreeing about + * the same request. + */ +export const REQUEST_METRICS_RESULTS = REQUEST_OUTCOME_CLASSES; /** * Closed recovery classes exported as Prometheus label values. * @@ -31,7 +41,7 @@ export const REQUEST_DURATION_BUCKETS_SECONDS = Object.freeze([0.1, 0.25, 0.5, 1 export const REQUEST_TTFT_BUCKETS_SECONDS = Object.freeze([0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10, 30] as const); export type RequestMetricsProtocol = typeof REQUEST_METRICS_PROTOCOLS[number]; -export type RequestMetricsResult = typeof REQUEST_METRICS_RESULTS[number]; +export type RequestMetricsResult = RequestOutcomeClass; export type RequestMetricsRecoveryClass = typeof REQUEST_METRICS_RECOVERY_CLASSES[number]; export interface RequestMetricFinalFact { @@ -84,18 +94,6 @@ function histograms(bounds: readonly number[]): HistogramCell[][] { )); } -function classifyResult(fact: RequestMetricFinalFact): RequestMetricsResult { - if (fact.closeReason === "client_cancel" || fact.status === 499) return "aborted"; - if (fact.terminalStatus === "failed") return "failed"; - if (fact.terminalStatus === "incomplete" - || fact.closeReason === "body_stall" - || fact.closeReason === "body_overflow") return "incomplete"; - if (fact.terminalStatus === "completed") return "completed"; - if (fact.terminalStatus === undefined - && (fact.status === 101 || (fact.status >= 200 && fact.status < 400))) return "completed"; - return "failed"; -} - /** * Metrics class for each shared failure cause. * @@ -176,7 +174,7 @@ export function createRequestMetricsOwner( return { recordFinalRequest(fact): void { const protocol: RequestMetricsProtocol = fact.protocol ?? "unknown"; - const result = classifyResult(fact); + const result = classifyRequestOutcome(fact); const protocolIndex = protocolCell(protocol); const resultIndex = resultCell(result); logicalRequests[protocolIndex]![resultIndex]! += 1; diff --git a/src/usage/log.ts b/src/usage/log.ts index b1bd2193e02..4a0be286bcb 100644 --- a/src/usage/log.ts +++ b/src/usage/log.ts @@ -12,6 +12,18 @@ import { normalizeRouteDecisionTrace, type RouteDecisionTraceV1 } from "../routi import { ACCOUNT_LOG_LABEL_RE, CODEX_ACCOUNT_LOG_LABEL_RE } from "../codex/account-label"; import { claudeCompatibilityReason, normalizeClaudeFeatureCodes, type ClaudeFeatureCode } from "../claude/compatibility"; import type { CodexWsStageRecord } from "../server/responses/codex-ws-wire"; +import { + ATTEMPT_RECOVERY_KIND_ROSTER, + ATTEMPT_RECOVERY_WITHHELD_ROSTER, + type AttemptRecoveryKind, + type AttemptRecoveryWithheld, + type RequestSpendTotals, +} from "./telemetry-contract"; + +// Re-exported so every existing importer keeps its path. The declarations moved to a leaf the +// dashboard can import without pulling node:fs and the config barrel into the browser build. +export { ATTEMPT_RECOVERY_KIND_ROSTER, ATTEMPT_RECOVERY_WITHHELD_ROSTER }; +export type { AttemptRecoveryKind, AttemptRecoveryWithheld, RequestSpendTotals }; export interface PersistedClaudeCompatibilityLog { decision: "shadow"; @@ -58,59 +70,6 @@ export function isCodexPoolAccountLogLabel(value: unknown): value is "main" | `p return value === "main" || (typeof value === "string" && CODEX_ACCOUNT_LOG_LABEL_RE.test(value)); } -/** - * Recovery kinds recorded per attempt in the usage log; the GUI renders localized labels - * for these wire values. - * - * The roster is the single statement of this vocabulary and the type is derived from it. It used - * to be written twice -- once as a union here, once as the read-back whitelist below -- and the - * two are not interchangeable: a member added only to the union is accepted by the compiler, - * written to disk, and then silently dropped by `normalizedAttempt`, so the row loses its reason - * on the next read. One declaration cannot drift from itself. - */ -export const ATTEMPT_RECOVERY_KIND_ROSTER = Object.freeze([ - "transient-5xx", - "connection-reset", - "oauth-401", - "key-401", - "key-429", - "rate-limit-429", - "anthropic-oauth-429", - "oauth-account-429", - "image-413", - "console-go-upload-retry", - "opaque-blob-rejection", - "empty-completion", - "reasoning-effort-downgrade", -] as const); - -export type AttemptRecoveryKind = typeof ATTEMPT_RECOVERY_KIND_ROSTER[number]; - -/** - * Why a recovery this request was otherwise willing to make did not happen. - * - * Recorded separately from `recoveryKinds` and from `sendCount`, because the question it - * answers is different from either. A log showing one physical send and no recovery kind used - * to be ambiguous: it could mean nothing was eligible, or that something was eligible and the - * send budget withheld it. Those need opposite follow-ups, and the second one was invisible - * (#5044). - * - * `sendCount` deliberately does not move for these. A refused attempt is not a physical send, - * and inflating the count to signal the refusal would corrupt the one number that means - * "requests this proxy actually made". - * - * Bounded vocabulary on purpose: it is a wire value a maintainer reads, never a credential, an - * account id, an upstream body, prompt content, or exception text. - * - * Declared as a roster for the same reason as {@link ATTEMPT_RECOVERY_KIND_ROSTER}. - */ -export const ATTEMPT_RECOVERY_WITHHELD_ROSTER = Object.freeze([ - "retry-send-budget", - "rotation-send-budget", -] as const); - -export type AttemptRecoveryWithheld = typeof ATTEMPT_RECOVERY_WITHHELD_ROSTER[number]; - /** Request-time upstream credential class, never a credential or account identifier. */ export type UsageCredentialSource = "grok-oauth" | "xai-api-key"; @@ -224,19 +183,7 @@ export interface PersistedUsageAttempt { * operator needs is the total that reached upstream carrying the full prompt. These fields are * that total, decomposed by how much of it is explained. */ -export interface PersistedRequestSpend { - /** Physical upstream sends summed across every attempt of this logical request, combo children included. */ - sends: number; - /** Sends whose attempt reached a terminal status, so the spend has a known outcome. */ - settled: number; - /** - * Sends charged with no terminal outcome behind them: an attempt abandoned mid-flight, or a - * budget charge no attempt row ever accounted for. Never folded into `settled` — an unexplained - * send is the exact quantity this record exists to make visible. - */ - unresolved: number; - /** Model sends the request execution budget charged. Absent when no budget was attached. */ - reserved?: number; +export interface PersistedRequestSpend extends RequestSpendTotals { /** Budget profile that produced `reserved`, so a count can be read against the policy it obeyed. */ policyVersion?: string; /** diff --git a/src/usage/request-outcome.ts b/src/usage/request-outcome.ts new file mode 100644 index 00000000000..9db60aee365 --- /dev/null +++ b/src/usage/request-outcome.ts @@ -0,0 +1,102 @@ +/** + * One terminal classification for a finished logical request, and the counts that go with it. + * + * Three surfaces answer "how did this request end" and they used to answer it three different + * ways. The durable row carries `terminalStatus` and `closeReason`; the Prometheus exporter had + * its own private `classifyResult`; the dashboard read the numeric HTTP status and nothing else. + * That is not a cosmetic difference. A turn cut short by `max_output_tokens` is durably + * `status: 200, terminalStatus: "incomplete"`, which the exporter reports as `incomplete` and the + * dashboard rendered as a green 200 — the operator and the metric disagreed about whether the + * user got an answer. + * + * The fix is not a third classifier. It is this one, which the exporter imports, the management + * payload carries, and the dashboard renders, so agreement is structural rather than a rule + * someone has to keep. + * + * Leaf module: its only import is a type, erased at runtime. + */ +import type { RequestSpendTotals } from "./telemetry-contract"; + +/** + * Ordered by how much of an answer the caller received. The order is not a ranking of severity; + * `aborted` is last because the caller chose it, not because it is the worst outcome. + */ +export const REQUEST_OUTCOME_CLASSES = Object.freeze([ + /** A terminal event settled the turn and the caller received the answer. */ + "completed", + /** The turn ended without an answer. */ + "failed", + /** The turn produced part of an answer and stopped. */ + "incomplete", + /** The caller went away before the turn finished. */ + "aborted", +] as const); + +export type RequestOutcomeClass = typeof REQUEST_OUTCOME_CLASSES[number]; + +/** + * The facts a terminal classification is allowed to read. + * + * Deliberately narrow, and deliberately NOT the whole durable row: an outcome that could consult + * a provider name or an error message would be a different answer per provider, which is how the + * three surfaces drifted apart in the first place. + */ +export interface RequestOutcomeFacts { + readonly status: number; + readonly terminalStatus?: string | undefined; + readonly closeReason?: "terminal" | "client_cancel" | "non_stream" | "body_stall" | "body_overflow" | undefined; +} + +/** + * Classify one finished logical request. + * + * Semantic terminal facts are read BEFORE the numeric status, which is the whole point. An HTTP + * 200 that carried an incomplete terminal is incomplete; a 502 that carried an incomplete + * terminal is also incomplete, and reading the status first would have called them success and + * failure. The numeric status is consulted only when no terminal event was recorded at all. + */ +export function classifyRequestOutcome(facts: RequestOutcomeFacts): RequestOutcomeClass { + if (facts.closeReason === "client_cancel" || facts.status === 499) return "aborted"; + if (facts.terminalStatus === "failed") return "failed"; + if (facts.terminalStatus === "incomplete" + || facts.closeReason === "body_stall" + || facts.closeReason === "body_overflow") return "incomplete"; + if (facts.terminalStatus === "completed") return "completed"; + if (facts.terminalStatus === undefined + && (facts.status === 101 || (facts.status >= 200 && facts.status < 400))) return "completed"; + return "failed"; +} + +/** A count is reportable only when the writer recorded a non-negative integer. */ +function reportableCount(value: number | undefined): number { + return typeof value === "number" && Number.isInteger(value) && value >= 0 ? value : 0; +} + +/** + * Physical sends a finished request made, as the one number every surface shows. + * + * This READS the recorded total rather than recomputing one. An earlier draft returned + * `max(sends, reserved)` on the reasoning that a budget charge with no attempt row behind it is + * still a send that left — true, but it made the dashboard report four where the exporter, which + * sums the same attempts the recorder summed, reported three. Two defensible formulas are still + * two answers. The recorder already decided this, and `unresolved` below is where a charge with + * no attempt behind it becomes visible. + */ +export function requestPhysicalSends(spend: RequestSpendTotals | undefined): number { + return reportableCount(spend?.sends); +} + +/** + * Sends whose attempt reached a terminal status, and sends that did not. + * + * Kept beside {@link requestPhysicalSends} because an operator reading a send total needs to know + * how much of it is explained. An unresolved send is the quantity a duplicate-send incident shows + * up in, and folding it into the total is what made #4546 invisible for so long. + */ +export function requestSettledSends(spend: RequestSpendTotals | undefined): number { + return reportableCount(spend?.settled); +} + +export function requestUnresolvedSends(spend: RequestSpendTotals | undefined): number { + return reportableCount(spend?.unresolved); +} diff --git a/src/usage/telemetry-contract.ts b/src/usage/telemetry-contract.ts new file mode 100644 index 00000000000..21e716cb580 --- /dev/null +++ b/src/usage/telemetry-contract.ts @@ -0,0 +1,83 @@ +/** + * The telemetry vocabulary both the proxy and the dashboard read. + * + * This module has NO imports, and that is its entire job. The dashboard is a separate TypeScript + * project with `erasableSyntaxOnly`, and a type-only import still pulls the imported file's whole + * import graph into that project. Importing these names from `./log` therefore dragged + * `node:fs`, `node:crypto` and the config barrel into the browser build, where a parameter + * property in `src/config/atomic-write.ts` fails to compile. The names below are the ones a + * browser legitimately needs, so they live where a browser can reach them. + * + * Anything added here must stay free of imports. A contract that acquires a dependency stops + * being a contract. + */ + +/** + * Recovery kinds recorded per attempt in the usage log; the dashboard renders localized labels + * for these wire values. + * + * The roster is the single statement of this vocabulary and the type is derived from it. It was + * written twice once -- as a union and as the read-back whitelist -- and the two are not + * interchangeable: a member added only to the union compiles, is written to disk, and is dropped + * on the next read, so the row loses the field that says why it recovered. One declaration cannot + * drift from itself, and the dashboard now reads this one rather than keeping a third copy. + */ +export const ATTEMPT_RECOVERY_KIND_ROSTER = Object.freeze([ + "transient-5xx", + "connection-reset", + "oauth-401", + "key-401", + "key-429", + "rate-limit-429", + "anthropic-oauth-429", + "oauth-account-429", + "image-413", + "console-go-upload-retry", + "opaque-blob-rejection", + "empty-completion", + "reasoning-effort-downgrade", +] as const); + +export type AttemptRecoveryKind = typeof ATTEMPT_RECOVERY_KIND_ROSTER[number]; + +/** + * Why a recovery this request was otherwise willing to make did not happen. + * + * Recorded separately from `recoveryKinds` and from `sendCount` because the question it answers + * is different from either. A log showing one physical send and no recovery kind used to be + * ambiguous: nothing was eligible, or something was and the send budget withheld it. Those need + * opposite follow-ups and the second was invisible (#5044). + * + * `sendCount` deliberately does not move for these. A refused attempt is not a physical send, and + * inflating the count to signal the refusal would corrupt the one number that means "requests this + * proxy actually made". + */ +export const ATTEMPT_RECOVERY_WITHHELD_ROSTER = Object.freeze([ + "retry-send-budget", + "rotation-send-budget", +] as const); + +export type AttemptRecoveryWithheld = typeof ATTEMPT_RECOVERY_WITHHELD_ROSTER[number]; + +/** + * What one logical request spent upstream, decomposed by how much of it is explained. + * + * The counting half of the durable spend record, without the routing detail that sits beside it. + * Every surface that reports a send total reads these three numbers and none of them recomputes a + * total of its own -- a recomputed total is how the exporter and the dashboard ended up reporting + * different send counts for the same request. + */ +export interface RequestSpendTotals { + /** Physical upstream sends summed across every attempt, combo children included. */ + sends: number; + /** Sends whose attempt reached a terminal status, so the spend has a known outcome. */ + settled: number; + /** + * Sends charged with no terminal outcome behind them: an attempt abandoned mid-flight, or a + * budget charge no attempt row ever accounted for. Never folded into `settled` -- an unexplained + * send is the exact quantity this record exists to make visible. + */ + unresolved: number; + /** Model sends the request execution budget charged. Absent when no budget was attached. */ + reserved?: number; +} diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 646d7e75263..af1849fefe8 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1033,6 +1033,7 @@ "request-log-conversation.test.ts": "usage", "request-log-estimate-cap.test.ts": "usage", "request-log.test.ts": "usage", + "request-outcome-agreement.test.ts": "usage", "request-pacing.test.ts": "usage", "reserve-auth-context.test.ts": "codex-integration", "reserve-availability.test.ts": "codex-integration", diff --git a/tests/usage/request-outcome-agreement.test.ts b/tests/usage/request-outcome-agreement.test.ts new file mode 100644 index 00000000000..b5b30744e9e --- /dev/null +++ b/tests/usage/request-outcome-agreement.test.ts @@ -0,0 +1,219 @@ +import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { repoPath } from "../helpers/repo-root"; +import { + REQUEST_OUTCOME_CLASSES, + classifyRequestOutcome, + requestPhysicalSends, + requestSettledSends, + requestUnresolvedSends, + type RequestOutcomeFacts, +} from "../../src/usage/request-outcome"; +import { + REQUEST_METRICS_RESULTS, + createRequestMetricsOwner, +} from "../../src/server/request-metrics"; +import { ATTEMPT_RECOVERY_KIND_ROSTER } from "../../src/usage/telemetry-contract"; + +/** + * The durable ledger, the Prometheus exporter and the dashboard have to answer "how did this + * request end" and "how many times did it reach upstream" the same way. They did not: the + * exporter kept a private classifier and the dashboard read the numeric status alone, so an + * incomplete 200 was a metric incident and a green row at the same time. + * + * None of these cases can be satisfied by a request that returned 200 -- several of them are + * specifically about a 200 that must NOT read as success. + */ + +const LOCALES = ["en", "ko", "ja", "zh", "zh-TW", "de", "fr", "ru", "tr", "vi"] as const; + +function sampleValue(snapshot: string, series: string): number { + const line = snapshot.split("\n").find(row => row.startsWith(series + " ")); + return line === undefined ? Number.NaN : Number(line.slice(series.length + 1)); +} + +/** Every combination a terminal can arrive in, built from the declared vocabularies. */ +const TERMINAL_STATUSES = [undefined, "completed", "failed", "incomplete"] as const; +const CLOSE_REASONS = [ + undefined, "terminal", "client_cancel", "non_stream", "body_stall", "body_overflow", +] as const; +const STATUSES = [101, 200, 204, 399, 400, 429, 499, 500, 502] as const; + +describe("terminal classification is stated once", () => { + test("the exporter labels every fact exactly as the shared classifier does", () => { + const disagreements: string[] = []; + for (const status of STATUSES) { + for (const terminalStatus of TERMINAL_STATUSES) { + for (const closeReason of CLOSE_REASONS) { + const facts: RequestOutcomeFacts = { + status, + ...(terminalStatus ? { terminalStatus } : {}), + ...(closeReason ? { closeReason } : {}), + }; + const metrics = createRequestMetricsOwner(1); + metrics.recordFinalRequest({ + protocol: "responses", + durationMs: 1, + status, + ...(terminalStatus ? { terminalStatus } : {}), + ...(closeReason ? { closeReason } : {}), + }); + const snapshot = metrics.snapshot(); + const expected = classifyRequestOutcome(facts); + const observed = REQUEST_METRICS_RESULTS.filter(result => sampleValue( + snapshot, + `opencodex_logical_requests_total{protocol="responses",result="${result}"}`, + ) === 1); + if (observed.length !== 1 || observed[0] !== expected) { + disagreements.push(`${status}/${terminalStatus ?? "-"}/${closeReason ?? "-"}: ` + + `exporter ${observed.join("+") || "none"} != ${expected}`); + } + } + } + } + expect(disagreements).toEqual([]); + }); + + test("the exporter's label set IS the shared vocabulary, not a copy of it", () => { + expect(REQUEST_METRICS_RESULTS).toBe(REQUEST_OUTCOME_CLASSES); + }); + + /** + * The three cases the disagreement actually showed up in. Written as literals because each one + * is a specific incident shape, not a member of a roster that could grow. + */ + test("a 200 that never delivered an answer does not read as success", () => { + expect(classifyRequestOutcome({ status: 200, terminalStatus: "incomplete" })).toBe("incomplete"); + expect(classifyRequestOutcome({ status: 502, terminalStatus: "incomplete" })).toBe("incomplete"); + expect(classifyRequestOutcome({ status: 200, closeReason: "client_cancel" })).toBe("aborted"); + expect(classifyRequestOutcome({ status: 200 })).toBe("completed"); + }); + + test("the dashboard calls the shared classifier instead of reading the status", () => { + const page = readFileSync(repoPath("gui", "src", "pages", "Logs.tsx"), "utf8"); + expect(page).toContain("classifyRequestOutcome"); + expect(page).toContain("request-outcome"); + }); +}); + +describe("send totals agree across surfaces", () => { + /** + * The budget charged four sends while the attempt rows account for three. Both numbers are + * real and they answer different questions, so the surfaces have to agree about WHICH one the + * send total is. An earlier draft returned max(sends, reserved) here, which is defensible on + * its own and made the dashboard say four while the exporter said three -- two defensible + * formulas are still two answers. + */ + const spend = { sends: 3, settled: 3, unresolved: 1, reserved: 4 }; + + test("the reported total is the recorded one, with the unexplained part beside it", () => { + expect(requestPhysicalSends(spend)).toBe(3); + expect(requestSettledSends(spend)).toBe(3); + expect(requestUnresolvedSends(spend)).toBe(1); + }); + + test("the exporter's send total is the number the dashboard shows", () => { + const metrics = createRequestMetricsOwner(1); + metrics.recordFinalRequest({ + protocol: "responses", + status: 200, + durationMs: 1, + terminalStatus: "completed", + attempts: [ + { sendCount: 2, recoveryKinds: [] }, + { sendCount: 1, recoveryKinds: ["connection-reset"] }, + ], + spendSends: spend.sends, + }); + const exported = sampleValue( + metrics.snapshot(), + 'opencodex_physical_sends_total{protocol="responses"}', + ); + expect(exported).toBe(requestPhysicalSends(spend)); + }); + + test("an absent or malformed spend record reports nothing rather than guessing", () => { + expect(requestPhysicalSends(undefined)).toBe(0); + expect(requestUnresolvedSends(undefined)).toBe(0); + expect(requestPhysicalSends({ sends: -2, settled: -1, unresolved: 0 })).toBe(0); + }); + + test("the dashboard shows the send total and the unresolved remainder", () => { + const page = readFileSync(repoPath("gui", "src", "pages", "Logs.tsx"), "utf8"); + expect(page).toContain("requestPhysicalSends"); + expect(page).toContain("requestUnresolvedSends"); + }); +}); + +describe("the dashboard reaches only browser-safe contract modules", () => { + /** + * A type-only import still pulls the imported file's whole import graph into the dashboard's + * TypeScript project, and that project sets `erasableSyntaxOnly`. Importing these names from + * `src/usage/log.ts` dragged `node:fs`, `node:crypto` and the config barrel into the browser + * build, where a parameter property fails to compile. The page must reach the leaf instead. + */ + test("it imports the vocabulary from the contract leaf, not the ledger module", () => { + const page = readFileSync(repoPath("gui", "src", "pages", "Logs.tsx"), "utf8"); + expect(page).toContain("src/usage/telemetry-contract"); + expect(page).not.toContain("src/usage/log"); + }); + + test("the contract leaf has no imports at all", () => { + const contract = readFileSync(repoPath("src", "usage", "telemetry-contract.ts"), "utf8"); + expect(contract.match(/^\s*import\s/gm)).toBeNull(); + }); + + test("the outcome module reaches nothing but the contract", () => { + const outcome = readFileSync(repoPath("src", "usage", "request-outcome.ts"), "utf8"); + const specifiers = [...outcome.matchAll(/from "([^"]+)"/g)].map(match => match[1]!); + expect(specifiers).toEqual(["./telemetry-contract"]); + }); +}); + +describe("the dashboard recovery roster cannot drift from the durable one", () => { + /** + * The defect this replaces: the page declared its own nine-member union while the ledger wrote + * thirteen, so four real causes rendered as "Unknown recovery reason". A source oracle rather + * than a type check, because the page is compiled by a different project. + */ + test("every durable recovery kind has a dashboard label", () => { + const page = readFileSync(repoPath("gui", "src", "pages", "Logs.tsx"), "utf8"); + const block = page.slice(page.indexOf("const RECOVERY_KIND_KEYS"), page.indexOf("} as const satisfies Record !block.includes(`"${kind}":`)); + expect(missing).toEqual([]); + }); + + test("the page derives the union rather than restating it", () => { + const page = readFileSync(repoPath("gui", "src", "pages", "Logs.tsx"), "utf8"); + expect(page).toContain("import type { AttemptRecoveryKind"); + expect(page).not.toContain('type AttemptRecoveryKind ='); + }); + + test("every label key the page names exists in all ten catalogs", () => { + const page = readFileSync(repoPath("gui", "src", "pages", "Logs.tsx"), "utf8"); + const keys = [...new Set([...page.matchAll(/"(logs\.detail\.(?:attempt\.recovery|outcome|sends)\.[a-zA-Z0-9]+)"/g)] + .map(match => match[1]!))]; + expect(keys.length).toBeGreaterThan(ATTEMPT_RECOVERY_KIND_ROSTER.length); + const gaps: string[] = []; + for (const locale of LOCALES) { + const catalog = readFileSync(repoPath("gui", "src", "i18n", `${locale}.ts`), "utf8"); + for (const key of keys) if (!catalog.includes(`"${key}"`)) gaps.push(`${locale}:${key}`); + } + expect(gaps).toEqual([]); + }); +}); + +describe("the exporter stays bounded", () => { + test("no series carries a user, model, account or request identifier", () => { + const metrics = createRequestMetricsOwner(1); + for (let index = 0; index < 32; index += 1) { + metrics.recordFinalRequest({ + protocol: "responses", status: 200, durationMs: 5, terminalStatus: "completed", + attempts: [{ sendCount: 1, recoveryKinds: ["rate-limit-429"] }], + }); + } + const snapshot = metrics.snapshot(); + const labels = [...new Set([...snapshot.matchAll(/([a-z_]+)="/g)].map(match => match[1]!))]; + expect(labels.sort()).toEqual(["le", "protocol", "recovery", "result"]); + }); +});