Skip to content

feat: add Plus tier as separate category in ProviderLimits - #126

Merged
diegosouzapw merged 1 commit into
diegosouzapw:mainfrom
nyatoru:fix/codex-plam
Feb 25, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:mainfrom
nyatoru:fix/codex-plam

Conversation

@nyatoru

@nyatoru nyatoru commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @nyatoru, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a new 'Plus' tier within the ProviderLimits feature. This enhancement improves the granularity of plan categorization and display, allowing the application to distinguish and present 'Plus' and 'Paid' plans as a separate category from 'Pro' plans in the dashboard's usage section. The change impacts both the UI filters and the underlying logic for normalizing plan tiers.

Highlights

  • New 'Plus' Tier Introduction: A new 'Plus' tier has been added to the TIER_FILTERS array, allowing it to be displayed as a distinct category in the ProviderLimits UI.
  • Plan Normalization Logic Update: The normalizePlanTier utility function has been refactored to correctly identify and categorize plans containing 'PLUS' or 'PAID' as the new 'Plus' tier, separating them from the 'Pro' tier.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • src/app/(dashboard)/dashboard/usage/components/ProviderLimits/index.tsx
    • Added 'plus' as a new filter option in the TIER_FILTERS array.
  • src/app/(dashboard)/dashboard/usage/components/ProviderLimits/utils.tsx
    • Modified the normalizePlanTier function to create a distinct categorization for 'Plus' and 'Paid' plans.
    • Adjusted the 'Pro' tier condition to exclude 'Plus' and 'Paid' keywords.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request successfully introduces a new 'Plus' tier category within the ProviderLimits component. The changes correctly update the tier filters in index.tsx and refine the normalizePlanTier logic in utils.tsx to distinguish the 'Plus' tier from 'Pro'. The ranking and labeling for the new tier are logically placed within the existing hierarchy.

Comment on lines +236 to +237
if (upper.includes("PLUS") || upper.includes("PAID")) {
return { key: "plus", label: "Plus", variant: "secondary", rank: 2, raw };

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

The normalizePlanTier function assigns variant: "secondary" to the new 'Plus' tier. However, the Badge component (src/shared/components/Badge.tsx) does not have a secondary variant defined in its variants object. This will result in the badge for the 'Plus' tier not displaying correctly, as the styling classes will be undefined. Please define the secondary variant in Badge.tsx to ensure proper styling.

@diegosouzapw
diegosouzapw merged commit af34095 into diegosouzapw:main Feb 25, 2026
10 checks passed
diegosouzapw added a commit that referenced this pull request Mar 7, 2026
Approved: Clean, well-scoped change that correctly separates Plus/Paid tier from Pro in ProviderLimits.
i1hwan added a commit to i1hwan/ApexRoute that referenced this pull request May 7, 2026
4 valid blockers from Copilot's 2026-05-07 review on commit 8483e8a:

1. RoutingBadge.getExcludedI18nKey: scoreAccount can exclude with
   reasons "session<5%" / "weekly<5%" but the i18n switch only mapped
   "quota_exhausted_unknown_reset" to routingPriorityExcludedQuota,
   so quota-low rows fell through to "Unknown". Now any reason
   containing "<5%" maps to the quota-localized label.

2. i18n routingScoreBase label "Base score (avg)" was misleading after
   v7 changed baseScore from arithmetic mean to Math.max(...).
   Updated all 32 locales (en + ko hand-translated, 30 placeholders
   updated to "Base score (max)"; ko: "기본 점수 (최댓값)").

3. QuotaVisualization.pickWindow: the previous "<key> " prefix match
   could collide with model-specific windows like "weekly Sonnet (7d)"
   depending on object iteration order. Now does an exact-match /
   parenthesised match in pass 1 ("weekly (7d)" / "session (5h)") and
   only falls back to the unparenthesised prefix in pass 2 — so the
   per-row Weekly mini-bar always reflects the overall window when
   present in the cache.

4. ProviderLimits row was double-rendering session/weekly windows:
   QuotaVisualization (mini-bars) AND quota.quotas.map (per-model bars)
   both included them. Added isOverallWindowName helper and filter the
   map() iteration so per-model bars exclude session/weekly entries.

Outdated comments (auto-flagged by Copilot but already fixed in earlier
commits, verified by direct code inspection):
  - 70625f7 expectedBase assertion (fixed in this PR's earlier
    api-usage-provider-limits update; line 152 has the assertion)
  - 70625f7 caches override (fixed via _ignoredExtraCaches destructure
    in route.ts:190)
  - 70625f7 single-account banner (fixed: group.length === 0)
  - 94cd28d unsafe casts (already removed in route.ts:95-100)
  - 8483e8a diegosouzapw#126 banner scroll-to (already fixed by 25a52df)
  - 8483e8a RoutingBadge null-check uses Number.isFinite (correct)

Non-blocking performance/accessibility comments deferred:
  - refreshRoutingOnly debounce (perf nit, separate PR if needed)
  - disclaimer info icon focusable (a11y nit, separate PR)

Verify: prettier ✓ eslint ✓ typecheck:core ✓ typecheck:noimplicit:core ✓
test:unit 2821/2821 PASS ✓
i1hwan added a commit to i1hwan/ApexRoute that referenced this pull request May 7, 2026
…ransparency banner, dual quota bars + routing v7 burn-pressure max (#25)

* feat(strategy): add normalizeConfiguredStrategy helper

Returns the input value if it appears in SETTINGS_FALLBACK_STRATEGY_VALUES,
otherwise "fill-first". Mirrors auth.ts:571's runtime fall-through behavior
for unrecognized configured values, but in a single shared helper that the
upcoming dashboard routing-preview API can consume without importing from
auth.ts.

Naming intent: this surfaces the user's configured fallback strategy, NOT
the strategy auth actually executes for any given request. Auth's per-
provider dispatch only explicitly branches on round-robin / p2c / random /
least-used / cost-optimized / strict-random / earliest-reset-first; the
other six valid configured values fall through to fill-first behavior.
The dashboard banner displays the configured value with that disclaimer.

7 new tests cover all 13 valid values + null + undefined + empty + garbage
+ case sensitivity. Tests pass; lint and typecheck:core/noimplicit:core
clean.

First commit of feat/provider-limits-priority-v3.1 (see
.sisyphus/plans/provider-limits-priority-and-autorefresh.md).

* chore(i18n): add usage-page keys for routing badge, banner, and auto-refresh (32 locales)

30 new keys under usage namespace covering:
- routing priority badge labels (Next, P{rank}, excluded reasons)
- routing score breakdown tooltip rows
- transparency banner strings + disclaimer about model-locked routes
- auto-refresh toggle + interval selector

en and ko hand-translated; 30 other locales seeded with English
placeholder values via a one-shot Node script. next-intl 4.x default
behavior renders namespace.key string for missing keys (no crash), so
the placeholders only prevent users from seeing raw keys until proper
translations arrive.

Lint and prettier clean. settings-i18n-keys test still passes.

Lands before commit 3 (API exposure) and commits 4-7 (UI components)
so that no UI commit references a key that doesn't yet exist.

Second commit of feat/provider-limits-priority-v3.1.

* feat(api): expose routing preview + configuredRoutingStrategy on /api/usage/provider-limits

GET and POST /api/usage/provider-limits now additionally return:
- configuredRoutingStrategy: normalized via the new
  normalizeConfiguredStrategy helper (always one of the 13 known values).
- routing: a Record<connectionId, RoutingPreviewEntry> map.

Routing preview construction (inline computeRouting helper, ~110 LOC):
- Group connections by provider.
- For earliest-reset-first: pre-filter inactive / rate-limited / terminal
  rows with the SAME predicates auth.ts:431-442 uses (isAccountUnavailable
  from open-sse/services/accountFallback, isTerminalConnectionStatus from
  src/sse/services/accountTerminalStatus). The pre-filter prevents the
  dashboard from labeling production-ineligible rows as "Next". Surviving
  rows go through the existing scoreAccount(); ranks are dense 1..N
  per provider over non-excluded entries; scoring-excluded entries get
  rank:null with the precise reason from scoreAccount.
- For other strategies: every entry has rank:null. Inactive / rate-limited
  / terminal still surface as excluded so the user sees red rows; we do
  NOT compute a numeric rank because auth's dispatch for these strategies
  is not score-based.

Per Oracle v3 review:
- Field name configuredRoutingStrategy (not "routingStrategy") to be
  honest about what is exposed: this is what the user configured, not
  what auth executes (auth's dispatch only branches on 6 + ERF; the
  other 6 valid configured values fall through to fill-first behavior).
- Per-request model context (auth.ts:439 isModelLocked) is deliberately
  NOT replicated. The dashboard has no request model. This makes the
  preview a quota-priority preview, not a full request-level prediction.
- isAccountUnavailable imported from @omniroute/open-sse/services/
  accountFallback (verified against auth.ts:11), not @/sse/services/auth.
- rateLimitedUntil accepts string | number (epoch); coerced via
  rateLimitedSentinel before isAccountUnavailable.

Touched files:
- src/app/api/usage/provider-limits/route.ts: 44 → 234 LOC.
- docs/openapi.yaml: new path entry under /api/usage/provider-limits
  documenting GET and POST shapes.
- tests/unit/api-usage-provider-limits.test.mjs: new, 10 tests covering
  ERF dense ranks, inactive (boolean + numeric 0), rate_limited (string +
  epoch number), terminal, multi-provider grouping, breakdown shape,
  non-ERF excluded surfacing, defensive entry skipping.

Verification: prettier, eslint, typecheck:core, typecheck:noimplicit:core,
docs:sync, full test:unit 2811/2811 (was 2794) — all green.

Third commit of feat/provider-limits-priority-v3.1.

* feat(dashboard): RoutingBadge component on Provider Limits rows

Renders next to the existing tier badge in each account row:
- ERF rank == 1 → solid "Next" chip (primary tint).
- ERF rank > 1 → ghost "P{rank}" chip.
- Excluded → ghost chip with diagonal stripe overlay + localized reason
  (inactive / rate_limited / terminal / quota_exhausted_unknown_reset
  / unavailable).
- Non-ERF eligible row → renders nothing (no rank to surface).

Hover/focus reveals a self-contained tooltip with the full routing score
breakdown (session %, weekly %, session/weekly points, base score,
penalties, final score). Built without the shared Tooltip primitive
because that primitive uses whitespace-nowrap and can't render a
multi-line table; the badge owns its own positioned popover with
whitespace-pre-line and a min-width.

Wiring in ProviderLimits/index.tsx:
- New state: routingByConnection (RoutingPreviewMap) + configuredRoutingStrategy.
- fetchCachedProviderLimits parses both new fields from GET response.
- refreshAll parses both from POST response.
- New refreshRoutingOnly() helper does a lightweight GET after every
  per-row /api/usage/[connectionId] success, so the badge stays fresh
  without changing the per-connection endpoint shape.

Verification: prettier, eslint, typecheck:core, typecheck:noimplicit:core
clean. (UI behavior is best verified manually after deployment.)

Fourth commit of feat/provider-limits-priority-v3.1.

* feat(dashboard): RoutingTransparencyBanner above account list

Compact info row between the page header and the tier-filter chips:

  Configured: <strategy> · Next per provider: Claude (acct-A), GLM (acct-B)
                                                                          ℹ

For ERF strategy, lists each provider that has 2+ accounts with the next
account it would pick. The provider/account pair becomes a clickable
button that scrolls to that provider's first row via data-connection-id.
For all-excluded providers it renders "(all excluded)". For non-ERF
strategies it shows only the configured strategy.

The trailing info icon hovers to the disclaimer that this is a
quota-priority preview, not a full prediction (per-request model lockout
isn't reproduced).

Account names truncated to 16 chars with ellipsis. Uses pickMaskedDisplayValue
for the same masked email/name presentation the row already uses.

Wiring in index.tsx:
- New import + new state value (configuredRoutingStrategy was previously
  setter-only).
- Banner rendered after the header div, before tier filters.
- Each row gets data-connection-id={conn.id} so the banner's scroll
  target finds it.

Verification: prettier, eslint, typecheck:core, typecheck:noimplicit:core
clean.

Fifth commit of feat/provider-limits-priority-v3.1.

* feat(dashboard): AutoRefreshControl with race-safe visibility-aware polling

User-configurable Refresh All cadence next to the existing manual button.

UI:
- Checkbox toggle "Auto refresh" + select dropdown 1m / 2m / 5m / 10m.
- Persisted to localStorage:
    omniroute:limits:autoRefresh:enabled
    omniroute:limits:autoRefresh:intervalMs
- Dropdown disabled when toggle is off.

Behavior:
- Polls only when document.visibilityState === "visible".
- visibilitychange listener triggers an immediate refresh on tab focus
  (hidden -> visible transition), then resumes the interval cadence.
- inFlightRef prevents overlap with manual Refresh All or another auto
  refresh that hasn't returned yet.
- lastTriggerAtRef enforces a 1-second same-tick guard against the race
  where setInterval and visibilitychange fire near-simultaneously.
- onTriggerRef captures the latest onTrigger callback identity without
  re-installing the interval on every parent re-render.

SSR-safe: typeof window / typeof document guards on every browser API.
Initial state read happens in useEffect (post-mount) to avoid hydration
mismatch.

Verification: prettier, eslint, typecheck:core, typecheck:noimplicit:core
clean.

Sixth commit of feat/provider-limits-priority-v3.1.

* feat(dashboard): dual session/weekly QuotaVisualization on each row

New compact mini-bar block rendered before the existing per-model bars:

  Session remaining ████████░░░░░░░░░░░░░░  78%
  Weekly  remaining ████░░░░░░░░░░░░░░░░░░  35%

Picks the matching window from quota.quotas by name match against
"session" / "weekly" (with tolerance for "session(...)", "weekly N day",
etc. labels). Renders nothing if neither window is present in the data
(non-OAuth providers, custom self-hosted, etc.).

Color buckets identical to the existing per-model bar (>50% green,
>20% yellow, ≤20% red).

Burning rate intentionally not in this commit per v3.1 plan §1; the
quota_snapshots history exists but plumbing it requires a new GET
endpoint and a sample-noise gate that doesn't fit this PR scope.

Inserted as the first child of the quota-bar branch using a fragment
wrapper; preserves the existing per-model bar markup verbatim.

Verification: prettier, eslint, typecheck:core, typecheck:noimplicit:core
clean.

Seventh commit of feat/provider-limits-priority-v3.1.

* chore(release): bump 3.7.1 → 3.8.0 and CHANGELOG

Minor feature release for the Provider Limits routing-priority badge,
configurable auto-refresh, transparency banner, and dual session/weekly
quota bars. Verification across the stack:

- prettier ✅
- eslint ✅
- typecheck:core ✅
- typecheck:noimplicit:core ✅
- docs:sync ✅ (3.8.0 across package.json, openapi.yaml, CHANGELOG)
- full test:unit 2811/2811 ✅ (was 2794, added 7 normalize +
  10 routing-preview)

Tag after merge: apex-v2.1.0.

Eighth and final commit of feat/provider-limits-priority-v3.1.

* fix(api): preserve full caches map in /api/usage/provider-limits POST response

Copilot review of PR #25 caught a regression introduced by the original
buildResponseBody implementation: spreading `...extra` LAST allowed the
partial-failure result from syncAllProviderLimits (which only contains
successfully refreshed connections in result.caches) to override the full
disk cache map returned by getCachedProviderLimitsMap(). After Refresh All
where some upstream calls fail, the dashboard's applyCachedQuotaState
received a partial map and the failed connections' quota rows would
disappear from the UI.

Pre-PR baseline put `caches: getCachedProviderLimitsMap()` last, so the
full map always won. PR #25's buildResponseBody accidentally inverted that.

Fix: extract a pure mergeIntoResponseBody(extra, base) helper. It
explicitly drops `caches`, `routing`, and `configuredRoutingStrategy`
from `extra` via destructuring, then spreads `base` (authoritative
fields) LAST. Symmetrically protects routing/configuredRoutingStrategy
in case a future caller passes those in `extra` too.

Two new unit tests on the pure helper:
1. partial-sync result preserves both A and B in caches; errors map
   passes through restExtra
2. defensive: even an adversarial extra with phantom caches/routing/
   strategy keys cannot leak into the response

Test approach uses only the pure helper — no DB, no settings, no
connections, no syncAllProviderLimits invocation.

Closes Copilot PR #25 review comment PRRC_kwDOR_WnW868DIIW.

Plan: .sisyphus/plans/pr25-fix-plan.md commit 9 (Issue 1).

* fix(dashboard): RoutingTransparencyBanner clicks scroll to nextConn, not first row

Copilot review of PR #25 caught: the banner displayed `accountName`
derived from `nextConn` (the connection with `isNext: true`) but its
onClick scrolled to `firstConnId` which was `group[0]?.id` — the first
connection in the source-order list, which may differ from nextConn.
User clicks the banner expecting to jump to the displayed account, but
lands on a different one.

Renamed NextEntryByProvider.firstConnId → nextConnId, set it to
nextConn?.id ?? null. JSX now renders the clickable button only when
nextConnId is non-null; when it's null (unusual edge case where no
isNext was found in the routing map), the entry renders as a plain
non-clickable span instead of scrolling to a stale `group[0]`.

Closes Copilot PR #25 review comment PRRC_kwDOR_WnW868DIKK.

Plan: .sisyphus/plans/pr25-fix-plan.md commit 10 (Issue 2).

* feat(dashboard): RoutingTransparencyBanner shows single-account providers + fixes cold-cache misclassification

Copilot review of PR #25 caught: the banner's collectNextPerProvider
guard `if (group.length < 2) continue;` excluded providers with exactly
one account, even though that one account IS the next pick. The user's
original framing was "여러 계정 있을 때" but transparency consistency
benefits from showing all providers — when only one account exists for
a provider, it's still useful to confirm "Next per provider: Claude
(only-acct-X)".

Self-review of PR #25 also surfaced an edge case in the same loop
(Issue 9): when the routing map has no entry for any connection in a
provider group (e.g., right after server boot before quotaCache is
populated), nonExcludedCount stayed 0 and allExcluded was set to true —
falsely claiming "(all excluded)" when the truth is "no data yet".

Fix:
1. Lower the guard to `group.length === 0` (always-skip empty groups
   only).
2. Track both nonExcludedCount AND excludedCount during the per-group
   scan, plus a derived hasAnyEntry boolean.
3. allExcluded becomes `hasAnyEntry && nonExcludedCount === 0`, so:
   - has entries, all excluded  → "(all excluded)"
   - has entries, some eligible → render account name
   - no entries (cold cache)    → render "(—)" via existing
                                  `accountName ?? "—"` fallback

Closes Copilot PR #25 review comment PRRC_kwDOR_WnW868DIJu.

Plan: .sisyphus/plans/pr25-fix-plan.md commit 11 (Issues 4 + 9).

* refactor(api): extract shared RoutingPreview types + collapse duplicate eligibility check

Self-review of PR #25 surfaced three coupled cleanup opportunities:

Issue 3 — Type drift risk: RoutingPreviewEntry and RoutingPreviewBreakdown
were declared in TWO places: route.ts and RoutingBadge.tsx, with
RoutingTransparencyBanner.tsx importing from RoutingBadge. Future change
to backend shape would silently pass frontend type-check because frontend
read its own (stale) declaration.

Issue 7 — rateLimitedSentinel was unnecessary: the helper converted epoch
number → ISO string before passing to isAccountUnavailable, but the function
already does `new Date(unavailableUntil).getTime()` (accountFallback.ts:660-663),
which accepts strings AND numbers natively.

Issue 8 — computeRouting's ERF and non-ERF branches duplicated the
inactive/rate_limited/terminal cascade verbatim.

Fix:
1. New canonical contract `src/shared/contracts/routingPreview.ts`
   exports RoutingPreviewBreakdown / RoutingPreviewEntry / RoutingPreviewMap.
2. route.ts imports from contracts + re-exports for backwards compat with
   any existing importer.
3. RoutingBadge.tsx imports from contracts + re-exports both types so
   index.tsx's `import RoutingBadge, { type RoutingPreviewEntry } from
   "./RoutingBadge"` continues to compile unchanged.
4. RoutingTransparencyBanner.tsx imports directly from contracts (was
   importing from RoutingBadge).
5. rateLimitedSentinel deleted; checkEligibility calls
   isAccountUnavailable(c.rateLimitedUntil as never) directly.
6. checkEligibility(c, strategy) extracted as the single source of truth
   for the inactive/rate_limited/terminal cascade. Both ERF and non-ERF
   branches in computeRouting now call it.

Plan: .sisyphus/plans/pr25-fix-plan.md commit 12 (Issues 3, 7, 8).

* test(api): assert baseScore equals avg of known track scores (Copilot PR #25 review)

Copilot review of PR #25 caught: the existing test computed
`expectedBase` but never asserted it against the actual
`entry.breakdown.baseScore`. As written it only checked that baseScore
is a number, so a regression in scoreAccount's averaging math (line 331
of earliestResetFirst.ts) would not be caught by the test.

Additionally, the old `expectedBase` formula was wrong — it weighted
points by remainingPct, but the actual baseScore in earliestResetFirst.ts:331
is `trackScores.reduce((a,b) => a+b, 0) / trackScores.length` where each
trackScore is the points value (NOT pct-weighted).

Fix: replace the type-only assertion with a math assertion using the
correct averaging formula, with floating-point tolerance (1e-9). Also
handles the edge case where no tracks have known scores (baseScore = 0).
A code comment cites earliestResetFirst.ts:331 as the formula source.

Closes Copilot PR #25 review comment PRRC_kwDOR_WnW868DIHb.

Plan: .sisyphus/plans/pr25-fix-plan.md commit 13 (Issue 5).

* refactor(dashboard): extract shared getBarColor + name TOOLTIP_CLOSE_DELAY_MS constant

Self-review of PR #25 surfaced two small DRY violations:

Issue 10 — getBarColor was duplicated identically in:
  - src/app/(dashboard)/dashboard/usage/components/ProviderLimits/index.tsx
  - src/app/(dashboard)/dashboard/usage/components/ProviderLimits/QuotaVisualization.tsx
  with the same >50/>20 thresholds and same hex colors.

Issue 11 — RoutingBadge.tsx tooltip close delay was a magic number
`setTimeout(... , 80)` with no inline rationale. Plan v3.1 §11 required
naming the constant with a 1-line intent comment.

Fix:
1. New `quotaColors.ts` exports QUOTA_BAR_GREEN_THRESHOLD,
   QUOTA_BAR_YELLOW_THRESHOLD, QuotaBarColors, getBarColor.
2. index.tsx and QuotaVisualization.tsx both import from quotaColors.ts
   and remove their local copies. The dashboard's runtime behavior is
   identical — same thresholds, same hex colors.
3. RoutingBadge.tsx: extracted `const TOOLTIP_CLOSE_DELAY_MS = 80` at
   module top with a 2-line comment explaining the brief grace period
   for cursor traversal between badge and tooltip. Value preserved
   exactly from PR #25.

Plan: .sisyphus/plans/pr25-fix-plan.md commit 14 (Issues 10, 11).

* refactor(api): filter computeRouting inputs to USAGE_SUPPORTED_PROVIDERS

Self-review of PR #25 surfaced a small efficiency / consistency issue:
auth.ts:325 and syncAllProviderLimits:325 both call
getProviderConnections({ isActive: true }), but the routing-preview
route used getProviderConnections({}) and passed every row (including
free providers like Qoder/Pollinations) into computeRouting.

The dashboard's frontend already filters to USAGE_SUPPORTED_PROVIDERS
in filteredConnections, so any routing entry for unsupported providers
was silently discarded. Wasted backend work + potential schema
mismatches if free providers later add testStatus/rateLimitedUntil
semantics that the routing logic doesn't handle.

Fix: at module top, build a Set<string> from USAGE_SUPPORTED_PROVIDERS
(cast as readonly string[]). At the start of computeRouting's group-by
loop, skip any connection whose provider is not in the set. O(1) lookup
per connection.

Test: new "skips connections whose provider is not in
USAGE_SUPPORTED_PROVIDERS" test exercises 4 unsupported providers
(qoder, pollinations, totally-fake-provider) plus 1 supported (claude)
and asserts the routing map only contains the supported entry.

Plan: .sisyphus/plans/pr25-fix-plan.md commit 15 (Issue 12).

* fix: Copilot re-review (cast cleanup, tooltip null safety, route.ts type strictness)

Address 3 HIGH inline comments from Copilot re-review (94cd28d) and 2 pre-existing TS errors uncovered while fixing them.

## Copilot inline comments addressed

- (#2) route.ts:95-98 — Removed unnecessary type casts `as never` and
  `as Parameters<typeof isTerminalConnectionStatus>[0]` from
  `checkEligibility`. The callees accept compatible structural types
  (isAccountUnavailable is untyped, isTerminalConnectionStatus only needs
  { testStatus?: string | null }).

- (#3, #4) RoutingBadge.tsx:107-122 — Replaced `!== null` checks with
  `Number.isFinite()` to fix tooltip rendering bug. Old check
  `entry.breakdown?.X !== null` evaluated to `true` when breakdown was
  undefined, leading formatNum(undefined) to return '—' which then had
  '%' appended → '—%'. Number.isFinite correctly rejects undefined,
  null, NaN, and Infinity in one expression.

## Pre-existing TS errors fixed (introduced in PR #25 first commit, missed by CI)

- route.ts:128 (TS2345) — `scoreAccount(c)` failed because
  `ConnectionRow.isActive: boolean | number | null` was incompatible
  with `ConnectionLike.isActive?: boolean`. Now normalize via:
  `isActive: c.isActive === false || c.isActive === 0 ? false : undefined`
  preserving 'inactive' signal without manufacturing positive 'true'.
  Also normalize rateLimitedUntil with finite-number guard around
  toISOString to prevent RangeError on NaN/Infinity input.

- route.ts:193 (TS2352) — `as ConnectionRow[]` cast on
  `getProviderConnections()` (returns JsonRecord[]) replaced with
  explicit `as unknown as ConnectionRow[]` to acknowledge the
  dynamic-shape boundary.

## Guardrail

- tsconfig.typecheck-core.json — Added route.ts to `files` whitelist
  so future regressions are caught by CI typecheck:core.

Verified by Oracle (3 review rounds: identified Number.isFinite gap and
toISOString throw risk; final APPROVED_WITH_NOTES).
LSP diagnostics: clean on all 3 modified files.

Closes Copilot inline comments PRRC #2/#3/#4 (94cd28d review).

* revert: drop provider-limits route.ts from typecheck-core whitelist

Adding provider-limits/route.ts to tsconfig.typecheck-core.json's files
whitelist (commit f4e3125) caused tsc to follow the transitive import
graph through @omniroute/open-sse/services/accountFallback into the
entire open-sse/* tree, exposing 10+ pre-existing TS errors in
open-sse/executors/{antigravity,base,cliproxyapi,cloudflare-ai}.ts and
open-sse/config/forwardingKeywordRules.ts. CI Lint job failed.

These errors predate this PR and are out of scope. Reverting only the
whitelist entry; the route.ts type strictness fixes (scoreAccount
normalize, getProviderConnections cast) and the Copilot re-review fixes
remain.

Follow-up: dedicated PR to fix open-sse/* TS errors and add this file
to the whitelist as a guardrail.

* feat(routing): v7 burn-rate pressure max — fixes mean-dilution defect

v6 used arithmetic mean across session and weekly tracks. In production
this dilutes the more-urgent track with the calmer track, picking accounts
where one track is hot but the dominant burn target is on the OTHER account.

User scenario (2026-05-07 dashboard):
  APEXATGNU: session 33%/0h34m, weekly 91%/6d17h
  GNUMAX:    session 29%/1h54m, weekly 63%/1d1h
v6 picked APEXATGNU (mean 1982 > 1840) instead of GNUMAX (the account
about to waste 63% weekly in 1 day).

v7 changes the cross-track combination from arithmetic mean to max():
  pressure(Q, t, W) = Q × clamp(W/t, 1, URGENCY_CAP)
  trackScore = pressure(...)
  baseScore  = max(sessionPressure, weeklyPressure)

The two windows are independent ledgers (verified via anthropics/claude-code
issues #54750, #52135, #40513). Treating them as independent burn-pressure
signals and routing to the more perishable one matches user intent
"burn what's about to expire".

Penalty rebalance: PENALTY_BACKOFF_WEIGHT 100 -> 101 so a fully-backed-off
paid account is strictly below self-hosted score=0 (was tie at 0).

All v6 invariants preserved:
  - F1 self-hosted score=0 fallback
  - F2 per-model weekly windows ignored
  - F3 Q=100 + null resetAt -> max urgency
  - F4 multiplicative scoring + penalty layer

sessionTimePoints/weeklyTimePoints retained as @deprecated for diagnostic
UIs and external callers; v7 scoring path uses pressure() directly.

Tests updated:
  - 7 new V7 RED tests (V7-2 through V7-8 plus V7-direct)
  - 17 existing assertions recalculated using returned track.secondsToReset
    (not seeded value) to eliminate deltaSec() floor flakiness
  - api-usage-provider-limits.test.mjs:132 mean -> max assertion

Verify chain: prettier / eslint / typecheck:core / typecheck:noimplicit:core /
test:unit (2821/2821 PASS) / docs:sync. Version 3.8.0 -> 3.8.1.

Plan: .sisyphus/plans/routing-strategy-v7.md
Oracle review: APPROVED with revisions (bg_8fb7b507)
Momus review: v1->v4 with all blockers fixed (bg_8729c574, bg_d075f71b,
              bg_4c59db7e, bg_21ed0a44)

* fix: address Copilot review on v7 (PR #25 review round 4)

4 valid blockers from Copilot's 2026-05-07 review on commit 8483e8a:

1. RoutingBadge.getExcludedI18nKey: scoreAccount can exclude with
   reasons "session<5%" / "weekly<5%" but the i18n switch only mapped
   "quota_exhausted_unknown_reset" to routingPriorityExcludedQuota,
   so quota-low rows fell through to "Unknown". Now any reason
   containing "<5%" maps to the quota-localized label.

2. i18n routingScoreBase label "Base score (avg)" was misleading after
   v7 changed baseScore from arithmetic mean to Math.max(...).
   Updated all 32 locales (en + ko hand-translated, 30 placeholders
   updated to "Base score (max)"; ko: "기본 점수 (최댓값)").

3. QuotaVisualization.pickWindow: the previous "<key> " prefix match
   could collide with model-specific windows like "weekly Sonnet (7d)"
   depending on object iteration order. Now does an exact-match /
   parenthesised match in pass 1 ("weekly (7d)" / "session (5h)") and
   only falls back to the unparenthesised prefix in pass 2 — so the
   per-row Weekly mini-bar always reflects the overall window when
   present in the cache.

4. ProviderLimits row was double-rendering session/weekly windows:
   QuotaVisualization (mini-bars) AND quota.quotas.map (per-model bars)
   both included them. Added isOverallWindowName helper and filter the
   map() iteration so per-model bars exclude session/weekly entries.

Outdated comments (auto-flagged by Copilot but already fixed in earlier
commits, verified by direct code inspection):
  - 70625f7 expectedBase assertion (fixed in this PR's earlier
    api-usage-provider-limits update; line 152 has the assertion)
  - 70625f7 caches override (fixed via _ignoredExtraCaches destructure
    in route.ts:190)
  - 70625f7 single-account banner (fixed: group.length === 0)
  - 94cd28d unsafe casts (already removed in route.ts:95-100)
  - 8483e8a diegosouzapw#126 banner scroll-to (already fixed by 25a52df)
  - 8483e8a RoutingBadge null-check uses Number.isFinite (correct)

Non-blocking performance/accessibility comments deferred:
  - refreshRoutingOnly debounce (perf nit, separate PR if needed)
  - disclaimer info icon focusable (a11y nit, separate PR)

Verify: prettier ✓ eslint ✓ typecheck:core ✓ typecheck:noimplicit:core ✓
test:unit 2821/2821 PASS ✓

* test(routing): tolerate deltaSec() floor drift in v7 score assertions

CI on linux node 20/22 hit "github-style account with no session window
scores from weekly only" assertion fail at 1e-9 tolerance because:

  - test calls scoreWeeklyTrack(...) → captures w.secondsToReset = N
  - test computes expectedW = pressure(Q, N, W)
  - test calls scoreAccount(...) which internally calls
    scoreWeeklyTrack(...) again → may see secondsToReset = N-1
    (Date.now drifted by ≥1ms across calls, deltaSec floors)
  - assertion `Math.abs(result.score - expectedW) < 1e-9` fails because
    pressure(Q, N-1, W) − pressure(Q, N, W) ≈ Q × W/(N(N-1)) ≫ 1e-9

Fix: introduce withinPressureMaxBand() / pressureDriftBand() helpers
that build inclusive [lo, hi] band tolerating one floor() step in either
direction, then assert result.score ∈ band. Applied to the 3 affected
tests (single-track ghacct, F2-2 max-not-mean, F4-3 idle abundance).

Same machine-local test passes at 1e-9 because Date.now resolution +
test ordering rarely cross floor boundary; CI hosts with slower clocks
do cross. CI assertion now stable.

Verify: prettier ✓ eslint ✓ typecheck:core ✓ typecheck:noimplicit:core ✓
test:unit 2821/2821 PASS ✓

* test(routing): widen deltaSec drift band to +/-1 second

Earlier fix only checked sec vs sec-1 but the second call can see
sec+1 too (depends on Date.now() millisecond rollover ordering).
F2-2 hit this on linux node 20: result.score=220.588 fell just below
the lo=220.59 boundary because the band was built one-directional.

Now pressureDriftBand checks {sec-1, sec, sec+1} and takes the full
[min, max] envelope. All 56 strategy tests still pass locally.

* fix: address Copilot review round 5 (PR #25 review on fa4d02f)

3 valid findings from Copilot's 2026-05-07 14:39 review:

1. RoutingTransparencyBanner.scrollToConnection: querySelector built a
   CSS attribute selector with raw connectionId. Most ids today are
   safe UUIDs, but custom OAuth/openai-compat ids could contain quotes
   or brackets that throw at parse. Now CSS.escape()s the value with a
   regex fallback for older browsers, and try/catch on the call so an
   unexpected throw silently no-ops instead of breaking the click
   handler.

2. RoutingBadge tooltip a11y: tooltip used role="tooltip" but had no
   programmatic association with the focusable badge, so screen readers
   may not announce the breakdown on focus. Added useId() + matching
   id on the tooltip span and aria-describedby on the focus target
   (only while open=true, so the description disappears with the
   tooltip itself).

3. earliestResetFirst.pressure() doc mismatch: code returns Q×CAP for
   any non-finite t (including Number.POSITIVE_INFINITY emitted by the
   F3 fresh-quota branch), but the doc described t>W → multiplier
   floored at 1, which contradicted the Infinity case. Doc now lists
   t=null OR non-finite as the same max-urgency branch and clarifies
   that t>W finite is the multiplier=1 floor case. Code unchanged
   (preserved as more conservative fallback).

Verify: prettier ✓ eslint ✓ typecheck:core ✓ typecheck:noimplicit:core ✓
test:unit 2821/2821 PASS ✓
diegosouzapw added a commit that referenced this pull request Jul 22, 2026
- fast-uri ^3.1.3 (root + electron overrides) — GHSA host confusion via IDN (#131, #126, high)
- hono ^4.12.27 (bump existing 4.12.25 override) — JSX context isolation / cx() XSS / v1 adapter req drop (#128/#129/#130, medium)
- @hono/node-server ^2.0.5 — serve-static path traversal (#127, medium); major bump, MCP transport verified
- body-parser ^2.3.0 — DoS on invalid limit (#125, low), via express 5

All four packages now clear in `npm audit`; lockfile-lint OK; vuln-ratchet advisory count reduced.
Electron lockfile updated for the second fast-uri site.
diegosouzapw added a commit that referenced this pull request Jul 22, 2026
- fast-uri ^3.1.3 (root + electron) — host confusion via IDN (#131/#126, high)
- hono ^4.12.27 — JSX ctx isolation / cx() XSS / v1 adapter req drop (#128/#129/#130, medium)
- @hono/node-server ^2.0.5 — serve-static path traversal (#127, medium); MCP uses only getRequestListener, not serve-static
- body-parser ^2.3.0 — DoS on invalid limit (#125, low)

Resolved: fast-uri 3.1.4, hono 4.12.31, @hono/node-server 2.0.11, body-parser 2.3.0. All clear in npm audit; lockfile-lint OK.
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…osouzapw#8066)

- fast-uri ^3.1.3 (root + electron overrides) — GHSA host confusion via IDN (diegosouzapw#131, diegosouzapw#126, high)
- hono ^4.12.27 (bump existing 4.12.25 override) — JSX context isolation / cx() XSS / v1 adapter req drop (diegosouzapw#128/diegosouzapw#129/diegosouzapw#130, medium)
- @hono/node-server ^2.0.5 — serve-static path traversal (diegosouzapw#127, medium); major bump, MCP transport verified
- body-parser ^2.3.0 — DoS on invalid limit (diegosouzapw#125, low), via express 5

All four packages now clear in `npm audit`; lockfile-lint OK; vuln-ratchet advisory count reduced.
Electron lockfile updated for the second fast-uri site.
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
…osouzapw#8067)

- fast-uri ^3.1.3 (root + electron) — host confusion via IDN (diegosouzapw#131/diegosouzapw#126, high)
- hono ^4.12.27 — JSX ctx isolation / cx() XSS / v1 adapter req drop (diegosouzapw#128/diegosouzapw#129/diegosouzapw#130, medium)
- @hono/node-server ^2.0.5 — serve-static path traversal (diegosouzapw#127, medium); MCP uses only getRequestListener, not serve-static
- body-parser ^2.3.0 — DoS on invalid limit (diegosouzapw#125, low)

Resolved: fast-uri 3.1.4, hono 4.12.31, @hono/node-server 2.0.11, body-parser 2.3.0. All clear in npm audit; lockfile-lint OK.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
Approved: Clean, well-scoped change that correctly separates Plus/Paid tier from Pro in ProviderLimits.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…osouzapw#8067)

- fast-uri ^3.1.3 (root + electron) — host confusion via IDN (diegosouzapw#131/diegosouzapw#126, high)
- hono ^4.12.27 — JSX ctx isolation / cx() XSS / v1 adapter req drop (diegosouzapw#128/diegosouzapw#129/diegosouzapw#130, medium)
- @hono/node-server ^2.0.5 — serve-static path traversal (diegosouzapw#127, medium); MCP uses only getRequestListener, not serve-static
- body-parser ^2.3.0 — DoS on invalid limit (diegosouzapw#125, low)

Resolved: fast-uri 3.1.4, hono 4.12.31, @hono/node-server 2.0.11, body-parser 2.3.0. All clear in npm audit; lockfile-lint OK.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…osouzapw#8066)

- fast-uri ^3.1.3 (root + electron overrides) — GHSA host confusion via IDN (diegosouzapw#131, diegosouzapw#126, high)
- hono ^4.12.27 (bump existing 4.12.25 override) — JSX context isolation / cx() XSS / v1 adapter req drop (diegosouzapw#128/diegosouzapw#129/diegosouzapw#130, medium)
- @hono/node-server ^2.0.5 — serve-static path traversal (diegosouzapw#127, medium); major bump, MCP transport verified
- body-parser ^2.3.0 — DoS on invalid limit (diegosouzapw#125, low), via express 5

All four packages now clear in `npm audit`; lockfile-lint OK; vuln-ratchet advisory count reduced.
Electron lockfile updated for the second fast-uri site.
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.

2 participants