fix: resolve late train review findings and regression gaps

devlog/_plan/261003_train_recovery_d/010_late_review.md

@@ -0,0 +1,56 @@
+# Lane D late-review follow-up
+
+This follow-up starts from landed dev `3ada270f3a0d955ad02b6a4f8f74b0412906bcb1`.
+The earlier verification record describes its inspected scenarios, not a claim
+that late review could find no further defect. Existing Doctor fixes remain intact.
+
+## Comment dispositions
+
+| Comment | Disposition | Evidence and change |
+| --- | --- | --- |
+| 4172062340 | Accepted | `gui/src/pages/integrations/FileIntegrationPage.tsx` reuses the omission-aware review path. No draft sends no defaults map; an explicit empty map still clears defaults. The GUI regression exercises Update and Save / review through preview and commit. |
+| 4172062382 | Accepted | `src/clients/config-export/droid.ts` filters inherited defaults against the current normalized model's declared efforts. Refresh removes unsupported headers and preserves compatible peers. Regressions cover incompatible, empty and unknown ladders plus explicit clearing. Ownership checks are unchanged. |
+| 4172062352 | Accepted | `gui/src/pages/Usage.tsx` shares a finite-number guard across summary, tables, tooltips and assistive text. Malformed rates are unavailable. Regressions cover null, strings and nonfinite values. |
+| 4172062362 | Accepted | `gui/src/use-model-visibility.ts` treats JSON integration details as optional after a successful write. Empty/non-JSON success still reconciles against the catalog; transport failures retain their existing error path. Regressions cover 204 and text responses. |
+| 4172062394 | Rebutted | `structure/dashboard-and-usage.md`, the English dashboard guide, `usage.throughput.title` and `src/usage/summary.ts` explicitly define qualifying **attempts**, with one sample for a legacy row without attempts. Changing this to distinct requests would change the accepted contract. No aggregation semantics changed. |
+| 4172062396 | Accepted | `tests/usage/usage-throughput.test.ts` identifies the second request's model and asserts two model rows at 10 and 15 tok/s with one sample each, alongside the weighted provider/summary rate. |
+
+## Focused verification
+
+From `gui/`:
+
+```sh
+bun test --isolate ./tests/models-visibility-queue.test.tsx ./tests/usage-custom-range.test.tsx
+bun test --isolate ./tests/integrations-surfaces.test.tsx
+```
+
+The first command reproduced seven failures before the display/response fixes
+(39 passed); after repair it passed **46/0**. The second passed **58/0**.
+Droid regressions were added before its source edit but were not run red;
+no Droid red-green claim is made.
+
+From the repository root:
+
+```sh
+bun test ./tests/usage/usage-throughput.test.ts
+bun test ./tests/clients/droid-managed-reasoning-defaults.test.ts ./tests/server/management-droid-reasoning-defaults.test.ts ./tests/responses/droid-reasoning-defaults.test.ts
+```
+
+These passed **5/0** and **44/0**, respectively. All batches were sequential;
+none reported skips. Structure, file-size ratchet and privacy checks passed.
+
+Three fixture-browser regressions exercised malformed string throughput across
+all three displays and a 204 visibility write through successful reconciliation.
+The Droid no-draft review also opened confirmation with the defaults map omitted.
+All passed; the throughput/visibility run had no uncaught page errors. Screenshots were inspected and
+remain outside tracked source. An initial browser assertion also selected cache
+cells; narrowing its selector to throughput corrected the harness, not the product.
+
+Broad typechecks, builds, full suites, Windows/native and real-client acceptance
+remain unrun locally. Final PR/CI and production-readiness judgment belong to the
+coordinator. No push, PR, merge, installation or release is part of this lane.
+
+Independent inherited review of the final source/tests/docs returned **PASS**,
+with no blocking findings; it supported all five accepted fixes/test improvements
+and the attempt-count contract rebuttal. This is a technical review, not final
+production-readiness approval.

docs-site/src/content/docs/guides/integrations.md

@@ -667,10 +667,12 @@ against its own effort list, so an incompatible first target does not remove it
 from a compatible fallback.
 
 A saved default that the routed model no longer supports is ignored for requests.
-Clear it in the panel, then review and confirm the change to remove it from Droid.
+If the connected model's declared effort list no longer includes the saved value,
+it is omitted from the panel's defaults and removed from the managed row on refresh.
+Reviewing without editing lets OpenCodex preserve the remaining supported defaults.
 
 Refresh preserves defaults while the exact `provider/model` selector remains
-connected. Renaming a provider, model, or combo alias replaces that managed row
+connected and declares the saved effort. Renaming a provider, model, or combo alias replaces that managed row
 and clears its default; choose a default for the renamed row again. Disable
 removes the defaults with the managed model rows, and Undo restores the saved
 rows and their defaults together.

docs-site/src/content/docs/guides/web-dashboard.md

@@ -181,7 +181,7 @@ For a custom usage interval, the server must confirm the exact requested start a
 If an older running proxy does not support those bounds, the dashboard and CLI reject its report;
 upgrade and restart that proxy before retrying. Resetting a manual model price affects only that
 model, preserving other rates saved independently.
-The **Usage** summary and Models/Providers tables show end-to-end output throughput from usage history for the selected range and filters: summed output tokens divided by summed wall-clock seconds, not an average of individual rates. The sample count is measured attempts (or requests for legacy rows without attempts); samples lacking positive finite output tokens or duration are excluded. An em dash means no sample qualified. This includes pre-decode waiting and is not estimated decode speed.
+The **Usage** summary and Models/Providers tables show end-to-end output throughput from usage history for the selected range and filters: summed output tokens divided by summed wall-clock seconds, not an average of individual rates. The sample count is measured attempts (or requests for legacy rows without attempts); samples lacking positive finite output tokens or duration are excluded. An em dash means no sample qualified or the server returned an unusable rate. This includes pre-decode waiting and is not estimated decode speed.
 
 The **Usage** Models and Providers tables show the estimated priced portion for each row. Requests
 without a matching price or usable usage are counted as excluded beside that amount when the proxy

docs-site/src/content/docs/ko/guides/integrations.md

@@ -273,10 +273,11 @@ GitHub Copilot 데스크톱 앱에서 opencodex를 OpenAI 호환 모델 프로
 확인하므로, 첫 대상이 지원하지 않아도 지원하는 다음 대상으로 전달됩니다.
 
 저장된 기본값을 라우팅된 모델이 더 이상 지원하지 않으면 요청에는 적용하지
-않습니다. Droid 설정에서도 제거하려면 패널에서 지운 뒤 변경 내용을 검토하고
-확인하세요.
+않습니다. 현재 모델의 effort 목록에서 빠진 값은 패널의 기본값에서도 제외되며,
+새로고침할 때 관리 행에서 제거됩니다. 편집 없이 변경 내용을 검토하면 여전히
+지원되는 기본값은 유지됩니다.
 
-새로고침은 정확히 같은 `provider/model` 선택자가 계속 연결된 동안 기본값을
+새로고침은 정확히 같은 `provider/model` 선택자가 계속 연결되고 저장된 effort를 지원하는 동안 기본값을
 유지합니다. 프로바이더, 모델, 콤보 별칭의 이름을 바꾸면 관리 행이 교체되고
 기본값이 지워지므로 새 행에서 다시 선택하세요. 연동을 해제하면 관리되는 모델
 행과 기본값이 함께 제거되며, 되돌리기는 저장된 행과 기본값을 함께 복원합니다.

docs-site/src/content/docs/reference/cli/providers-accounts.md

@@ -121,7 +121,7 @@ reauthentication flow above. A second `401` does not start another refresh/retry
 A `403` asking to verify the account quarantines that credential as `needs-reauth(verify)`;
 when account failover is enabled, the request can retry on another eligible account. Complete
 Google's account verification, then run `ocx login google-antigravity`. Silent token refresh
-does not clear this verification requirement. If OpenCodex cannot save the quarantine, this adapter exchange preserves the original `403` without another recovery send; the account has not been durably quarantined. An enclosing combo or policy route can still apply its existing fallback rules.
+does not clear this verification requirement. If OpenCodex cannot save the quarantine, this adapter exchange preserves the original `403` without another recovery send; the account has not been durably quarantined. If a sibling request cannot be built or admitted for sending, the exchange delivers the original `403` through normal error formatting. An enclosing combo or policy route can still apply its existing fallback rules.
 
 A proxy that is already running picks up the new credential without a restart: the CLI asks it to
 reload that one provider from disk, and the request carries no credential of its own. If the

gui/src/pages/Usage.tsx

@@ -211,8 +211,12 @@ function cacheHitRateTitle(model: UsageModel, locale: Locale, t: TFn): string |
   });
 }
 
+function hasThroughput(value: unknown): value is number {
+  return typeof value === "number" && Number.isFinite(value);
+}
+
 function throughputTitle(row: { throughputTokensPerSec?: number; throughputSamples?: number }, t: TFn): string {
-  return Number.isFinite(row.throughputTokensPerSec)
+  return hasThroughput(row.throughputTokensPerSec)
     ? t("usage.throughput.title", { samples: row.throughputSamples ?? 0 })
     : t("usage.throughput.unmeasured");
 }
@@ -498,7 +502,7 @@ function UsageSummaryCards({
     </div>
       <div className="usage-cost-row" role="note" title={throughputTitle(summary, t)}>
         <span className="muted">{t("usage.col.tokPerSec")}</span>
-        <span className="stat-value mono usage-cost-value">{summary.throughputTokensPerSec?.toFixed(1) ?? t("usage.unavailable")}</span>
+        <span className="stat-value mono usage-cost-value">{hasThroughput(summary.throughputTokensPerSec) ? summary.throughputTokensPerSec.toFixed(1) : t("usage.unavailable")}</span>
         <span className="muted text-caption">{throughputTitle(summary, t)}</span>
       </div>
       {summary.estimatedCostUsd !== undefined && (
@@ -811,9 +815,9 @@ function UsageModelsTable({
                 <td className="num mono">{formatTokens(model.outputTokens, locale)}</td>
                 <td className="num mono" title={throughputNote}>
                   <span className="usage-hit-rate">
-                    {typeof model.throughputTokensPerSec === "number" ? `${model.throughputTokensPerSec.toFixed(1)} tok/s` : unavailable}
+                    {hasThroughput(model.throughputTokensPerSec) ? `${model.throughputTokensPerSec.toFixed(1)} tok/s` : unavailable}
                   </span>
-                  {typeof model.throughputTokensPerSec === "number" && <span className="sr-only">{throughputNote}</span>}
+                  {hasThroughput(model.throughputTokensPerSec) && <span className="sr-only">{throughputNote}</span>}
                 </td>
                 <td className="num mono">{formatOptionalTokens(model.cacheReadInputTokens ?? model.cachedInputTokens, locale, unavailable)}</td>
                 <td className="num mono">{formatOptionalTokens(model.cacheCreationInputTokens, locale, unavailable)}</td>
@@ -896,9 +900,9 @@ function UsageProvidersTable({
               <td className="num mono">{formatTokens(provider.totalTokens, locale)}</td>
               <td className="num mono" title={throughputTitle(provider, t)}>
                 <span className="usage-hit-rate">
-                  {typeof provider.throughputTokensPerSec === "number" ? `${provider.throughputTokensPerSec.toFixed(1)} tok/s` : unavailable}
+                  {hasThroughput(provider.throughputTokensPerSec) ? `${provider.throughputTokensPerSec.toFixed(1)} tok/s` : unavailable}
                 </span>
-                {typeof provider.throughputTokensPerSec === "number" && <span className="sr-only">{throughputTitle(provider, t)}</span>}
+                {hasThroughput(provider.throughputTokensPerSec) && <span className="sr-only">{throughputTitle(provider, t)}</span>}
               </td>
               <td className="num"><UsageListPrice row={provider} locale={locale} t={t} /></td>
               <td><div className="usage-bar"><div className="usage-bar-fill" style={{ width: `${Math.round(provider.shareRatio * 100)}%` }} /></div></td>

gui/src/pages/integrations/FileIntegrationPage.tsx

@@ -351,9 +351,7 @@ function FileIntegrationControls({
             : status.droidReasoning.defaults}
           disabled={pending || plannedMutation !== null}
           onChange={values => setDroidReasoningDraft({ scopeKey, values })}
-          onReview={() => void requestMutation("apply", (droidReasoningDraft?.scopeKey === scopeKey
-            ? droidReasoningDraft.values
-            : status.droidReasoning?.defaults) ?? {})}
+          onReview={() => void requestMutation("apply")}
         />
       )}
 

gui/src/use-model-visibility.ts

@@ -57,7 +57,9 @@ export function useModelVisibility(apiBase: string, options: Options) {
           if (current.active) {
             if (!response.ok) error = "models.saveFailed";
             else {
-              const body: unknown = await response.json();
+              // Integration refresh details are optional; a successful empty/legacy response
+              // still reconciles visibility through the authoritative catalog read below.
+              const body: unknown = await response.json().catch(() => undefined);
               if (current.active) callbacks.current.onResponse(body);
             }
           }

gui/tests/integrations-surfaces.test.tsx

@@ -350,7 +350,7 @@ test("Droid reasoning defaults use one frozen snapshot for review and commit", a
   expect(container.querySelector<HTMLButtonElement>('[role="combobox"]')?.textContent).toContain("low");
 });
 
-test("Droid refresh omits unsupported saved defaults while an explicit edit stays frozen through confirm", async () => {
+test.each(["Update", "Save / review changes"])("Droid %s omits unsupported saved defaults while an explicit edit stays frozen through confirm", async (action) => {
   stateResponse = () => json(status({
     clientId: "droid",
     state: "stale",
@@ -362,14 +362,15 @@ test("Droid refresh omits unsupported saved defaults while an explicit edit stay
   }));
   await mountClient(true, "droid");
 
-  await act(async () => { buttonByText("Update")!.click(); });
+  await act(async () => { buttonByText(action)!.click(); });
   await act(async () => { await new Promise<void>(resolve => testWindow.setTimeout(resolve, 20)); });
   const refreshPreview = requests.find(request => request.method === "POST" && request.url.endsWith("/api/client-integrations/preview"));
   expect(refreshPreview?.body).not.toHaveProperty("droidReasoningDefaults");
 
-  const dialog = container.querySelector("dialog[open]")!;
-  const close = Array.from(dialog.querySelectorAll("button")).find(button => button.textContent?.trim() === "Close") as HTMLButtonElement;
-  await act(async () => { close.click(); });
+  await confirmDialog("Apply");
+  const unchangedMutation = requests.find(request => request.method === "PUT" && request.url.endsWith("/api/client-integrations/droid"));
+  expect(unchangedMutation).toBeDefined();
+  expect(unchangedMutation?.body).not.toHaveProperty("droidReasoningDefaults");
 
   const selector = container.querySelector<HTMLButtonElement>('[role="combobox"]')!;
   await act(async () => { selector.click(); });
@@ -382,10 +383,27 @@ test("Droid refresh omits unsupported saved defaults while an explicit edit stay
   expect(previews[1]?.body).toMatchObject({ droidReasoningDefaults: { "openai/gpt-test": "low" } });
   expect(container.querySelector<HTMLButtonElement>('[role="combobox"]')?.disabled).toBe(true);
   await confirmDialog("Apply");
-  const mutation = requests.find(request => request.method === "PUT" && request.url.endsWith("/api/client-integrations/droid"));
+  const mutation = requests.filter(request => request.method === "PUT" && request.url.endsWith("/api/client-integrations/droid")).at(-1);
   expect(mutation?.body).toMatchObject({ droidReasoningDefaults: { "openai/gpt-test": "low" } });
 });
 
+test("Droid explicitly clearing its last default sends an empty map for review and commit", async () => {
+  stateResponse = () => json(status({ clientId: "droid", droidReasoning: {
+    models: [{ model: "openai/gpt-test", label: "Test model", efforts: ["low"] }],
+    defaults: { "openai/gpt-test": "low" },
+  } }));
+  await mountClient(true, "droid");
+  await act(async () => { container.querySelector<HTMLButtonElement>('[role="combobox"]')!.click(); });
+  const clear = [...testWindow.document.querySelectorAll('[role="option"]')].find(option => option.textContent === "No default") as HTMLButtonElement;
+  await act(async () => { clear.click(); });
+  await act(async () => { buttonByText("Save / review changes")!.click(); });
+  const preview = requests.find(request => request.method === "POST" && request.url.endsWith("/api/client-integrations/preview"));
+  expect(preview?.body).toHaveProperty("droidReasoningDefaults", {});
+  await confirmDialog("Apply");
+  const mutation = requests.find(request => request.method === "PUT" && request.url.endsWith("/api/client-integrations/droid"));
+  expect(mutation?.body).toHaveProperty("droidReasoningDefaults", {});
+});
+
 function buttons(): HTMLButtonElement[] {
   return Array.from(container.querySelectorAll("button")) as unknown as HTMLButtonElement[];
 }

gui/tests/models-visibility-queue.test.tsx

@@ -161,6 +161,17 @@ test("a saved write whose reconciliation fails never publishes success", async (
   expect(container.querySelector(".action-toast")?.className).toContain("notice-err");
 });
 
+for (const response of [() => new Response(null, { status: 204 }), () => new Response("saved")]) {
+  test("a successful visibility write without JSON reconciles without a network error", async () => {
+    await mount();
+    await click("a");
+    await settle(response());
+    expect(pressed("a")).toBe("false");
+    expect(container.querySelector(".action-toast")?.className).toContain("notice-ok");
+    expect(container.querySelector(".action-toast")?.textContent).not.toContain("Network");
+  });
+}
+
 test("a click during reconciliation survives the old read and starts another ordered write", async () => {
   await mount();
   await click("a");

gui/tests/usage-custom-range.test.tsx

@@ -570,3 +570,25 @@ test("throughput renders in summary, Models and Providers with honest missing-sa
     expect(rows[1]?.querySelector(`[title="${en["usage.throughput.unmeasured"]}"]`)?.textContent).toContain("—");
   }
 });
+
+for (const invalid of [null, "20", "Infinity", NaN, Infinity, -Infinity]) {
+  test(`invalid throughput ${String(invalid)} is unavailable in every display`, async () => {
+    await mount();
+    const base = report(requests[0], "invalid-throughput-model");
+    const metric = { throughputTokensPerSec: invalid, throughputSamples: 2 };
+    const data = { ...base, summary: { ...base.summary, ...metric },
+      models: [{ ...base.models[0], ...metric }], providers: [{ ...base.models[0], ...metric }],
+    };
+    // A custom response keeps nonfinite values visible at the GUI boundary; JSON encodes them as null.
+    const response = Response.json(data);
+    response.json = async () => data;
+    await act(async () => { requests[0].resolve(response); });
+    const title = en["usage.throughput.unmeasured"];
+    expect(container.querySelector(`.usage-cost-row[title="${title}"] .stat-value`)?.textContent).toBe("—");
+    for (const section of ["models", "providers"]) {
+      const cell = container.querySelector(`#usage-section-${section} tbody td[title="${title}"]`);
+      expect(cell?.textContent).toBe("—");
+      expect(cell?.querySelector(".sr-only")).toBeNull();
+    }
+  });
+}