Skip to content

feat(web): /snippets page — pipeline-ab Arm B (STREAMLINED)#10

Open
eksno wants to merge 2 commits into
alphafrom
feat/snippets-arm-b
Open

feat(web): /snippets page — pipeline-ab Arm B (STREAMLINED)#10
eksno wants to merge 2 commits into
alphafrom
feat/snippets-arm-b

Conversation

@eksno

@eksno eksno commented Jun 12, 2026

Copy link
Copy Markdown
Member

EXPERIMENT PR — DO NOT MERGE. pipeline-ab A/B experiment artifact (Arm B (STREAMLINED)).

Streamlined lean layout: Composer 2.5 hands + review-reconcile red-team review only (no mid-process ceremony, no QA).

Both arms build the IDENTICAL /snippets feature from ONE shared plan. This PR stays OPEN for the operator to evaluate by eye against its live Railway preview. Implementation commits land here next.


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

@railway-app railway-app Bot temporarily deployed to testing / testing-pr-10 June 12, 2026 19:14 Destroyed
@railway-app

railway-app Bot commented Jun 12, 2026

Copy link
Copy Markdown

🚅 Deployed to the testing-pr-10 environment in testing

Service Status Web Updated (UTC)
testing ✅ Success (View Logs) Web Jun 12, 2026 at 7:33 pm

A /snippets route rendering a gallery of syntax-highlighted code cards,
highlighted at build time so the browser ships zero highlighter JS.

Highlighting (src/lib/highlight.ts): Shiki via a css-variables theme, so
every token color resolves through a --shiki-* CSS variable defined under
:root and .dark in app.css (OKLCH, no hardcoded hex that breaks in one
theme). Done in a server load (+page.server.ts) so Shiki + grammars + WASM
never enter the client bundle -- the snippets client chunk stays ~8 kB
instead of ~230 kB.

Each card has a copy-to-clipboard button (raw source, SSR-guarded
clipboard helper) and an auto-dismiss toast (in-tree store with full timer
cleanup -- every setTimeout has a matching clearTimeout path; the icon
reset timer and toast timers are cleared on unmount). 3 snippets (TS /
Svelte / shell), wired into the site-header nav.

Adds vitest + jsdom and a dedicated vitest.config.ts; tests cover the
snippet data integrity, the lang mapping + highlighter output shape incl.
a hex-absence assertion, the clipboard success/SSR/failure paths, and the
toast auto-dismiss + timer-cleanup contract. shiki is a dependency;
vitest + jsdom are devDependencies.

Code authored by Cursor Composer 2.5 (test-firewalled) per ADR 0042;
designed, tested, reviewed, and gated by the Opus seat, which moved the
highlight load server-side to keep Shiki out of the client bundle.

Station-Trailer-Version: 1
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@eksno, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 59 minutes and 59 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7f384c22-f5a7-447a-bf69-102cb3783659

📥 Commits

Reviewing files that changed from the base of the PR and between c20b967 and 7e2b736.

⛔ Files ignored due to path filters (1)
  • web/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (16)
  • web/package.json
  • web/src/app.css
  • web/src/lib/clipboard.test.ts
  • web/src/lib/clipboard.ts
  • web/src/lib/components/site-header.svelte
  • web/src/lib/components/snippet-card.svelte
  • web/src/lib/components/toaster.svelte
  • web/src/lib/highlight.test.ts
  • web/src/lib/highlight.ts
  • web/src/lib/snippets.test.ts
  • web/src/lib/snippets.ts
  • web/src/lib/toast.test.ts
  • web/src/lib/toast.ts
  • web/src/routes/snippets/+page.server.ts
  • web/src/routes/snippets/+page.svelte
  • web/vitest.config.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/snippets-arm-b

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@eksno

eksno commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

Final review (advisory -- not a merge gate)

Arm B (lean) -- whole-diff red-team review: /snippets

Scope: git diff origin/alpha...HEAD on feat/snippets-arm-b. The complete
assembled change was read in full (17 files). Reviewed against the shared plan
snippets-plan.md and the original ask. Issues-only; each lens red-teamed.
Tests/check/build were NOT re-run (pre-gated by the strong seat per brief).

This arm is clean. No blocking or act-worthy correctness/standards/spec defects
were found. The items below are the only things surfaced, and all are minor
judgement calls left for the operator -- none was auto-fixed.

Lens 1 -- Spec gaps (against plan + original ask)

None.

Red-team notes (checked, NOT findings):

  • Plan named src/routes/snippets/+page.ts; impl ships +page.server.ts. The
    plan (S3b) explicitly permitted implementer judgement on the load path, and
    the original ask's bundle-size concern is better served by +page.server.ts
    (Shiki/grammars/WASM provably never enter the client graph). The route omits
    an explicit export const ssr but inherits ssr = true from +layout.ts,
    which is required for the server load to run at prerender -- correct. This is
    an improvement over the literal plan text, not a gap.
  • +page.server.ts exports prerender = true (valid on a server load); the
    global layout already prerenders, so the route prerenders to static
    already-highlighted HTML. Contract satisfied.
  • All hard-contract artifacts present and named exactly: highlightToHtml,
    toShikiLang, Lang, the full --shiki-* var set under BOTH :root and
    .dark, 3 snippets (>=1 each ts/svelte/shell), nav link, 4 vitest files.

Lens 2 -- Standards violations

None.

Red-team notes (checked, NOT findings):

  • Svelte 5 runes only ($props/$state/$effect); no export let, no onMount.
  • ASCII clean across all new files (verified: zero non-ASCII bytes); -- used
    in titles/meta, not em-dash.
  • Import paths use the established $lib/components/ui/*/index.js + import * as Card forms.
  • Plan S10 mentions cn(), but the new card/toaster compose only static class
    strings (no conditional/merge case), and site-header's existing pattern uses
    a class array, not cn(). cn() would add nothing here -- its absence is
    correct, not a violation.
  • {@html snippet.html} carries an inline justification comment + a scoped
    eslint-disable-next-line svelte/no-at-html-tags; source is our own static,
    server-highlighted content. Acceptable and documented.

Lens 3 -- Quality problems

None act-worthy. One cosmetic nit (left for judgement):

  • [minor/cosmetic] snippet-card.svelte: Card.Content applies its own
    horizontal padding and the inner code wrapper adds p-4, so the code block
    is double-padded relative to the card edge. Purely visual, behavior-
    preserving, and a taste call -- not a defect. Left.

Red-team notes (checked clean, NOT findings):

  • Timer/leak defense is complete on every path: card icon-reset timer cleared
    in $effect(() => () => clearTimeout(resetTimer)) and re-cleared before each
    reschedule; toast store clears per-toast on dismiss, and the Toaster's
    $effect cleanup calls _clearAllTimers() on unmount. No setTimeout
    without a matching clearTimeout path.
  • Theme-correctness: no hardcoded hex anywhere in new CSS/markup (verified by
    scan; only oklch(...) + token vars). Every syntax color resolves through a
    --shiki-* var defined under BOTH :root and .dark; --shiki-background: transparent lets the code block inherit the bg-muted/50 surface.
    createCssVariablesTheme() registers under the name css-variables, so the
    theme: 'css-variables' lookup in codeToHtml resolves correctly (verified
    against the installed shiki).
  • SSR/prerender safety: copyToClipboard guards typeof navigator === 'undefined'; Toaster renders an empty list server-side (store starts [])
    so prerender emits an empty container -- no hydration mismatch. Highlighting
    runs only in the server load.
  • Client-bundle: highlighter is confined to +page.server.ts; the browser
    receives only static strings. Singleton highlighter promise (_hp ??=) is
    reused across snippets.
  • Copy writes the RAW snippet.code, not the rendered HTML.
  • Tests are genuine: the hex-absence regex assertion directly encodes the anti-
    hardcoded-color requirement (run on ts AND shell); the toast test uses fake
    timers to prove auto-dismiss at TTL and that _clearAllTimers stops pending
    fires; clipboard test covers success, SSR-undefined, and reject paths.
  • No dead code; no unused imports.

Verdict

Clean change. Meets the plan and the original ask, follows repo conventions,
and defends the three failure modes the card targets (theme-flip correctness,
timer leaks, SSR safety). The single surfaced item is a cosmetic padding taste
call. Advisory only -- this arm does not block the ship.

@eksno

eksno commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

Arm B (lean) -- review disposition

Acted on

None. No cheap, clearly-correct, behavior-preserving fix was warranted -- the
change is clean across all three lenses, so no edits, no fix commits, and the
test/check/build gate was not re-triggered (n/a).

Left for your judgement

  • Code-block double padding (snippet-card.svelte): Card.Content's own
    padding plus the inner wrapper's p-4 nests the code block one step in from
    the card edge. Cosmetic only; tighten to a single padding layer if you prefer
    the snippet flush to the card. Left because it is a visual taste call, not a
    correctness issue.

Pass-2 (report-only): not run -- no fix was applied, so the assembled whole is
unchanged from pass-1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant