Repository navigation
refactor(dashboard): dead CSS, duplication and oversized components - #585
Conversation
…ponents
Housekeeping pass over the Svelte dashboard. No intended behavior change
apart from one fix noted below.
Dead CSS: dashboard.css drops 2618 -> 2400 lines. Removed an Alpine.js
[x-cloak] leftover, an unwired tab-transition, five budget-bar-fill-period-*
colors, two .is-editor-open compounds nothing sets, an empty media block and
nine other unreachable rules. Svelte's unused-selector warning cannot see
these: they are either global or sit in components whose class attribute is
built from an expression.
Fix: AuditPane's request/response direction arrows were meant to be tinted
accent/blue, but the global modifier (.audit-pane-icon-request, 0,1,0) lost
to the scoped base (.audit-pane-icon.svelte-hash, 0,2,0) and both rendered
muted grey. The modifiers now live in the owning component as compound
:global selectors so they stay reachable for dynamically composed classes
and outrank the base. The cascade trap is documented in CONVENTIONS.md.
Extracted: NoDataIllustration (a 60-line SVG copy-pasted byte-identically
into 3 pages), FilterInput (the search block repeated in 11 pages),
CopyButton, AuditEntry{Summary,Metadata}/AuditPaneTabs (AuditEntryRow
622 -> 71), WorkflowIdBadge, DatePickerCalendar + datePickerLogic
(DatePicker 576 -> 293). Deleted pages/models/TableIcon.svelte, a
hand-rolled duplicate of the lucide Icon atom that gave models/ different
pencil/trash glyphs from the rest of the app.
Deduped: 11 groups of identical functions are now zero -- four identical
{error:{message}} extractors (new $lib/api/errors.js, kept free of
rune imports so pure page logic and node:test can use it), the chart
tooltip/tick fragments, the 11-color palette that existed in three places
with labelColor's hash duplicated verbatim, browserStorage, formatDateParam,
formatNumber, splitCommaList, mcpServerStatus, qualifiedModelName, the two
workflow connector twins, and the debounce shape in three components.
Markup: WorkflowChart's nine near-identical nodes collapse into one snippet,
ProviderStatusCard's eight label/value rows into another, Sidebar's three
theme buttons into an each block, and two dead single-child layout wrappers
are gone.
Tests: 317 -> 330. The new date-picker tests caught a bug introduced while
extracting calendarDays (a loop bound re-evaluated each iteration produced
38-40 grid cells instead of 42).
Findings, including what was deliberately left alone and why, are in
docs/dev/2026-07-25_dashboard-refactor.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
.pagination-btn was the app's base button class, worn by ~37 elements that have nothing to do with pagination -- every primary action read class="pagination-btn pagination-btn-primary pagination-btn-with-icon". Renamed across 36 files (157 occurrences) to .btn, .btn-primary, .btn-danger, .btn-danger-outline and .btn-with-icon, and documented the pairing rule above the base rule. The .pagination container class is unchanged. The rename is token-exact: no other class contains the substring "pagination-btn", and the new .btn-danger does not collide with the existing .table-action-btn-danger (a different class token). Verified against a live gateway that every variant -- primary, danger, danger-outline, plain and with-icon -- renders as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Too many files changed for review. ( Bypass the limit by tagging |
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR centralizes dashboard utilities, error handling, filters, icons, buttons, chart styling, storage, and debounce behavior. It extracts audit-log and date-picker components, refactors workflow and overview rendering, updates global CSS, and adds unit tests and conventions documentation. ChangesDashboard refactor
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Drops docs/dev/2026-07-25_dashboard-refactor.md. Its removal/extraction tables duplicated the two commit messages and the PR description, which stay attached to the diff; a dated narration of one PR goes stale as soon as the code moves. The two durable warnings move to where someone would actually hit them: - dashboard.css now explains why the light palette is written twice and why light-dark() must not collapse it (chartTheme.js reads those variables via getComputedStyle, which would hand Chart.js the literal "light-dark(...)" token stream). Replaces a TODO blaming the missing CSS preprocessor, which was not the actual blocker. - CONVENTIONS.md notes why a component with computed state classes cannot hand its status CSS to a child component (the compiler prunes what it cannot see in the child's markup). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/dashboard/CONVENTIONS.md`:
- Around line 164-172: Update the Utils section of CONVENTIONS.md to document
the shared format.js exports splitCommaList and formatDateParam, alongside the
existing utility entries. Describe them as the canonical helpers for comma-list
parsing and date-parameter formatting, without changing unrelated documentation.
In `@web/dashboard/src/lib/api/errors.js`:
- Around line 6-9: Update errorPayloadMessage so string messages are normalized
with whitespace trimmed before checking whether they are non-empty and returning
them; use fallback for whitespace-only values, matching errorMessage behavior.
In `@web/dashboard/src/lib/components/atoms/NoDataIllustration.svelte`:
- Around line 8-14: Update the SVG accessibility attributes in
NoDataIllustration so an empty label removes the aria-label and marks the
illustration decorative with aria-hidden. Preserve the existing named-image
behavior when label is provided, using the label value rather than the “No data”
fallback.
In `@web/dashboard/src/lib/components/organisms/Sidebar.svelte`:
- Around line 87-107: Update the desktop theme buttons in the themes each-block
to include aria-pressed based on whether themeStore.theme matches theme.value.
Update the mobile theme-toggle-mobile button’s aria-label and title to describe
the action and current activeTheme, such as “Change theme (currently Light
theme),” rather than only the current theme name.
In `@web/dashboard/src/pages/auth-keys/authKeysLogic.js`:
- Around line 104-108: Replace the separate labelChipStyle/labelColor import and
export declarations in authKeysLogic.js with a single top-of-file re-export from
chartTheme.js, positioned after the existing initial import. Remove the trailing
import/export block while preserving both exported symbols.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7ef2360c-169d-4d31-bc62-0fdfb4fbb0f7
⛔ Files ignored due to path filters (5)
internal/admin/dashboard/static/dist/assets/index-B2NLf2EL.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/assets/index-D6wCbLbq.cssis excluded by!**/dist/**internal/admin/dashboard/static/dist/assets/index-hhannOLz.cssis excluded by!**/dist/**internal/admin/dashboard/static/dist/assets/index-yem6AgYM.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (104)
docs/dev/2026-07-25_dashboard-refactor.mdweb/dashboard/CONVENTIONS.mdweb/dashboard/src/lib/api/client.jsweb/dashboard/src/lib/api/errors.jsweb/dashboard/src/lib/components/atoms/CopyButton.svelteweb/dashboard/src/lib/components/atoms/Modal.svelteweb/dashboard/src/lib/components/atoms/NoDataIllustration.svelteweb/dashboard/src/lib/components/molecules/DatePicker.svelteweb/dashboard/src/lib/components/molecules/DatePickerCalendar.svelteweb/dashboard/src/lib/components/molecules/FilterInput.svelteweb/dashboard/src/lib/components/molecules/Pagination.svelteweb/dashboard/src/lib/components/molecules/datePickerLogic.jsweb/dashboard/src/lib/components/organisms/AuthBanner.svelteweb/dashboard/src/lib/components/organisms/AuthDialog.svelteweb/dashboard/src/lib/components/organisms/Sidebar.svelteweb/dashboard/src/lib/components/organisms/TypedConfirmationDialog.svelteweb/dashboard/src/lib/stores/dateRange.svelte.jsweb/dashboard/src/lib/stores/timezone.svelte.jsweb/dashboard/src/lib/stores/ui.svelte.jsweb/dashboard/src/lib/utils/chartTheme.jsweb/dashboard/src/lib/utils/debounce.jsweb/dashboard/src/lib/utils/format.jsweb/dashboard/src/lib/utils/storage.jsweb/dashboard/src/pages/audit-logs/AuditEntryMetadata.svelteweb/dashboard/src/pages/audit-logs/AuditEntryRow.svelteweb/dashboard/src/pages/audit-logs/AuditEntrySummary.svelteweb/dashboard/src/pages/audit-logs/AuditFilters.svelteweb/dashboard/src/pages/audit-logs/AuditLogsPage.svelteweb/dashboard/src/pages/audit-logs/AuditPane.svelteweb/dashboard/src/pages/audit-logs/AuditPaneTabs.svelteweb/dashboard/src/pages/audit-logs/audit-logic.jsweb/dashboard/src/pages/auth-keys/AuthKeyEditor.svelteweb/dashboard/src/pages/auth-keys/AuthKeyLabelsEditor.svelteweb/dashboard/src/pages/auth-keys/AuthKeysPage.svelteweb/dashboard/src/pages/auth-keys/authKeys.svelte.jsweb/dashboard/src/pages/auth-keys/authKeysLogic.jsweb/dashboard/src/pages/budgets/BudgetEditor.svelteweb/dashboard/src/pages/budgets/BudgetsPage.svelteweb/dashboard/src/pages/guardrails/GuardrailEditor.svelteweb/dashboard/src/pages/guardrails/GuardrailList.svelteweb/dashboard/src/pages/guardrails/guardrails-logic.jsweb/dashboard/src/pages/guardrails/guardrails.svelte.jsweb/dashboard/src/pages/mcp-servers/McpCatalogModal.svelteweb/dashboard/src/pages/mcp-servers/McpServerEditor.svelteweb/dashboard/src/pages/mcp-servers/McpServersPage.svelteweb/dashboard/src/pages/mcp-servers/mcp-servers.jsweb/dashboard/src/pages/mcp-servers/mcpServers.svelte.jsweb/dashboard/src/pages/models/FailoverDrafts.svelteweb/dashboard/src/pages/models/FailoverEditor.svelteweb/dashboard/src/pages/models/ModelGlobalActions.svelteweb/dashboard/src/pages/models/ModelRow.svelteweb/dashboard/src/pages/models/ModelTable.svelteweb/dashboard/src/pages/models/ModelsPage.svelteweb/dashboard/src/pages/models/PricingOverrideEditor.svelteweb/dashboard/src/pages/models/TableIcon.svelteweb/dashboard/src/pages/models/VirtualModelEditor.svelteweb/dashboard/src/pages/models/VmTargetRow.svelteweb/dashboard/src/pages/models/failover-logic.jsweb/dashboard/src/pages/overview/AuditStatsCharts.svelteweb/dashboard/src/pages/overview/LiveTokens.svelteweb/dashboard/src/pages/overview/ProviderStatusCard.svelteweb/dashboard/src/pages/overview/SummaryCards.svelteweb/dashboard/src/pages/overview/UsageChart.svelteweb/dashboard/src/pages/overview/auditStatsLogic.jsweb/dashboard/src/pages/overview/chartStyle.jsweb/dashboard/src/pages/overview/liveTokensLogic.jsweb/dashboard/src/pages/overview/mcpOverviewLogic.jsweb/dashboard/src/pages/overview/overviewChartLogic.jsweb/dashboard/src/pages/overview/overviewState.svelte.jsweb/dashboard/src/pages/providers-config/ProviderCredentialEditor.svelteweb/dashboard/src/pages/providers-config/ProvidersConfigPage.svelteweb/dashboard/src/pages/providers-config/providersConfig.svelte.jsweb/dashboard/src/pages/providers-config/providersConfigLogic.jsweb/dashboard/src/pages/rate-limits/RateLimitEditor.svelteweb/dashboard/src/pages/rate-limits/RateLimitInspector.svelteweb/dashboard/src/pages/rate-limits/RateLimitsPage.svelteweb/dashboard/src/pages/settings/BudgetResetSettings.svelteweb/dashboard/src/pages/settings/BudgetSettings.svelteweb/dashboard/src/pages/settings/FailoverSettings.svelteweb/dashboard/src/pages/settings/PricingRecalculation.svelteweb/dashboard/src/pages/settings/RuntimeRefresh.svelteweb/dashboard/src/pages/settings/TaggingSettings.svelteweb/dashboard/src/pages/settings/TimezoneSettings.svelteweb/dashboard/src/pages/settings/pricing-logic.jsweb/dashboard/src/pages/usage/FacetFilters.svelteweb/dashboard/src/pages/usage/UsageBreakdownChart.svelteweb/dashboard/src/pages/usage/UsageLog.svelteweb/dashboard/src/pages/usage/usage-chart-config.jsweb/dashboard/src/pages/usage/usage-helpers.jsweb/dashboard/src/pages/workflows/WorkflowCard.svelteweb/dashboard/src/pages/workflows/WorkflowChart.svelteweb/dashboard/src/pages/workflows/WorkflowEditor.svelteweb/dashboard/src/pages/workflows/WorkflowIdBadge.svelteweb/dashboard/src/pages/workflows/WorkflowList.svelteweb/dashboard/src/pages/workflows/WorkflowsPage.svelteweb/dashboard/src/pages/workflows/workflowChartLogic.jsweb/dashboard/src/styles/dashboard.cssweb/dashboard/tests/api-errors.test.jsweb/dashboard/tests/auth-keys.test.jsweb/dashboard/tests/date-picker-logic.test.jsweb/dashboard/tests/debounce.test.jsweb/dashboard/tests/guardrails.test.jsweb/dashboard/tests/mcp-servers.test.jsweb/dashboard/tests/providers-config.test.js
💤 Files with no reviewable changes (6)
- web/dashboard/src/pages/workflows/WorkflowList.svelte
- web/dashboard/src/pages/models/TableIcon.svelte
- web/dashboard/src/lib/components/atoms/Modal.svelte
- web/dashboard/src/pages/guardrails/guardrails-logic.js
- web/dashboard/tests/auth-keys.test.js
- web/dashboard/tests/guardrails.test.js
The theme control is a self-contained widget with two presentations of one choice: a three-way pill when there is room, a single cycling button when there isn't. It now lives in ThemeToggle.svelte and takes a `compact` prop. This moves coupling out of Sidebar rather than adding it. Sidebar previously reached into the widget with four rules to swap the two forms -- two under .sidebar-collapsed and two in the 768px breakpoint. ThemeToggle now owns both branches, sitting next to each other so it is obvious they express the same intent, and neither component needs :global() to reach the other. Sidebar 435 -> 334 lines. Not extracted: the individual .theme-btn inside the loop. It is used once, its styling assumes the pill around it (transparent, borderless, sized as a segment), and pulling it out would split .theme-btn:focus-visible away from its siblings and duplicate the focus-ring declaration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The "Foundation API" section had grown to 134 of 226 lines: a hand-maintained catalogue of every store method, component prop and util export. Nothing enforced it, so it drifted from the source, and it buried the rules that actually matter among prop lists. Every component already documents its own props in a header comment, so the catalogue was duplication with a shelf life. Foundation is now an index -- what exists, so nobody re-implements it -- and points at the header comments for detail. The rules that are not discoverable from a signature all stay: the result.stale contract, global 401 handling, never fetch /admin directly, why errors.js has no Svelte-runtime imports, Modal's autofocus attribute, ChartCanvas's build-in-an-effect semantics. Also: - The three scope-hash traps move out of the CSS rule into their own numbered rule; "no new npm dependencies" is promoted from a bullet in the middle of the page-conventions list to a golden rule. - Documents that pure-logic .js files must use relative imports rather than $lib, because node:test runs them without Vite so the alias does not resolve -- and therefore cannot import a .svelte.js store. - Verification lists the npm scripts rather than a raw npx invocation. 226 -> 146 lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removes 53 comments that carried no information a reader couldn't get from
the line below them:
- 45x "/* Styles owned by this component (moved from dashboard.css). */",
the first line of nearly every scoped <style> block. It is a leftover from
the Svelte port: "moved from dashboard.css" is git history, and "owned by
this component" is what a scoped style block means -- CONVENTIONS rule 2
already says that is what belongs there.
- Section comments naming the component they are already inside
(/* Category Tabs */ in ModelsPage, /* Usage Log Section */ in UsageLog,
/* Contribution Calendar */ in ContributionCalendar).
- Markup labels restating the element below (<!-- Auth Error Banner -->
above <AuthBanner />, <!-- Filter --> above the filter toolbar).
- Two header comments restating the filename ("// Workflows page.",
"// Rate Limits page.").
Deliberately kept: the section dividers in dashboard.css that head a run of
rules (/* Table */, /* Content */), header comments that add context beyond
the filename, and every markup comment stating a behavior ("Clicking a
highlighted body snippet opens the Interactions drawer", "two or more targets
turn the redirect into a load balancer").
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CodeRabbit review of #585. Four accepted, one accepted in part. - errorPayloadMessage now trims and treats a whitespace-only message as absent, matching errorMessage. The two sit side by side in errors.js and the comment claims they do the same thing, so the difference was a trap; a blank string is also useless to show a user. Covered by a new test. - NoDataIllustration with label="" is now genuinely decorative (role/aria-label dropped, aria-hidden set) instead of still announcing "No data" over an empty state that already says so in text. That option is documented but currently unused, so this is the documented behavior catching up with the code. - ThemeToggle exposes its selection: aria-pressed per segment and role="group" on the pill, matching the SegmentedControl atom's ARIA. The state was previously conveyed by background color alone. - The narrow theme button cycles rather than selects, so it is now labelled for its action -- "Change theme (currently Light theme)" -- instead of announcing only the current theme, which read as a state, not a control. - CONVENTIONS.md: widened the format.js description to cover date params and comma lists. Not re-enumerating every export -- that catalogue is what the previous commit deliberately removed -- but the one-line index has to hint that splitCommaList lives there, since two pages had already reimplemented it. Declined: collapsing the remaining `import` + `export {}` pairs into `export ... from`. That was the original form; it creates no local binding, so the five modules that also call the symbol internally (mcp-servers, providersConfigLogic, failover-logic, mcpOverviewLogic, pricing-logic) break at runtime. Applied only to authKeysLogic and usage-helpers, which re-export without using the symbols. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Thanks — went through all five. Four accepted, one accepted in part, in 67806a7.
Mobile theme button label — accepted. It cycles rather than selects, so announcing Collapse
|
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Housekeeping pass over the Svelte dashboard: 69 files, +689 / −2657 in source (plus the regenerated
dist/). No intended behavior change apart from one fix, called out below.Dead CSS
dashboard.cssdrops 2618 → 2400 lines. Removed 14 unreachable rule groups: an Alpine.js[x-cloak]leftover, an unwired tab-transition, fivebudget-bar-fill-period-*colours, two.is-editor-opencompounds nothing sets, an empty@mediablock,.editor-modal-shell-wide(two components carry comments saying theModalatom has no wide variant — it doesn't), and more.Svelte's unused-selector warning can't see any of these: they're either global or sit in components whose
classattribute is built from an expression, which makes the compiler conservative.One fix
AuditPane's request/response direction arrows are meant to be tinted accent/blue (there's a comment saying so), but the global modifier.audit-pane-icon-request(0,1,0) lost to the scoped base.audit-pane-icon.svelte-hash(0,2,0) and both rendered muted grey. Confirmed against the previously committed compiled CSS. The modifiers now live in the owning component as compound:globalselectors, so they stay reachable for dynamically composed classes and outrank the base. This is the one visible change in the PR. The cascade trap is now documented inCONVENTIONS.md.Extracted
NoDataIllustrationFilterInputCopyButtonAuditEntry{Summary,Metadata}+AuditPaneTabsAuditEntryRow, 622 → 71 linesWorkflowIdBadgeWorkflowChartDatePickerCalendar+datePickerLogicDatePicker, 576 → 293Deleted
pages/models/TableIcon.svelte— a hand-rolled name→SVG switch duplicating the lucideIconatom. The rest of the app already rendered the same table actions viaIcon, somodels/had visibly different pencil/trash glyphs from API Keys and MCP Servers.Deduplicated
A scan for identical function bodies found 11 groups; all are now zero. Notably four byte-identical
{error:{message}}extractors (plus four near-identical test blocks), the chart tooltip/tick fragments in two modules, and the 11-colour palette in three places withlabelColor's djb2 hash duplicated verbatim.New
$lib/api/errors.jsexists rather than living inclient.jsbecauseclient.jsimports rune-based stores, which pure page logic andnode:testcan't load.Renamed
.pagination-btn→.btnIt was the app's base button class on ~37 elements unrelated to pagination — every primary action read
class="pagination-btn pagination-btn-primary pagination-btn-with-icon". Renamed across 36 files (157 occurrences);.pagination(the container) is unchanged. Token-exact, and.btn-dangerdoesn't collide with the pre-existing.table-action-btn-danger.Verification
svelte-check: 0 errors, 0 warningsnpm run buildclean; embeddeddist/regenerated and in syncThe new date-picker tests immediately caught a bug I'd introduced while extracting
calendarDays:for (let d = 1; d <= 42 - days.length; d++)re-evaluates its bound each iteration, so the grid came out at 38–40 cells instead of 42. Fixed.Review notes
Reviewing commit-by-commit is easier than the combined diff — the second commit is a pure mechanical rename.
Three files remain slightly over the 400-line guideline (
WorkflowChart575,Sidebar435,ProviderStatusCard429). The remainder in each is a flat, commented status/state CSS matrix; splitting is blocked because the state classes are computed strings, so a child component's scoped rules would be pruned — now noted inCONVENTIONS.md. The light/dark token duplication also has to stay:light-dark()would breakchartTheme.cssVar(), which reads those variables back withgetComputedStyle— now explained at the top ofdashboard.css, replacing a TODO that blamed the missing CSS preprocessor.🤖 Generated with Claude Code
Summary by CodeRabbit
FilterInput,NoDataIllustration,CopyButton, andThemeTogglecomponents (plus a workflow ID badge).