site: SvelteKit rewrite (composable, prerendered, a11y)#33
Conversation
Composable section-block architecture, prerendered Cloudflare deploy, build-time typed site data replacing build.ts templating, a11y first-class. No i18n / redesign / docs system (YAGNI).
Replace the hand-rolled build.ts/dev.ts + single 211-line
index.html with SvelteKit 2 (Svelte 5) + adapter-cloudflare,
fully prerendered. Seven self-contained section components
(Wordmark/Install/Demo/Completion/Speaks/Why/SiteFooter); pages
are thin compositions, re-splittable in one file. Build-time
typed site data from workspace Cargo.toml (scripts/gen-site-data)
replaces {{var}} templating — zero placeholders in output.
Terminal transcripts rendered via whitespace-exact string
literals so the formatter can't corrupt the ASCII. a11y: skip
link, polite live region for copy + route-change focus,
prefers-reduced-motion, AA contrast tokens. Old pipeline/source
removed (recoverable on master).
svelte-check: 0 errors / 0 warnings. Build prerenders / /demo
/completion /why with zero {{}} placeholders, verified.
wrangler.jsonc pointed at the dead ./dist; repoint to the adapter-cloudflare worker (.svelte-kit/cloudflare/_worker.js + ASSETS) so `wrangler deploy` and CF preview builds work. Add a vitest guard asserting every install/completion command string is derived from the Cargo.toml-sourced package names (no hardcoded drift). Verified: dry-run reads 37 assets, ASSETS bound; test 2/2.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
runner | 09ebe0e | Commit Preview URL Branch Preview URL |
May 20 2026, 11:42 AM |
📝 WalkthroughRewrites the marketing site to SvelteKit 2 (Svelte 5) with
Standardizes the marketing site on SvelteKit prerendering, increases type-safety and test coverage, fixes inline-markdown/tokenization edge cases, improves accessibility, and simplifies build/deploy by adopting Vite/SvelteKit + Cloudflare adapter while preserving existing visual design and terminal presentation. WalkthroughReplace custom Bun build/dev with SvelteKit + Vite; add Cargo.toml→generated.ts prebuild, typed site data and commands, accessibility helpers and CopyButton, new Svelte components and routes (including changelog), consolidated styling, prerender integration tests, and Cloudflare Wrangler updates. ChangesSite Migration & Component Architecture
Sequence DiagramssequenceDiagram
participant CLI as gen-site-data.ts
participant Parser as parseCargoToml
participant Renderer as renderGenerated
participant FS as filesystem (write generated.ts)
participant Svelte as SvelteKit/Vite
participant Browser as User Browser
CLI->>Parser: read workspace Cargo.toml
Parser-->>Renderer: GenData object
Renderer->>FS: write site/src/lib/content/generated.ts
Svelte->>FS: import generated.ts at build/prerender
Svelte-->>Browser: prerendered HTML pages served
🚥 Pre-merge checks | ✅ 5 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
✨ Simplify code
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. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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: 6
🤖 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 `@site/package.json`:
- Around line 12-34: Update the package.json "version" field (currently "0.0.0")
to a SemVer MINOR bump reflecting the added backward-compatible functionality
(e.g., "0.1.0"); locate the "version" key in package.json and change its value
accordingly so the package version is consistent with the PR changes.
In `@site/src/app.css`:
- Around line 20-31: The root CSS token block violates Stylelint: normalize
keyword casing and adjust empty-line-before declarations in the :root
declarations. Make any keyword values lowercase (check and set the color-scheme
value and any other keyword tokens to lowercase) and ensure the block follows
your declaration-empty-line-before rule (either add a single blank line after
the opening brace or remove an extra blank line so the first declaration meets
your configured rule). Update the declarations such as --mono, --radius-sm,
--ring-width, --dur-fast, --lh-base, --ls-caps, --text-micro, --text-xs and
color-scheme accordingly so Stylelint no longer flags value-keyword-case or
declaration-empty-line-before.
In `@site/src/lib/content/site.test.ts`:
- Line 15: The test currently asserts
expect(site.version).toMatch(/^\d+\.\d+\.\d+/) which allows trailing junk;
tighten the regex used in the test (site.version) to fully anchor and validate a
semantic version (for example change to /^\d+\.\d+\.\d+$/ or a fuller semver
pattern like /^\d+\.\d+\.\d+(?:-[0-9A-Za-z-.]+)?(?:\+[0-9A-Za-z-.]+)?$/) so
invalid versions with extra characters no longer pass.
In `@site/src/lib/sections/SiteFooter.svelte`:
- Line 11: Replace the hardcoded "/blob/master/CHANGELOG.md" in
SiteFooter.svelte with a branch-aware path using a new defaultBranch property
(e.g., construct link as "{site.repo}/blob/{site.defaultBranch}/CHANGELOG.md");
add defaultBranch to the SiteData shape and populate it at build time (or
extract it from Cargo.toml metadata) and fall back to a sensible default like
"main" if missing so the changelog link no longer assumes "master".
In `@site/src/lib/sections/Speaks.svelte`:
- Line 5: In Speaks.svelte, the hardcoded counts in the <h3> headings (e.g. the
"Package managers · 12" <h3> and the two other similar headings) must be removed
or made dynamic: either delete the "· N" suffix from those <h3> elements, or
replace each hardcoded number with a computed value derived from the
corresponding array length (e.g. use the list's .length for the package
managers, languages, tools list rendering). Update all three instances in the
component so the displayed counts reflect the source arrays (or are omitted)
rather than fixed numbers.
In `@site/src/routes/`+page.svelte:
- Around line 10-19: Add missing Open Graph meta tags inside the existing
<svelte:head> block alongside the current og:title and og:description: include a
meta property="og:url" with content set to the canonical URL
("https://runner.kjanat.dev/") and a meta property="og:type" with
content="website" so social platforms can correctly resolve the page URL and
type; place these near the other og:* tags referencing the existing title,
description, and site.npmName variables.
🪄 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
Run ID: dae98fd0-9485-4ecf-9d79-84250303162a
⛔ Files ignored due to path filters (3)
bun.lockis excluded by!**/*.lockdocs/superpowers/specs/2026-05-19-site-sveltekit-rewrite-design.mdis excluded by!docs/superpowers/**site/static/icon.svgis excluded by!**/*.svg
📒 Files selected for processing (39)
package.jsonsite/.gitignoresite/_headerssite/biome.jsonsite/build.tssite/dev.tssite/package.jsonsite/scripts/gen-site-data.tssite/src/404.htmlsite/src/app.csssite/src/app.d.tssite/src/app.htmlsite/src/app/copy.tssite/src/index.htmlsite/src/lib/announce.svelte.tssite/src/lib/components/CopyButton.sveltesite/src/lib/content/site.test.tssite/src/lib/content/site.tssite/src/lib/sections/Completion.sveltesite/src/lib/sections/Demo.sveltesite/src/lib/sections/Install.sveltesite/src/lib/sections/SiteFooter.sveltesite/src/lib/sections/Speaks.sveltesite/src/lib/sections/Why.sveltesite/src/lib/sections/Wordmark.sveltesite/src/routes/+error.sveltesite/src/routes/+layout.sveltesite/src/routes/+layout.tssite/src/routes/+page.sveltesite/src/routes/completion/+page.sveltesite/src/routes/demo/+page.sveltesite/src/routes/why/+page.sveltesite/src/styles/404.csssite/src/styles/base.csssite/static/robots.txtsite/svelte.config.jssite/tsconfig.jsonsite/vite.config.tssite/wrangler.jsonc
💤 Files with no reviewable changes (8)
- site/src/app/copy.ts
- site/biome.json
- site/src/index.html
- site/src/404.html
- site/src/styles/base.css
- site/src/styles/404.css
- site/dev.ts
- site/build.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 18000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (rust)
- GitHub Check: Workers Builds: runner
🧰 Additional context used
📓 Path-based instructions (1)
@(package.json|pyproject.toml|setup.py|Cargo.toml|go.mod|pom.xml|build.gradle|VERSION)
📄 CodeRabbit inference engine (Custom checks)
@(package.json|pyproject.toml|setup.py|Cargo.toml|go.mod|pom.xml|build.gradle|VERSION): If any source code files (excluding tests, docs, CI, markdown, or comments-only changes) are modified, then a version field MUST be updated in one of the following files if present in the repo: package.json, pyproject.toml, setup.py, Cargo.toml, go.mod, pom.xml, build.gradle, or a VERSION file.
The new version MUST follow SemVer (MAJOR.MINOR.PATCH). If the PR introduces breaking changes (removal or renaming of public APIs, changes to function signatures, deleted exported symbols, or incompatible config changes), MAJOR must increment. If it adds backward-compatible functionality, MINOR must increment. If it only fixes bugs without changing public APIs, PATCH must increment.
Files:
package.json
🪛 HTMLHint (1.9.2)
site/src/app.html
[error] 9-9: <title> must be present in tag.
(title-require)
🪛 Stylelint (17.11.1)
site/src/app.css
[error] 31-31: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 20-20: Expected "Menlo" to be "menlo" (value-keyword-case)
(value-keyword-case)
[error] 20-20: Expected "Consolas" to be "consolas" (value-keyword-case)
(value-keyword-case)
🔇 Additional comments (31)
site/src/lib/content/site.ts (1)
1-38: LGTM!site/src/lib/content/site.test.ts (1)
1-14: LGTM!Also applies to: 18-28
site/src/lib/announce.svelte.ts (1)
1-17: LGTM!site/src/lib/components/CopyButton.svelte (1)
1-54: LGTM!site/src/lib/sections/Wordmark.svelte (1)
1-17: LGTM!site/src/lib/sections/Install.svelte (1)
1-42: LGTM!site/src/lib/sections/Completion.svelte (1)
1-51: LGTM!site/src/lib/sections/Demo.svelte (1)
1-32: LGTM!site/src/lib/sections/Why.svelte (1)
1-23: LGTM!site/src/lib/sections/SiteFooter.svelte (1)
1-5: LGTM!site/src/routes/+page.svelte (2)
1-8: LGTM!
21-29: LGTM!site/src/routes/completion/+page.svelte (2)
1-4: LGTM!
6-16: LGTM!site/src/routes/demo/+page.svelte (2)
1-5: LGTM!
7-19: LGTM!site/src/routes/why/+page.svelte (1)
1-15: LGTM!site/src/routes/+error.svelte (2)
1-3: LGTM!
5-16: LGTM!site/scripts/gen-site-data.ts (1)
1-95: LGTM!package.json (1)
10-11: LGTM!site/.gitignore (1)
1-9: LGTM!site/svelte.config.js (1)
1-20: LGTM!site/vite.config.ts (1)
1-10: LGTM!site/tsconfig.json (1)
2-14: LGTM!site/wrangler.jsonc (1)
5-12: LGTM!site/src/app.html (1)
1-13: LGTM!site/src/app.d.ts (1)
1-7: LGTM!site/src/routes/+layout.svelte (1)
1-60: LGTM!site/src/routes/+layout.ts (1)
1-4: LGTM!site/src/app.css (1)
1-19: LGTM!Also applies to: 33-43, 52-67, 68-91, 103-116, 119-120, 128-134, 136-143, 157-166, 279-303, 434-440
| "type": "module", | ||
| "scripts": { | ||
| "bd": "bun --silent run build", | ||
| "build": "bun build.ts", | ||
| "check": "biome check", | ||
| "prebuild": "bun run gen", | ||
| "build": "vite build", | ||
| "check": "bun run gen && svelte-kit sync && svelte-check --tsconfig ./tsconfig.json", | ||
| "deploy": "bun run build && wrangler deploy", | ||
| "dev": "bun run dev.ts", | ||
| "dev": "bun run gen && vite dev", | ||
| "fmt": "bunx dprint fmt", | ||
| "lint": "biome lint", | ||
| "prestart": "bun run build", | ||
| "start": "wrangler preview", | ||
| "tail": "wrangler tail", | ||
| "typecheck": "tsc --noEmit" | ||
| "gen": "bun scripts/gen-site-data.ts", | ||
| "preview": "vite preview", | ||
| "test": "bun run gen && vitest run" | ||
| }, | ||
| "devDependencies": { | ||
| "@biomejs/biome": "^2.4.14", | ||
| "@sveltejs/adapter-cloudflare": "^7.2.4", | ||
| "@sveltejs/kit": "^2.49.2", | ||
| "@sveltejs/vite-plugin-svelte": "^7.1.2", | ||
| "@types/bun": "^1.3.13", | ||
| "typescript": "^6.0.3", | ||
| "wrangler": "^4.87.0" | ||
| "svelte": "^5.43.2", | ||
| "svelte-check": "^4.4.1", | ||
| "typescript": "^6", | ||
| "vite": "^8.0.13", | ||
| "vitest": "^4.1.6", | ||
| "wrangler": "^4.93.0" |
There was a problem hiding this comment.
Ahoy — bump package version for this feature rewrite.
Line 3 still has version: "0.0.0" even though this PR adds backward-compatible functionality across source files; that should be a SemVer MINOR increment.
Suggested fix
- "version": "0.0.0",
+ "version": "0.1.0",As per coding guidelines @(package.json|pyproject.toml|setup.py|Cargo.toml|go.mod|pom.xml|build.gradle|VERSION): If any source code files ... are modified, then a version field MUST be updated ... If it adds backward-compatible functionality, MINOR must increment.
🤖 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 `@site/package.json` around lines 12 - 34, Update the package.json "version"
field (currently "0.0.0") to a SemVer MINOR bump reflecting the added
backward-compatible functionality (e.g., "0.1.0"); locate the "version" key in
package.json and change its value accordingly so the package version is
consistent with the PR changes.
| --mono: ui-monospace, Menlo, Consolas, monospace; | ||
| --radius-sm: 2px; | ||
| --radius-md: 4px; | ||
| --ring-width: 2px; | ||
| --ring-offset: 3px; | ||
| --dur-fast: 80ms; | ||
| --dur-base: 120ms; | ||
| --lh-base: 1.55; | ||
| --ls-caps: 0.16em; | ||
| --text-micro: 0.7rem; | ||
| --text-xs: 0.75rem; | ||
| color-scheme: light dark; |
There was a problem hiding this comment.
Fix Stylelint blockers in the root token block.
Line 20 and Line 31 violate configured Stylelint rules (value-keyword-case and declaration-empty-line-before), which can fail CI.
Suggested patch
- --mono: ui-monospace, Menlo, Consolas, monospace;
+ --mono: ui-monospace, "Menlo", "Consolas", monospace;
--radius-sm: 2px;
--radius-md: 4px;
--ring-width: 2px;
--ring-offset: 3px;
--dur-fast: 80ms;
--dur-base: 120ms;
--lh-base: 1.55;
--ls-caps: 0.16em;
--text-micro: 0.7rem;
--text-xs: 0.75rem;
+
color-scheme: light dark;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --mono: ui-monospace, Menlo, Consolas, monospace; | |
| --radius-sm: 2px; | |
| --radius-md: 4px; | |
| --ring-width: 2px; | |
| --ring-offset: 3px; | |
| --dur-fast: 80ms; | |
| --dur-base: 120ms; | |
| --lh-base: 1.55; | |
| --ls-caps: 0.16em; | |
| --text-micro: 0.7rem; | |
| --text-xs: 0.75rem; | |
| color-scheme: light dark; | |
| --mono: ui-monospace, "Menlo", "Consolas", monospace; | |
| --radius-sm: 2px; | |
| --radius-md: 4px; | |
| --ring-width: 2px; | |
| --ring-offset: 3px; | |
| --dur-fast: 80ms; | |
| --dur-base: 120ms; | |
| --lh-base: 1.55; | |
| --ls-caps: 0.16em; | |
| --text-micro: 0.7rem; | |
| --text-xs: 0.75rem; | |
| color-scheme: light dark; |
🧰 Tools
🪛 Stylelint (17.11.1)
[error] 31-31: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 20-20: Expected "Menlo" to be "menlo" (value-keyword-case)
(value-keyword-case)
[error] 20-20: Expected "Consolas" to be "consolas" (value-keyword-case)
(value-keyword-case)
🤖 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 `@site/src/app.css` around lines 20 - 31, The root CSS token block violates
Stylelint: normalize keyword casing and adjust empty-line-before declarations in
the :root declarations. Make any keyword values lowercase (check and set the
color-scheme value and any other keyword tokens to lowercase) and ensure the
block follows your declaration-empty-line-before rule (either add a single blank
line after the opening brace or remove an extra blank line so the first
declaration meets your configured rule). Update the declarations such as --mono,
--radius-sm, --ring-width, --dur-fast, --lh-base, --ls-caps, --text-micro,
--text-xs and color-scheme accordingly so Stylelint no longer flags
value-keyword-case or declaration-empty-line-before.
Split gen-site-data into a pure, I/O-free site-data.ts (parseCargoToml throws CargoParseError on missing/empty version|name|repository; renderGenerated) and a thin Bun CLI wrapper. Add vitest coverage: canonical parse, .git/trailing-slash strip, npm→crate fallback, MIT default, and the failure path the spec required (missing/empty required field → throw, never a stale/empty value). Scripts get their own Bun-typed scripts/tsconfig.json and are excluded from the app tsconfig, fixing the node:path ts2591 flood. vitest include extended to scripts/. Stale build.ts-era site/dist removed. svelte-check 0/0; tsc -p scripts 0; vitest 13/13; build prerenders all routes.
- nav.site: bare top margin let the 6.5rem/lh-0.95 wordmark glyphs render over the demo/completion/why links. Give it 3.25rem bottom separation + relative/z-index. Fixes the visible overlap. - Speaks: counts now derived from arrays (length), can't drift. - site.test: anchor the version regex to a full semver pattern. - +page: add og:url + og:type. Skipped (with reason): package.json version stays 0.0.0 — private, never published, field has no consumer. app.css "stylelint": no stylelint configured in the project; keyword values already lowercase. SiteFooter /blob/master/: master IS the repo default branch, link is correct — a configurable field is unwarranted scope. svelte-check 0/0; vitest 13/13; build prerenders all routes.
Asserts the shipped HTML, not just the data layer: every route
emitted, zero {{placeholders}}, title+canonical per page, full OG
set on /, command strings derived from canonical names, no
dangling internal links, footer attribution intact. Builds once
then reuses output. svelte-check 0/0; 20 tests pass.
The blinking caret sat after the last completion row, so the suggestion list read as command output. Move it onto the `runner <TAB>` input line at the tab position and drop the trailing one — the menu now clearly renders below a live prompt.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@site/scripts/site-data.ts`:
- Line 73: The normalization for repo (derived from repoRaw) fails for values
ending in ".git/" because it removes ".git" before trimming slashes; update the
logic in the site-data normalization (the repo assignment using repoRaw) to
first trim trailing slashes from repoRaw and then strip a trailing ".git" (or
use a single regex that removes either ".git" or ".git/"), so repo and
downstream repoShort/URL computations never retain a ".git" suffix.
🪄 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
Run ID: 82548807-faa9-4cf3-b3a1-1f6afab5b8fc
📒 Files selected for processing (10)
site/scripts/gen-site-data.tssite/scripts/site-data.test.tssite/scripts/site-data.tssite/scripts/tsconfig.jsonsite/src/app.csssite/src/lib/content/site.test.tssite/src/lib/sections/Speaks.sveltesite/src/routes/+page.sveltesite/tsconfig.jsonsite/vite.config.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 18000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: verify
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
- GitHub Check: Workers Builds: runner
🧰 Additional context used
🪛 ast-grep (0.42.2)
site/scripts/site-data.test.ts
[warning] 52-52: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${field} = .*$, "m")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 60-60: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${field} = .*$, "m")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 Stylelint (17.11.1)
site/src/app.css
[error] 31-31: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 20-20: Expected "Menlo" to be "menlo" (value-keyword-case)
(value-keyword-case)
[error] 20-20: Expected "Consolas" to be "consolas" (value-keyword-case)
(value-keyword-case)
🔇 Additional comments (10)
site/src/lib/sections/Speaks.svelte (1)
2-3: Avast! Ye've fixed it perfectly, matey!Har har! The old complaint about them hardcoded counts be resolved! Now the headings pull their numbers straight from the array lengths, so when ye add or remove items the counts update themselves. Beautiful work, this be exactly what we asked fer!
Also applies to: 43-43, 51-51, 59-59
site/src/routes/+page.svelte (1)
19-20: Yo-ho-ho! The Open Graph treasure be complete!Arrr, ye've added the
og:urlandog:typetags just as we asked! Now yer pages'll share pretty as a mermaid on Facebook and Discord. Well done, sailor!site/src/app.css (1)
7-481: Shiver me timbers, this CSS be shipshape!LGTM!
site/scripts/gen-site-data.ts (1)
1-36: LGTM!site/scripts/tsconfig.json (1)
1-16: LGTM!site/tsconfig.json (1)
2-21: LGTM!site/vite.config.ts (1)
1-13: LGTM!site/src/lib/content/site.test.ts (1)
1-29: LGTM!site/scripts/site-data.ts (1)
1-72: LGTM!Also applies to: 74-106
site/scripts/site-data.test.ts (1)
1-74: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@site/src/lib/sections/Completion.svelte`:
- Around line 29-31: The element with id "completion-tag" used as the section
label is a <span>; replace it with a real heading element (e.g., <h2> or
appropriate level) keeping the id "completion-tag" and the class "section-tag"
so the existing <section aria-labelledby="completion-tag"> keeps working and
improves semantic navigation for screen readers and keyboard users.
In `@site/src/prerender.test.ts`:
- Around line 28-33: The current build-guard only checks for pagesDir/index.html
using existsSync, which allows missing route files to slip through and causes
readFileSync in the loop (html.set) to throw ENOENT; change the guard to verify
that every expected route file from routes exists (e.g., iterate
Object.values(routes) and check existsSync(join(pagesDir, file)) for each), and
only skip execSync("bun run build", { cwd: root, stdio: "inherit" }) when all
files are present; otherwise run the build so readFileSync and html.set succeed.
🪄 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
Run ID: 686ff9f5-7610-4a8b-a531-b0f1f6635b32
📒 Files selected for processing (2)
site/src/lib/sections/Completion.sveltesite/src/prerender.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 18000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (rust)
- GitHub Check: Workers Builds: runner
🔇 Additional comments (6)
site/src/lib/sections/Completion.svelte (1)
1-27: LGTM!Also applies to: 32-50
site/src/prerender.test.ts (5)
1-6: LGTM!
14-23: LGTM!
36-47: LGTM!
49-72: LGTM!
74-95: LGTM!
app.css reduced to what genuinely can't be component-scoped:
tokens, resets, base elements, and the shared utilities used by
multiple components — .section-tag, .tagline, .meta, .install,
hr.rule, .wordmark (also the 404 page), and the entire .term*
terminal theme (Demo/Completion inject transcripts via {@html};
Svelte scoped styles never reach {@html} content, so .term* MUST
stay global).
Single-owner rules moved into the owning component's scoped
<style>: .copy* (CopyButton), .matrix* (Speaks), .why* (Why),
footer* (SiteFooter), nav.site/.skip-link (+layout). Selector-
faithful; selector accounting confirms nothing dropped. .wordmark
kept global after catching it is shared with +error.svelte.
svelte-check 0/0; 20 tests; build prerenders all routes.
Records why .term* and shared display classes (incl. .wordmark, shared with the 404) must stay global, the selector-accounting sweep to re-run after scoping, and an ordered near-term roadmap (tokens.css extraction next, then axe a11y, then a visual- regression guard) so the refactor isn't delayed indefinitely.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
/changelog renders the workspace CHANGELOG, no gen script and no
generated file: Vite inlines CHANGELOG.md via `?raw` (resolved
from source, survives bundling, HMR), a prerendered
+page.server.ts parses it once at build, output is baked static.
server.fs.allow:['..'] permits the out-of-root import.
Pure parser (src/lib/changelog.ts): folds 2-space wrapped
continuation bullets, skips [Unreleased] + the trailing
reference-link/comment block (only after releases start — the
preamble [label]: defs are skipped, not treated as terminator).
renderInline escapes HTML then reintroduces a fixed allow-list
(code/bold/autolinked URLs); {@html} is safe by construction +
covered by tests. Composable Changelog route + nav link; scoped
structure, {@html} code/a inherit global per DISCOVERIES.
Tests: parser (fold/skip/trailer), renderInline (escape/code/
bold/url/no-js-pseudo), prerender invariants extended to
/changelog (scope-tolerant assertions; {{ }} leak check excludes
changelog since it documents the retired build.ts templating).
svelte-check 0/0; 29 tests; build prerenders all routes.
ESLint flat config (eslint-plugin-svelte v3, typescript-eslint,
Svelte 5). Plain array — no tseslint.config() helper (its variadic
overload is deprecated). `lint` script; `eslint .` passes clean.
Reviewed rule-offs (documented in config, not per-line): every
{@html} renders trusted build-time content via escape-first /
whitespace-exact renderers; no-navigation-without-resolve N/A on a
prerendered domain-root site (no base/i18n; external links can't
resolve()).
Valid review fixes: CopyButton drop redundant catch assignment
(no-useless-assignment); site-data repo normalization handles
`.git/` (trim slashes before stripping .git); prerender test
build-guard checks every route file (not just index) so a partial
build can't ENOENT; DISCOVERIES sweep uses grep -F (selectors have
regex-special dots).
lint clean; svelte-check 0/0; 29 tests; build prerenders all.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@site/src/lib/changelog.ts`:
- Line 114: The bold replacement in the s.replace call currently forbids any
asterisks inside because it uses a negated-star character class; update the
regex used in that replace (the line assigning back to variable s) to use a
non-greedy dot pattern that matches any characters minimally up to the next
double-asterisk (and enable the dotAll/"s" flag if bold may span newlines) so
sequences like **Changed the * wildcard behavior** are correctly captured as
bold.
In `@site/vite.config.ts`:
- Around line 6-8: The dev server's file-system allowlist is too broad — change
the server.fs.allow value in vite.config.ts from allowing the entire parent
directory ("..") to only the specific file needed ("../CHANGELOG.md") so only
the workspace-root CHANGELOG.md can be imported via the /changelog route; update
the server.fs.allow entry referenced in the config to the single file path to
enforce least privilege.
🪄 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
Run ID: 3848a3ff-113a-4bd0-810d-2a3c42738fa9
📒 Files selected for processing (7)
site/src/lib/changelog.test.tssite/src/lib/changelog.tssite/src/prerender.test.tssite/src/routes/+layout.sveltesite/src/routes/changelog/+page.server.tssite/src/routes/changelog/+page.sveltesite/vite.config.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 18000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: verify
- GitHub Check: Workers Builds: runner
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
site/src/prerender.test.ts (2)
29-34: Build guard still checks only one prerender file.Line 29 only gates on
index.html; if another expected route file is missing, Line 33 can still throwENOENT. This was already called out in earlier review feedback and appears unresolved.
1-28: LGTM!Also applies to: 35-116
site/src/routes/+layout.svelte (1)
1-95: LGTM!site/src/routes/changelog/+page.server.ts (1)
1-18: LGTM!site/src/routes/changelog/+page.svelte (1)
1-91: LGTM!site/vite.config.ts (2)
1-2: Aye, these imports be ship-shape!Using
vitest/configferdefineConfiggives ye proper type inference fer both Vite and Vitest properties. Smartly rigged, that be.
9-15: Har har! These test configs be perfectly calibrated!Yer include patterns cover both
srcandscriptsdirectories as befits the project structure, and settin'environment: "node"be spot-on fer server-side prerender validation, parser tests, and codegen verification. No browser shenanigans needed here!site/src/lib/changelog.ts (3)
10-28: LGTM, ye scallywag!Yer interfaces be shipshape, and those regex patterns be seaworthy fer keepin' a changelog. The
REF_OR_COMMENT_REbe particularly clever fer skippin' the preamble but breakin' on the trailer. Arrr!
30-98: LGTM, ye magnificent sea dog!Yer
parseChangelogbe a thing o' beauty! TheflushItemhelper be clever, the Unreleased skippin' be spot-on (case-insensitive, no less!), the continuation-line foldin' be proper, and that reference-link trailer detection be workin' like a charm. This parser be tighter than a ship's riggin' in a storm!
100-113: LGTM, ye security-minded buccaneer!Yer HTML escapin' first, then reintroducin' the allow-list be the proper way to keep them XSS scallywags at bay! And that
rel="noopener noreferrer"on the autolinked URLs be fine defense against malicious sites. The{@html}be safe by construction here. Well done!Also applies to: 115-120
site/src/lib/changelog.test.ts (1)
1-83: LGTM, ye glorious test-writin' privateer!Shiver me timbers, this be some fine test coverage! Ye've got:
- Unreleased skippin' verified ⚓
- Version and date capture tested 🏴☠️
- Wrapped continuation lines folded proper 📜
- Multiple groups per release preserved ⚔️
- Reference-link trailer stoppin' the parse 🛑
- HTML escapin' workin' right 🔒
- Code spans, bold, and autolinks renderin' correctly 🎨
- And ye even blocked them treacherous
javascript:pseudo-URLs! 🦜This test suite be tighter than a treasure chest, and I be happier than a pirate with a fresh bottle o' rum! Well done, matey!
| // Allow importing the workspace-root CHANGELOG.md (?raw) from the | ||
| // /changelog route — it sits one level above the site root. | ||
| server: { fs: { allow: [".."] } }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Vite server.fs.allow configuration - can it accept specific file paths or only directories?
💡 Result:
Vite’s server.fs.allow can accept specific file paths as well as directories (it is not limited to directories-only). Vite’s official server options docs state that both directories and files can be provided in server.fs.allow, and the examples show entries like '/path/to/custom/allow_file.demo' alongside allow_directory values.[1][2] Note: There have been historical issues where providing an individual file didn’t behave as expected (e.g., Vite reporting the file as outside the allow list even when it appeared in the configured list).[3] However, the current docs explicitly describe support for files in the allow list.[1][2]
Citations:
- 1: https://vite.dev/config/server-options
- 2: https://v4.vitejs.dev/config/server-options
- 3: Vite fs allow not respecting individual files vitejs/vite#5689
Trim that allowlist from the whole parent directory down to just the file ye need, matey.
The allow: [".."] be openin' the dev server to yer entire parent directory when ye only need the CHANGELOG.md. Vite supports specific file paths in server.fs.allow, so batten this down to ["../CHANGELOG.md"] instead. Low effort, proper security, and it follows the principle of least privilege—which be the mark of a tidy codebase, savvy?
(Fair warnin': older Vite versions had quirks with file-level allowlists, but current docs explicitly support it.)
🤖 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 `@site/vite.config.ts` around lines 6 - 8, The dev server's file-system
allowlist is too broad — change the server.fs.allow value in vite.config.ts from
allowing the entire parent directory ("..") to only the specific file needed
("../CHANGELOG.md") so only the workspace-root CHANGELOG.md can be imported via
the /changelog route; update the server.fs.allow entry referenced in the config
to the single file path to enforce least privilege.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
site/package.json (1)
12-41:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAhoy — bump the package version before merge.
Line 3 is still
0.0.0while this PR adds backward-compatible functionality across source files; this needs a SemVer MINOR bump (for example,0.1.0).As per coding guidelines
@(package.json|pyproject.toml|setup.py|Cargo.toml|go.mod|pom.xml|build.gradle|VERSION): If any source code files ... are modified, then a version field MUST be updated ... If it adds backward-compatible functionality, MINOR must increment.🤖 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 `@site/package.json` around lines 12 - 41, Update the package.json "version" field (currently 0.0.0) to a SemVer MINOR bump (e.g., 0.1.0) because this PR adds backward-compatible functionality; locate the top-level "version" key in package.json and change its value accordingly before merging.
🤖 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 `@site/src/prerender.test.ts`:
- Around line 26-34: The current beforeAll guard only checks for file existence
(using routes, pagesDir, existsSync) so stale prerender artifacts can pass;
update beforeAll to force a build in CI or add mtime freshness checks: detect CI
via process.env.CI and call execSync("bun run build", { cwd: root, stdio:
"inherit" }) unconditionally there, and for local runs replace the simple
existsSync test with a modification-time comparison (fs.statSync/mtime) between
each generated file (join(pagesDir, file)) and relevant source files (or a
single known source timestamp) so stale files trigger execSync; keep symbols
routes, pagesDir, beforeAll, existsSync, execSync and root to locate the change.
---
Duplicate comments:
In `@site/package.json`:
- Around line 12-41: Update the package.json "version" field (currently 0.0.0)
to a SemVer MINOR bump (e.g., 0.1.0) because this PR adds backward-compatible
functionality; locate the top-level "version" key in package.json and change its
value accordingly before merging.
🪄 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
Run ID: 7860add1-fa48-4ea7-97aa-109aa7eb9b59
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
package.jsonsite/DISCOVERIES.mdsite/eslint.config.jssite/package.jsonsite/scripts/site-data.tssite/src/lib/components/CopyButton.sveltesite/src/prerender.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 18000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: runner
- GitHub Check: Analyze (rust)
🧰 Additional context used
📓 Path-based instructions (1)
@(package.json|pyproject.toml|setup.py|Cargo.toml|go.mod|pom.xml|build.gradle|VERSION)
📄 CodeRabbit inference engine (Custom checks)
@(package.json|pyproject.toml|setup.py|Cargo.toml|go.mod|pom.xml|build.gradle|VERSION): If any source code files (excluding tests, docs, CI, markdown, or comments-only changes) are modified, then a version field MUST be updated in one of the following files if present in the repo: package.json, pyproject.toml, setup.py, Cargo.toml, go.mod, pom.xml, build.gradle, or a VERSION file.
The new version MUST follow SemVer (MAJOR.MINOR.PATCH). If the PR introduces breaking changes (removal or renaming of public APIs, changes to function signatures, deleted exported symbols, or incompatible config changes), MAJOR must increment. If it adds backward-compatible functionality, MINOR must increment. If it only fixes bugs without changing public APIs, PATCH must increment.
Files:
package.json
🔇 Additional comments (5)
package.json (1)
10-11: LGTM!site/DISCOVERIES.md (1)
1-47: LGTM!site/eslint.config.js (1)
1-51: LGTM!site/src/lib/components/CopyButton.svelte (1)
1-162: LGTM!site/scripts/site-data.ts (1)
1-107: LGTM!
The two `"rule": "off"` entries were suppressions — against the
no-lint-suppression standard. Deleted them and fixed the actual
violations instead:
- no-at-html-tags (changelog): replaced renderInline()+{@html}
with tokenizeInline() → typed tokens rendered as real elements
via a new InlineMd component. Svelte auto-escapes; zero XSS
surface; no {@html}.
- no-navigation-without-resolve: internal links use resolve()
from $app/paths (nav route-ids + trailing-slash-aware
aria-current); external links carry rel="external …" (the
rule's designed, semantically-correct mechanism — full
navigation off-site, not a suppression).
- require-each-key: keyed the token loop.
Tests updated to tokenizeInline (lossless/escape/no-js-pseudo).
svelte-check 0/0; 30 tests; build prerenders all routes.
Remaining: the Demo/Completion terminal transcripts still use
{@html} (whitespace-exact ASCII) — handled next, not suppressed.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@site/src/lib/sections/Install.svelte`:
- Line 28: The copied curl command in Install.svelte uses a non-breaking space
in the split prop (split={{ pre: "curl -fsSL ", shrink: installerUrl, post: " |
sh" }}), which can break pasted shell input; replace the NBSP after "-fsSL" with
a normal ASCII space (or compute the pre string using "curl -fsSL "
programmatically) so the generated command uses a regular space character and
splits/executes correctly when pasted.
🪄 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
Run ID: 2146f544-a156-4cb5-b224-61077a1fb461
📒 Files selected for processing (11)
site/eslint.config.jssite/src/lib/changelog.test.tssite/src/lib/changelog.tssite/src/lib/components/InlineMd.sveltesite/src/lib/sections/Install.sveltesite/src/lib/sections/SiteFooter.sveltesite/src/lib/sections/Wordmark.sveltesite/src/routes/+error.sveltesite/src/routes/+layout.sveltesite/src/routes/+page.sveltesite/src/routes/changelog/+page.svelte
📜 Review details
⏰ Context from checks skipped due to timeout of 18000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: verify
- GitHub Check: Workers Builds: runner
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (rust)
🔇 Additional comments (12)
site/src/lib/sections/SiteFooter.svelte (2)
12-12: Branch name is still hardcoded in the changelog URL.Line 12 still assumes
master; this was already raised and remains unresolved.
9-22: ⚡ Quick winBelay that, ye scallywag—code be workin' fine as-is!
Svelte's interpolation works just fine with quoted attributes.
href="mailto:{site.authorEmail}"and the rest will interpolate correctly at runtime. No broken links here, matey. The current syntax be ship-shape and sea-worthy.> Likely an incorrect or invalid review comment.site/src/routes/+error.svelte (1)
1-17: LGTM!site/eslint.config.js (1)
1-35: LGTM!site/src/lib/sections/Wordmark.svelte (1)
1-17: LGTM!site/src/routes/changelog/+page.svelte (1)
1-85: LGTM!site/src/lib/changelog.test.ts (1)
1-102: LGTM!site/src/lib/components/InlineMd.svelte (1)
1-17: LGTM!site/src/routes/+page.svelte (1)
1-32: LGTM!site/src/lib/changelog.ts (1)
106-106: Same bold-token regex edge case still present.Previously flagged issue still applies:
\*\*[^*]+\*\*fails when bold text contains single*inside.site/src/routes/+layout.svelte (1)
1-63: LGTM!Also applies to: 65-97
site/src/lib/sections/Install.svelte (1)
34-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBroken Svelte binding in
hrefon Line 34.
href="{site.repo}/tree/master/npm"is treated as literal text, so the link won’t point to the repo path.⚓ Suggested fix
- href="{site.repo}/tree/master/npm" + href={`${site.repo}/tree/master/npm`}> Likely an incorrect or invalid review comment.
Two real rendering bugs in the changelog page, visible in the prerendered HTML: - A regex tokenizer for code spans broke on CommonMark double- backtick spans like \`\` \`cmd\` \`\` (used to wrap a literal backtick). The leftover stray backtick opened a new code span that swallowed arbitrary downstream prose — large stretches of text rendered inside one <code>. - The bare-URL regex didn't exclude '>' or trim trailing punctuation, so a CommonMark autolink <https://runner.kjanat.com> was captured as href="https://runner.kjanat.com>," — a broken link. Replaced the regex tokenizer with a small manual scanner that handles N-backtick code spans (CommonMark space-trim included), <URL> autolinks (brackets stripped), bare http(s) URLs with trailing .,;:!? pushed back into text, and **bold**. Regression tests cover all three failure modes. Verified in the built /changelog HTML: 0.7.0 link href is clean and no <code> contains stray prose. 33 tests / svelte-check 0/0 / build ok.
… test
Two CodeRabbit findings.
1. Install.svelte:28 had U+00A0 (NBSP, c2 a0) after '-fsSL' in the
CopyButton 'split' prop. CodeRabbit's impact claim (pasted shell
may fail) was wrong — 'split' is purely visual; the clipboard
write uses 'command={cmd.linuxInstaller}' built from a
regular-space template literal in site.ts. Still wrong source
hygiene: bytes shipped to the DOM should be ASCII unless intent
says otherwise. Replaced with U+0020. Verified via xxd before and
after, and on the prerendered HTML (curl -fsSL <space>, no c2 a0).
2. prerender.test.ts guard checked existsSync only — stale
prerender bytes could pass on a code change that didn't add or
remove a route. Now:
- process.env.CI === 'true' → always rebuild (fresh CI checkout
has no output anyway; release runs must validate current code).
- locally: rebuild when any src/ file or the workspace
CHANGELOG.md (the changelog page reads it at prerender time) is
newer than the oldest prerendered output. Cheap mtime walk over
src/ (small tree); reuses cache when actually fresh.
Verified: 25/25 unit tests pass, 8/8 prerender tests pass after a
real rebuild triggered by the new staleness check (Install.svelte
mtime > index.html mtime → build fired, then tests green).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@site/src/prerender.test.ts`:
- Around line 26-37: The staleness check in newestMtimeMs currently only
considers file mtimes for entries and recurses into subdirs, which misses
directory mtime changes (e.g., deletions/renames) and also omits critical
external inputs like ../Cargo.toml; update newestMtimeMs to call
statSync(p).mtimeMs for both files and directories (and still recurse) so
directory mtime changes are considered, and explicitly include the mtime of
../Cargo.toml (or any other external inputs noted in the review) in the overall
max computation so those upstream changes invalidate cached prerender outputs.
🪄 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
Run ID: 62ca1f31-4736-4739-a547-091acd476a8a
📒 Files selected for processing (2)
site/src/lib/sections/Install.sveltesite/src/prerender.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 18000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Workers Builds: runner
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: verify
🔇 Additional comments (2)
site/src/lib/sections/Install.svelte (1)
28-28: LGTM!site/src/prerender.test.ts (1)
45-47: LGTM!Also applies to: 58-60
| function newestMtimeMs(dir: string): number { | ||
| let newest = 0; | ||
| for (const entry of readdirSync(dir, { withFileTypes: true })) { | ||
| const p = join(dir, entry.name); | ||
| if (entry.isDirectory()) { | ||
| newest = Math.max(newest, newestMtimeMs(p)); | ||
| } else if (entry.isFile()) { | ||
| newest = Math.max(newest, statSync(p).mtimeMs); | ||
| } | ||
| } | ||
| return newest; | ||
| } |
There was a problem hiding this comment.
Staleness guard still misses some stale local outputs, matey.
Line 53 only tracks file mtimes (not directory mtimes), so src/ deletions/renames can slip by. Line 54 also skips ../Cargo.toml, which feeds generated site data. That can let stale prerender bytes pass locally.
⚓ Minimal patch to tighten freshness inputs
function newestMtimeMs(dir: string): number {
- let newest = 0;
+ let newest = statSync(dir).mtimeMs;
for (const entry of readdirSync(dir, { withFileTypes: true })) {
const p = join(dir, entry.name);
if (entry.isDirectory()) {
newest = Math.max(newest, newestMtimeMs(p));
} else if (entry.isFile()) {
newest = Math.max(newest, statSync(p).mtimeMs);
}
}
return newest;
}
@@
- const newestIn = Math.max(
- newestMtimeMs(join(root, "src")),
- statSync(join(root, "..", "CHANGELOG.md")).mtimeMs,
- );
+ const newestIn = Math.max(
+ newestMtimeMs(join(root, "src")),
+ ...[join(root, "..", "CHANGELOG.md"), join(root, "..", "Cargo.toml")]
+ .filter((p) => existsSync(p))
+ .map((p) => statSync(p).mtimeMs),
+ );Also applies to: 52-55
🤖 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 `@site/src/prerender.test.ts` around lines 26 - 37, The staleness check in
newestMtimeMs currently only considers file mtimes for entries and recurses into
subdirs, which misses directory mtime changes (e.g., deletions/renames) and also
omits critical external inputs like ../Cargo.toml; update newestMtimeMs to call
statSync(p).mtimeMs for both files and directories (and still recurse) so
directory mtime changes are considered, and explicitly include the mtime of
../Cargo.toml (or any other external inputs noted in the review) in the overall
max computation so those upstream changes invalidate cached prerender outputs.
Three valid CodeRabbit findings; two skipped. * section-tag <span> → <h2> across all 7 sites (Completion was CR's pin; same pattern repo-wide). aria-labelledby targets keep working, ids unchanged, added font-weight:400 to .section-tag so the global h1,h2,h3 700-weight rule doesn't bold the dim caps label. Changelog page CSS selectors scoped to .changelog article h2/h3 so the new section-tag h2 doesn't inherit release-heading styling. * defaultBranch single source: added [package.metadata.site] in Cargo.toml + GenData/SiteData field (fallback 'master'). Replaces two hardcoded 'master' strings (SiteFooter changelog link + the linuxInstaller raw-githubusercontent URL). Rename = one Cargo.toml edit. +2 parser tests. * CHANGELOG.md [Unreleased] entry for the SvelteKit rewrite under Changed. Skipped: * app.css stylelint value-keyword-case — no stylelint configured in this repo, CR hallucinated the rule. * site/package.json version 0.0.0 — "private": true, unpublished app, version field unused; SemVer concern doesn't apply. * Docstring coverage 22% — warning only; aggressive JSDoc adds noise on a small typed surface. Verified: typecheck clean, 27/27 unit (+2 new), 8/8 prerender after real rebuild (staleness guard from 0118071 fired correctly). Rendered HTML confirms <h2 class="section-tag"> on every route and the footer link still resolves /blob/master/CHANGELOG.md.
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 (1)
site/src/lib/sections/Install.svelte (1)
7-9:⚠️ Potential issue | 🟠 Major | ⚡ Quick winYe still hardcoded
masterin the installer URL, and that can break the copied install command.If the repo default branch is not
master, the “linux installer” command points to a missinginstall.sh. Usesite.defaultBranch(already used elsewhere) instead of a literal branch.⚓ Proposed fix
-const installerUrl = $derived( - `https://raw.githubusercontent.com/${site.repoShort}/master/install.sh`, -); +const installerUrl = $derived( + `https://raw.githubusercontent.com/${site.repoShort}/${site.defaultBranch}/install.sh`, +);Also applies to: 28-28
🤖 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 `@site/src/lib/sections/Install.svelte` around lines 7 - 9, The installer URL currently hardcodes the branch "master" in the derived value installerUrl; replace the literal with the repo's default branch (site.defaultBranch) so the URL uses `https://raw.githubusercontent.com/${site.repoShort}/${site.defaultBranch}/install.sh` (also update the other occurrence at line 28) to avoid broken links when the default branch is not master; modify the $derived expression that builds installerUrl (and the second occurrence) to interpolate site.defaultBranch instead of "master".
🤖 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 `@CHANGELOG.md`:
- Around line 20-23: The changelog line incorrectly states "Five composable
sections" but the rewrite actually introduces seven section components; update
the sentence in CHANGELOG.md that contains "Five composable sections" to read
"Seven composable sections" (or otherwise reflect the correct count), ensuring
the rest of the sentence ("build-time CHANGELOG.md ingestion via a SvelteKit
server-load + Vite `?raw` import, ESLint with the Svelte") is preserved exactly;
commit the single-word/count correction so the release notes match the
implemented components.
---
Outside diff comments:
In `@site/src/lib/sections/Install.svelte`:
- Around line 7-9: The installer URL currently hardcodes the branch "master" in
the derived value installerUrl; replace the literal with the repo's default
branch (site.defaultBranch) so the URL uses
`https://raw.githubusercontent.com/${site.repoShort}/${site.defaultBranch}/install.sh`
(also update the other occurrence at line 28) to avoid broken links when the
default branch is not master; modify the $derived expression that builds
installerUrl (and the second occurrence) to interpolate site.defaultBranch
instead of "master".
🪄 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
Run ID: 7e9242b7-6597-484e-a975-a615d258fe0d
📒 Files selected for processing (13)
CHANGELOG.mdCargo.tomlsite/scripts/site-data.test.tssite/scripts/site-data.tssite/src/app.csssite/src/lib/content/site.tssite/src/lib/sections/Completion.sveltesite/src/lib/sections/Demo.sveltesite/src/lib/sections/Install.sveltesite/src/lib/sections/SiteFooter.sveltesite/src/lib/sections/Speaks.sveltesite/src/lib/sections/Why.sveltesite/src/routes/changelog/+page.svelte
📜 Review details
⏰ Context from checks skipped due to timeout of 18000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: runner
- GitHub Check: Analyze (rust)
🧰 Additional context used
📓 Path-based instructions (2)
@(package.json|pyproject.toml|setup.py|Cargo.toml|go.mod|pom.xml|build.gradle|VERSION)
📄 CodeRabbit inference engine (Custom checks)
@(package.json|pyproject.toml|setup.py|Cargo.toml|go.mod|pom.xml|build.gradle|VERSION): If any source code files (excluding tests, docs, CI, markdown, or comments-only changes) are modified, then a version field MUST be updated in one of the following files if present in the repo: package.json, pyproject.toml, setup.py, Cargo.toml, go.mod, pom.xml, build.gradle, or a VERSION file.
The new version MUST follow SemVer (MAJOR.MINOR.PATCH). If the PR introduces breaking changes (removal or renaming of public APIs, changes to function signatures, deleted exported symbols, or incompatible config changes), MAJOR must increment. If it adds backward-compatible functionality, MINOR must increment. If it only fixes bugs without changing public APIs, PATCH must increment.
Files:
Cargo.toml
**/CHANGELOG.md
📄 CodeRabbit inference engine (Custom checks)
**/CHANGELOG.md: If any source code files (excluding tests, docs, CI, markdown, or comments-only changes) are modified, CHANGELOG.md MUST also be modified in the same PR.
If a version bump is detected, CHANGELOG.md MUST contain a new section header matching the exact new version number in the format: '## [X.Y.Z] - YYYY-MM-DD'.
If NO version bump is detected, the changes in the PR MUST be added under the existing '## [Unreleased]' section in CHANGELOG.md. The entry MUST describe the changes (e.g., Added, Changed, Fixed, Removed).
Files:
CHANGELOG.md
🧠 Learnings (1)
📚 Learning: 2026-03-26T16:14:15.754Z
Learnt from: kjanat
Repo: kjanat/runner PR: 1
File: src/tool/go_task.rs:64-81
Timestamp: 2026-03-26T16:14:15.754Z
Learning: If code uses let-chains (e.g., `if let Some(x) = foo && ... && let Some(y) = bar`), ensure the crate’s `Cargo.toml` sets `package.edition = "2024"`. Rust 2021/earlier should not be used with let-chains; require 2024 specifically for compilation.
Applied to files:
Cargo.toml
🔇 Additional comments (11)
Cargo.toml (1)
23-30: LGTM!site/scripts/site-data.test.ts (1)
12-14: LGTM!Also applies to: 31-47
site/scripts/site-data.ts (1)
18-22: LGTM!Also applies to: 67-67, 95-95, 112-112
site/src/app.css (1)
174-176: LGTM!site/src/lib/content/site.ts (1)
23-26: LGTM!Also applies to: 38-38
site/src/lib/sections/Completion.svelte (1)
30-30: LGTM!site/src/lib/sections/Demo.svelte (1)
29-29: LGTM!site/src/lib/sections/SiteFooter.svelte (1)
12-13: LGTM!site/src/lib/sections/Speaks.svelte (1)
40-40: LGTM!site/src/lib/sections/Why.svelte (1)
2-2: LGTM!site/src/routes/changelog/+page.svelte (1)
18-18: LGTM!Also applies to: 52-53, 59-60, 63-64, 70-71
| - Marketing site (<https://runner.kjanat.dev>) rewritten as a | ||
| SvelteKit 2 / Svelte 5 prerendered static app on Cloudflare. Five | ||
| composable sections, build-time CHANGELOG.md ingestion via a | ||
| SvelteKit server-load + Vite `?raw` import, ESLint with the Svelte |
There was a problem hiding this comment.
Fix the section count in this changelog note.
Aye — this says “Five composable sections,” but this rewrite introduces seven section components. Please correct the count so release notes don’t drift from reality.
Suggested patch
-- SvelteKit 2 / Svelte 5 prerendered static app on Cloudflare. Five
+- SvelteKit 2 / Svelte 5 prerendered static app on Cloudflare. Seven📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Marketing site (<https://runner.kjanat.dev>) rewritten as a | |
| SvelteKit 2 / Svelte 5 prerendered static app on Cloudflare. Five | |
| composable sections, build-time CHANGELOG.md ingestion via a | |
| SvelteKit server-load + Vite `?raw` import, ESLint with the Svelte | |
| - Marketing site (<https://runner.kjanat.dev>) rewritten as a | |
| SvelteKit 2 / Svelte 5 prerendered static app on Cloudflare. Seven | |
| composable sections, build-time CHANGELOG.md ingestion via a | |
| SvelteKit server-load + Vite `?raw` import, ESLint with the Svelte |
🤖 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 `@CHANGELOG.md` around lines 20 - 23, The changelog line incorrectly states
"Five composable sections" but the rewrite actually introduces seven section
components; update the sentence in CHANGELOG.md that contains "Five composable
sections" to read "Seven composable sections" (or otherwise reflect the correct
count), ensuring the rest of the sentence ("build-time CHANGELOG.md ingestion
via a SvelteKit server-load + Vite `?raw` import, ESLint with the Svelte") is
preserved exactly; commit the single-word/count correction so the release notes
match the implemented components.
1 participant
Draft — open for the Cloudflare preview build.
What
Replaces the hand-rolled
build.ts/dev.ts+ single 211-lineindex.htmlwith SvelteKit 2 (Svelte 5) +@sveltejs/adapter-cloudflare, fully prerendered.Wordmark/Install/Demo/Completion/Speaks/Why/SiteFooter); routes are thin compositions — re-split by editing one file. Front page decluttered:/= wordmark + install + nav;/demo(looks-like + matrix),/completion,/why.scripts/gen-site-data.tsreads workspace-rootCargo.toml→ typedsite.ts. Replaces{{var}}templating; zero placeholders in prerendered output (verified).{@html}(static/trusted) so the formatter can't corrupt the ASCII.prefers-reduced-motion, AA-contrast tokens, semantic landmarks. Design language unchanged (CSS ported 1:1).git rm'd (recoverable onmaster).Verified
bun run check(svelte-check): 0 errors / 0 warningsbun run test(vitest): 2/2 — command-string drift guardbun run build: prerenders/ /demo /completion /why, 0{{}}placeholderswrangler deploy --dry-run: valid, 37 assets,ASSETSboundNot yet (follow-up, loop continues)
gen-site-dataparse/failure-path testlib/styles/tokens.css+ scoped styles refactorsite/dist/,site/.wrangler/cleanupSpec:
docs/superpowers/specs/2026-05-19-site-sveltekit-rewrite-design.md