Skip to content

feat: add static site generation mode#147

Open
rohilsurana wants to merge 5 commits into
mainfrom
feat/static-site-mode
Open

feat: add static site generation mode#147
rohilsurana wants to merge 5 commits into
mainfrom
feat/static-site-mode

Conversation

@rohilsurana
Copy link
Copy Markdown
Member

@rohilsurana rohilsurana commented Jun 3, 2026

Summary

  • Adds --preset static to chronicle build that produces a fully self-contained SPA deployable to any static host (GitHub Pages, S3, Netlify, Cloudflare Pages) with no server, no SSR, no database
  • Client-side search using MiniSearch over pre-built JSON index replaces the server-side SQLite FTS5 search
  • API playground makes direct browser requests to the API server in static mode (no CORS proxy needed) with user-friendly CORS error handling
  • Pre-generates page metadata JSON, search index, API specs, sitemap.xml, robots.txt, llms.txt, OG images (PNG via Satori+Sharp), optimized images (WebP), and markdown files for all pages and API operations
  • Reads meta.json files for folder titles and ordering in the sidebar tree
  • Handles content root redirects (index_page config) and env var substitution (${API_SERVER_URL}) gracefully

Performance optimizations

  • <picture> tags with webp <source> and original fallback for static builds — browsers get optimized format automatically
  • Mermaid component lazy-loaded via React.lazy — 2MB of diagram chunks only downloaded when mermaid code blocks are rendered (applies to both SSR and static)

Usage

# Basic static build
chronicle build --preset static

# With API server URL baked in
API_SERVER_URL=https://api.example.com chronicle build --preset static

# Serve the output
npx serve .output/public --single

Output structure

.output/public/
├── index.html              (SPA shell)
├── assets/                 (Vite JS/CSS bundle)
├── data/
│   ├── pages/*.json        (page metadata, replaces /api/page)
│   ├── search.json         (search index, replaces /api/search)
│   └── specs/*.json        (API specs, replaces /api/specs)
├── _content/               (optimized images — webp + original fallback)
├── og/*.png                (OG images)
├── *.md                    (markdown files for docs and API operations)
├── sitemap.xml
├── robots.txt
└── llms.txt

Files changed

File Change
static-generate.ts New — core static generation (page data, search index, specs, sitemap, robots, llms, OG images, image optimization, markdown files, SPA shell)
entry-static.tsx New — client entry using createRoot (no hydration), fetches from static JSON
build.ts Static preset uses vite.build() then runs static generator
vite-config.ts Switches client entry, skips DB config, disables Nitro prerender for static
page-context.tsx Routes data fetches through /data/ paths in static mode, content root redirect
preload.ts No-ops search prefetch in static mode, uses static page data paths
search.tsx Adds MiniSearch client-side search with lazy-loaded index
playground-dialog.tsx Direct fetch to API server in static mode with CORS error handling
openapi.ts Exports resolveDocument for static generator's $ref resolution
image.tsx <picture> tag with webp source for static mode
code.tsx Lazy-load Mermaid component via React.lazy
index.tsx Lazy-load Mermaid in MDX component map

Test plan

  • 14 Playwright e2e tests for static site (page rendering, navigation, search, data files, SEO files, 404 handling)
  • Full broken-assets scan across 180 pages — 0 broken images
  • 171 unit tests pass (no regressions)
  • SSR build still works unchanged
  • Verified on real documentation site (180 pages, 414 images, API specs with $ref resolution)
  • Visual comparison of static vs SSR — pixel-perfect match for docs and developer pages

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 3, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
chronicle Ready Ready Preview, Comment Jun 3, 2026 8:04pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a --preset static build mode with a static-site generator, static runtime entry that boots from embedded page data, client-side static-mode branches (search, API fetching, image handling), CLI/Vite wiring, and Playwright E2E tests for static sites.

Changes

Static Site Mode Implementation

Layer / File(s) Summary
Build infrastructure and CLI integration
packages/chronicle/package.json, packages/chronicle/playwright.config.ts, packages/chronicle/src/cli/commands/build.ts, packages/chronicle/src/server/vite-config.ts
Adds dev dependencies (@playwright/test, minisearch), Playwright config, CLI support for --preset static, and Vite config that exports isStaticPreset() and conditionally wires static entry, manifest, and Nitro options.
Static site generation pipeline
packages/chronicle/src/cli/commands/static-generate.ts
Implements generateStaticSite() to scan .content, parse frontmatter/headings/images, build an ordered page tree with prev/next, load API specs, emit per-page JSON and data/search.json (including API ops), produce spec bundles, sitemap/robots/llms, OG images, optimize/copy assets, and generate SPA index.html with embedded window.__PAGE_DATA__.
Static runtime bootloader and data fetching
packages/chronicle/src/server/entry-static.tsx, packages/chronicle/src/lib/openapi.ts
Adds entry-static.tsx that reads embedded page data, resolves routes and client redirects (including index_page), conditionally fetches static JSON page/spec files, dynamically loads MDX modules, and renders the app; exports resolveDocument for spec resolution.
Static mode detection and conditional data fetching
packages/chronicle/src/lib/page-context.tsx, packages/chronicle/src/lib/preload.ts
Runtime isStaticMode() detection branches fetchApiSpecs/fetchPageData to /data/specs/ and /data/pages/ JSON files; PageProvider may redirect single-segment doc routes to configured index pages; preload skips search suggestion prefetch in static mode.
Client-side static mode routing
packages/chronicle/src/components/ui/search.tsx, packages/chronicle/src/components/api/playground-dialog.tsx
Search uses minisearch with cached /data/search.json and heading anchors in static mode; API playground adds sendDirect() and sendViaProxy() with handleSend() choosing behavior by isStaticMode().
MDX lazy Mermaid & static images
packages/chronicle/src/components/mdx/*
Mermaid renderer is lazy-loaded with Suspense; MDX image component serves a .webp <source> inside a <picture> for local non-SVG images in static mode.
End-to-end test coverage
packages/chronicle/tests/e2e/static-site.e2e.ts, packages/chronicle/tests/e2e/check-broken-assets.e2e.ts
Playwright tests validate static-site rendering, navigation, search, static JSON endpoints, sitemap/robots/llms, browser globals, public assets, 404 behavior, and detect broken images/failed requests producing a report.

Sequence Diagram(s)

sequenceDiagram
  participant Index as index.html
  participant Window as window.__PAGE_DATA__
  participant Entry as entry-static.tsx
  participant Pages as /data/pages/*.json
  participant Specs as /data/specs/*.json
  participant MDX as MDX modules
  participant App as React App

  Index->>Window: read embedded __PAGE_DATA__
  Entry->>Entry: resolve current route
  Entry->>Pages: fetch page JSON (docs)
  Pages-->>Entry: return page data
  Entry->>Specs: fetch spec JSON (api)
  Specs-->>Entry: return specs
  Entry->>MDX: load MDX module for page
  MDX-->>Entry: rendered content
  Entry->>App: render React app with initial props
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • raystack/chronicle#42: Modifies Mermaid rendering in the MDX pipeline; related to lazy Mermaid changes.
  • raystack/chronicle#92: Changes search and preload behavior; intersects with static-mode search and preload branching.
  • raystack/chronicle#24: Earlier Vite/Nitro/CLI build flow changes that this PR extends for the static preset.

Suggested reviewers

  • rohanchkrabrty
  • paanSinghCoder
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 3.08% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: add static site generation mode' accurately and concisely summarizes the main feature added in this changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description is directly related to the changeset, detailing the static site generation feature being implemented across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/static-site-mode

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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/chronicle/src/components/api/playground-dialog.tsx (1)

220-267: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add missing useCallback dependencies to avoid stale request payloads.

handleSend reads jsonMode and bodyJsonStr, but they are missing from the dependency list, so requests can be sent with outdated body mode/content.

Proposed fix
-  }, [specName, method, path, serverUrl, pathValues, queryValues, getAuthHeaders, headerValues, bodyValues, body])
+  }, [specName, method, path, serverUrl, pathValues, queryValues, getAuthHeaders, headerValues, bodyValues, body, jsonMode, bodyJsonStr])
🤖 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 `@packages/chronicle/src/components/api/playground-dialog.tsx` around lines 220
- 267, The handleSend useCallback captures jsonMode and bodyJsonStr but they are
not listed in the dependency array, causing stale request bodies; update the
dependency array on the useCallback that defines handleSend (the function named
handleSend) to include jsonMode and bodyJsonStr so the callback is recreated
when either the JSON mode or the JSON string changes.
🧹 Nitpick comments (2)
packages/chronicle/tests/e2e/compare-static-ssr.e2e.ts (1)

26-30: ⚡ Quick win

Ensure contexts always close with try/finally

If navigation/evaluate/screenshot fails, ctxSSR.close() and ctxStatic.close() may be skipped, which can leak resources across tests.

Suggested refactor
   test(`compare: ${pg.name}`, async ({ browser }) => {
     const ctxSSR = await browser.newContext();
     const ctxStatic = await browser.newContext();
-    const pageSSR = await ctxSSR.newPage();
-    const pageStatic = await ctxStatic.newPage();
+    try {
+      const pageSSR = await ctxSSR.newPage();
+      const pageStatic = await ctxStatic.newPage();
@@
-    await ctxSSR.close();
-    await ctxStatic.close();
+    } finally {
+      await ctxSSR.close();
+      await ctxStatic.close();
+    }
@@
   });

Also applies to: 114-115

🤖 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 `@packages/chronicle/tests/e2e/compare-static-ssr.e2e.ts` around lines 26 - 30,
The test opens Playwright contexts/pages (ctxSSR, ctxStatic, pageSSR,
pageStatic) without guaranteeing cleanup; wrap the
navigation/evaluate/screenshot logic in try/finally blocks so both contexts are
always closed (and pages closed) even on failure — e.g., create ctxSSR/ctxStatic
and pages, then use try { ... } finally { await pageSSR?.close(); await
pageStatic?.close(); await ctxSSR?.close(); await ctxStatic?.close(); } to
ensure no resource leak; apply same pattern to the other occurrence around the
lines referencing ctxSSR/ctxStatic (and pages) at 114-115.
packages/chronicle/src/server/entry-static.tsx (1)

16-16: ⚡ Quick win

Use the configured @/* alias for internal imports.

Line 16 should use the project alias instead of a relative path to stay consistent with repo conventions.

Proposed change
-import { App } from './App';
+import { App } from '`@/server/App`';

As per coding guidelines **/*.{ts,tsx}: Use path alias @/*./src/* configured in tsconfig and vite.

🤖 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 `@packages/chronicle/src/server/entry-static.tsx` at line 16, Import in
entry-static.tsx is using a relative path; change the import of App to use the
project path alias instead (replace "import { App } from './App';" with the
alias form e.g. "import { App } from '`@/server/App`' or '`@/App`' as appropriate").
Update the import statement referencing the App symbol so it resolves via the
configured `@/`* alias consistent with tsconfig/vite settings and repo
conventions.
🤖 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 `@packages/chronicle/src/cli/commands/build.ts`:
- Around line 42-43: The code currently hardcodes outputDir =
path.resolve(projectRoot, '.output/public'), which breaks non-default static
presets; update the build command to derive the output path from the active
preset/config instead of a fixed string: locate where the preset or
static-capable preset is selected (references to outputDir and projectRoot in
build.ts) and read the preset's configured output root (e.g., a property or
method on the preset like outputRoot, outputDir, getOutputDir(projectRoot) or
similar config in the preset object) and resolve that against projectRoot; fall
back to the existing '.output/public' only if the preset provides no output
path. Ensure the variable name stays outputDir so downstream code remains
unchanged.

In `@packages/chronicle/src/cli/commands/static-generate.ts`:
- Around line 544-546: The code uses page.frontmatter.lastModified directly with
toISOString(), which throws for invalid dates; fix by creating a Date from
page.frontmatter.lastModified (e.g., const d = new
Date(page.frontmatter.lastModified)), check its validity via
isFinite(d.getTime()) or !isNaN(d.getTime()), and only produce the lastmod
string when the date is valid; otherwise leave lastmod as an empty string —
update the lastmod construction in static-generate.ts to use this validated Date
logic.
- Around line 619-623: The remote font fetch call (the fetch that assigns to
response and fontData in static-generate.ts) has no timeout and can hang builds;
wrap the fetch in an AbortController with a short timeout (e.g., 5–10s) so the
request is aborted if it exceeds the limit, clear the timer on success, and
handle the abort error in the existing catch to fall back gracefully (e.g., use
a local font or skip). Specifically, modify the block where response = await
fetch(...) and fontData = await response.arrayBuffer() to create an
AbortController, pass controller.signal to fetch, set a setTimeout that calls
controller.abort(), and ensure the timeout is cleared after response is
received.
- Around line 804-860: The code currently allows an empty entryJs when
readViteManifest() returns falsy or no entry with isEntry is found, producing a
broken index.html; after the manifest-parsing loop in static-generate.ts check
for manifest existence and that entryJs is non-empty and if not either throw a
descriptive Error (e.g. "Vite manifest missing or no entry chunk found") or call
process.exit(1) after logging the manifest/manifest keys; update the logic
around readViteManifest, manifest and entryJs to fail fast and surface the
underlying manifest contents to aid debugging.
- Around line 696-716: The code resolves image sources using relativePath from
markdown and directly calls path.resolve(contentDir, relativePath), which allows
path traversal; update the image handling in the loop that uses variables
imgUrl, relativePath, srcPath (and the isSvg branch) to validate the resolved
path: after computing srcPath = path.resolve(contentDir, relativePath) compute a
normalized relative = path.relative(contentDir, srcPath) and skip (or log and
continue) if that relative startsWith('..') or path.isAbsolute(relative)
indicates srcPath is outside contentDir; apply the same guard before copying
SVGs and before converting rasters to webp so any ../ escapes are rejected.

In `@packages/chronicle/src/components/ui/search.tsx`:
- Around line 119-134: The current filters (in the docs branch and the results
branch around ms.search) use startsWith which wrongly matches tags like "v1" to
"/v10/..."; change the check to ensure the tag matches a full path segment by
using a boundary-aware test (e.g., replace the startsWith logic with a regex
like new RegExp(`^/${tag}(?:/|$)`) or compare the first path segment via
url.split('/')[1] === tag); update the filtering logic in both places (the docs
filter that uses docs.filter and the results.filter after ms.search) to use this
boundary-aware check (reference variables/functions: docs, results, ms.search,
tag, query).

In `@packages/chronicle/tests/e2e/check-broken-assets.e2e.ts`:
- Around line 30-31: Remove the swallow of navigation errors so broken assets
fail the test: stop catching errors from page.goto (reference: page.goto) and
let the navigation throw on failure or explicitly rethrow the caught error; then
after the asset-check/report step (the code that writes the report referenced
around lines 48-55), add an assertion that the reported failures count is zero
(e.g., assert failuresCount === 0 or expect(failuresCount).toBe(0)) so the test
fails when any broken assets are found. Ensure page.waitForTimeout remains only
for pacing, not masking navigation errors.
- Around line 6-8: Replace the machine-specific absolute pagesDir and the
import-time fs.readdirSync call: stop hardcoding pagesDir and defer reading
files until test runtime. Construct pagesDir using a project-relative resolution
(e.g., path.resolve(process.cwd(), '…/public/data/pages') or __dirname-based
path) and move the readdirSync logic out of module top-level into the test setup
or a helper function so pageFiles is computed at runtime (refer to pagesDir and
pageFiles to locate and change the code).

In `@packages/chronicle/tests/e2e/compare-static-ssr.e2e.ts`:
- Around line 117-119: The test currently only checks ssrSnap and staticSnap are
not null; update it to actually compare their contents by replacing the two
non-null assertions with a structural/content assertion: e.g., normalize both
snapshots (trim, collapse whitespace, or parse into DOM via cheerio) and assert
equality with expect(ssrSnapNormalized).toEqual(staticSnapNormalized) or, if you
prefer tolerance, compute a diff and assert it falls within acceptable bounds.
Locate the variables ssrSnap and staticSnap in the compare-static-ssr.e2e.ts
test and perform normalization/parsing before the final expect so the test fails
on real content drift.

In `@packages/chronicle/tests/e2e/static-site.e2e.ts`:
- Around line 34-40: Replace the conditional visibility guards with explicit
assertions so tests fail when elements are missing: instead of wrapping the
navigation/assertions in if (await installationLink.isVisible()) { ... }, call
an explicit visibility assertion (e.g., await
expect(installationLink).toBeVisible()) before clicking, then perform
installationLink.click(), await page.waitForTimeout(...), assert URL with
expect(page.url()).toContain('/docs/guides/installation') and check
page.textContent('body') contains 'Installation'; apply the same change to the
second block (the block around lines 52-57) so its link is asserted visible
before interacting.
- Around line 43-49: The test "search dialog opens with Ctrl+K" currently calls
page.keyboard.press('Meta+k') which is macOS-specific; change it to send a
platform-aware shortcut by detecting the OS (e.g., process.platform === 'darwin'
? 'Meta+k' : 'Control+k') before calling page.keyboard.press so the test uses
Meta+k on mac and Control+k on other systems; update the call site where
page.keyboard.press is invoked in this test to use the computed modifier string.

---

Outside diff comments:
In `@packages/chronicle/src/components/api/playground-dialog.tsx`:
- Around line 220-267: The handleSend useCallback captures jsonMode and
bodyJsonStr but they are not listed in the dependency array, causing stale
request bodies; update the dependency array on the useCallback that defines
handleSend (the function named handleSend) to include jsonMode and bodyJsonStr
so the callback is recreated when either the JSON mode or the JSON string
changes.

---

Nitpick comments:
In `@packages/chronicle/src/server/entry-static.tsx`:
- Line 16: Import in entry-static.tsx is using a relative path; change the
import of App to use the project path alias instead (replace "import { App }
from './App';" with the alias form e.g. "import { App } from '`@/server/App`' or
'`@/App`' as appropriate"). Update the import statement referencing the App symbol
so it resolves via the configured `@/`* alias consistent with tsconfig/vite
settings and repo conventions.

In `@packages/chronicle/tests/e2e/compare-static-ssr.e2e.ts`:
- Around line 26-30: The test opens Playwright contexts/pages (ctxSSR,
ctxStatic, pageSSR, pageStatic) without guaranteeing cleanup; wrap the
navigation/evaluate/screenshot logic in try/finally blocks so both contexts are
always closed (and pages closed) even on failure — e.g., create ctxSSR/ctxStatic
and pages, then use try { ... } finally { await pageSSR?.close(); await
pageStatic?.close(); await ctxSSR?.close(); await ctxStatic?.close(); } to
ensure no resource leak; apply same pattern to the other occurrence around the
lines referencing ctxSSR/ctxStatic (and pages) at 114-115.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9cd0e4ce-24d6-4fe9-a541-b831bd2e48e1

📥 Commits

Reviewing files that changed from the base of the PR and between 86cbba0 and fe0aee4.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (15)
  • docs/superpowers/specs/2026-06-03-static-site-mode-design.md
  • packages/chronicle/package.json
  • packages/chronicle/playwright.config.ts
  • packages/chronicle/src/cli/commands/build.ts
  • packages/chronicle/src/cli/commands/static-generate.ts
  • packages/chronicle/src/components/api/playground-dialog.tsx
  • packages/chronicle/src/components/ui/search.tsx
  • packages/chronicle/src/lib/openapi.ts
  • packages/chronicle/src/lib/page-context.tsx
  • packages/chronicle/src/lib/preload.ts
  • packages/chronicle/src/server/entry-static.tsx
  • packages/chronicle/src/server/vite-config.ts
  • packages/chronicle/tests/e2e/check-broken-assets.e2e.ts
  • packages/chronicle/tests/e2e/compare-static-ssr.e2e.ts
  • packages/chronicle/tests/e2e/static-site.e2e.ts

Comment thread packages/chronicle/src/cli/commands/build.ts
Comment thread packages/chronicle/src/cli/commands/static-generate.ts Outdated
Comment thread packages/chronicle/src/cli/commands/static-generate.ts
Comment thread packages/chronicle/src/cli/commands/static-generate.ts
Comment thread packages/chronicle/src/cli/commands/static-generate.ts
Comment thread packages/chronicle/tests/e2e/check-broken-assets.e2e.ts Outdated
Comment thread packages/chronicle/tests/e2e/check-broken-assets.e2e.ts
Comment thread packages/chronicle/tests/e2e/compare-static-ssr.e2e.ts Outdated
Comment thread packages/chronicle/tests/e2e/static-site.e2e.ts Outdated
Comment thread packages/chronicle/tests/e2e/static-site.e2e.ts Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/chronicle/tests/e2e/check-broken-assets.e2e.ts (1)

37-37: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Navigation errors are silently swallowed.

The .catch(() => {}) suppresses all navigation failures, so if a page returns 404 or times out, the test proceeds as if the page loaded successfully. This may hide legitimate static site generation issues.

If intentional (to audit all pages even when some fail), consider at minimum logging the error:

-    await page.goto(`${STATIC_URL}${pageUrl}`, { waitUntil: 'networkidle', timeout: 15000 }).catch(() => {});
+    await page.goto(`${STATIC_URL}${pageUrl}`, { waitUntil: 'networkidle', timeout: 15000 }).catch(e => {
+      failedRequests.push({ pageUrl, resource: `NAVIGATION_FAILED: ${e.message}` });
+    });
🤖 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 `@packages/chronicle/tests/e2e/check-broken-assets.e2e.ts` at line 37, The
navigation call in the test uses page.goto(`${STATIC_URL}${pageUrl}`, {
waitUntil: 'networkidle', timeout: 15000 }).catch(() => {}), which silently
swallows navigation failures; change this to surface errors instead — either
remove the .catch so failures fail the test, or replace it with a handler that
logs the thrown error (including pageUrl and error.message) and rethrows or
records a test assertion failure; update the call site in the
check-broken-assets.e2e.ts test (the page.goto invocation) so navigation errors
are not ignored.
♻️ Duplicate comments (1)
packages/chronicle/tests/e2e/check-broken-assets.e2e.ts (1)

59-59: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

No assertion on failedRequests — network failures pass silently.

The test collects failedRequests and writes them to the report, but never asserts on them. A page with broken JS/CSS assets will pass as long as images render. Consider adding an assertion:

   expect(broken, `Broken images found on ${broken.length} pages`).toHaveLength(0);
+  expect(failedRequests, `Failed requests on ${failedRequests.length} resources`).toHaveLength(0);
🤖 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 `@packages/chronicle/tests/e2e/check-broken-assets.e2e.ts` at line 59, The test
currently only asserts on the `broken` array and ignores `failedRequests`, so
network failures can pass silently; add an assertion that `failedRequests` is
empty (e.g., `expect(failedRequests, \`Network/asset request failures found on
${failedRequests.length} pages\`).toHaveLength(0)`) right after the existing
`expect(broken...)` line so any failed JS/CSS/image/network requests fail the
test and include a clear message with `failedRequests.length`.
🤖 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.

Outside diff comments:
In `@packages/chronicle/tests/e2e/check-broken-assets.e2e.ts`:
- Line 37: The navigation call in the test uses
page.goto(`${STATIC_URL}${pageUrl}`, { waitUntil: 'networkidle', timeout: 15000
}).catch(() => {}), which silently swallows navigation failures; change this to
surface errors instead — either remove the .catch so failures fail the test, or
replace it with a handler that logs the thrown error (including pageUrl and
error.message) and rethrows or records a test assertion failure; update the call
site in the check-broken-assets.e2e.ts test (the page.goto invocation) so
navigation errors are not ignored.

---

Duplicate comments:
In `@packages/chronicle/tests/e2e/check-broken-assets.e2e.ts`:
- Line 59: The test currently only asserts on the `broken` array and ignores
`failedRequests`, so network failures can pass silently; add an assertion that
`failedRequests` is empty (e.g., `expect(failedRequests, \`Network/asset request
failures found on ${failedRequests.length} pages\`).toHaveLength(0)`) right
after the existing `expect(broken...)` line so any failed JS/CSS/image/network
requests fail the test and include a clear message with `failedRequests.length`.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3062a2f5-7326-4b29-be9f-690070a74fdb

📥 Commits

Reviewing files that changed from the base of the PR and between fe0aee4 and be08bd6.

📒 Files selected for processing (4)
  • packages/chronicle/src/cli/commands/static-generate.ts
  • packages/chronicle/src/components/ui/search.tsx
  • packages/chronicle/tests/e2e/check-broken-assets.e2e.ts
  • packages/chronicle/tests/e2e/static-site.e2e.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/chronicle/src/components/ui/search.tsx
  • packages/chronicle/tests/e2e/static-site.e2e.ts

@rohilsurana rohilsurana requested a review from rsbh June 3, 2026 17:37
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
packages/chronicle/src/components/mdx/code.tsx (1)

6-6: ⚡ Quick win

Use the configured @/ alias in the lazy import.

Please switch this new TSX import from ./mermaid to the configured @/components/mdx/mermaid alias so module resolution stays consistent with the repo convention.

As per coding guidelines, **/*.{ts,tsx}: Use path alias @/*./src/* configured in tsconfig and vite.

🤖 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 `@packages/chronicle/src/components/mdx/code.tsx` at line 6, The lazy import
for the Mermaid component uses a relative path; update the import in the const
Mermaid = lazy(...) expression to use the repo alias instead: import from
"`@/components/mdx/mermaid`" and keep the .then(m => ({ default: m.Mermaid }))
wrapper so the lazy-loaded module still resolves the named export correctly.
packages/chronicle/src/components/mdx/index.tsx (1)

12-12: ⚡ Quick win

Use the configured @/ alias for this lazy import too.

This new dynamic import should use @/components/mdx/mermaid instead of ./mermaid to match the repo’s TSX import convention.

As per coding guidelines, **/*.{ts,tsx}: Use path alias @/*./src/* configured in tsconfig and vite.

🤖 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 `@packages/chronicle/src/components/mdx/index.tsx` at line 12, The lazy import
uses a relative path; update the dynamic import in the LazyMermaid declaration
to use the repo alias instead (replace './mermaid' with
'`@/components/mdx/mermaid`') so it follows the TSX path-alias convention; keep
the rest of the lazy(() => import(...).then(m => ({ default: m.Mermaid })))
expression unchanged and ensure the symbol LazyMermaid remains the same.
🤖 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.

Nitpick comments:
In `@packages/chronicle/src/components/mdx/code.tsx`:
- Line 6: The lazy import for the Mermaid component uses a relative path; update
the import in the const Mermaid = lazy(...) expression to use the repo alias
instead: import from "`@/components/mdx/mermaid`" and keep the .then(m => ({
default: m.Mermaid })) wrapper so the lazy-loaded module still resolves the
named export correctly.

In `@packages/chronicle/src/components/mdx/index.tsx`:
- Line 12: The lazy import uses a relative path; update the dynamic import in
the LazyMermaid declaration to use the repo alias instead (replace './mermaid'
with '`@/components/mdx/mermaid`') so it follows the TSX path-alias convention;
keep the rest of the lazy(() => import(...).then(m => ({ default: m.Mermaid })))
expression unchanged and ensure the symbol LazyMermaid remains the same.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7ed8872a-f3b6-4f58-b4e5-e72dd13873cb

📥 Commits

Reviewing files that changed from the base of the PR and between be08bd6 and 075cd86.

📒 Files selected for processing (3)
  • packages/chronicle/src/components/mdx/code.tsx
  • packages/chronicle/src/components/mdx/image.tsx
  • packages/chronicle/src/components/mdx/index.tsx

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