From 8a6f4e24b3dcbe16f73ae54e93207ac2115d8f90 Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 20 Sep 2026 17:06:41 +0900 Subject: [PATCH 1/4] refactor(usage): one terminal classification for a finished request Three surfaces answered "how did this request end" three different ways. The durable row carries terminalStatus and closeReason, the Prometheus exporter had its own private classifyResult, and the dashboard read the numeric HTTP status and nothing else. That is not cosmetic. A turn cut short by max_output_tokens is durably status 200 with terminalStatus "incomplete", which the exporter reports as incomplete and the dashboard rendered as a green 200: the metric and the operator disagreed about whether the user got an answer. Move the classifier into src/usage/request-outcome.ts and have the exporter import it, including its result label set, so the four strings are stated once. Semantic terminal facts are read before the numeric status, which is the whole point; the status is consulted only when no terminal event was recorded. The module also names the send totals a surface should show, because reporting sends without the unresolved remainder is how a duplicate-send incident stays invisible. It is a leaf: its only import is a type. --- src/lib/request-failure-model.ts | 2 +- src/server/request-metrics.ts | 28 ++++----- src/usage/log.ts | 79 ++++-------------------- src/usage/request-outcome.ts | 102 +++++++++++++++++++++++++++++++ src/usage/telemetry-contract.ts | 83 +++++++++++++++++++++++++ 5 files changed, 212 insertions(+), 82 deletions(-) create mode 100644 src/usage/request-outcome.ts create mode 100644 src/usage/telemetry-contract.ts 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; +} From 847c177484ae9d7dbf9438d86c601c52157f1efa Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 20 Sep 2026 17:09:12 +0900 Subject: [PATCH 2/4] fix(gui): make the logs page agree with the ledger and the exporter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Carries the rehydration half of #2366 — the half that brings the durable terminal facts out to where an operator reads them. Its separate attribution vocabulary is deliberately left behind, because the landed stage and cause model already owns that question and two vocabularies for one thing is the class of defect this batch exists to remove. The page classified every request by its numeric HTTP status alone and showed no send count at all, so it disagreed with both other surfaces about the same request. A turn cut short by max_output_tokens is durably incomplete and is reported incomplete by the exporter; the page rendered a green 200. The data was never missing — /api/logs spreads the whole durable entry — the page simply did not declare terminalStatus, closeReason or spend. It now declares them and calls the shared classifier rather than reimplementing the precedence, so agreement is structural instead of a rule someone maintains. It also shows the upstream send count, and names the unresolved remainder when there is one, because a send total without it is how a duplicate-send incident stays invisible. The recovery-kind union is now the durable roster instead of a copy. The copy had drifted to nine of thirteen members, so key-401, oauth-account-429, opaque-blob-rejection and reasoning-effort-downgrade each reached the operator as "Unknown recovery reason" — four real causes rendered as an absence of one. The satisfies clause makes the next added kind a typecheck failure here rather than a silent fallback, and the four missing labels are added across all ten catalogs. Co-authored-by: chilung --- gui/src/i18n/de.ts | 11 +++++++ gui/src/i18n/en.ts | 11 +++++++ gui/src/i18n/fr.ts | 11 +++++++ gui/src/i18n/ja.ts | 11 +++++++ gui/src/i18n/ko.ts | 11 +++++++ gui/src/i18n/ru.ts | 11 +++++++ gui/src/i18n/tr.ts | 11 +++++++ gui/src/i18n/vi.ts | 11 +++++++ gui/src/i18n/zh-TW.ts | 11 +++++++ gui/src/i18n/zh.ts | 11 +++++++ gui/src/pages/Logs.tsx | 72 +++++++++++++++++++++++++++++++++--------- 11 files changed, 167 insertions(+), 15 deletions(-) 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"} From 7a95b0a7290504bb19f8553c4eaf7b89a1e518eb Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 20 Sep 2026 17:11:27 +0900 Subject: [PATCH 3/4] test(usage): hold the three surfaces to one answer The exporter is driven over the full cross product of status, terminal status and close reason and its emitted result label is compared against the shared classifier, so the two cannot drift apart without a case objecting. The cases that actually broke are asserted by name as well: an incomplete 200 is not a success, and a cancelled 200 is aborted. A source oracle holds the dashboard to the same contract. It has to call the shared classifier rather than read the status, it has to show the send total and the unresolved remainder, and its recovery-label map has to cover every member of the durable roster. That last one is a source oracle rather than a type check because the page is compiled by a separate project, which is how the copy drifted to nine of thirteen members unnoticed in the first place. Every label key the page names is required to exist in all ten catalogs, so a new recovery kind cannot ship with an English label and nine blanks. One case asserts the exporter's whole label set is still protocol, result, recovery and le after thirty-two requests carrying recoveries, which is the bounded-cardinality promise stated as an assertion rather than a convention. --- scripts/test-layout/layout.json | 1 + tests/fixtures/test-layout-expected.json | 1 + tests/usage/request-outcome-agreement.test.ts | 219 ++++++++++++++++++ 3 files changed, 221 insertions(+) create mode 100644 tests/usage/request-outcome-agreement.test.ts 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/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"]); + }); +}); From 3b24c6c2a966f5bed9f022033acf127f56e54721 Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 20 Sep 2026 17:13:23 +0900 Subject: [PATCH 4/4] docs(devlog): record lane C2 and refresh the deferred dispositions Each item that did not land carries the reason that is true against current dev, not the one written a day ago. #3748's blocker is now narrower and more useful than "parallel store": the recorder does not yet record why a request finally failed, so there is nothing closed to group by. #3983's emission path turns out not to be ephemeral, because stderr is redirected to the service log under both launchd and systemd. #5063 has a concurrent-append data-loss window that the rename cannot see. Retention and masking are stated in one table rather than reimplemented, with the policy that projections inherit both instead of getting their own. --- .../030_lane_c2.md | 220 ++++++++++++++++++ 1 file changed, 220 insertions(+) create mode 100644 devlog/_plan/260920_meaning_preservation_batch/030_lane_c2.md 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.