Skip to content

fix(dashboard): make provider card warning indicators expose the interaction they advertise - #10448

Merged
diegosouzapw merged 3 commits into
release/v3.8.50from
fix/10261-provider-warning-badges
Aug 18, 2026
Merged

diegosouzapw merged 3 commits into
release/v3.8.50from
fix/10261-provider-warning-badges

Conversation

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Closes #10261

Root cause

src/app/(dashboard)/dashboard/providers/components/ProviderCard.tsx rendered two warning indicators that advertised interaction they did not implement:

  1. Usage-risk indicator (subscriptionRisk) — a plain <span title="… click for details"> with no onClick, no role, and no dialog. Clicking it did nothing.
  2. Connection warning-count badge — a static Badge with no title (no reasons exposed) and no click affordance, even though the reasons (failures, lastFailure, totalRequests) already exist per-connection in providerSpecificData.apiKeyHealth[] and are shown in EditConnectionModal.tsx.

Fix

  • The usage-risk indicator is now a real <button role... aria-haspopup="dialog"> that opens an accessible Modal (reuses the existing riskNotice copy, keyed by riskNoticeVariant).
  • The warning-count badge is wrapped in a keyboard- and pointer-interactive control (role="button", tabIndex=0, Enter/Space activation) exposing a sanitized reasons summary (max failure count + relative last-failure time — never raw upstream error text, Hard Rule fix(ui): fix Select dropdown dark theme inconsistency #12) via title/aria-label, and navigates to the provider detail/health view on activation.
  • The two indicators are now visually distinct: the risk indicator keeps the bare material-symbols-outlined glyph; the warning badge is the pill-shaped Badge component.
  • Both new interactive elements call preventDefault()/stopPropagation() so they don't trigger the card's wrapping <Link> navigation (existing pattern from handleTestClick/handleToggle).
  • page.tsx now aggregates a sanitized warningMaxFailures / warningLastFailureRelative summary from apiKeyHealth alongside the existing warning count.
  • Added providers.riskNotice.detailsTitle and providers.warningNotice.{tooltip,lastFailureSuffix,ariaLabel} to en.json.

Regression test

src/app/(dashboard)/dashboard/providers/components/__tests__/providerCardWarningIndicators.test.tsx (vitest, matches the tests/unit/**/*.test.tsx / dashboard/providers/**/__tests__/** include globs in vitest.config.ts):

  • risk indicator opens an accessible dialog on click, and never leaks credential-shaped strings
  • risk indicator is a real interactive control reachable and activatable by keyboard (focus + Enter)
  • warning badge exposes sanitized reasons + a distinct click/keyboard affordance (tabindex=0, no raw upstream error strings)
  • the two indicators use visually distinct styling
✓ src/app/.../__tests__/providerCardWarningIndicators.test.tsx (4 tests) — 4 passed

Gates run

  • npm run typecheck:core — clean
  • npx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files> — clean
  • node scripts/check/check-file-size.mjs — OK, no new violations on touched files
  • node scripts/check/check-complexity.mjs — OK (2456 vs baseline 2774)
  • node scripts/check/check-cognitive-complexity.mjs — OK (1104 vs baseline 1223)
  • node scripts/check/check-test-discovery.mjs — OK, new test file discovered
  • New regression test: 4/4 passing under npx vitest run
  • Pre-existing tests referencing ProviderCard (tests/unit/ui/providerCardHandle.test.tsx, highlightableProviderCard.test.tsx, providerCardKimiPartnerAccent.test.tsx, card-hover-visible.test.ts, providers-page-utils.test.ts) — confirmed by content inspection that none touch subscriptionRisk/warningCount/the changed lines; a devbox-saturated (13-way parallel fan-out) npm run test:unit run hit vitest worker-pool timeouts unrelated to this diff, consistent with known devbox-under-load flakiness, not a regression from this change.

⚠️ base-red inherited: #9985 — ESLint errors (2) from #10250 (unrelated i18n PT-PT translation commit; pre-existing on the release tip, not touched by this PR).

…raction they advertise

The usage-risk indicator (subscriptionRisk) promised "click for details" in its
tooltip but was a bare <span> with no onClick/role/dialog. The connection
warning-count badge exposed neither a title tooltip (reasons) nor any click
affordance, even though the reasons already exist in
providerSpecificData.apiKeyHealth[].

Turn the risk indicator into a real <button role/aria-haspopup="dialog"> that
opens an accessible Modal reusing the existing riskNotice copy, and wrap the
warning badge in a keyboard- and pointer-interactive control that surfaces a
sanitized reasons summary (max failure count + relative last-failure time,
never raw upstream error text) and navigates to the connection detail/health
view on activation. Both indicators are now visually distinct (bare icon vs.
pill Badge).

Closes #10261
… all 42 locales (#10261)

Real Vietnamese translations (vi.json has a strict no-__MISSING__-marker gate);
other 41 locales carry the sync-ui __MISSING__ placeholder pending the normal
translation pass.
@diegosouzapw

Copy link
Copy Markdown
Collaborator Author

Code review

Found 1 issue:

  1. The new Vitest regression test is under src/app/.../__tests__/, but the repository rule requires all unit, integration, ecosystem, and Vitest tests to live under tests/. Move src/app/(dashboard)/dashboard/providers/components/__tests__/providerCardWarningIndicators.test.tsx into a tests/ subtree while preserving the assertions.

Reference: https://github.com/diegosouzapw/OmniRoute/blob/85444272e521b4c12f3c2cf27c1f81dbf4041b2f/AGENTS.md#L247-L250

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw
diegosouzapw merged commit 8dc797f into release/v3.8.50 Aug 18, 2026
11 checks passed
@diegosouzapw
diegosouzapw deleted the fix/10261-provider-warning-badges branch August 19, 2026 18:47
diegosouzapw pushed a commit that referenced this pull request Aug 25, 2026
…en mcp shard

None of these are cycle regressions. The Vitest job runs test:vitest (mcp shard)
then test:vitest:ui; the mcp shard was failing on a missing glm-5.3-max and aborted
the job before the ui shard ever ran. Fixing that shard this cycle unmasked 34 ui
failures that had been broken since 18-23 Aug — four separate PRs that moved a
contract and updated their own tests but not their siblings.

- ProviderCard gained useRouter() in #10448; four test files render it without
  mocking next/navigation and died on 'invariant expected app router to be mounted'.
  The sibling created alongside #10448 already had the mock — it just was not
  applied to the other four consumers.

- SkillCoverage gained a required config category. The four fixtures in
  agent-skills-page still described only api/cli, so the component read
  config.have off undefined. Values were chosen per scenario rather than pasted:
  full coverage gets 2/2 so its bar stays emerald, the amber fixture gets 3/4 so it
  stays amber. CoverageBar renders api -> config -> cli, so the new bar lands in the
  MIDDLE and the cli assertions moved from index [1] to [2]; without that the cli
  checks would have passed while measuring the config bar. The aria test now pins
  all three bars.

- CliAgentsPage hardcoded AGENT_IDS, which had already drifted once (6 -> 8 with
  omp/letta, per its own comment) and drifted again with prime-agent (#11166). It is
  now derived from CLI_TOOLS. This is why an agent missing from that list is not
  cosmetic: it never enters the status map, defaults to not_installed, and adds a
  phantom card to the filter and count tests. Deriving keeps the fixture in sync by
  construction instead of waiting for the next agent.

- claudeTlsClient asserted proxyUrl was undefined inside a test literally named
  'falls back to env var when per-call proxyUrl not provided' — it pinned the old
  behaviour where testOverride bypassed proxy resolution. #10910 moved resolution
  ahead of the override on purpose ('so test overrides and the real path both see
  it'), so the assertion now checks the fallback the test name promises.

test:vitest:ui goes from 34 failures to 14. The remaining 14 sit in six files none
of this commit touches (AutoComboCatalog, CoolingConnectionsPanel, ProxyRegistryManager
x2, connectionsSearchFilter) plus one claudeTlsClient case that passes in isolation
and only fails in the full run — i.e. cross-file pollution. They need a clean
environment to judge: this devbox resolves part of its tree through a stray pnpm
store and has already produced one phantom failure count this cycle.

Refs #10692
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…raction they advertise (cheaper-inference#10448)

* fix(dashboard): make provider card warning indicators expose the interaction they advertise

The usage-risk indicator (subscriptionRisk) promised "click for details" in its
tooltip but was a bare <span> with no onClick/role/dialog. The connection
warning-count badge exposed neither a title tooltip (reasons) nor any click
affordance, even though the reasons already exist in
providerSpecificData.apiKeyHealth[].

Turn the risk indicator into a real <button role/aria-haspopup="dialog"> that
opens an accessible Modal reusing the existing riskNotice copy, and wrap the
warning badge in a keyboard- and pointer-interactive control that surfaces a
sanitized reasons summary (max failure count + relative last-failure time,
never raw upstream error text) and navigates to the connection detail/health
view on activation. Both indicators are now visually distinct (bare icon vs.
pill Badge).

Closes cheaper-inference#10261

* i18n(providers): sync riskNotice.detailsTitle + warningNotice keys to all 42 locales (cheaper-inference#10261)

Real Vietnamese translations (vi.json has a strict no-__MISSING__-marker gate);
other 41 locales carry the sync-ui __MISSING__ placeholder pending the normal
translation pass.

* test(dashboard): relocate provider warning regression test

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: adevwithpurpose <adevwithpurpose@users.noreply.github.com>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…en mcp shard

None of these are cycle regressions. The Vitest job runs test:vitest (mcp shard)
then test:vitest:ui; the mcp shard was failing on a missing glm-5.3-max and aborted
the job before the ui shard ever ran. Fixing that shard this cycle unmasked 34 ui
failures that had been broken since 18-23 Aug — four separate PRs that moved a
contract and updated their own tests but not their siblings.

- ProviderCard gained useRouter() in cheaper-inference#10448; four test files render it without
  mocking next/navigation and died on 'invariant expected app router to be mounted'.
  The sibling created alongside cheaper-inference#10448 already had the mock — it just was not
  applied to the other four consumers.

- SkillCoverage gained a required config category. The four fixtures in
  agent-skills-page still described only api/cli, so the component read
  config.have off undefined. Values were chosen per scenario rather than pasted:
  full coverage gets 2/2 so its bar stays emerald, the amber fixture gets 3/4 so it
  stays amber. CoverageBar renders api -> config -> cli, so the new bar lands in the
  MIDDLE and the cli assertions moved from index [1] to [2]; without that the cli
  checks would have passed while measuring the config bar. The aria test now pins
  all three bars.

- CliAgentsPage hardcoded AGENT_IDS, which had already drifted once (6 -> 8 with
  omp/letta, per its own comment) and drifted again with prime-agent (cheaper-inference#11166). It is
  now derived from CLI_TOOLS. This is why an agent missing from that list is not
  cosmetic: it never enters the status map, defaults to not_installed, and adds a
  phantom card to the filter and count tests. Deriving keeps the fixture in sync by
  construction instead of waiting for the next agent.

- claudeTlsClient asserted proxyUrl was undefined inside a test literally named
  'falls back to env var when per-call proxyUrl not provided' — it pinned the old
  behaviour where testOverride bypassed proxy resolution. cheaper-inference#10910 moved resolution
  ahead of the override on purpose ('so test overrides and the real path both see
  it'), so the assertion now checks the fallback the test name promises.

test:vitest:ui goes from 34 failures to 14. The remaining 14 sit in six files none
of this commit touches (AutoComboCatalog, CoolingConnectionsPanel, ProxyRegistryManager
x2, connectionsSearchFilter) plus one claudeTlsClient case that passes in isolation
and only fails in the full run — i.e. cross-file pollution. They need a clean
environment to judge: this devbox resolves part of its tree through a stray pnpm
store and has already produced one phantom failure count this cycle.

Refs cheaper-inference#10692
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(dashboard): provider warning badges should expose their details consistently

2 participants