Skip to content

fix(models): don't auto-hide rate-limited/timeout models on Test All #3849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
diegosouzapw merged 3 commits into from
Jun 14, 2026
Merged

fix(models): don't auto-hide rate-limited/timeout models on Test All #3849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc1e22f
fix(models): don't auto-hide rate-limited/timeout models on Test All
lukmanc405 Jun 14, 2026
622373e
Merge remote-tracking branch 'origin/release/v3.8.25' into fix/test-a…
diegosouzapw Jun 14, 2026
75adf6b
chore(quality): re-baseline providerPageHelpers for transient-hide gu…
lukmanc405 Jun 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions file-size-baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"src/app/(dashboard)/dashboard/providers/[id]/hooks/useProviderConnections.ts": 954,
"src/app/(dashboard)/dashboard/providers/[id]/hooks/useProviderModels.ts": 155,
"src/app/(dashboard)/dashboard/providers/[id]/hooks/useProviderSettings.ts": 264,
"src/app/(dashboard)/dashboard/providers/[id]/providerPageHelpers.ts": 939,
"src/app/(dashboard)/dashboard/providers/[id]/providerPageHelpers.ts": 955,
"src/app/(dashboard)/dashboard/providers/components/onboarding/ProviderOnboardingWizard.tsx": 906,
"src/app/(dashboard)/dashboard/providers/page.tsx": 1925,
"src/app/(dashboard)/dashboard/runtime/RuntimePageClient.tsx": 1198,
Expand Down Expand Up @@ -124,5 +124,6 @@
"_rebaseline_2026_06_13_v3825_prettier_reconcile": "Reconciliação tardia: o prettier do pre-commit reformatou 3 arquivos DEPOIS da medição de file-size dos PRs, inflando linhas além do baseline setado — OAuthModal.tsx 956→960 e providers.ts 3146→3147 (#3324), combo.ts 5162→5164 (#2743d). Bumps de reformatação automática (sem lógica nova). LIÇÃO: medir file-size pós-commit (pós-prettier), não antes.",
"_rebaseline_2026_06_14_3826_release_drift": "Re-baseline release/v3.8.25 drift already documented from #3809 owner changes: ProxyRegistryManager.tsx 1072→1089, sidebarVisibility.ts 990→1006, schemas.ts 2519→2522. This PR does not touch those source files; updating the frozen values restores Fast Quality Gates on the current release branch.",
"_rebaseline_2026_06_14_3825_combo_stickiness": "Re-baseline #3825 (sessionless combo stickiness + reasoning-aware readiness): combo.ts 5164→5198 (+34, pós-prettier). Crescimento = deriveComboSessionKey() + effectiveSessionId threading nos sites de read/write do pin server-side. streamReadinessPolicy.ts não-frozen (sob cap). Lógica coesa no handler de combo; não-extraível.",
"_rebaseline_2026_06_14_r3_3835_kiro_pricing": "PR #3835 own growth: pricing.ts 1470→1508 (+38 = missing Kiro pricing rows, claude-sonnet-4.6 etc., pure data). Also carries inherited release/v3.8.25 drift not yet frozen by prior r2 merges: RequestLoggerV2.tsx 1276→1282 (#3820 resizable log table) and combo.ts 5198→5203 (#3811 round-robin replay-response fix). This PR does not touch RequestLoggerV2/combo.ts source; updating the frozen values restores Fast Quality Gates on the current release branch."
"_rebaseline_2026_06_14_r3_3835_kiro_pricing": "PR #3835 own growth: pricing.ts 1470→1508 (+38 = missing Kiro pricing rows, claude-sonnet-4.6 etc., pure data). Also carries inherited release/v3.8.25 drift not yet frozen by prior r2 merges: RequestLoggerV2.tsx 1276→1282 (#3820 resizable log table) and combo.ts 5198→5203 (#3811 round-robin replay-response fix). This PR does not touch RequestLoggerV2/combo.ts source; updating the frozen values restores Fast Quality Gates on the current release branch.",
"_rebaseline_2026_06_14_r3_3849_transient_hide": "PR #3849 own growth: providerPageHelpers.ts 939→955 (+16 = expanded JSDoc on the auto-hide policy + transient-failure guard in evaluateTestAllEntry). Cohesive helper logic; not extractable."
}
28 changes: 22 additions & 6 deletions src/app/(dashboard)/dashboard/providers/[id]/providerPageHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,20 +120,36 @@ export interface TestAllModelOutcome {
* PassthroughModelsSection) derive — and then apply — the same per-model status.
* Previously test-all only counted ok/error for a toast and never updated
* `modelTestStatus`, so the icons stayed blank and users could not tell which
* model failed (unlike the single-model ▶ test). When `autoHideFailed` is on,
* ANY non-ok result is auto-hidden — including rate-limited / timed-out failures
* (the user opted for "hide every failure").
* model failed (unlike the single-model ▶ test).
*
* Auto-hide policy: when `autoHideFailed` is on, only NON-TRANSIENT failures are
* hidden. Transient failures (rate-limited, timeout) are surfaced as 'error' on
* the row icon but NOT hidden, because:
* - The provider may have been temporarily throttled during a parallel batch
* (a single Test All across 10+ models routinely trips per-account rate
* limits on subscription-tier APIs).
* - The model itself is not broken — a retry seconds later would succeed.
* - Hidden state persists across server restarts and silently removes the
* model from `/v1/models`, so a transient blip turns into a permanent
* catalog gap that the user can only recover from by editing the DB or
* hand-toggling each row.
*
* Genuine failures (`status:"error"` without a transient flag — e.g. upstream
* 400 "invalid model", schema mismatch, auth failure) ARE still auto-hidden,
* which is the intended use of the toggle.
*/
export function evaluateTestAllEntry(
entry: { status?: "ok" | "error"; rateLimited?: boolean; isTimeout?: boolean } | null | undefined,
autoHideFailed: boolean
): TestAllModelOutcome {
const ok = entry?.status === "ok";
const transient = Boolean(entry?.rateLimited || entry?.isTimeout);
return {
status: ok ? "ok" : "error",
// User opted for "hide every failure": any non-ok result is auto-hidden when
// the toggle is on, including rate-limited / timed-out failures.
shouldHide: !ok && autoHideFailed,
// Hide only persistent failures. Transient (rate-limited, timeout) are
// surfaced on the icon but kept visible so a single throttled batch test
// does not silently wipe the catalog.
shouldHide: !ok && autoHideFailed && !transient,
};
Comment on lines 145 to 153

@gemini-code-assist gemini-code-assist Bot Jun 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Critical Bug: Skipped or Untested Models are Silently Auto-Hidden

When a batch test is run, there are two scenarios where a model's test entry (entry) can be undefined or null:

  1. Early Stopping: The batch test runner in src/app/api/models/test-all/route.ts stops early if it encounters consecutive rate limits (CONSECUTIVE_RATE_LIMIT_STOP_THRESHOLD). Any remaining models in the list are skipped and will not have an entry in the response.
  2. API/Network Failures: If the /api/models/test-all request returns a non-200 response (e.g., a 500 error or validation error) but succeeds at the HTTP/JSON level, result.results will be undefined, meaning entry is undefined for all models.

Under the current implementation, if entry is undefined or null, ok is evaluated as false, and transient is false. If autoHideFailed is enabled, shouldHide evaluates to true. This causes the client to silently auto-hide all skipped or untested models, permanently removing them from the catalog and /v1/models!

To prevent this, we should only auto-hide a model if we have an explicit, confirmed failure status (status: "error") from the test runner.

  const ok = entry?.status === "ok";
  const isError = entry?.status === "error";
  const transient = Boolean(entry?.rateLimited || entry?.isTimeout);
  return {
    status: ok ? "ok" : "error",
    // Hide only persistent failures. Transient (rate-limited, timeout) are
    // surfaced on the icon but kept visible so a single throttled batch test
    // does not silently wipe the catalog. Untested/skipped models (where entry is
    // undefined/null due to early stop) must not be hidden.
    shouldHide: isError && autoHideFailed && !transient,
  };

}

Expand Down
13 changes: 9 additions & 4 deletions tests/unit/test-all-model-status.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,21 @@ test("failed entry maps to status 'error' and hides only when autoHide is on", (
});
});

test("rate-limited / timeout failures show 'error' and ARE auto-hidden (hide every failure)", () => {
test("rate-limited / timeout failures show 'error' but are NOT auto-hidden", () => {
// Rate-limited and timeout are TRANSIENT — the model itself is fine, the
// provider was just throttled during a parallel Test All. Hiding it would
// silently remove a working model from /v1/models with no recovery path
// short of manual DB edit or per-row eye-toggle. We surface the failure on
// the row icon (status: 'error') but keep the model visible.
assert.deepEqual(evaluateTestAllEntry({ status: "error", rateLimited: true }, true), {
status: "error",
shouldHide: true,
shouldHide: false,
});
assert.deepEqual(evaluateTestAllEntry({ status: "error", isTimeout: true }, true), {
status: "error",
shouldHide: true,
shouldHide: false,
});
// ...but only when the toggle is on
// Toggle off → still not hidden, of course.
assert.deepEqual(evaluateTestAllEntry({ status: "error", rateLimited: true }, false), {
status: "error",
shouldHide: false,
Comment on lines 51 to 53

@gemini-code-assist gemini-code-assist Bot Jun 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update Test for Missing/Null/Empty Entries

Since we should only auto-hide models with an explicit "error" status (to prevent skipped/untested models from being hidden when a batch test stops early), the test "missing / null / empty entry is treated as a failure" at the bottom of this file (lines 57-63) needs to be updated to assert shouldHide: false.

Currently, that test is outside the modified diff hunk, so we cannot provide a direct code suggestion for it, but please update it manually to:

test("missing / null / empty entry is treated as a failure but NOT hidden", () => {
  for (const entry of [undefined, null, {}]) {
    const out = evaluateTestAllEntry(entry, true);
    assert.equal(out.status, "error", `entry=${JSON.stringify(entry)} → error`);
    assert.equal(out.shouldHide, false);
  }
});

Expand Down