feat(docs): view-transition polish, branded search + 404, chrome pass - #313
Conversation
View transitions (astro-vtbot, SPA mode): - Direction-aware 150ms slide on <main>, keyed to sidebar order (<PageOrder/>), so next/prev pages slide like paging a book - <PageOffset name="main"/> kills the fake-scroll artifact when navigating away from a scrolled page - Page-title morph between doc pages via view-transition-name on the title-panel h1 (direct-child selector so markdown h1s can't duplicate the name and void the transition) - Self-hosted navigation progress bar on vtbot's loading() hook — vtbot's own <ProgressBar/> imports Swup's plugin from unpkg.com at runtime in visitors' browsers, which fails our supply-chain bar - Belt-and-braces reduced-motion guard on all VT pseudo-elements Starlight chrome: - Search dialog branded: surface/border/radius on the frame, code-bg input with accent focus, card-primitive result groups, accent <mark> highlights, blurred backdrop - Asides mapped onto brand hues (stock blue/purple/orange/red tokens were never remapped, so the first :::note would land off-palette) - kbd/mark prose treatments, accent anchor-link hover, thin branded scrollbars on inner scroll surfaces, mobile menu button kept on the surface scale instead of stock white inversion 404 page: "signal lost" treatment — landing atmosphere (grid + glow), mono 404, flatlining waveform, card quick-links; custom h1 replaces the hidden stock title panel so the page keeps exactly one heading. PostHog: capture_exceptions enabled (error tracking for docs JS); the SPA pageview setup (capture_pageview: history_change) was verified correct against the vtbot lifecycle docs and needs no change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review finding from the pre-push code reviewer: the new transition choreography, ProgressBar component, branded chrome, 404 redesign, and PostHog capture_exceptions had no [Unreleased] entry (AGENTS.md §Documentation Sync). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR refreshes the documentation site: adds runtime UX components (progress bars, reading progress), scroll/zoom improvements for Mermaid/tables, a global CSS/layout overhaul, sidebar and homepage restructuring, and splits the TypeScript SDK docs into focused topic pages with synchronized examples. ChangesDocs Site UX, Styling, and Content Refresh
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
github-actions
Bot
added
documentation
|
📚 Docs preview is live → https://47615bda-wavehouse-docs.wave-rf.workers.dev
|
…arch Review feedback on the PR preview: - Navigation choreography removed — the direction-aware slide, page-title morph, PageOrder and PageOffset are gone (too much motion for docs). The vtbot SPA base, the nav progress bar, and the reduced-motion view-transition guard stay. - New ReadingProgress.astro: a 2.5px accent line under the header filling with the scrolled-past fraction of the article body — the mobile answer to the desktop TOC's "where am I". Doc pages only (rendered from the sidebar footer branch), hides on short articles and under the open mobile menu, one rAF-throttled passive listener that survives view transitions. - 404 trace reworked into an EKG: heartbeat decaying into a flatline, drawn by a looping monitor-sweep dash animation (pathLength-normalized) over a live→dead gradient, with a dim static ghost underneath so the shape always reads, and a blinking sample dot. Static full trace for reduced motion. - Search round 2: group-title header band + transparent rows on one card surface, surface-tint row hover replacing Starlight's bare bright outline, synced inner corner radii, branded clear/load-more controls, and a gentle reduced-motion-aware pop on open. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rule PR preview feedback round 3: - Reading bar: 2px solid accent at 75% opacity (was a 2.5px glowing gradient), spanning only the content column — offset past the sidebar at >=50rem, hidden >=72rem where the right-rail TOC already answers "where am I". Fixes the line crossing the sidebar border and the unexpected full-viewport width. - Search dialog: the results list scrolls inside .dialog-frame with a stock chunky UA scrollbar against the rounded corner — now thin and brand-colored (dialog + frame). - Doc h2 section rule: the full-width border-bottom boxed every section against the content edge; replaced with a hairline that fades out to the right (same scannability, no grid-paper feel). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- New homepage section "Query it like a database. Subscribe to it like a socket." — Ingest / Query / Live-updates tabs showing @wavehouse/sdk snippets (insert, chainable query, liveQuery backfill+stream), each checked against the API documented in sdk.md. The SDK is a core selling point and the landing only showed curl. - Starlight <Tabs> restyled as a segmented control on the card surface (the underlined default was the last stock-looking chrome piece); horizontal scroll + thin scrollbar on narrow viewports. - The lone full-width "How it ships" card read as an orphan section — its content (single process, ClickHouse-only dependency) folds into the quickstart prose unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both pre-push reviewers (code + docs) flagged the landing "Live updates" snippet: wh.from(table) returns a TableRef, which has no .where()/ .orderBy()/.limit()/.liveQuery() — those live on QueryBuilder, entered only via .select(...)/.selectAll(). The snippet was faithfully copied from sdk.md's Live Queries example, which has carried the same latent bug (uncaught because no e2e test exercises that chain shape). Both now start the chain with .selectAll(); CHANGELOG gets a Fixed entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…group - Eighteen "> **Title.** body" blockquotes across api/pipes/ access-control/sdk/development become :::note/:::caution asides: the bold lead promotes to the aside label, sharp-edge content (at-least-once retry, 64 MiB cap, lockout footguns, latest-tag promotion) gets caution, behavioral clarifications get note. Bodies unchanged; short untitled clarification quotes stay blockquotes. - Sidebar: API Reference + TypeScript SDK move from "Guides" into a new "Reference" group — lookup material vs narrative. Slugs/URLs are untouched; llms.txt page order follows sidebar.ts as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Round-7 review findings: the two api.md blockquotes with a second ">"-bridged paragraph left artifacts after the aside conversion — the Insert-only rationale rendered as a detached gray blockquote below its note, and a stray empty blockquote sat between the FORMAT note and the 64 MiB caution. Continuation folded into the note body, stray ">" removed, artifact scan across all docs now clean. CHANGELOG count corrected to nineteen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The five ways a first session actually goes sideways — schema-refresh 404 on a fresh table, querying inside the flush window, fail-closed 403 on non-demo tables, port conflicts, cold-start timing — were each documented somewhere, but not where a new user hits them. All claims sourced from existing docs + deployments/compose/standalone.yaml (ports, WH_SCHEMA_REFRESH_INTERVAL, plain depends_on). The SDK pointer in step 4 becomes a :::tip aside, exercising the styled tip variant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two parallel design-review agents audited the full screenshot corpus (13 pages x widths x themes); confirmed-real findings fixed: - Tabs: Starlight's stock underline is a box-shadow on every tab — it survived the segmented-control restyle as a floating arc under each inactive pill. box-shadow: none on the base tab. - Search: results hard-clipped mid-glyph against the dialog's rounded bottom; a bottom-only mask scrim makes the cut read as intentional (the pinned input at the top of the frame is unaffected). - Header glass: 78% -> 88% bg opacity — scrolled body text stayed legible through it and visually collided with the search trigger. - Mobile header controls: the menu button was a heavy filled circle next to the bordered search trigger; both states now sit on the bordered-surface family. - Hero CTAs stack full-width + centered on <=30rem (ragged hug-widths). Rejected after verification: "h2 rule repeats between wrapped lines" (the landing's background grid, not the heading rule) and a stale "empty aside fragment" (screenshot predated 79617f0's fix). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
☀️ Overnight run — morning summaryPreview: https://d821250a-wavehouse-docs.wave-rf.workers.dev · 6 pushes, 10 review rounds (every push: local What changed since your feedback last night
Verified
Deliberately not done (your call)
🤖 Generated with Claude Code |
…ings A 15-agent visual audit (every page x 320-1440px x both themes, on the exhaustive screenshot corpus) after the report that many pages look broken on mobile. Four systemic root causes, all verified by hand before fixing: - Tables crushed instead of scrolling: fixed-layout + overflow-wrap: anywhere shredded identifiers at 390px (WH_P/ROME/THEU/S_PO/RT) and split tokens even at 1440. New ScrollHints.astro wraps each table in a framed scroll container with true auto layout (prose wraps first, scroll only when min-content demands); GitHub-style display:block scroll as the no-JS fallback. overflow-wrap: anywhere removed. - Mermaid diagrams scaled to ~3-7px-label thumbnails on phones: standalone svgs now wrapped in the same scroller, keeping readable size with pan (no-JS keeps scale-to-fit via the existing carve-out selector that stops matching once wrapped). Zoom lightbox: safe centering (plain centering made the overflow start side unreachable); coarse pointers get a visible zoom badge. - Translucent fixed chrome double-exposed while scrolling on mobile: header + Starlight's sticky On-this-page bar go opaque <50rem (glass stays on desktop). Removed the hand-written -webkit-backdrop-filter: it made LightningCSS treat prefixing as author-managed and DROP the standard backdrop-filter from the build (Firefox got no blur). - Wrapped headings drew the section rule through the text: Starlight renders headings display:inline inside .sl-heading-wrapper, so backgrounds fragment per line box (pre-branch border-bottom had the same bug) and heading margins no-oped. Rhythm + rule moved to the wrapper. Also: right-edge fade scrim on every horizontal scroller while content is hidden (data-wh-more, scroll-tracked), code ligatures disabled site-wide (!= rendered as one ≠ glyph in operator reference tables), footer keeps 3 columns at tablet widths, reading bar at 0.9 opacity, search dialog bottom scrim deepened. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
diagrams, aside titles, state-machine labels Second 15-agent audit over a regenerated corpus (mechanical overflow: 0 hits site-wide with the corrected detector). Confirmed fixed: chrome opacity, table shredding, heading rules, scroll affordances. Residuals fixed here: - Scrolling tables: per-column min-widths (6rem, last column min(26rem, 75vw)). Without floors auto layout crushed off-screen prose columns to ~100px — their 10-20 wrapped lines drove row height, leaving near-blank viewport-tall rows (config policy table was a full-screen empty frame at 390; tallest row now 438px). - Table code chips: white-space nowrap — `--url` broke at hyphens into what read as two separate flags (sdk codegen table). - Diagrams pin to Mermaid's stamped natural width inside the scroller (style.minWidth = style.maxWidth): the 540px CSS floor still let the widest ingest-pipeline diagrams shrink ~33% to ~5px labels. Verified 2148px natural = 2148px rendered at 390. - Aside titles: block flow + inline icon — Starlight's flex title made a trailing code chip its own flex item, detaching it from the sentence when the title wrapped. - Zoom lightbox stage joins the data-wh-more fade contract (it hard-clipped nodes at its edge with no scroll hint). - ingest-pipeline state-machine edge labels shortened; the long labels overlapped each other at every width (prose explains the semantics). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🔍 Rigorous audit round — response to "pages look broken on mobile"Preview: https://7983d615-wavehouse-docs.wave-rf.workers.dev You were right — my earlier sweep covered 5 pages and extrapolated. This round did it properly: Method. Exhaustive corpus: all 14 pages × {320, 360, 390, 414, 768, 1024, 1440}px × dark/light, with 390px captured as sequential viewport slices (full-page shots hide sticky-chrome bugs — they render the header once, at the top). Plus per-element mechanical overflow detection at every mobile width, and interactive states (search, menu, zoom lightbox, tabs). Then 15 auditor agents in parallel, one per page, over the corpus → 28 MUST / 32 SHOULD findings → fixes → fresh corpus + second 15-agent verification round → residuals fixed. The four systemic root causes (all verified by hand before fixing):
Also fixed along the way: code ligatures off site-wide ( Where it stands: second-round verification: mechanical overflow 0 hits site-wide; 7/15 pages fully clean, the rest down to NITs (mostly capture-harness artifacts like touch-emulation gaps). Gates: Remaining known papercuts (deliberately not churned at 3am): prose inline-code chips can still wrap mid-token in one spot at 390 ( 🤖 Generated with Claude Code |
Starlight ships `.sl-markdown-content li { overflow-wrap: anywhere }`
(so long URLs in list items never overflow) — which also licenses
mid-token splits of short code chips at phone widths across every list
on the site. break-word keeps the no-overflow guarantee but only splits
a token that can't fit on a line of its own.
Known nit kept on the record: the ingest-pipeline `users_<suite>` chip
in the (a / b / c) run still splits at the same point under break-word
in Chromium; the override still removes the shred-license everywhere
else.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e16fa9f4-b3d2-48fe-8f94-f3d803af8f6a
📒 Files selected for processing (20)
CHANGELOG.mddocs/src/components/Footer.astrodocs/src/components/Head.astrodocs/src/components/Hero.astrodocs/src/components/MermaidZoom.astrodocs/src/components/PostHog.astrodocs/src/components/ProgressBar.astrodocs/src/components/ReadingProgress.astrodocs/src/components/ScrollHints.astrodocs/src/config/sidebar.tsdocs/src/content/docs/404.mddocs/src/content/docs/access-control.mddocs/src/content/docs/api.mddocs/src/content/docs/development.mddocs/src/content/docs/getting-started.mddocs/src/content/docs/index.mdxdocs/src/content/docs/ingest-pipeline.mddocs/src/content/docs/pipes.mddocs/src/content/docs/sdk.mddocs/src/styles/global.css
📜 Review details
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI
🧰 Additional context used
📓 Path-based instructions (3)
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Markdown files must pass markdownlint style checks and misspell spell-checking
Files:
docs/src/content/docs/pipes.mddocs/src/content/docs/404.mddocs/src/content/docs/access-control.mddocs/src/content/docs/ingest-pipeline.mddocs/src/content/docs/development.mddocs/src/content/docs/getting-started.mddocs/src/content/docs/api.mdCHANGELOG.mddocs/src/content/docs/sdk.md
docs/src/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
docs/src/**/*.{md,mdx}: Mermaid diagrams must be authored vertically (defaultflowchart TB/TD) to fit Starlight content column (~46–58rem); reserveLRfor short chains (≤3–4 nodes); use<div class="diagram-pair">…</div>for side-by-side comparisons
Mermaid node labels should be short, using<br/>for second lines; lean on semantic node classes (:::wh,:::win,:::pain,:::fail,:::infra,:::neutral,:::store,:::client)
Files:
docs/src/content/docs/pipes.mddocs/src/content/docs/404.mddocs/src/content/docs/access-control.mddocs/src/content/docs/index.mdxdocs/src/content/docs/ingest-pipeline.mddocs/src/content/docs/development.mddocs/src/content/docs/getting-started.mddocs/src/content/docs/api.mddocs/src/content/docs/sdk.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript files must pass Biome linting and
tsctype checking
Files:
docs/src/config/sidebar.ts
🧠 Learnings (2)
📚 Learning: 2026-05-19T14:42:16.296Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 142
File: docs/src/styles/global.css:13-20
Timestamp: 2026-05-19T14:42:16.296Z
Learning: In the Wave-RF/WaveHouse repo, CSS review should treat Tailwind v4 at-rules (e.g., `theme`, `layer`, and `import ... layer()`) as intentional and valid. They are processed by `tailwindcss/vite` configured in `docs/astro.config.mjs` (`vite.plugins`), so you should not flag these directives in `docs/src/styles/**/*.css` as “unknown at-rules” during review.
Applied to files:
docs/src/styles/global.css
📚 Learning: 2026-05-19T14:42:16.296Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 142
File: docs/src/styles/global.css:13-20
Timestamp: 2026-05-19T14:42:16.296Z
Learning: In Wave-RF/WaveHouse, Tailwind v4 directives in CSS (e.g., `theme`, `layer`, and `import ... layer(...)`) are intentionally supported by the docs build pipeline (via `tailwindcss/vite` configured in `docs/astro.config.mjs`). During code review, do NOT flag these as “unknown at-rules” in files under `docs/src/styles/`; they should be allowed because they validate during the build.
Applied to files:
docs/src/styles/global.css
🪛 LanguageTool
docs/src/content/docs/api.md
[style] ~331-~331: A comma is missing here.
Context: ...e] ClickHouse's inline FORMAT clause (e.g. SELECT 1 FORMAT CSV or `… FORMAT Pret...
(EG_NO_COMMA)
[style] ~359-~359: Consider using the typographical ellipsis character here instead.
Context: ...e upstream URL, and the request body is {"sql": "..."} with no escape hatch for query-strin...
(ELLIPSIS)
docs/src/content/docs/sdk.md
[style] ~82-~82: ‘in preference to’ might be wordy. Consider a shorter alternative.
Context: ...are present the server reads the header in preference to the query parameter, and strips the `?t...
(EN_WORDINESS_PREMIUM_IN_PREFERENCE_TO)
🪛 Stylelint (17.12.0)
docs/src/styles/global.css
[error] 1152-1152: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 1157-1157: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 1162-1162: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 1167-1167: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 1442-1442: Expected "currentColor" to be "currentcolor" (value-keyword-case)
(value-keyword-case)
🔇 Additional comments (17)
docs/src/content/docs/404.md (3)
1-17: LGTM!
19-41: LGTM!
48-48: ⚡ Quick winVerify external link behavior is intentional.
The GitHub issues link opens in the same tab (no
target="_blank"), which navigates users away from the docs. If users are expected to open a new tab to report issues while keeping the docs visible, consider addingtarget="_blank"andrel="noopener noreferrer"for security.If same-tab navigation is intentional to avoid popup friction, the current implementation is fine.
Suggested enhancement if new-tab behavior is preferred
- <p class="wh-404__report">Followed a link that should have worked? <a href="https://github.com/Wave-RF/WaveHouse/issues/new">File an issue</a> — broken links are bugs.</p> + <p class="wh-404__report">Followed a link that should have worked? <a href="https://github.com/Wave-RF/WaveHouse/issues/new" target="_blank" rel="noopener noreferrer">File an issue</a> — broken links are bugs.</p>docs/src/config/sidebar.ts (1)
19-29: LGTM!docs/src/content/docs/index.mdx (3)
28-28: LGTM!
105-147: LGTM!
170-170: LGTM!docs/src/content/docs/getting-started.md (2)
78-80: LGTM!
94-103: LGTM!docs/src/content/docs/development.md (1)
535-537: LGTM!docs/src/content/docs/ingest-pipeline.md (1)
136-147: LGTM!docs/src/content/docs/access-control.md (1)
54-56: LGTM!Also applies to: 67-69, 220-222
docs/src/content/docs/pipes.md (1)
36-38: LGTM!Also applies to: 175-177
docs/src/content/docs/api.md (1)
71-73: LGTM!Also applies to: 179-183, 304-306, 330-336, 358-360, 404-406, 441-443, 540-542
docs/src/content/docs/sdk.md (1)
81-83: LGTM!Also applies to: 376-378, 595-597, 622-622
CHANGELOG.md (1)
23-26: LGTM!Also applies to: 42-43
docs/src/components/Hero.astro (1)
378-388: LGTM!
github-project-automation
Bot
moved this from Backlog
to In review
in WaveHouse Task Board
|
Correction to the audit summary's "remaining papercuts": the mid-token chip wrapping in list items is now fixed in 🤖 Generated with Claude Code |
…code readability Response to PR-review feedback on layout drift and horizontal scrolling: - Layout back to Starlight stock geometry: one 45rem content track at every width (was 45/50/58rem stepped), stock 18.75rem sidebar, prose sub-cap removed — text/tables/code now share the same right edge. The ≥100rem .main-frame cap is gone outright: auto margins on a flex item sized it to fit-content (column measured 544px instead of 720px at 1680px) while the fixed left sidebar stayed glued to the viewport edge. - Mermaid: desktop returns to scale-to-fit + click-to-zoom; the natural-width pin (minWidth = maxWidth) that forced wide diagrams into horizontal scrollers at every viewport is now phone-only via a --wh-natural-w custom property (pan beats 3px labels there). - Zoom lightbox: fit math sizes against the stage CONTENT box (subtract padding, floor fractional px) — fully-zoomed diagrams previously overshot by the padding and always showed both scrollbars. - Code blocks wrap (expressiveCode defaultProps.wrap) instead of scrolling; development.md Quick Reference drops the column-aligned trailing comments whose 39-space continuation indent guaranteed overflow under preserveIndent. - Table code wraps BETWEEN tokens: per-token nowrap spans split at spaces, <wbr> after slashes in paths — tokens stay atomic (no --url shred), JSON bodies/commands wrap at their spaces. No-JS keeps the nowrap+scroll fallback. - Table column floors apply only to tables measured as genuinely overflowing (data-wh-scrolls) — unconditional floors were CREATING scroll on tables that fit by wrapping. - configuration.md stacks each setting's YAML key over its env var in one column (the two widest columns in every table on the page). - Table readability: 14px top-aligned body cells, zebra rows, stronger hover. - Stylelint (CodeRabbit): blank line before declarations after custom properties in the aside variants; currentcolor keyword case. Verified: zero page-level horizontal overflow across 14 pages x 12 widths (320-2560) x both themes; at >=1024px every table and code block fits the column except two tables in the ~90px window at the exact 72rem breakpoint where stock Starlight squeezes the pane below 45rem. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🔧 Morning round — layout back to stock Starlight, sideways scroll eliminatedCommit: Point-by-point response to this morning's feedback: "Layout lines up weird" — confirmed, and it was measurably brokenEvery layout value diffed against stock Starlight. Deviations found:
The doc shell is now byte-for-byte stock Starlight geometry; the branding lives in colors/typography/chrome, none in layout math. Mermaid horizontal scrolling — regression found and fixedThe "natural-size diagrams" round pinned every diagram to its natural width ( Zoom modal phantom scrollbars — real bug, fixedThe fit math sized the diagram against Tables + code blocks overflowing — systematically eliminatedFour root causes, four fixes:
Verification: 14 pages × 12 widths (320–2560px) × both themes — zero page-level horizontal overflow anywhere. At ≥1024px every table and code block fits the column except two tables in the ~90px window at exactly the 72rem breakpoint, where stock Starlight itself squeezes the pane below 45rem (they fit again at 1280+). Phones keep the designed pan-inside-scroller behavior. Table readability passBody cells dropped to 14px with tightened line-height and top alignment, zebra striping replaces hover-only row tracking (hover stays, a step stronger). Preview "failing to load"Investigated api.md lengthAgreed — tracked by #302 (OpenAPI spec → rendered reference); not restructuring that page meanwhile. Splitting the SDK docssdk.md is 806 lines / 19 top-level sections. Split plan:
On multi-language: topic-first URLs with shared usage pages — when a second language lands, the usage pages grow Also in this pushCodeRabbit's stylelint findings fixed (blank-line-before-declaration ×4, 🤖 Posted on Eric's behalf by Claude Code |
Second response round to PR-review feedback: - SDK split: sdk.md (806 lines) becomes /sdk overview + five topic pages (queries, streaming, pipes, admin, reference) under a nested sidebar group. Topic-first URLs are the multi-language plan: a second language adds <Tabs syncKey="lang"> to the shared usage pages + its own setup page — no URL churn. Install is <Tabs syncKey="pkg"> (pnpm/npm/yarn/ bun/deno/CDN) and the landing hero's install chip follows the synced choice via Starlight's localStorage label. - Breakout layout: markdown body becomes a 3-track grid — prose capped at the stock 45rem in the centered middle track (title + prose share a left edge), while tables/code/diagrams span all tracks at fit-content: short blocks keep prose width and alignment, wide ones grow centered up to the main pane. Pane width depends only on the viewport, so the TOC never moves and the page never scrolls sideways. - Tables: header row emphasized (0.8125rem/650 on ink, stronger underline); wrap-vs-scroll is now a crowding heuristic — a >=4-column table averaging under ~8.5rem per column scrolls instead of wrapping every other word. Verified: 19 pages x 12 widths (320-2560) x both themes — zero page-level horizontal overflow; breakout confirmed at 1440/1920/2560 (title/prose left edges aligned, TOC position unchanged); hero chip verified for default/bun/deno/CDN cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The table cell still named docs/src/content/docs/sdk.md, deleted by the SDK docs split — a contributor adding an endpoint would be directed to a non-existent file (pre-push reviewer finding). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s optional Two stale claims surfaced by the docs reviewer's cross-checks: - development.md documented KEEP_RUNNING=true for make test-e2e, but the orchestrator reads no such variable and always tears down — removed. - sdk/streaming.md typed StreamSubscriber.initial as required; clients/ts/src/types.ts:49 declares it optional (initial?:). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🎯 Round 6 — the phantom fade, killed by constructionCommits: Why this artifact kept survivingYour "happens when the comment wraps" observation was the key. The fade depends on exactly where a line wraps at your specific width — when a wrap point lands after a run of preserved spaces (like the gap before a The fix is an invariant, not a thresholdA wrapped code block ( Your other two asks
The dedicated verification you asked forA single-purpose agent hunted exactly this artifact: 42,696 element scans, zero violations —
Verdict line from the agent: AlsoThe docs reviewer's cross-checks caught two stale claims while gating this round, fixed in 🤖 Posted on Eric's behalf by Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/src/components/ScrollHints.astro (1)
22-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDrop
<table>from the scroller selector.After
wrapScrollables()runs, JS-enhanced tables scroll on.wh-table-wrap, not on the<table>itself. Keeping.sl-markdown-content tableinSCROLLERSmakesupdateAll()stampdata-wh-more*on a non-scrolling child once, while the scroll listener only updates the wrapper. That leaves a stale right-edge mask behind after the user pans to the end.Suggested patch
- const SCROLLERS = - ".expressive-code pre, .sl-markdown-content table, .wh-scroll-x, .wh-zoom__stage"; + const SCROLLERS = + ".expressive-code pre, .wh-scroll-x, .wh-zoom__stage";docs/src/styles/global.css (1)
313-319:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExtend the mobile wrapper scroller rule to
classandstateMermaid diagrams.This block gives
class/stateSVGs the same phonemin-width: var(--wh-natural-w, 540px)asflowchart/sequence, but onlyflowchart/sequencewrappers getoverflow-x: auto. A wrappedclassorstatediagram on mobile can therefore keep the 540px floor without a local scroller when JS is unavailable.Suggested patch
.sl-markdown-content :where(p, div):has(> svg[aria-roledescription^="flowchart"]), - .sl-markdown-content :where(p, div):has(> svg[aria-roledescription^="sequence"]) { + .sl-markdown-content :where(p, div):has(> svg[aria-roledescription^="sequence"]), + .sl-markdown-content :where(p, div):has(> svg[aria-roledescription^="class"]), + .sl-markdown-content :where(p, div):has(> svg[aria-roledescription^="state"]) { overflow-x: auto; overflow-y: visible; scroll-padding-inline: 1rem; scrollbar-width: thin; }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 19b3ddfa-fc13-48ce-bca1-d427b439eabc
📒 Files selected for processing (8)
CHANGELOG.mddocs/src/components/ScrollHints.astrodocs/src/content/docs/access-control.mdxdocs/src/content/docs/deployment.mddocs/src/content/docs/development.mddocs/src/content/docs/sdk/pipes.mddocs/src/content/docs/sdk/streaming.mddocs/src/styles/global.css
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
docs/src/content/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
docs/src/content/**/*.{md,mdx}: Author Mermaid diagrams vertically (top-downflowchart TB/TD) to fit the Starlight content column (~46–58rem) at legible size; reserveLRfor short chains only (≤3–4 nodes)
Keep Mermaid node labels short; use<br/>for second line rather than one long line; lean on semantic node classes (wh,win,pain,fail,infra,neutral,store,client) and CSS variables
Never sit two large Mermaid diagrams side-by-side; wrap comparisons in<div class="diagram-pair">…</div>to stack vertically so each gets the full column width
Mermaid diagrams are click-to-zoom on the site (viadocs/src/components/MermaidZoom.astro), so fine detail is recoverable, but author diagrams legibly for the first view — don't ship an illegible inline diagram relying on zoom
Files:
docs/src/content/docs/sdk/pipes.mddocs/src/content/docs/deployment.mddocs/src/content/docs/development.mddocs/src/content/docs/sdk/streaming.mddocs/src/content/docs/access-control.mdx
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Validate locally before every push by running `make ci` — do not use CI as your first feedback loop
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Run `/prepush` to discover pre-push reviewers and run them until each returns `ship_it` before pushing a PR-branch
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Every code change must update its documentation and `CHANGELOG.md` in the same PR
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Address and resolve every review finding with a substantive reply, fix, or tracked issue; never silently drop a review comment
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Create PRs with `gh pr create --draft` only — never use `gh pr ready` or approve; only humans transition draft → ready-for-review
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: PR title must pass Conventional Commits gate: `<type>(scope): <subject>` ≤ 72 chars, subject lowercase-first with no trailing period; validate with `scripts/lint-pr-title.sh`
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Never force-push or rebase a PR branch — use `git merge origin/main` to absorb upstream changes
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Never hand-write markers or use `--no-verify` — if tempted, the gate is wrong-shaped for the situation; fix the gate instead
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Comment the *why*, not the *what* — add comments only when the reason is not obvious; keep to 1–2 lines and match file density; remove comments that merely restate the code
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: DRY — maintain one source of truth; before adding logic look for an existing helper, type, or constant to reuse; factor duplicated rules into one place
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Leave the codebase neater than you found it — fix stale comments, typos, misnamed locals, and dead code in passing, keeping cleanups in the same spirit as your change; defer large/risky cleanups to a tracking issue
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Aim for 80%+ test coverage on new code; project-wide CI-enforced minimum is 80% (merged unit + integration + e2e); per-suite minima: unit 70%, integration 12%, e2e 50%, SDK 50%
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Every new function should have corresponding test cases; run `make lint` and `make test` before considering work complete
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Run integration tests via `make test-integration` (Docker testcontainers; do NOT `make deps-up` first)
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Any notable change: update `CHANGELOG.md` under `[Unreleased]` section in the same PR
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: When backend API changes, update TypeScript SDK: add typed client methods in `clients/ts/src/`, update SDK docs under `docs/src/content/docs/sdk/`, and update API reference tree
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: When JWT auth or role extraction changes, update TypeScript SDK auth handling in `clients/ts/src/http.ts` and types in `clients/ts/src/client.ts`
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: When `EventMessage` or ingest event format changes, update TypeScript SDK payload types in `clients/ts/src/`; regenerate SDK types via codegen CLI
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: When structured query AST changes, update TypeScript SDK query-builder types and methods in `clients/ts/src/query-builder.ts`
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: When live-query aggregation classification changes, update TypeScript SDK live-query helpers in `clients/ts/src/stream/`
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: When named pipes API changes, update TypeScript SDK in `clients/ts/src/pipes.ts`
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: When policy or access-control logic changes, update TypeScript SDK in `clients/ts/src/policy.ts`
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: When ClickHouse schema-driven types change, re-run SDK codegen CLI and commit regenerated types
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Use Conventional Commits types (`feat`, `fix`, `docs`, `refactor`, `test`, `chore`, `ci`, `deps`, `build`, `perf`, `revert`, `style`) for commit messages and PR titles (≤ 72 chars)
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: No `CODEOWNERS` file — admin approval is enforced natively by the `main branch protection` ruleset's `required_reviewers` rule requesting the `Wave-RF/wavehouse-admins` team
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: `CONTRIBUTING.md` Conventional Commits type list must stay in sync with the regex in `scripts/lint-pr-title.sh` (the single source of truth used by both local gate and required `PR housekeeping` check)
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-06-10T18:17:03.723Z
Learning: Dependency vulnerability scanning: run `govulncheck ./...` on every push/PR (enforced by CI); Dependabot opens weekly grouped PRs with no auto-merge (human admin approval required)
📚 Learning: 2026-06-10T15:01:09.027Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 312
File: docs/src/content/docs/development.md:0-0
Timestamp: 2026-06-10T15:01:09.027Z
Learning: In this repo’s Markdown review (all .md files), do not flag capitalization/style issues for literal paths starting with ".github/" (or any substring that is a path beginning with ".github/"). Treat ".github" as the correct lowercase dotfile directory name, even when it appears inside prose or code spans; automated checks such as LanguageTool’s "(GITHUB)" rule commonly produce false positives for this literal filesystem path.
Applied to files:
docs/src/content/docs/sdk/pipes.mdCHANGELOG.mddocs/src/content/docs/deployment.mddocs/src/content/docs/development.mddocs/src/content/docs/sdk/streaming.md
📚 Learning: 2026-05-19T14:42:16.296Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 142
File: docs/src/styles/global.css:13-20
Timestamp: 2026-05-19T14:42:16.296Z
Learning: In the Wave-RF/WaveHouse repo, CSS review should treat Tailwind v4 at-rules (e.g., `theme`, `layer`, and `import ... layer()`) as intentional and valid. They are processed by `tailwindcss/vite` configured in `docs/astro.config.mjs` (`vite.plugins`), so you should not flag these directives in `docs/src/styles/**/*.css` as “unknown at-rules” during review.
Applied to files:
docs/src/styles/global.css
📚 Learning: 2026-05-19T14:42:16.296Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 142
File: docs/src/styles/global.css:13-20
Timestamp: 2026-05-19T14:42:16.296Z
Learning: In Wave-RF/WaveHouse, Tailwind v4 directives in CSS (e.g., `theme`, `layer`, and `import ... layer(...)`) are intentionally supported by the docs build pipeline (via `tailwindcss/vite` configured in `docs/astro.config.mjs`). During code review, do NOT flag these as “unknown at-rules” in files under `docs/src/styles/`; they should be allowed because they validate during the build.
Applied to files:
docs/src/styles/global.css
🔇 Additional comments (19)
docs/src/content/docs/sdk/streaming.md (1)
146-146: LGTM!docs/src/content/docs/sdk/pipes.md (1)
17-19: LGTM!CHANGELOG.md (1)
149-149: LGTM!docs/src/content/docs/development.md (3)
288-315: LGTM!
358-361: LGTM!
550-552: LGTM!docs/src/content/docs/access-control.mdx (4)
8-8: LGTM!Also applies to: 56-58, 69-71, 340-342
18-30: LGTM!
85-118: LGTM!Also applies to: 126-147, 160-186, 216-248, 261-287, 303-324, 377-390, 398-468
266-267: LGTM!Also applies to: 269-270, 381-382, 401-404
docs/src/content/docs/deployment.md (8)
34-35: LGTM!
121-122: LGTM!
156-157: LGTM!
176-180: LGTM!
206-210: LGTM!
317-318: LGTM!
385-386: LGTM!
425-428: LGTM!docs/src/styles/global.css (1)
1173-1176: Avoid reintroducing the manual-webkit-backdrop-filtertwin for the search dialog backdrop
site-search dialog::backdropcurrently sets bothbackdrop-filterand-webkit-backdrop-filter; since the project notes that this pattern makes the CSS pipeline drop the unprefixedbackdrop-filter(Firefox blur breakage), either remove the-webkit-backdrop-filterdeclaration here or confirm the emitted CSS still keepsbackdrop-filterfor this rule.
CodeRabbit round addressing (1 inline + 2 outside-diff + 1 verify): - classifyCode() now classifies code-ONLY starlight-tabs groups as a unit: all panels are measured (hidden ones get an inline display:block during the batched read — layout never reaches paint mid-batch) and the group pins to its widest panel via --wh-natural-w so the frame keeps one width across tab switches; matching breakout CSS rule. - Root cause found while wiring it: the natural-width probe set white-space:pre on the pre, but EC wraps on the DESCENDANT .code spans (pre.wrap .ec-line .code), so every default-wrap block measured its wrapped width and could never earn the wide track (only wrap=false blocks ever widened). The wrap class now comes off for the batched measurement; long-lined wrap blocks across api/development/sdk pages widen as the earn-the-width rule always intended. - Tab switches refresh edge fades — a pre revealed from [hidden] measured 0x0 when fades were last stamped. - Bare table left the SCROLLERS list: after wrapScrollables() tables scroll on .wh-table-wrap, so a fade stamped on the non-scrolling <table> itself could never clear. - The phone Mermaid wrapper-scroller rule extends to class/state diagrams (they had the 540px no-JS min-width floor without the matching overflow-x:auto). - Search-dialog backdrop drops the reintroduced hand-written -webkit-backdrop-filter twin (LightningCSS author-managed-prefix trap); emitted CSS verified — standard property kept, twin auto-emitted. Verified on the production build with Playwright: 19 pages x 5 widths (375-2560) — zero element overflow, zero fade-on-wrap, zero stale fades; tab-switch width drift 0px; the configuration and access-control format switchers take the wide track at >=1680px. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@coderabbitai The two outside-diff-range comments and the backdrop-filter verification from the latest review are addressed in 9f1f343:
The inline comment about tabbed code never earning the wide track is fixed in the same commit (details in the thread reply): code-only tab groups now classify as a unit, and fixing it surfaced a deeper measurement bug that had kept every default- 🤖 Generated with Claude Code |
|
Tip For best results, initiate chat on the files or code changes.
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
github-project-automation
Bot
moved this from In review
to Done
in WaveHouse Task Board
Addresses every thread @EricAndrechek left on #434. Docs — the documented decision, restored. PR #313 settled that SDK pages are topic-first: shared usage pages grow <Tabs syncKey="lang"> as languages land, each language keeps its own setup page, and the topic URLs never churn. The Go SDK's first draft deleted that comment and shipped a parallel /sdk/go/* tree with TypeScript left un-prefixed at the root of /sdk. Undone: - /sdk/{queries,streaming,pipes,admin,reference} are now .mdx, each carrying one <Tabs syncKey="lang"> block per section — shared prose outside, language-specific code and caveats inside. Nothing was dropped in the merge; both languages' full content is preserved. - /sdk/typescript (was the root /sdk) and /sdk/go are the per-language setup/caveats pages. /sdk is now a language-neutral overview. - The sidebar comment is back, and stronger: it says what a third language costs (one setup page, one TabItem per topic) and what it must not do (a parallel tree; a language at the root of /sdk). - AGENTS.md gains a §SDK docs layout section so the rule is enforceable rather than a comment one agent can delete. - SDK mentions across README, why-wavehouse, getting-started, the docs homepage, the 404 page and the footer drop the per-SDK dependency-count trivia and just name the clients we publish. The dependency posture stays where it is a real difference: the Go page's own comparison list. Make targets — one family, no new ad-hoc names. - test-ts -> test-sdk-ts, test-go-sdk -> test-sdk-go, plus test-sdk which runs both and test-sdk-go-e2e for the live-server suite. - test-conformance-ts is gone as a target: each language's half of the wire-format suite now rides that language's target, which is already how the Go half worked. - `make test` is the test-unit alias it was before this PR. - All four SDK targets use gotestsum and honor ARGS/V=1 like every other Go suite — the nested module has no tool directives, so they resolve the binary with `go tool -n gotestsum` from the root module first. Static checks — one entry point per tool, not per module. - lint-go-sdk and verify-go-sdk are deleted. lint-go now lints both modules (two invocations, because golangci-lint is module-scoped); verify-go-sdk was pure duplication — gofumpt is already fmt-go's job and `go vet` is golangci-lint's govet linter. - tidy covers the nested go.mod too, so verify checks exactly what fix rewrites. That closes the tidy half of #437; vulncheck remains open. - go-mod-download warms both modules, so anything reaching into clients/go inherits the prereq. - fix-go is one step per line instead of a five-command && chain, and a comment says the `cd` is the only difference between its two golangci-lint lines. Coverage — .testcoverage.yml now explains why the Go SDK has one gate key where the TS SDK has three (one instrumented suite vs two), and #518 tracks running the Go e2e suite from the orchestrator so it earns the same shape. Refs #434, #437, #518
1 participant
What
A broad polish pass over the docs site, building on #309's astro-vtbot SPA integration — evolved across several overnight cycles from PR-preview feedback. (The direction-aware page slides + title morph from round 1 were tried and ripped out on review — too much motion for docs.)
Progress feedback
loading()hook: vtbot's own<ProgressBar/>imports Swup's plugin from unpkg.com at runtime in every visitor's browser (verified in the built bundle), which fails our supply-chain bar.ReadingProgress.astro) — a 2px accent line under the header filling with how far through the article you've scrolled; spans only the content column, hidden ≥72rem where the right-rail TOC takes over. The mobile answer to "where am I on this page".Starlight chrome
<mark>hits, branded clear/load-more controls.:::note/:::cautionasides (sharp edges → caution, clarifications → note).kbd/mark, accent anchor hover, inner-surface scrollbars, mobile menu button kept on the surface scale, h2 section rule fades out instead of boxing each section.Landing page
clients/ts/srcby reviewers. This surfaced a latent bug in sdk.md's Live Queries example (chained.where()offfrom(), which has no builder methods) — fixed in both places.Docs IA + content
404 — "signal lost"
Landing atmosphere, mono 404, an EKG trace (heartbeat decaying to flatline, redrawn by a looping monitor-sweep over a live→dead gradient, dim static ghost underneath, blinking sample dot; static for reduced motion), card quick links, exactly one accessible heading.
PostHog
history_change+ init sentinel); addedcapture_exceptions: truefor error tracking on docs JS.Verification
make cigreen + both pre-push reviewersship_iton every push (9 review rounds; 2 iterate rounds caught real bugs — the sdk.md live-query defect and two aside-conversion artifacts — both fixed).Deliberately not done: splitting api.md/sdk.md (URL churn + redirects felt wrong to ship overnight without feedback — happy to do as a follow-up), Steps adoption on getting-started (would drop the numbered sections from the TOC).
🤖 Generated with Claude Code