Conversation
📝 WalkthroughWalkthroughAdds breadcrumb UI and integrates it into the Page theme, introduces a client Search component + styles and a new advanced search API route, expands ThemePageProps to include Changes
Sequence DiagramsequenceDiagram
participant Client as Client (Search UI)
participant API as /api/search (Route)
participant Source as Content Source
participant Page as Page module (structuredData loader)
Client->>API: GET /api/search?q=...
API->>Source: source.getPages()
Source-->>API: pages[]
API->>Page: for page with load() -> invoke load()
Page-->>API: structuredData (or error)
API-->>Client: search indexes (id,url,title,description,structuredData)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/chronicle/src/components/ui/breadcrumbs.tsx`:
- Around line 11-23: findInTree currently compares only the last URL segment
(item.url?.split('/').pop()) which causes incorrect matches for pages with the
same filename in different folders; modify findInTree to accept the currentPath
(or an accumulatedPath) and match full URLs (e.g., compare item.url to the
corresponding prefix/suffix of currentPath) instead of just the final segment,
update the function signature (findInTree(items: PageTreeItem[], segment:
string, currentPath: string) or findInTree(items, accumulatedPath)) and adjust
all call sites to pass currentPath so the lookup uses the full path context and
returns the correct PageTreeItem.
🧹 Nitpick comments (1)
packages/chronicle/src/components/ui/breadcrumbs.tsx (1)
28-28: Consider making the base path configurable.The
/docsbase path is hardcoded. If the documentation root ever changes or needs to be configurable per deployment, this would require code changes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/chronicle/src/app/api/search/route.ts`:
- Around line 34-37: The block that calls data.load() inside the Promise.all can
throw and will reject the whole batch; wrap the await data.load() call in a
try-catch so failures for a single page are handled without breaking the entire
Promise.all. Specifically, inside the async mapper where you access
structuredData and call data.load(), catch errors from await data.load() (or
from loaded.structuredData) and either skip that page, set structuredData to
null/empty, and log the error with context (e.g., page id or data source) so the
search route remains available while individual load failures are isolated.
🧹 Nitpick comments (2)
packages/chronicle/src/components/ui/search.tsx (2)
46-48: Keyboard shortcut hint only shows macOS modifier.The trigger button displays
⌘ Kwhich is Mac-specific, but the actual shortcut handler (line 32) supports bothmetaKey(Mac) andctrlKey(Windows/Linux). Consider detecting the platform to show the appropriate hint.💡 Suggested approach for cross-platform hint
+const isMac = typeof navigator !== 'undefined' && navigator.platform.toUpperCase().indexOf('MAC') >= 0; +const shortcutHint = isMac ? '⌘ K' : 'Ctrl K'; + return ( <> - <Button variant="outline" color="neutral" onClick={() => setOpen(true)} className={styles.trigger} trailingIcon={<kbd className={styles.kbd}>⌘ K</kbd>}> + <Button variant="outline" color="neutral" onClick={() => setOpen(true)} className={styles.trigger} trailingIcon={<kbd className={styles.kbd}>{shortcutHint}</kbd>}> <Text>Search...</Text> </Button>Note: Platform detection should be done in an effect or memoized to avoid SSR mismatches.
117-125: Consider edge case handling ingetPageTitle.The function works for typical URLs but returns an empty string for root URLs (e.g.,
/). If root pages are indexed, this could result in empty titles being displayed.💡 Suggested improvement
function getPageTitle(url: string): string { const path = url.split("#")[0]; const segments = path.split("/").filter(Boolean); const lastSegment = segments[segments.length - 1] || ""; + if (!lastSegment) return "Home"; return lastSegment .split("-") .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) .join(" "); }
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/chronicle/src/app/api/search/route.ts`:
- Around line 4-24: Replace the locally redefined types StructuredDataHeading,
StructuredDataContent, StructuredData (and any local variants used by
PageData.load) with the canonical types exported by fumadocs-core: import the
types (e.g. StructuredHeading, StructuredContent, StructuredData) from
'fumadocs-core/search/server' and use those names instead of the local
interfaces; ensure StructuredContent.heading uses the library's string | null
type and make StructuredData.headings and StructuredData.contents required
arrays as per fumadocs-core so PageData.structuredData and the load() return
type align with the upstream contract.
🧹 Nitpick comments (1)
packages/chronicle/src/app/api/search/route.ts (1)
31-31: Type assertion should have runtime validation or documented schema.The cast
page.data as PageDataassumes the data from fumadocs-mdx frontmatter matches the expected shape. While the code has fallbacks (??operators), there's no validation that required properties exist. Either add explicit property checks before use or document the required frontmatter schema that all MDX files must follow. Note: fumadocs-core doesn't export a typedPageDatainterface; this is intentionally user-defined based on your frontmatter structure.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/chronicle/package.json`:
- Line 31: Remove unused lodash and `@types/lodash` entries from the chronicle
package.json by deleting the "lodash" dependency (currently listed in
dependencies) and the "@types/lodash" entry (currently in devDependencies);
after removing them, run the package manager's install (npm/yarn/pnpm) to update
the lockfile and ensure no import/use of lodash remains in the chronicle
codebase (search for "lodash" or "_" to confirm).
🧹 Nitpick comments (1)
packages/chronicle/src/components/ui/search.tsx (1)
47-49: Consider splitting the long button line for readability.The trigger button JSX is quite long on a single line, which can be harder to read and maintain.
♻️ Suggested formatting
- <Button variant="outline" color="neutral" onClick={() => setOpen(true)} className={styles.trigger} trailingIcon={<kbd className={styles.kbd}>{isMacOs ? "⌘" : "Ctrl"} K</kbd>}> - <Text>Search...</Text> - </Button> + <Button + variant="outline" + color="neutral" + onClick={() => setOpen(true)} + className={styles.trigger} + trailingIcon={ + <kbd className={styles.kbd}>{isMacOs ? "⌘" : "Ctrl"} K</kbd> + } + > + <Text>Search...</Text> + </Button>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/chronicle/src/lib/config.ts`:
- Around line 14-16: getConfigPath is inconsistent with loadConfig's directory
resolution: extract the directory-resolution logic into a single helper (or have
getConfigPath call loadConfig's resolver) that uses contentDir ??
process.env.CHRONICLE_CONTENT_DIR ?? './content' and then join with CONFIG_FILE;
update both getConfigPath and any other path helpers (the code around the
loadConfig function and the similar logic at lines ~37-38) to reuse that
resolver so they always point to the same config file location (reference
symbols: loadConfig, getConfigPath, CONFIG_FILE).
🧹 Nitpick comments (1)
packages/chronicle/src/themes/default/Layout.tsx (1)
12-13: Remove debug logging from the client layout.Line 13 logs the full config on every render, which is noisy and can expose config details in the browser console.
🧹 Suggested fix
- console.log(config);
Summary by CodeRabbit
New Features
Improvements
Style
Chores