Repository navigation
feat(dashboard): unify Provider Limits row design (Oracle two-stage approved) - #27
Merged
Merged
Conversation
…nment + header sizing User report after PR #26 (3.8.5) Docker deploy: 5 visual inconsistencies on /dashboard/limits — (1) Next pill baseline drift in account header rows, (2) Session/Weekly mini-bars rendered with a different vocabulary than the per-model bars on the same row (plain text label vs colored pill, font-mono vs font-semibold, rounded-full vs rounded-sm, vertical stack vs horizontal row), (3) percentage typography mismatch, (4) Refresh All / Auto-refresh / interval-select height drift, (5) Configured banner text/icon baseline mismatch. Design unification (per-model bar adopted as canonical, Session/Weekly mini-bars absorb that vocabulary): - F5 RoutingBadge: collapsed two-layer wrapper into a single inline-flex items-center span carrying ref / tabIndex / focus handlers / striped className. Tooltip portal geometry preserved (wrapperRef still measures the visible badge box). - F1+F2 QuotaVisualization: replaced MiniBar with OverallQuotaRow that matches per-model bar markup exactly — colored pill label, ⏱ countdown, h-1.5 rounded-sm bar, font-semibold percent. Outer container removed; Fragment returns two rows that sit as equal flex-wrap peers next to per-model bars in the same parent. Per-model bar's i>0 border-l/pl-3/ml-1 separator removed; gap spacing alone now separates all rows. - F3 Header controls: Refresh All button, AutoRefreshControl wrapper, and inner interval select all set to inline-flex items-center h-8. - F4 RoutingTransparencyBanner: items-start → items-center with leading-snug on the container, leading-none align-middle on material icons, align-baseline leading-snug on the dotted-underline inline button. i18n: new keys sessionQuotaLabel / weeklyQuotaLabel — en (Session/Weekly) + ko (세션/주간) hand-translated, 30 other locales seeded with English fallback (next-intl does not auto-fall-back to default messages, so explicit copies are required not redundant). Oracle two-stage verification (per user instruction Plan + Implementation): - Stage 1 (plan, ses_1fbbfa2a5ffe6xzkdsplPMRSOp): NEEDS_REVISION on 7 items. All applied in plan v2: U1 root-cause re-traced (double wrapper, not striped padding); F2 layout decision unified (flatten + remove all separators); F3 sizing h-8 (not h-9, to preserve header rhythm); i18n English fallback explicit; F4 leading-none on icons; new test required; F5 className on single wrapper (no Badge style-prop change). - Stage 2 (implementation, ses_1fbb494e4ffe7BxOUFFzU8g6dm): NEEDS_REVISION on 2 real defects + 1 minor. ALL FIXED before this commit: - Defect B: pickWindow accepted 'weekly(7d)' no-space form but isOverallWindowName did not, so the parent !isOverallWindowName filter in index.tsx would let the same quota render twice (overall mini-bar + per-model bar). Fixed by adding no-space match to isOverallWindowName. Cross-function parity test added in pickwindow.test.mjs. - Defect C: QuotaVisualization conditionally rendered each row based on its own existence, so when only one of session/weekly was present the other row's placeholder was lost (previous MiniBar always rendered both rows). Fixed by always rendering both OverallQuotaRow elements; the pct=null branch now serves as the absent-window placeholder. New integration tests cover only-session, only-weekly, and only-per-model cases. - Minor I: CHANGELOG referenced align-middle on banner icons while the code did not include it. Resolved by adding align-middle to both route and info icon classNames (also helps baseline alignment). Test count: 2852/2852 PASS (was 2841; +11 net — 9 quota-visualization-design + 2 pickwindow-parity). Version 3.8.5 → 3.8.6.
There was a problem hiding this comment.
Pull request overview
This PR unifies the visual design and layout vocabulary of the Provider Limits row on /dashboard/limits by making the overall Session/Weekly quota rows match the per-model bar style, while also fixing an overall-window name parity bug (weekly(7d)), aligning header control sizing, and tightening banner baseline alignment. It also adds i18n keys for the new labels, adds targeted unit tests, and bumps the version to 3.8.6 (including OpenAPI + changelog).
Changes:
- Refactors
QuotaVisualizationto render unifiedOverallQuotaRowrows (Session/Weekly) and fixesisOverallWindowNameto recognize no-space canonical forms (weekly(7d)). - Unifies dashboard UI alignment: single-wrapper
RoutingBadge, consistenth-8header controls, and centeredRoutingTransparencyBannericon/text/button baselines. - Adds i18n keys (
sessionQuotaLabel,weeklyQuotaLabel) across locales, adds new/updated unit tests, and bumps version/documentation to3.8.6.
Commands / tests (review):
- Commands run in this review: none (not executed here).
- PR description reports: eslint/typecheck ok; unit tests
2852/2852passing.
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/app/(dashboard)/dashboard/usage/components/ProviderLimits/QuotaVisualization.tsx |
Introduces OverallQuotaRow, always-renders Session+Weekly pair, and expands isOverallWindowName to handle no-space canonical forms. |
src/app/(dashboard)/dashboard/usage/components/ProviderLimits/index.tsx |
Aligns Refresh button sizing and removes per-model row separators to match the new unified layout. |
src/app/(dashboard)/dashboard/usage/components/ProviderLimits/AutoRefreshControl.tsx |
Normalizes auto-refresh control/select height to h-8. |
src/app/(dashboard)/dashboard/usage/components/ProviderLimits/RoutingBadge.tsx |
Collapses to a single focusable wrapper span for consistent baseline/tooltip anchoring. |
src/app/(dashboard)/dashboard/usage/components/ProviderLimits/RoutingTransparencyBanner.tsx |
Centers items and adjusts icon/button leading/alignment for baseline consistency. |
tests/unit/quota-visualization-design.test.mjs |
New tests to lock the unified overall-row markup/behavior and always-render-both-rows behavior. |
tests/unit/quota-visualization-pickwindow.test.mjs |
Adds regression/invariant tests for no-space canonical forms and pickWindow/isOverall parity. |
src/i18n/messages/*.json |
Adds usage.sessionQuotaLabel / usage.weeklyQuotaLabel to all locales (ko translated; others English). |
package.json |
Bumps version to 3.8.6. |
docs/openapi.yaml |
Bumps API version to 3.8.6. |
CHANGELOG.md |
Adds 3.8.6 release notes. |
Comment on lines
+18
to
+22
| // Oracle audit (ses_1fbb494e4ffe7BxOUFFzU8g6dm — defect B): pickWindow accepts | ||
| // both "weekly (7d)" and "weekly(7d)" via startsWith("windowKey(") at line 47, | ||
| // so isOverallWindowName MUST accept the same no-space canonical forms or the | ||
| // parent !isOverallWindowName filter in index.tsx will let "weekly(7d)" leak | ||
| // into per-model rendering AND OverallQuotaRow simultaneously. |
Owner
Author
There was a problem hiding this comment.
Fixed in a2d3d6c. Removed the hard-coded line number from the test comment. The rationale now describes the operator itself (startsWith with windowKey + open-paren) rather than its location, and the cross-function parity invariant is enforced by the dedicated pickWindow / isOverallWindowName parity test below — so no future drift in the production file can silently invalidate the comment.
Comment on lines
+34
to
+38
| assert.match( | ||
| html, | ||
| /class="text-\[11px\] font-semibold py-0\.5 px-2 rounded whitespace-nowrap min-w-\[60px\] text-center"/, | ||
| "label pill must use the per-model bar className signature" | ||
| ); |
Owner
Author
There was a problem hiding this comment.
Fixed in a2d3d6c. Replaced the full-string class="..." regex with a per-class assertHasClasses helper. Each required Tailwind class is asserted independently with regex metacharacter escaping (so min-w-[60px] matches literally) and word-boundary anchors (so adjacent classes do not produce false positives). Harmless refactors (class reordering, JIT output drift, addition of equivalent classes) no longer break the tests; missing required classes or the presence of forbidden classes (rounded-full, font-mono) still fail.
…ness, 2 nits)
R1-1: removed hard-coded production-code line number ('at line 47') from the
test comment in pickwindow.test.mjs. The behavior is now described by its
function/operator (startsWith with windowKey + open paren), independent of
where that operator sits in the implementation file. The dedicated parity
test below enforces the cross-function invariant directly, so the test does
not depend on any specific implementation line number.
R1-2: replaced full-string className regex matches in design.test.mjs with
a per-class assertHasClasses helper. Each Tailwind utility class is asserted
independently, so harmless refactors (Tailwind class reordering, addition
of equivalent classes, JIT output drift) no longer break the tests. The
helper escapes regex metacharacters in class names like 'min-w-[60px]' and
matches on word boundaries, so adjacent classes do not produce false
positives. Tests still fail when a required class is genuinely removed or
when a forbidden class (rounded-full, font-mono) is present.
2852/2852 unit tests still pass.
i1hwan
added a commit
that referenced
this pull request
May 9, 2026
…redact PII from CHANGELOG R3-1 (defect): touchSession(sessionId, connectionId) compat shim was updating lastActive BEFORE forwarding to bindSessionConnection. That made the forwarded call always see 'within affinity window' (because lastActive had just been refreshed to Date.now()), so a session idle >5 minutes would mis-fire the within-window alarm on its first compat-shim rebind. The shim now binds first (so the binding decision sees the pre-touch lastActive). Only when an entry pre-existed is requestCount incremented — first_bind already sets it to 1, so a fresh session via the compat shim sees requestCount == 1 (preserves the original 1-touch == 1-request semantics that session-manager.test.mjs relies on). R3-2 (defect): selectByEarliestResetFirst was binding fall-through selections itself with a hard-coded source: 'fall_through'. Since the strategy ran BEFORE chat.ts:548's post-credential bind, the actual within-window rebind always carried the wrong source — the emergency_fallback alarm suppression added in stage-2 audit could never engage in the emergency retry path. The strategy no longer binds. chat.ts:548 is now the single binding site (single source of truth) and selects source: 'emergency_fallback' vs 'explicit_post_credential' from runtimeOptions.emergencyFallbackTried, so the suppression works as intended. The unused bindSessionConnection import was removed from the strategy. Test count: 2875/2875 PASS (was 2874; +1 new compat-shim first-bind contract test). PII REDACTION (urgent user demand): CHANGELOG.md previously contained real account names (APEXATGNU, GNUMAX) in production observation narratives across multiple [3.8.x] sections. All occurrences have been replaced with generic placeholders (Account A / Account B). The same substitution was applied to .sisyphus/plans/*.md (workspace-only, not git-tracked, but kept consistent for future readers). PR #28 body on GitHub has also been edited to use the generic names. Merged commit messages on main (PR #25 / #26 / #27) cannot be redacted without a force-push to main, which is forbidden by AGENTS.md (incident cheaper-inference#1386 on 2026-04-18); their messages remain as-is on origin/main.
i1hwan
added a commit
that referenced
this pull request
May 9, 2026
…Oracle two-stage verified) (#28) * fix(routing,session): real session-affinity fix \u2014 D1 + D2 + D3 (Oracle two-stage verified) Production observation on 3.8.5/3.8.6: a Claude OAuth session bound to APEXATGNU switched to GNUMAX only 4 seconds after the last successful APEXA request, on the same opencode-supplied x-session-affinity header, losing 147k tokens of cache_write. Oracle hard-debug (ses_1fa7165c0...) identified three independent design defects, all fixed here. D1: extractExternalSessionId now recognises x-session-affinity (between the generic forms and the omniroute-specific alias). Each header source gets a hyphenated namespace prefix (ext-xsa: / ext-omr:) so that a legacy generic value of e.g. 'xsa:foo' cannot collide with a newer x-session-affinity: 'foo' request. The opencode CLI's stable session continuity signal now flows through to bound-conn lookup; fingerprint fallback only kicks in when no header is present. D2: touchSession is split. touchSession(sessionId) is now a pure activity update (lastActive, requestCount). All connection binding mutations go through bindSessionConnection(sessionId, connectionId, { source }) which returns a BindResult with reasons no_session_id / first_bind / no_change / rebind_within_window / rebind_after_window. A within-window rebind (the diagnostic anomaly) emits a structured log.warn unless source is 'emergency_fallback' (legitimate retry path in chat.ts). Critically, null oldConnectionId is treated as first_bind (NOT rebind_within_window) so the production flow of touchSession() followed by bindSessionConnection() does not raise the alarm on every new session's normal first request. The deprecated 2-arg touchSession remains as a one-shot-warned compat shim for tests/legacy callers. D3: selectByEarliestResetFirst and isAffinityValid accept an optional provider argument plumbed from auth.ts. For provider 'claude' (Anthropic OAuth lane), the urgent-alt heuristic break (PR #26 v8) is suppressed unless boundScore <= 0 (genuinely unusable via accumulated backoff). Other break paths (terminal, rate_limited, quota_exhausted, low-quota < 15%, session_expired, affinity_window_passed) are unaffected. Other providers retain the original break behaviour. Trace logs added at every routing decision: log.debug 'kept' on affinity preserved, log.warn 'broken' with bound/altscore on heuristic break, and log.debug 'urgent-alt break suppressed by cache-sensitive gate' with gate='claude_urgent_alt_suppressed' when the Claude gate fires. A single log line during production traffic now answers 'did affinity break, and why?' deterministically. Oracle two-stage verification: - Stage 1 plan v1 (ses_1f85345c8...): NEEDS_REVISION on 7 items. - Stage 1 plan v2 (ses_1f84ae893...): NEEDS_REVISION (narrow) on 5 wording-mismatch items. - Stage 2 implementation (ses_1f83ab6ed...): NEEDS_REVISION on 4 real defects + 1 nit. Defect A namespace collision, defect B critical null-first-bind misclassification, defect D emergency-fallback alarm noise, defect H lazy-require silent swallow, nit E gate trace log reason. ALL FIVE FIXED in this commit. New tests: SA-12 / SA-13 / SA-14 (Claude block / score=0 allow / GitHub allow), session-manager-affinity-headers (9 tests incl. collision-proof namespace), session-manager-binding (10 tests incl. null-first-bind and emergency_fallback). Test count: 2874/2874 PASS (was 2852; +22 net). Version 3.8.6 -> 3.8.7. * docs(session,test): address Copilot PR #28 review (3 doc/test nits) R1-1: CHANGELOG line 31-32 said '8 tests' for both new session-manager test files, but line 34 (and the actual files) say 9 + 10. Reconciled to 9 (affinity-headers) and 10 (binding); aligned with the +22 net total. R1-2: the 'rebind_after_window' test was named as if it deterministically asserted the after-window classification, but it only asserts the weaker 'one of the two rebind variants' contract because it does not advance real time. Test renamed to 'rebind to a different conn always succeeds and updates the bound conn (window classification is wall-clock dependent)' so the assertion matches the name. The deterministic within-window branch is exercised by the previous test in the same file. R1-3: SA-2 comment claimed the quotaCache lookup keys on conn.provider, but the cache is keyed by connectionId only — the provider string is descriptive metadata, not part of the lookup key. Comment corrected to state that the Claude gate decision is driven entirely by the selectByEarliestResetFirst third argument, not by any cache-key provider field. No production code change. 2874/2874 unit tests still pass. * fix(routing,docs): address Copilot PR #28 R2 review (1 defect + 3 nits) R2-1 (defect): selectByEarliestResetFirst was logging at warn level for EVERY isAffinityValid invalid reason — including expected non-heuristic ones like affinity_window_passed / rate_limited / terminal / quota_exhausted_unknown_reset / inactive / track-excluded. That polluted production warn-level logs with normal routing events. Now only the two HEURISTIC break reasons (affinity_break_low_quota / affinity_break_p1_too_urgent) emit at warn level — those are the diagnostic alarm cases where the smart-affinity logic actively chose to break a healthy binding. All other invalid reasons drop to debug. R2-2 (changelog): aligned the [3.8.7] wording with the actual emission ("ONLY when affinity is broken via the heuristic rules" + explicit list of expected reasons that drop to debug). R2-3 (test comment): the binding test note 'logger may be unavailable in this test environment' was stale after the PR's switch to a static logger import. Updated to the real reason: keeping the unit test decoupled from logger side effects, with verification of the warning emission deferred to an integration test. R2-4 (test comment): the affinity-headers test rationale was slightly off: collision-proofing comes from the prefixes themselves differing ('ext-xsa:' vs 'ext:'), not from any restriction on what legacy raw values can begin with. Reworded to make the proof source explicit. No production behavior change beyond R2-1's log-level gating. 2874/2874 unit tests still pass. * fix(routing,session): address Copilot PR #28 R3 review (2 defects) + redact PII from CHANGELOG R3-1 (defect): touchSession(sessionId, connectionId) compat shim was updating lastActive BEFORE forwarding to bindSessionConnection. That made the forwarded call always see 'within affinity window' (because lastActive had just been refreshed to Date.now()), so a session idle >5 minutes would mis-fire the within-window alarm on its first compat-shim rebind. The shim now binds first (so the binding decision sees the pre-touch lastActive). Only when an entry pre-existed is requestCount incremented — first_bind already sets it to 1, so a fresh session via the compat shim sees requestCount == 1 (preserves the original 1-touch == 1-request semantics that session-manager.test.mjs relies on). R3-2 (defect): selectByEarliestResetFirst was binding fall-through selections itself with a hard-coded source: 'fall_through'. Since the strategy ran BEFORE chat.ts:548's post-credential bind, the actual within-window rebind always carried the wrong source — the emergency_fallback alarm suppression added in stage-2 audit could never engage in the emergency retry path. The strategy no longer binds. chat.ts:548 is now the single binding site (single source of truth) and selects source: 'emergency_fallback' vs 'explicit_post_credential' from runtimeOptions.emergencyFallbackTried, so the suppression works as intended. The unused bindSessionConnection import was removed from the strategy. Test count: 2875/2875 PASS (was 2874; +1 new compat-shim first-bind contract test). PII REDACTION (urgent user demand): CHANGELOG.md previously contained real account names (APEXATGNU, GNUMAX) in production observation narratives across multiple [3.8.x] sections. All occurrences have been replaced with generic placeholders (Account A / Account B). The same substitution was applied to .sisyphus/plans/*.md (workspace-only, not git-tracked, but kept consistent for future readers). PR #28 body on GitHub has also been edited to use the generic names. Merged commit messages on main (PR #25 / #26 / #27) cannot be redacted without a force-push to main, which is forbidden by AGENTS.md (incident cheaper-inference#1386 on 2026-04-18); their messages remain as-is on origin/main.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
User report after PR #26 (3.8.5) merged + Docker deploy: the Provider Limits row on
/dashboard/limitshad 5 visual inconsistencies:Nextpill drifts off the tier-badge baseline (especially in the striped excluded variant).font-monovsfont-semibold,rounded-fullvsrounded-sm, vertical stack vs horizontal row) — they read as two disconnected groups.font-mono w-9vs per-modelfont-semibold min-w-[32px]).Refresh Allbutton, Auto-refresh toggle, and interval<select>have inconsistent heights (px-3.5 py-1.5vspy-1vs intrinsic Toggle).Configured(RoutingTransparencyBanner) box has icon / text / inline-button baseline mismatches.The user explicitly asked: "기존 디자인을 신규 디자인으로 옮기던 해서 통일감을 만들어줘. 레이아웃 개편을 좀 해라."
Approach
Per-model bar adopted as canonical; Session / Weekly mini-bars absorb that vocabulary.
<span ref>+<span tabIndex>) → single<span>carryingref/tabIndex/ focus handlers /aria-describedby/ striped className. Tooltip portal geometry preserved (wrapperRef.getBoundingClientRect()still measures the visible badge box).OverallQuotaRowwith the per-model markup signature: pill labeltext-[11px] font-semibold py-0.5 px-2 rounded whitespace-nowrap min-w-[60px] text-center,text-[10px] text-text-muted ⏱ {countdown},h-1.5 rounded-smbar,text-[11px] font-semibold min-w-[32px] text-rightpercent. Outer container removed;Fragmentreturns the two rows as equal flex-wrap peers next to per-model bars in the same parent. Per-model barborder-l/pl-3/ml-1(i>0) separator removed; only gap-spacing now separates rows. Always renders both Session AND Weekly even when one window is absent (the other gets the muted em-dash placeholder) — keeps the visual pair stable across providers.Refresh All, AutoRefresh wrapper, and inner select all set toinline-flex items-center h-8.items-start→items-centerwithleading-snugon the container,leading-none align-middleon material icons,align-baseline leading-snugon the dotted-underline inline button.i18n: new keys
sessionQuotaLabel/weeklyQuotaLabel, en (Session/Weekly) + ko (세션/주간) hand-translated, 30 other locales seeded with English fallback (next-intl does not auto-fall-back to default messages, so explicit copies are required).Oracle two-stage verification (user-requested)
Stage 1 — Plan (
ses_1fbbfa2a5ffe6xzkdsplPMRSOp): NEEDS_REVISION, 7 items, all applied in plan v2 before implementation:h-8, noth-9, to preserve existing header rhythm.leading-noneon icons explicitly.styleprop change.Stage 2 — Implementation (
ses_1fbb494e4ffe7BxOUFFzU8g6dm): NEEDS_REVISION, 2 real defects + 1 minor. All fixed in this commit:pickWindowacceptedweekly(7d)no-space form butisOverallWindowNamedid not, so the parent!isOverallWindowNamefilter inindex.tsxwould let the same quota double-render (overall mini-bar + per-model bar). Fix: extendisOverallWindowNameto match no-space forms; add cross-function parity test.QuotaVisualizationconditionally rendered each row based on its own existence, dropping the missing-window placeholder. Fix: always render both rows;pct=nullbranch is now the placeholder.align-middlewhile code did not include it. Fix: addedalign-middleto both banner icon classNames.Test plan
Verification
Plan + reviews
.sisyphus/plans/limits-dashboard-design-unification.md(Oracle plan APPROVED v2 after 7 revisions; Oracle implementation APPROVED after 3 revisions)omni2.jpeg(post-PR-feat(dashboard): limits page polish + smart session affinity #26 Docker screenshot from user) drives the diagnosis in plan §1.1Branch / Commit
feat/limits-dashboard-design-unification(fromorigin/mainat42a5da4f, the squash result of PR feat(dashboard): limits page polish + smart session affinity #26)2f47a4a4covers all 5 areas + 2 Oracle audit rounds + tests + version bumpOut of scope
sessionRemaining/weeklyRemainingi18n keys (kept across 32 locales for backwards-compat)Badgecomponentstyleprop addition (would be a design-system change; out of scope here)