feat(sql-compare): model filter + V-layout hover polish - #59
Merged
Conversation
- HTML bugs: colspan 11→12 in index-themes.html; fix </div>→</section>
+ missing grid-area closing tag in index-v2-dock.html
- stopBtn2 (#stopBtn2 in tabs layout) wired to stopBenchmark in app.js
and synced in applyButtonState so enabled/disabled state is correct
- Add missing A1/A2/A3 HTML shells (static/index-a1.html, a2, a3) and
fix variants/index.html links from /index-aX.html → /static/index-aX.html
- CSS: replace deprecated word-break:break-word with overflow-wrap in
v1-instrument, v2-dock, v2-monitor, v3-startup, v3-tabs, sql-grid
- CSS: add clip-path:inset(50%) alongside clip:rect() in .sr-only rules
(v1-instrument, v3-startup)
- a3.css: add ::marker{display:none} for Firefox alongside ::-webkit-details-marker
- a2.css: replace hardcoded rgba(255,255,255) values with CSS token vars
- v4-terminal.css: raise --text-dim/#text-muted to WCAG AA contrast;
add html[data-design="v2…v6"] channel palette overrides
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ntrast, a11y labels - v4-overlay.html/app.js: label[role="button"] now responds to Enter/Space - v4-overlay.css: mobile .grid-viewport inset restored (0 0 60px 0 instead of auto bottom) - c1-clean-lab.css: raise --text-dim #9CA3AF → #767676 for WCAG AA 4.5:1 on white - index-a2.html: add aria-label to single-letter layout nav links Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Dead code — never linked from production, no nav switcher, no routes. Deleted 7 files (index-a1/a2/a3.html, index-themes.html, variants/a1/a2/a3.css), removed A2 tab-switching from app.js, cleaned up variants/index.html. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Each V1-V4 layout now has a second standalone CSS theme (Instrument/Monitor/ Startup/Terminal) loaded disabled by default. A "Theme" button in the nav switches between them; choice persists via localStorage. Both font families preloaded in the Google Fonts URL. - V1 Grid Hero ↔ Instrument (Fira Code / Fira Sans) - V2 Dock ↔ Monitor (IBM Plex Sans / JetBrains Mono) - V3 Tabs ↔ Startup (Space Grotesk / DM Sans) - V4 Overlay ↔ Terminal (Share Tech Mono / Fira Code) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t toggle The v1-instrument/v2-monitor/v3-startup/v4-terminal CSS files target the classic sidebar layout, not the V-layout HTML structures. Removed broken per-V-layout toggle. Added a <select> theme switcher on index.html instead — switches between style.css and 4 alternatives; choice persists via localStorage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sql-grid.css: full D5 Arctic Light override block (sticky headers, hover states, compare-head, result chips, pass/fail cells on white bg). Per-channel tinted hover + compare-head rows for D1-D4 and D6. channels-d456.css: fix sql-compare-bar visibility on D5 light theme. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
app.js: - v2-dock: bind settings slideout toggle/open/close via data-dock-action attributes (replaces inline onclick, fixes CSP violation) - v3-tabs: sync aria-selected on label[role="tab"] with radio :checked state for screen reader tab announcements - label[role="button|tab"]: extend keyboard handler to cover v3 tab labels - theme toggle: apply persisted theme on first load unconditionally so label/classes always match stylesheet state index-v2-dock.html: rebuilt dock with slideout settings panel, SVG icons, model chip list, dock__btn pattern index-v3-tabs.html: tab labels gain role="tab" + aria-controls; setup tab restructured into card grid (Endpoints / Models / Settings) index-v4-overlay.html: minor fixes variants/index.html: note copy update Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…hover/polish Universal filter in compare bar, anchored over Model/Run column. Search-icon SVG, visibility counter, MutationObserver guarded against microtask loop. Per-channel CSS polish in channels-d456.css and sql-grid.css with focus rings, accent borders, v5 readable border, v6 monospace amber. Hover ring in V-layouts: transform:scale was clipped by overflow:auto on .grid-area__sql/.sql-compare-area; replaced with inset box-shadow + filter:brightness. Plus: add #sqlCompareContainer to index-v3-tabs.html, remove inline onclick on v4-overlay compareRunsBtn, align variants/index.html note with shared switcher, add index-compare-filter.html as parallel classic entry URL.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds four new layout HTML pages, a design-variant catalog, runtime controls for theme and compare UI, and a large set of shared and variant-specific styles for the SQL grid, page layouts, and visual themes. ChangesLayout and Theme Variants
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Stylelint (17.13.0)static/variants/v4-terminal.cssError: ENOENT: no such file or directory, open '/.stylelintrc.json' static/variants/sql-grid.cssError: ENOENT: no such file or directory, open '/.stylelintrc.json' Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (2)
static/variants/sql-grid.css (2)
33-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant variable redeclaration.
--sql-model-column-width: 360pxis already defined in:root(Line 6) with the same value; this local redeclaration on.sql-result-scrollis dead weight.🤖 Prompt for 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. In `@static/variants/sql-grid.css` at line 33, The `.sql-result-scroll` rule is redundantly redeclaring `--sql-model-column-width` with the same value already set in `:root`; remove this local override and keep the shared definition as the single source of truth.
404-504: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDuplicated v5 SQL-heatmap re-theme across two files.
This entire
data-design="v5"heatmap block largely duplicates the v5 overrides inchannels-d456.css(Lines 540-708): sticky thead/model column, body cells, compare-head rows, result-count chips, and result cells. Maintaining two copies invites divergence (the hoverbox-shadowalready differs). Consider consolidating the v5 heatmap rules into a single source of truth.🤖 Prompt for 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. In `@static/variants/sql-grid.css` around lines 404 - 504, The v5 SQL heatmap styling is duplicated between this block and the existing rules in channels-d456.css, which will cause the two versions to drift. Consolidate the shared data-design="v5" overrides into a single source of truth and have sql-grid.css reuse that shared styling instead of maintaining a second copy; keep the unique symbols like .sql-result-table, .sql-result-cell, .sql-compare-bar, and .sql-result-count aligned with the centralized rules.
🤖 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 `@static/index-compare-filter.html`:
- Around line 341-343: The empty-state row in the history table undercounts the
number of columns, since the `<thead>` for `historyBody` spans 12 columns but
the placeholder cell currently uses `colspan="11"`. Update the empty-state
`<td>` in `historyBody` to match the table structure used by the other variant
shells, keeping the colspan aligned with the 12-column header.
In `@static/index-v3-tabs.html`:
- Line 284: The sqlCompareContainer id is duplicated across tab containers, so
app.js methods like document.getElementById('sqlCompareContainer') and
$('sqlCompareContainer') only bind to the first match and send compare rendering
to the wrong tab. Make the compare container unique per tab in
static/index-v3-tabs.html, then update the matching lookup in app.js (and any
related compare/render helpers) to target the intended container by its unique
id.
In `@static/variants/c3-warm-instrument.css`:
- Around line 45-92: The c3 warm instrument styles still quote the single-token
font family Sora, which violates the font-family-name-quotes rule. Update the
font-family declarations in the relevant selectors in this stylesheet (the base
heading rule, html[data-style="c3"] .section-title, and html[data-style="c3"]
.metric-card .value) to use Sora without quotes, keeping the rest of the
declarations unchanged.
In `@static/variants/channels-d456.css`:
- Around line 52-156: The stylesheet uses quoted single-token font-family names,
which violates the font-family-name-quotes rule. Update the font-family
declarations in the affected rules (including the JetBrains Mono stack and the
html[data-design="v5"] h1/.section-title rule) to remove quotes around Inter
while keeping multi-word names quoted as needed.
In `@static/variants/sql-grid.css`:
- Line 542: The font-family declaration in the SQL grid styles uses quotes
around the single-token family name Inter, which violates the project’s
font-family-name-quotes rule. Update the font-family value in the relevant CSS
rule in static/variants/sql-grid.css so Inter is unquoted, while keeping the
rest of the fallback stack unchanged.
In `@static/variants/v1-gridhero.css`:
- Around line 402-408: The checkbox-based toggle control in the toolbar is being
fully hidden, which breaks keyboard and assistive-tech access for the
Scan/Models and History triggers. Update the `toolbar__dropdown-chk` handling so
the underlying input remains accessible and can still be focused/announced, or
replace the label-driven toggle pattern with real button controls and style them
accordingly. Use the existing `toolbar__dropdown-chk` and related toggle styles
as the place to adjust visibility/focus behavior.
In `@static/variants/v1-instrument.css`:
- Around line 7-26: The six-channel design toggle is not wired into this
stylesheet, so selecting v2-v6 has no visual effect and only channels 1-3 get
active-state styling. Update the `:root` palette usage in this variant to
respond to `html[data-design="vN"]` set by
`document.documentElement.dataset.design`, and make sure the channel selector
styling covers all six design buttons so every option shows an active swatch
when selected.
- Line 894: The CSS in v1-instrument should stop using the deprecated
word-break: break-word rule and switch to overflow-wrap for long-content
wrapping. Update the affected style rules in static/variants/v1-instrument.css,
replacing each word-break: break-word usage with overflow-wrap: anywhere and add
word-break: normal where necessary to preserve wrapping behavior. Use the
existing selectors in this stylesheet to locate the three rules and keep the
visual behavior unchanged.
In `@static/variants/v2-monitor.css`:
- Line 1008: The styles in the affected selectors use deprecated word-break:
break-word, so update each matching rule in the stylesheet to use overflow-wrap:
break-word instead while keeping the existing white-space behavior. Apply this
in the .conv-text, .sql-comment-cell, and .sql-mismatch-table td declarations so
the wrapping intent stays the same and the Stylelint warning is removed.
- Around line 58-61: The `.sr-only` visually-hidden rule is using deprecated
`clip`, which Stylelint flags. Update the `.sr-only` definition to use the
modern `clip-path` approach in the same selector, keeping `clip` only if you
want a legacy fallback. Locate and adjust the `sr-only` class in the stylesheet
so the visually-hidden pattern remains intact without the deprecated property.
In `@static/variants/v3-startup.css`:
- Around line 76-79: The .sr-only rule in v3-startup.css should drop the
deprecated clip declaration since clip-path already handles the hiding. Update
the .sr-only style block to remove clip: rect(0, 0, 0, 0) and keep the rest of
the accessibility-hiding properties unchanged.
- Around line 1330-1343: The SQL compare hover styling in .sql-result-cell is
still using transform: scale(1.3), which causes clipping and jitter inside the
scroll container. Update the .sql-result-cell and .sql-result-cell:hover rules
in the v3 startup CSS to avoid changing the element’s box size; use a
non-layout-affecting hover treatment such as an inset ring, brightness, or
similar visual emphasis instead. Keep the existing cursor/transition behavior,
but remove the scaling so hover effects stay contained within the scroll area.
In `@static/variants/v3-tabs.css`:
- Around line 1284-1295: The .sql-detail-modal pre rule uses the deprecated
word-break: break-word value, which should be updated to a Stylelint-safe
wrapping approach. In the v3-tabs stylesheet, replace that declaration with
overflow-wrap: anywhere and keep the rest of the pre styling intact; use the
.sql-detail-modal pre selector to locate the rule.
- Around line 1256-1307: The SQL detail modal styles are targeting outdated
selectors, so the runtime elements rendered by app.js are not getting styled.
Update the modal CSS in the SQL detail modal block to match the actual DOM
classes used by the modal: .sql-detail-modal-header, .sql-detail-modal-title,
.sql-detail-modal-subtitle, and .sql-detail-modal-close. Remove reliance on h3,
pre, and .close-btn here, and apply the intended layout/spacing/typography to
the matching modal container and header elements.
In `@static/variants/v4-terminal.css`:
- Around line 195-206: The glow on the heading/card styles is still hard-coded
to a green rgba value, so it does not follow the active design palette. Update
the affected `h1`/card rules in `v4-terminal.css` to use the existing
`--accent-glow` token instead of the literal shadow color, and apply the same
change to the other matching blocks referenced by this style so all variants
inherit the themed glow consistently.
- Around line 1191-1197: The hover scaling is still being clipped because
.sql-result-scroll retains overflow-x: auto, so edge cells in the SQL grids will
crop on hover. Update the scroll container styling in .sql-result-scroll (and
the matching SQL grid variant block) so the hover scale can extend beyond the
horizontal edges, while preserving the intended scrolling behavior. Use the
existing .sql-result-scroll selector and related SQL grid styles as the place to
make the overflow adjustment.
- Around line 95-112: The v5 light theme currently leaves SQL heatmap glyphs
with dark-on-dark or white-on-light contrast issues because
`.sql-result-cell.error` still inherits `color: var(--bg)` and the pass/fail
states remain on white. Update the v5-specific styles in
`static/variants/v4-terminal.css` for the SQL result cell states so the glyph
colors are explicitly dark enough for Arctic Light, matching the existing v5
overrides already added in `static/variants/v4-overlay.css` for the same heatmap
states. Make sure the affected `.sql-result-cell` variants use the proper v5
text/contrast tokens rather than relying on the generic defaults.
---
Nitpick comments:
In `@static/variants/sql-grid.css`:
- Line 33: The `.sql-result-scroll` rule is redundantly redeclaring
`--sql-model-column-width` with the same value already set in `:root`; remove
this local override and keep the shared definition as the single source of
truth.
- Around line 404-504: The v5 SQL heatmap styling is duplicated between this
block and the existing rules in channels-d456.css, which will cause the two
versions to drift. Consolidate the shared data-design="v5" overrides into a
single source of truth and have sql-grid.css reuse that shared styling instead
of maintaining a second copy; keep the unique symbols like .sql-result-table,
.sql-result-cell, .sql-compare-bar, and .sql-result-count aligned with the
centralized rules.
🪄 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: CHILL
Plan: Pro Plus
Run ID: f26a0c81-c935-46cf-bd0f-bd910ca38df5
📒 Files selected for processing (21)
index.htmlstatic/app.jsstatic/index-compare-filter.htmlstatic/index-v1-gridhero.htmlstatic/index-v2-dock.htmlstatic/index-v3-tabs.htmlstatic/index-v4-overlay.htmlstatic/variants/c1-clean-lab.cssstatic/variants/c2-signal-monitor.cssstatic/variants/c3-warm-instrument.cssstatic/variants/channels-d456.cssstatic/variants/index.htmlstatic/variants/sql-grid.cssstatic/variants/v1-gridhero.cssstatic/variants/v1-instrument.cssstatic/variants/v2-dock.cssstatic/variants/v2-monitor.cssstatic/variants/v3-startup.cssstatic/variants/v3-tabs.cssstatic/variants/v4-overlay.cssstatic/variants/v4-terminal.css
added 4 commits
June 28, 2026 00:41
# Conflicts: # static/app.js # static/index-v1-gridhero.html # static/index-v3-tabs.html # static/variants/c3-warm-instrument.css # static/variants/channels-d456.css # static/variants/sql-grid.css # static/variants/v1-gridhero.css # static/variants/v1-instrument.css # static/variants/v2-dock.css # static/variants/v2-monitor.css # static/variants/v3-startup.css # static/variants/v3-tabs.css # static/variants/v4-terminal.css
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
1 participant
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.
Summary
sr-onlyhelper fix.app.js/v3-tabs.css; no additional code change was needed there.Changed files
static/style.cssstatic/index-v1-gridhero.htmlstatic/variants/v1-gridhero.cssstatic/variants/v1-instrument.cssstatic/variants/v2-monitor.cssstatic/variants/v3-startup.cssstatic/variants/v4-terminal.cssstatic/variants/sql-grid.cssstatic/variants/c3-warm-instrument.cssstatic/variants/channels-d456.cssValidation
git diff --check✅Notes
static/app.jsalready handles the v3 compare tab flow.Summary by CodeRabbit