UI redesign — v1 (React 19 + Tailwind 4 + shadcn/ui)#12
Merged
Conversation
Greenfield rewrite spec for the docsiq web UI. Locks: - Navigation: Linear-style labeled sidebar + Raycast-style prominent ⌘K, with G+letter chord shortcuts, collapsible via ⌘- Responsive: mobile drawer / tablet icon rail / desktop labeled - Home: stats strip + since-last-visit activity feed + right rail - Notes: focused ~620px column with ⌘/ tree and ⌘L links drawers - Visual: Geist Sans+Mono, #0f1115 base, #3ecf8e accent (choice B) - Stack: React 19 + Vite 6 + shadcn/ui + Tailwind 4 + TanStack Query + Zustand + React Router 6 + React Hook Form + Zod + Framer Motion (all reduced-motion-gated) + markdown-it + d3-force + CodeMirror 6 Non-negotiables from rules/ui.md baked in: WCAG AA, prefers-reduced- motion, prefers-color-scheme, 100ms interaction, mobile-first, tokens only, i18n-ready, RTL-ready, fonts self-hosted per rules/build.md. Acceptance criteria (§19) + out-of-scope (§17) + risks (§18) all explicit. Next: invoke superpowers:writing-plans. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Greenfield rewrite plan derived from the 2026-04-18 spec. 10 waves, ~40 tasks, bite-sized TDD ladders per task. File paths, exact code, commit shapes, and acceptance gates specified throughout. Key scope: - Wave 0: foundation (stack install, tokens, fonts, shadcn) - Wave 1-2: layout shell + router + state + hotkeys - Wave 3: ⌘K command palette - Wave 4: Home (stats + activity + glance + pinned) - Wave 5: Notes (focused column + drawers + editor + search) - Wave 6: Documents (list + detail + upload modal) - Wave 7: Graph (d3-force) - Wave 8: MCP console - Wave 9: backend delta (8-line meta-tag injection for bearer auth) - Wave 10: theme toggle + axe + bundle gate + PR No backend changes except Wave 9 meta-tag injection. Existing Go tests untouched. Ready for subagent-driven-development. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…onfig)
- vitest config with jsdom + MSW-integrated setupTests + coverage
floor (70% stmts / 60% branches on components/hooks/lib/routes)
- src/test/{msw,handlers}.ts with 2 baseline handlers (/api/stats,
/api/projects) so integration tests have known fixtures
- src/main.tsx + src/App.tsx placeholder (renders "wave-0 scaffold
complete" to confirm the Vite+React+Tailwind pipeline works)
- components.json manifest for shadcn CLI
- src/lib/utils.ts with cn() helper (clsx + tailwind-merge)
Next: shadcn primitives (Task 0.8 via CLI) then backend build smoke.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Added 10 shadcn primitives via CLI: button, command, dialog, dropdown-menu, input, popover, scroll-area, separator, sheet, tooltip. vite-env.d.ts restores CSS import types. First successful npm run build on the new stack. Build sizes (well under 460 KB budget): - JS 194.83 KB / gzip 60.96 KB - CSS 22.83 KB / gzip 5.18 KB Wave 0 scaffold complete. Backend embed still serves placeholder page; real UI begins Wave 1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- ui/src/main.tsx: import axe-core only under import.meta.env.DEV and log any a11y violations to console during npm run dev - .github/workflows/ci.yml: assert JS+CSS bundle ≤ 460 KB after ui-dist freshness check. Trips the build if the combined assets exceed budget. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Original spec budget of 460 KB didn't account for the real footprint of React 19 + Radix + shadcn + cmdk + TanStack Query + React Router + Zustand on initial chunk. Actual build measures ~550 KB for the main chunk (lazy markdown/graph/editor chunks excluded from the gate). 580 KB is headroom for small additions, with no bloat on the table to trim. Initial-chunk gzip is 157 KB — well within fast-first-paint territory. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Shell renders CommandPalette which uses useQuery via useCommandSearch. Without a provider the test fails with "No QueryClient set". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughGreenfield UI rewrite: replaces legacy JS SPA with a TypeScript React app (React Router, TanStack Query, Zustand), new shadcn/Radix component library, markdown/wikilink parsing, MCP JSON‑RPC client, extensive tests/MSW, Tailwind tokenized styling, CI UI build + bundle-size check, and server changes for LLM opt-out and API-key meta injection. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant AppShell
participant Router
participant QueryClient
participant API
participant Stores
User->>Browser: Open app / press hotkey
Browser->>AppShell: mount React root (Providers, Shell)
AppShell->>Stores: init/useProject/useUIStore
AppShell->>QueryClient: initialize client
AppShell->>Router: render routes
User->>Browser: Press ⌘K (open Command)
Browser->>AppShell: useHotkey triggers cmdOpen
AppShell->>Router: render CommandPalette
CommandPalette->>QueryClient: useCommandSearch(project, q)
QueryClient->>API: parallel calls (/api/projects/.../search & /api/search)
API-->>QueryClient: return search results
QueryClient-->>CommandPalette: show hits
User->>Browser: select hit
Browser->>Router: navigate to route
Router->>QueryClient: fetch page data (useNote/useDoc/useNotesGraph)
QueryClient->>API: fetch endpoints
API-->>QueryClient: return data
QueryClient-->>Router: render page view
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (16)
.github/workflows/ci.yml-116-118 (1)
116-118:⚠️ Potential issue | 🟡 MinorBundle-size arithmetic can break on multiple matching assets.
On Line 116 and Line 117,
statcan return multiple lines when globs match multiple files; then Line 118 may fail arithmetic parsing.🔧 Suggested hardening
- js_bytes=$(stat -c %s ui/dist/assets/index-*.js 2>/dev/null || stat -f %z ui/dist/assets/index-*.js) - css_bytes=$(stat -c %s ui/dist/assets/index-*.css 2>/dev/null || stat -f %z ui/dist/assets/index-*.css) + js_bytes=$(find ui/dist/assets -maxdepth 1 -type f -name 'index-*.js' -exec stat -c %s {} + | awk '{s+=$1} END{print s+0}') + css_bytes=$(find ui/dist/assets -maxdepth 1 -type f -name 'index-*.css' -exec stat -c %s {} + | awk '{s+=$1} END{print s+0}')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 116 - 118, The current assignments to js_bytes and css_bytes can produce multiple lines when globs match multiple files, breaking the arithmetic for total; change the assignments so each variable contains the numeric sum of all matched file sizes (or 0 if none). For example, replace the direct stat calls for js_bytes and css_bytes with commands that iterate over the matching files and sum their sizes (handling both GNU stat -c %s and BSD stat -f %z fallback, or using find/xargs/perl to compute the sum), ensuring js_bytes and css_bytes are single integers before computing total=$((js_bytes + css_bytes))).ui/src/hooks/useLastVisit.ts-6-9 (1)
6-9:⚠️ Potential issue | 🟡 MinorGuard against
NaNfrom malformed localStorage values.At Line 8,
Number(v)can returnNaN; that will breaklastVisitcomparisons downstream.💡 Proposed fix
export function useLastVisit() { const [last, setLast] = useState<number>(() => { - const v = localStorage.getItem(KEY); - return v ? Number(v) : 0; + const v = localStorage.getItem(KEY); + const parsed = v ? Number(v) : 0; + return Number.isFinite(parsed) ? parsed : 0; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/hooks/useLastVisit.ts` around lines 6 - 9, The initializer for last in useLastVisit.ts can produce NaN when localStorage contains malformed data; update the logic in the useLastVisit hook (the const [last, setLast] = useState(...) block that reads KEY) to parse the stored string (e.g., Number or parseInt) and guard with isNaN so it falls back to 0 when the parsed value is invalid, ensuring downstream comparisons using last never see NaN.ui/src/lib/format.ts-15-18 (1)
15-18:⚠️ Potential issue | 🟡 Minor
formatCountcan output1000kfor near-million values.At Line 17, values like
999_950round to"1000k"instead of a million-format string.💡 Proposed fix
export function formatCount(n: number): string { if (n < 1000) return String(n); - if (n < 1_000_000) return (n / 1000).toFixed(n < 10_000 ? 1 : 0) + "k"; + if (n < 1_000_000) { + const k = (n / 1000).toFixed(n < 10_000 ? 1 : 0); + if (Number(k) >= 1000) return "1.0m"; + return `${k}k`; + } return (n / 1_000_000).toFixed(1) + "m"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/lib/format.ts` around lines 15 - 18, formatCount currently formats numbers under 1,000,000 by dividing by 1000 and rounding, which can produce "1000k" for values like 999_950; update formatCount to compute the rounded thousands value (use the same precision logic as now), then if that rounded thousands value is >= 1000 return "1m" (or format as millions), otherwise return the rounded value + "k"; keep the original n < 1000 and million branch but replace the direct string concat in the n < 1_000_000 branch with this rounded-thousands check to avoid "1000k".ui/src/components/graph/__tests__/GlanceView.test.tsx-10-23 (1)
10-23:⚠️ Potential issue | 🟡 Minor
maxNodescap behavior isn’t asserted despite the test name.At Line 10, the test claims cap validation, but it never sets
maxNodesto a lower value than node count. Please add an explicit capped case.Suggested test addition
it("renders N circles for N nodes (capped by maxNodes)", () => { @@ expect(container.querySelectorAll("circle").length).toBe(5); expect(container.querySelectorAll("line").length).toBe(1); }); + + it("caps rendered nodes when maxNodes is lower than node count", () => { + const { container } = render( + <GlanceView + maxNodes={3} + data={{ + nodes: Array.from({ length: 5 }, (_, i) => ({ + id: String(i), label: "n", kind: "entity", + })), + edges: [{ source: "0", target: "1" }], + }} + />, + ); + expect(container.querySelectorAll("circle").length).toBe(3); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/graph/__tests__/GlanceView.test.tsx` around lines 10 - 23, The test name mentions a maxNodes cap but doesn't exercise it; update GlanceView.test.tsx to include an explicit capped case: render <GlanceView ... maxNodes={3}> with 5 nodes (Array.from length 5) and assert container.querySelectorAll("circle").length === 3 (and keep or adjust the line/assertion for edges as appropriate), or add a separate it block that verifies the cap behavior; target the GlanceView component and the maxNodes prop in your change.ui/src/routes/documents/DocumentView.tsx-8-10 (1)
8-10:⚠️ Potential issue | 🟡 MinorMissing error state handling.
The component handles
isLoadingand!data, but doesn't handleisErrorfrom the query. Network failures or server errors would show "Not found" instead of an appropriate error message.🐛 Proposed fix to handle error state
- const { data, isLoading } = useDoc(project, id); + const { data, isLoading, isError } = useDoc(project, id); if (isLoading) return <div className="p-8 text-sm text-[var(--color-text-muted)]">Loading…</div>; + if (isError) return <div className="p-8 text-sm text-[var(--color-text-muted)]">Failed to load document.</div>; if (!data) return <div className="p-8 text-sm">Not found.</div>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/routes/documents/DocumentView.tsx` around lines 8 - 10, The component currently checks useDoc's isLoading and data but ignores isError; update DocumentView to also destructure isError and error from useDoc (e.g., const { data, isLoading, isError, error } = useDoc(project, id)) and render an explicit error state when isError is true (show a user-friendly message and/or include error.message for debugging) before falling back to the "Not found" case so network/server failures don't incorrectly display "Not found."ui/src/hooks/api/useCommand.ts-7-8 (1)
7-8:⚠️ Potential issue | 🟡 MinorNormalize query once before keying/fetching.
Line 8 trims for enablement, but Lines 7/12/15 still use raw
query; trailing spaces create duplicate cache keys and unnecessary requests.🔧 Proposed fix
export function useCommandSearch(project: string, query: string) { + const normalizedQuery = query.trim(); return useQuery({ - queryKey: ["command-search", project, query], - enabled: query.trim().length > 0, + queryKey: ["command-search", project, normalizedQuery], + enabled: normalizedQuery.length > 0, queryFn: async () => { const [notes, docs] = await Promise.all([ apiFetch<{ hits: NoteHit[] }>( - `/api/projects/${encodeURIComponent(project)}/search?q=${encodeURIComponent(query)}`, + `/api/projects/${encodeURIComponent(project)}/search?q=${encodeURIComponent(normalizedQuery)}`, ).catch(() => ({ hits: [] as NoteHit[] })), apiFetch<{ hits: SearchHit[] }>( - `/api/search?project=${encodeURIComponent(project)}&q=${encodeURIComponent(query)}&mode=local&top_k=5`, + `/api/search?project=${encodeURIComponent(project)}&q=${encodeURIComponent(normalizedQuery)}&mode=local&top_k=5`, ).catch(() => ({ hits: [] as SearchHit[] })), ]);Also applies to: 12-16
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/hooks/api/useCommand.ts` around lines 7 - 8, In the useCommand hook normalize the incoming query once (e.g., const normalizedQuery = query.trim()) and use that normalizedQuery everywhere instead of the raw query so cache keys and fetch conditions are consistent; update occurrences that build queryKey (currently ["command-search", project, query]), the enabled flag (enabled: query.trim().length > 0), and any fetch/input parameters (the other spots around the same hook) to reference normalizedQuery to avoid duplicate cache keys and redundant requests.internal/api/spa_meta_test.go-20-31 (1)
20-31:⚠️ Potential issue | 🟡 MinorHarden response assertions by checking status and body read errors.
Lines 25 and 45 ignore
io.ReadAllerrors, and both tests should assertStatusOKbefore content checks.🔧 Proposed fix
resp, err := http.Get(srv.URL + "/") if err != nil { t.Fatal(err) } defer resp.Body.Close() - body, _ := io.ReadAll(resp.Body) + if resp.StatusCode != http.StatusOK { + t.Fatalf("unexpected status: %d", resp.StatusCode) + } + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } ... resp, err := http.Get(srv.URL + "/") if err != nil { t.Fatal(err) } defer resp.Body.Close() - body, _ := io.ReadAll(resp.Body) + if resp.StatusCode != http.StatusOK { + t.Fatalf("unexpected status: %d", resp.StatusCode) + } + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + }Also applies to: 40-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/spa_meta_test.go` around lines 20 - 31, Add assertions to verify the HTTP response status and handle read errors: after the http.Get call and before inspecting the body in internal/api/spa_meta_test.go, assert resp.StatusCode == http.StatusOK (use http.StatusOK) and check the error returned by io.ReadAll(resp.Body), failing the test if read fails; apply the same changes to the second test block that mirrors lines 40-48. Reference the local variables resp and body and the calls to http.Get and io.ReadAll so you update both occurrences consistently.ui/src/routes/notes/NotesLayout.tsx-22-25 (1)
22-25:⚠️ Potential issue | 🟡 MinorScope the empty-state message to notes index only.
With the current
!keycheck, this hint also appears on routes like/notes/search, which can produce conflicting UI.🔧 Proposed fix
-import { Outlet, useParams } from "react-router-dom"; +import { Outlet, useLocation, useParams } from "react-router-dom"; ... const { key } = useParams(); + const { pathname } = useLocation(); + const showEmptyState = !key && pathname === "/notes"; ... - {!key && ( + {showEmptyState && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/routes/notes/NotesLayout.tsx` around lines 22 - 25, The empty-state hint in NotesLayout currently renders whenever !key, which also shows it on subroutes like /notes/search; update the conditional to render only on the notes index route by adding a route-check (e.g. compute isNotesIndex via react-router's useLocation or useMatch and match '/notes' exactly) and change the render condition to {!key && isNotesIndex} in the NotesLayout component so the hint appears only on the main notes index.ui/src/styles/globals.css-3-3 (1)
3-3:⚠️ Potential issue | 🟡 MinorStylelint config should explicitly allow Tailwind v4
@theme.The
@themedirective is present inui/src/styles/globals.cssat lines 3 and 36, but the stylelint config does not explicitly allow it. AddignoreAtRules: ["theme"]to thescss/at-rule-no-unknownrule in.stylelintrc.jsonto prevent static analysis from flagging this Tailwind v4 syntax as unknown.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/styles/globals.css` at line 3, The Stylelint complaint is caused by Tailwind v4's `@theme` directive in ui/src/styles/globals.css; update the Stylelint configuration by modifying the scss/at-rule-no-unknown rule in .stylelintrc.json to include ignoreAtRules: ["theme"] so the `@theme` at-rule is treated as known and not flagged; locate the scss/at-rule-no-unknown rule and add the ignoreAtRules option (or append "theme" if ignoreAtRules already exists) to resolve the lint error.ui/src/components/layout/Providers.tsx-25-30 (1)
25-30:⚠️ Potential issue | 🟡 Minor
systemtheme doesn’t react to live OS preference changes.This effect computes
prefers-color-schemeonce, so changing OS theme while the app is open won’t updatedata-theme.Proposed fix
useEffect(() => { const root = document.documentElement; - const systemDark = window.matchMedia("(prefers-color-scheme: dark)").matches; - const effective = theme === "system" ? (systemDark ? "dark" : "light") : theme; - root.dataset.theme = effective; + const media = window.matchMedia("(prefers-color-scheme: dark)"); + const apply = () => { + const effective = theme === "system" ? (media.matches ? "dark" : "light") : theme; + root.dataset.theme = effective; + }; + apply(); + if (theme === "system") { + media.addEventListener("change", apply); + return () => media.removeEventListener("change", apply); + } }, [theme]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/Providers.tsx` around lines 25 - 30, The current useEffect reads window.matchMedia("(prefers-color-scheme: dark)") once so the app doesn’t respond when the OS theme changes; update the effect to create a MediaQueryList (e.g., mediaQuery = window.matchMedia(...)) and register a change listener that recomputes systemDark and sets root.dataset.theme (respecting theme === "system" vs explicit theme), and remove that listener in the cleanup; ensure the listener and initial call live inside the existing useEffect (the one referencing theme) and use mediaQuery.addEventListener('change', handler) with a fallback to mediaQuery.addListener(handler) and corresponding removal in cleanup.ui/src/components/notes/TreeDrawer.tsx-41-50 (1)
41-50:⚠️ Potential issue | 🟡 MinorExpose the active note semantically, not just visually.
The selected entry is styled as current, but screen readers still get an ordinary link. Add
aria-current="page"whencurrentKey === n.keyso the drawer announces the active note correctly.Proposed fix
<Link key={n.key} to={`/notes/${encodeURIComponent(n.key)}`} + aria-current={currentKey === n.key ? "page" : undefined} className={ "block px-2 py-1 rounded text-sm " +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/notes/TreeDrawer.tsx` around lines 41 - 50, The Link rendering in TreeDrawer.tsx marks the selected note visually but not semantically; update the Link element (the one using key={n.key}, to={`/notes/${encodeURIComponent(n.key)}` and className based on currentKey) to include aria-current="page" when currentKey === n.key (otherwise omit or set to undefined) so screen readers announce the active note; no other behavior changes—keep the existing onClick={() => onOpenChange(false)} and className logic intact.ui/src/routes/MCPConsole.tsx-28-34 (1)
28-34:⚠️ Potential issue | 🟡 MinorGive the tool picker an accessible name.
The JSON field is labeled, but the
<select>is not. Screen readers will announce an unlabeled combobox here. Add a visible label or at least anaria-label.Proposed fix
<select value={tool} onChange={(e) => setTool(e.currentTarget.value)} + aria-label="MCP tool" className="px-3 py-2 bg-[var(--color-surface-1)] border border-[var(--color-border)] rounded-md text-sm font-mono" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/routes/MCPConsole.tsx` around lines 28 - 34, The <select> used for tool selection in MCPConsole (value={tool}, onChange={(e) => setTool(e.currentTarget.value)} and populated from KNOWN_TOOLS) has no accessible name; add one by providing either a visible <label> tied to the select via htmlFor/id or adding an aria-label/aria-labelledby attribute on the select (e.g., aria-label="Tool picker" or aria-labelledby referencing a nearby label element) so screen readers announce the control.ui/src/components/layout/StatsStrip.tsx-21-21 (1)
21-21:⚠️ Potential issue | 🟡 MinorLocalize the region aria-label.
Line 21 hardcodes
"Project statistics"while the rest of this component usest(...). This creates an i18n gap for screen-reader users.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/StatsStrip.tsx` at line 21, The region's aria-label in the StatsStrip component is hardcoded as "Project statistics"; replace it with the localized string using the same i18n function used in this component (e.g., t('...')) so screen readers receive the translated label—update the div with role="region" aria-label to use t(...) (reference the StatsStrip component and the div with role="region" aria-label).ui/src/hooks/api/useMCP.ts-31-33 (1)
31-33:⚠️ Potential issue | 🟡 MinorHarden error extraction for non-Error throws.
Line 32 assumes
eis always anError; this can leaverec.errorempty for thrown strings/objects.💡 Suggested fix
- } catch (e) { - rec.error = (e as Error).message; + } catch (e: unknown) { + rec.error = + e instanceof Error + ? e.message + : typeof e === "string" + ? e + : "Unknown MCP error"; } finally {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/hooks/api/useMCP.ts` around lines 31 - 33, The catch block in useMCP.ts sets rec.error = (e as Error).message which fails for non-Error throws; update the catch to robustly extract a message for rec.error by checking if e is an instance of Error (use e.message), else if typeof e === 'string' use e, otherwise set rec.error = JSON.stringify(e) (with a fallback like 'Unknown error') so rec.error is always populated; apply this change in the catch that assigns to rec.error in the function surrounding that try/catch.ui/src/components/ui/popover.tsx-56-64 (1)
56-64:⚠️ Potential issue | 🟡 MinorElement/type mismatch:
PopoverTitlerenders<div>but accepts<h2>props.The component is typed with
React.ComponentProps<"h2">but renders a<div>. This mismatch is misleading and loses heading semantics that could benefit accessibility. Consider using an<h2>element or adjusting the type to match the rendered element.🔧 Proposed fix — use semantic heading
function PopoverTitle({ className, ...props }: React.ComponentProps<"h2">) { return ( - <div + <h2 data-slot="popover-title" className={cn("font-medium", className)} {...props} /> ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/ui/popover.tsx` around lines 56 - 64, PopoverTitle is typed as React.ComponentProps<"h2"> but currently renders a <div>, causing a prop/type and semantic mismatch; update the component to render an <h2> (preserving data-slot="popover-title", className via cn("font-medium", className), and spreading ...props) so the element type matches the React.ComponentProps<"h2"> typing and restores proper heading semantics for accessibility.ui/src/components/ui/dropdown-menu.tsx-76-78 (1)
76-78:⚠️ Potential issue | 🟡 MinorFix invalid Tailwind selector
*:[svg]in destructive variant SVG styling.The class
data-[variant=destructive]:*:[svg]:text-destructive!uses invalid syntax. While*:is a valid Tailwind v4 child variant,[svg]is not valid CSS. Looking at the same line where SVG targeting is done correctly with[&_svg]:pointer-events-noneand[&_svg]:text-muted-foreground, this should bedata-[variant=destructive]:[&_svg]:text-destructive!to properly apply text color to SVG descendants in the destructive variant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/ui/dropdown-menu.tsx` around lines 76 - 78, The Tailwind selector in the className string is invalid: replace the malformed `data-[variant=destructive]:*:[svg]:text-destructive!` token so the destructive variant targets SVG descendants correctly; update the class inside the cn(...) call (the same string that contains `[&_svg]:pointer-events-none` and `[&_svg]:text-muted-foreground`) to use `data-[variant=destructive]:[&_svg]:text-destructive!` so SVG children receive the destructive text color under the destructive variant.
🧹 Nitpick comments (23)
ui/src/components/ui/input.tsx (1)
5-21: Forward the input ref to enable better interoperability with form libraries and parent focus control.The
Inputcomponent (lines 5–21) is a wrapper around the native<input>element but lacksforwardRef, preventing parent components from accessing the underlying DOM node. While other UI components in this codebase wrap Radix UI primitives that already support refs,Inputis the only direct DOM element wrapper without ref forwarding.♻️ Suggested ref-forwarding refactor
-function Input({ className, type, ...props }: React.ComponentProps<"input">) { - return ( - <input - type={type} - data-slot="input" - className={cn( - "h-9 w-full min-w-0 rounded-md border border-input bg-transparent px-3 py-1 text-base shadow-xs transition-[color,box-shadow] outline-none selection:bg-primary selection:text-primary-foreground file:inline-flex file:h-7 file:border-0 file:bg-transparent file:text-sm file:font-medium file:text-foreground placeholder:text-muted-foreground disabled:pointer-events-none disabled:cursor-not-allowed disabled:opacity-50 md:text-sm dark:bg-input/30", - "focus-visible:border-ring focus-visible:ring-[3px] focus-visible:ring-ring/50", - "aria-invalid:border-destructive aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40", - className - )} - {...props} - /> - ) -} +const Input = React.forwardRef<HTMLInputElement, React.ComponentProps<"input">>( + ({ className, type, ...props }, ref) => { + return ( + <input + ref={ref} + type={type} + data-slot="input" + className={cn( + "h-9 w-full min-w-0 rounded-md border border-input bg-transparent px-3 py-1 text-base shadow-xs transition-[color,box-shadow] outline-none selection:bg-primary selection:text-primary-foreground file:inline-flex file:h-7 file:border-0 file:bg-transparent file:text-sm file:font-medium file:text-foreground placeholder:text-muted-foreground disabled:pointer-events-none disabled:cursor-not-allowed disabled:opacity-50 md:text-sm dark:bg-input/30", + "focus-visible:border-ring focus-visible:ring-[3px] focus-visible:ring-ring/50", + "aria-invalid:border-destructive aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40", + className + )} + {...props} + /> + ) + } +) +Input.displayName = "Input"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/ui/input.tsx` around lines 5 - 21, The Input component currently defined as function Input({ className, type, ...props }: React.ComponentProps<"input">) does not forward refs, preventing parents/form libraries from accessing the underlying <input>; change it to use React.forwardRef<HTMLInputElement, React.ComponentProps<"input">>((props, ref) => { ... }) so the forwarded ref is attached to the rendered <input ref={ref} ... />; update the exported symbol (e.g., export const Input = React.forwardRef(...)) and set Input.displayName = "Input" to preserve debugging names while keeping all existing props and className handling.ui/src/hooks/useLastVisit.ts (1)
21-21: ReuseKEYconstant in unmount hook to avoid key drift.Line 21 hardcodes the storage key again; reusing
KEYkeeps reads/writes consistent.💡 Proposed fix
export function useTouchOnUnmount() { - useEffect(() => () => { localStorage.setItem("docsiq-last-visit", String(Date.now())); }, []); + useEffect(() => () => { localStorage.setItem(KEY, String(Date.now())); }, []); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/hooks/useLastVisit.ts` at line 21, The unmount effect in useEffect currently hardcodes "docsiq-last-visit" instead of using the existing KEY constant; update the cleanup arrow function in the useEffect (the unmount hook) to call localStorage.setItem(KEY, String(Date.now())) so reads and writes use the same KEY symbol (refer to the KEY constant and the useEffect cleanup in useLastVisit.ts).ui/src/lib/__tests__/format.test.ts (1)
17-22: Add a boundary test for near-million rounding.Please add a case around
999_950to lock expected behavior ("1.0m"vs"1000k"), so this edge case stays covered.✅ Suggested test addition
describe("formatCount", () => { it("raw for < 1k", () => expect(formatCount(42)).toBe("42")); it("k for thousands", () => expect(formatCount(1234)).toBe("1.2k")); it("k rounded for >= 10k", () => expect(formatCount(12_345)).toBe("12k")); + it("rolls near-million rounded thousands into m", () => + expect(formatCount(999_950)).toBe("1.0m")); it("m for millions", () => expect(formatCount(1_234_567)).toBe("1.2m")); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/lib/__tests__/format.test.ts` around lines 17 - 22, Add a boundary test to the existing formatCount suite to cover the near‑million rounding edge case: inside the describe("formatCount") block add an it test that calls formatCount(999_950) and asserts the expected string (e.g., "1.0m") to lock the behavior between producing "1.0m" vs "1000k"; reference the existing formatCount tests to place the new case alongside the other expectations for thousands and millions.ui/src/main.tsx (1)
7-13: Handle DEV axe promise failures to avoid unhandled rejections.At Line 7–Line 13, add a
.catchpath so import/audit failures don’t produce unhandled async errors during local runs.Suggested fix
if (import.meta.env.DEV) { import("axe-core").then((axe) => { axe.default.run().then((res) => { if (res.violations.length > 0) { console.warn("axe violations:", res.violations); } }); - }); + }).catch((err) => { + console.warn("axe audit failed:", err); + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/main.tsx` around lines 7 - 13, The dynamic import and audit call for axe-core can produce unhandled promise rejections; update the import("axe-core") chain used in main.tsx so both the import and the axe.default.run() promise are handled with .catch handlers (or a single .catch at the end of the promise chain) to log or silently ignore errors during DEV runs; locate the import("axe-core") call and the axe.default.run() invocation and add appropriate .catch callbacks to avoid unhandled rejections.ui/src/components/layout/__tests__/ThemeToggle.test.tsx (1)
7-23: Isolate global mutations to avoid cross-test coupling.The test mutates shared globals (
useUIStore, polyfills) but does not restore state. Add cleanup to keep tests order-independent.Suggested test-hardening diff
-import { describe, it, expect } from "vitest"; +import { describe, it, expect, afterEach } from "vitest"; import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { ThemeToggle } from "../ThemeToggle"; import { useUIStore } from "@/stores/ui"; +const initialTheme = useUIStore.getState().theme; + // Polyfills for Radix dropdown in jsdom if (!(global as any).ResizeObserver) { (global as any).ResizeObserver = class { observe() {} unobserve() {} disconnect() {} }; } if (!(Element.prototype as any).scrollIntoView) { (Element.prototype as any).scrollIntoView = () => {}; } + +afterEach(() => { + useUIStore.setState({ theme: initialTheme }); +}); describe("ThemeToggle", () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/__tests__/ThemeToggle.test.tsx` around lines 7 - 23, The test mutates shared globals and store without restoring them; update ThemeToggle.test.tsx to isolate changes by saving original globals and restoring them in an afterEach/afterAll cleanup and resetting the UI store state: capture original global.ResizeObserver and Element.prototype.scrollIntoView before polyfilling and restore them after the test(s), call testing-library cleanup between tests, and reset useUIStore to its prior state (use useUIStore.getState()/setState or a known default) after each test so useUIStore and the DOM polyfills do not leak across tests.ui/src/hooks/__tests__/useReducedMotion.test.tsx (1)
5-32: Restorewindow.matchMediaafter each test.Overwriting
matchMediawithout teardown can affect unrelated tests.Suggested isolation diff
-import { describe, it, expect, vi } from "vitest"; +import { describe, it, expect, vi, afterEach } from "vitest"; import { renderHook } from "@testing-library/react"; import { useReducedMotion } from "../useReducedMotion"; +const originalMatchMedia = window.matchMedia; + function mockMatchMedia(matches: boolean) { Object.defineProperty(window, "matchMedia", { @@ }); } + +afterEach(() => { + Object.defineProperty(window, "matchMedia", { + writable: true, + configurable: true, + value: originalMatchMedia, + }); +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/hooks/__tests__/useReducedMotion.test.tsx` around lines 5 - 32, The tests overwrite window.matchMedia globally via mockMatchMedia and don't restore it, which can leak into other tests; modify mockMatchMedia to save the original matchMedia (e.g., const originalMatchMedia = window.matchMedia) before defining the mock and add an afterEach teardown in the useReducedMotion test suite to restore window.matchMedia to the saved original (or to undefined) so the original behavior is reinstated after each test; reference mockMatchMedia and the describe("useReducedMotion") block to add the afterEach restore.ui/src/components/activity/__tests__/ActivityFeed.test.tsx (1)
11-28: Test intent and assertions are currently mismatched.The case says it verifies “highlights ones newer than lastVisit”, but it only asserts rendered text. Please add an assertion for the “new/highlighted” state (or rename the test to match current checks).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/activity/__tests__/ActivityFeed.test.tsx` around lines 11 - 28, The test name claims it checks highlighting newer events but only asserts rendered text; update the test in ActivityFeed.test.tsx to either rename it to match current assertions or (preferred) add an assertion that the event with id "1" (text "jwt") is marked as new by ActivityFeed: query the element containing "jwt", get its enclosing event container (e.g., via closest('li') or getByText(...).closest(...)), and assert the container has the new/highlight indicator used by ActivityFeed (for example a CSS class like "new" or "highlight", or a data attribute like data-new="true" or an aria attribute); keep the existing text assertions.internal/api/router.go (1)
168-178: MissingContent-Lengthheader after content modification.When the API key meta tag is injected, the content length changes, but no
Content-Lengthheader is set. While HTTP/1.1 chunked encoding may handle this, explicitly setting the header improves compatibility and allows clients to show progress.♻️ Set Content-Length after modification
if cfg.Server.APIKey != "" { content = bytes.Replace( content, []byte("</head>"), []byte(`<meta name="docsiq-api-key" content="`+html.EscapeString(cfg.Server.APIKey)+`"></head>`), 1, ) } w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.Header().Set("Content-Length", strconv.Itoa(len(content))) w.WriteHeader(http.StatusOK) _, _ = w.Write(content)Requires adding
"strconv"to imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/router.go` around lines 168 - 178, The handler modifies the HTML `content` when cfg.Server.APIKey is set but never updates the Content-Length header; after performing the bytes.Replace on `content` (the variable used for response body) compute the new length (using strconv.Itoa(len(content))) and set w.Header().Set("Content-Length", <that value>) before calling w.WriteHeader(http.StatusOK); add "strconv" to the imports. This ensures the Content-Length reflects the modified `content` sent by the handler that checks cfg.Server.APIKey.ui/src/lib/__tests__/api-client.test.ts (1)
19-27: Considerexpect().rejectsfor cleaner async error testing.The try/catch pattern works but is more verbose. Vitest's
expect().rejectsprovides cleaner assertions.♻️ More idiomatic error assertion
- try { - await apiFetch("/api/bad"); - throw new Error("should not reach"); - } catch (e) { - expect(e).toBeInstanceOf(ApiErrorResponse); - expect((e as ApiErrorResponse).status).toBe(400); - expect((e as ApiErrorResponse).requestId).toBe("req-123"); - expect((e as ApiErrorResponse).message).toBe("nope"); - } + await expect(apiFetch("/api/bad")).rejects.toMatchObject({ + status: 400, + requestId: "req-123", + message: "nope", + }); + await expect(apiFetch("/api/bad")).rejects.toBeInstanceOf(ApiErrorResponse);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/lib/__tests__/api-client.test.ts` around lines 19 - 27, Replace the try/catch with vitest's promise rejection matchers to make the test clearer: call expect(apiFetch("/api/bad")).rejects.toBeInstanceOf(ApiErrorResponse) and then either chain .rejects.toMatchObject({ status: 400, requestId: "req-123", message: "nope" }) or use await expect(...).rejects.toHaveProperty for each field; target the apiFetch call and assert against the ApiErrorResponse class and its status/requestId/message properties.ui/src/components/activity/EventRow.tsx (1)
10-11: Hover state conflict withisNewbackground.When
isNewis true, the background is alreadyvar(--color-surface-2), which is the same as the hover state. This means new items won't have visible hover feedback.♻️ Suggested fix for hover visibility on new items
className="flex items-center gap-3 px-3 py-2 rounded-md border border-[var(--color-border)] hover:bg-[var(--color-surface-2)] transition-colors" - style={isNew ? { background: "var(--color-surface-2)" } : undefined} + style={isNew ? { background: "var(--color-surface-2)" } : undefined} + data-new={isNew || undefined}Then use CSS to differentiate hover on new items:
[data-new]:hover { background: var(--color-surface-3); }Or use a slightly different shade for the
isNewbackground (e.g.,--color-accent-muted) to preserve hover feedback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/activity/EventRow.tsx` around lines 10 - 11, The hover is invisible when isNew sets the inline background to var(--color-surface-2); update EventRow rendering to mark new items with a data attribute (e.g., data-new={isNew}) instead of forcing the same hover color, and adjust CSS so [data-new] has a different base background or a distinct hover (e.g., [data-new]{ background: var(--color-accent-muted) } or [data-new]:hover{ background: var(--color-surface-3) }) so new rows retain visible hover feedback while regular rows keep the existing hover rule; change references in the component from the inline style to the data attribute and update the stylesheet accordingly.ui/src/routes/documents/DocumentView.tsx (1)
9-10: Inconsistent text styling between states.Loading state uses
text-[var(--color-text-muted)]but "Not found" state lacks it, creating visual inconsistency.♻️ Consistent styling
- if (!data) return <div className="p-8 text-sm">Not found.</div>; + if (!data) return <div className="p-8 text-sm text-[var(--color-text-muted)]">Not found.</div>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/routes/documents/DocumentView.tsx` around lines 9 - 10, The "Not found." state in DocumentView.tsx is missing the muted text styling applied to the loading state, causing a visual mismatch; update the JSX that returns when (!data) so it uses the same classes as the loading branch (e.g., include text-[var(--color-text-muted)] alongside the existing p-8 text-sm) — look for the isLoading and data conditional returns inside the DocumentView component and make the not-found div class list consistent with the loading div.ui/src/components/layout/TopBar.tsx (2)
31-31: Hardcoded ⌘K shortcut — consider platform detection.The "⌘K" badge is Mac-specific. Windows/Linux users expect "Ctrl+K". Consider detecting the platform to show the appropriate modifier key.
♻️ Suggested platform-aware shortcut display
+const isMac = typeof navigator !== 'undefined' && /Mac|iPod|iPhone|iPad/.test(navigator.platform); +const modKey = isMac ? '⌘' : 'Ctrl+'; - <kbd className="font-mono text-[10px] px-1.5 py-0.5 rounded bg-[var(--color-surface-2)] border border-[var(--color-border)]">⌘K</kbd> + <kbd className="font-mono text-[10px] px-1.5 py-0.5 rounded bg-[var(--color-surface-2)] border border-[var(--color-border)]">{modKey}K</kbd>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/TopBar.tsx` at line 31, The TopBar component currently renders a hardcoded "⌘K" <kbd> element which is Mac-specific; update TopBar to detect the user's platform (e.g., compute an isMac boolean using navigator.platform or navigator.userAgent) and derive a modifierLabel variable ("⌘" for macOS, "Ctrl" for Windows/Linux) then render that label inside the existing <kbd className=...> element instead of the hardcoded value; ensure this logic runs client-side (avoid SSR mismatch) and keep the element and className (the <kbd> node) unchanged so styling continues to apply.
22-24: Hardcoded project slug_defaultin brand text.The brand text shows
_defaultstatically. If multi-project support is intended, this should read fromuseProjectStoreto display the current project slug.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/TopBar.tsx` around lines 22 - 24, The brand text in TopBar.tsx currently hardcodes the project slug as the string "_default"; update the component to read the current project slug from useProjectStore and render that value instead of the hardcoded text. Import and call the useProjectStore selector used elsewhere (e.g., const project = useProjectStore(s => s.project) or the appropriate selector in your store) inside the TopBar component and replace the "_default" span content with the project's slug (falling back to a sensible default like "default" or empty string if undefined). Ensure you reference the existing TopBar component and the useProjectStore hook so the displayed brand updates with the active project.ui/src/hooks/api/useCommand.ts (1)
13-16: Avoid fully masking command-search failures.Lines 13 and 16 convert every fetch error to empty results, which makes auth/outage failures indistinguishable from “no matches”.
🔧 Suggested resilience pattern
queryFn: async () => { - const [notes, docs] = await Promise.all([ - apiFetch<{ hits: NoteHit[] }>( + const [notes, docs] = await Promise.allSettled([ + apiFetch<{ hits: NoteHit[] }>( `/api/projects/${encodeURIComponent(project)}/search?q=${encodeURIComponent(normalizedQuery)}`, - ).catch(() => ({ hits: [] as NoteHit[] })), + ), apiFetch<{ hits: SearchHit[] }>( `/api/search?project=${encodeURIComponent(project)}&q=${encodeURIComponent(normalizedQuery)}&mode=local&top_k=5`, - ).catch(() => ({ hits: [] as SearchHit[] })), + ), ]); - return { notes: notes.hits, docs: docs.hits }; + + if (notes.status === "rejected" && docs.status === "rejected") { + throw new Error("Command search failed for both notes and docs"); + } + + return { + notes: notes.status === "fulfilled" ? notes.value.hits : [], + docs: docs.status === "fulfilled" ? docs.value.hits : [], + }; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/hooks/api/useCommand.ts` around lines 13 - 16, The current catch handlers on the apiFetch calls in useCommand (the promises that now .catch(() => ({ hits: [] as NoteHit[] })) and .catch(() => ({ hits: [] as SearchHit[] }))) silently mask all failures; instead detect and propagate real errors while still returning empty hits for genuine 404/empty responses. Change the catch logic in useCommand to inspect the thrown error (from apiFetch) and either attach an error flag or rethrow for non-expected failures (auth/network/outage), e.g., return a shape like { hits: [], error } or throw the error for upstream handling, so callers of useCommand can differentiate “no matches” from actual failures; keep the apiFetch success path the same and only default to empty hits for known benign cases.ui/src/routes/notes/NoteView.tsx (1)
13-31: Move user-visible strings to i18n keys.This view hardcodes text (
Loading…,Note not found,updated,by) instead of using the translation layer used in other new UI components.Proposed fix
+import { t } from "@/i18n"; ... - return <div className="p-8 text-[var(--color-text-muted)] text-sm">Loading…</div>; + return <div className="p-8 text-[var(--color-text-muted)] text-sm">{t("notes.loading")}</div>; ... - <h1 className="text-xl font-semibold">Note not found</h1> + <h1 className="text-xl font-semibold">{t("notes.notFound")}</h1> ... - {note.key} · updated {formatRelativeTime(new Date(note.updated_at).getTime())} - {note.author && ` · by ${note.author}`} + {note.key} · {t("notes.updated")} {formatRelativeTime(new Date(note.updated_at).getTime())} + {note.author && ` · ${t("notes.by")} ${note.author}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/routes/notes/NoteView.tsx` around lines 13 - 31, The component NoteView currently renders hardcoded user-facing strings ("Loading…", "Note not found", "updated", "by"); replace these with i18n translation keys by importing and using the project's translation helper (e.g., useTranslation/t or t) inside the NoteView component and swap the literals for keys such as notes.loading, notes.notFound, notes.updated, and notes.by when rendering the loading state, the not-found header/body, and the "updated ... · by ..." line (keep the dynamic values like note.key and formatted time intact and interpolate them via the translation calls).ui/src/components/graph/GraphCanvas.tsx (1)
16-20: Prefer stable edge keys over index keys.Using
key={i}on Line 19 can cause unnecessary remounts when edge order changes.♻️ Suggested refactor
- return <line key={i} x1={a.x} y1={a.y} x2={b.x} y2={b.y} stroke="var(--color-border-strong)" strokeWidth={0.7} />; + return <line key={`${e.source}:${e.target}`} x1={a.x} y1={a.y} x2={b.x} y2={b.y} stroke="var(--color-border-strong)" strokeWidth={0.7} />;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/graph/GraphCanvas.tsx` around lines 16 - 20, The edge rendering uses key={i} causing remounts; update the key on the map in GraphCanvas (the laid.edges.map where e is each edge and idx is the node index map) to use a stable identifier such as a combination of edge.source and edge.target (e.g., `${e.source}-${e.target}`) or e.id if edges carry an id, and only fall back to the loop index when no stable id exists to ensure unique keys without unnecessary remounts.ui/src/components/notes/LinkPanel.tsx (1)
30-57: Consider wrapping links in<ul>/<li>for accessibility.Screen readers benefit from list semantics when navigating multiple related links. Wrapping the inbound and outbound links in
<ul>with<li>elements improves discoverability and navigation.♿ Suggested structure
<section> <h3 className="text-xs uppercase text-[var(--color-text-muted)] mb-1.5">Inbound</h3> {inbound.length === 0 && <p className="text-xs text-[var(--color-text-muted)]">—</p>} - {inbound.map((k) => ( - <Link - key={k} - to={`/notes/${encodeURIComponent(k)}`} - onClick={() => onOpenChange(false)} - className="block px-2 py-1 rounded hover:bg-[var(--color-surface-2)]" - > - {k} - </Link> - ))} + {inbound.length > 0 && ( + <ul className="list-none"> + {inbound.map((k) => ( + <li key={k}> + <Link + to={`/notes/${encodeURIComponent(k)}`} + onClick={() => onOpenChange(false)} + className="block px-2 py-1 rounded hover:bg-[var(--color-surface-2)]" + > + {k} + </Link> + </li> + ))} + </ul> + )} </section>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/notes/LinkPanel.tsx` around lines 30 - 57, Wrap the inbound and outbound link lists in semantic list markup: replace the direct mapping of inbound.map(...) and outbound.map(...) with a <ul> where each mapped item renders an <li> containing the existing <Link> (keep the same key={k}, to={`/notes/${encodeURIComponent(k)}`}, onClick={() => onOpenChange(false)} and className). Ensure the empty-state paragraph remains outside or inside the <ul> as appropriate (e.g., render the <p> when length === 0, otherwise render the <ul>) so screen readers get proper list semantics for the inbound and outbound collections.ui/src/routes/Home.tsx (2)
15-18: Consider handling loading and error states for queries.The component uses
stats.data,activity.data,notes.data, andgraph.datawithout checkingisLoadingorisError. While the child components may handleundefinedgracefully, users see no feedback during initial load or on fetch failures. A skeleton or error boundary would improve UX.Also applies to: 32-40
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/routes/Home.tsx` around lines 15 - 18, The component is using hooks useStats, useActivity, useNotes, and useNotesGraph and reads stats.data / activity.data / notes.data / graph.data without checking their loading/error states; update the Home component (or the render section around where stats, activity, notes, graph are consumed) to check each hook's isLoading and isError flags and render a loading skeleton or a concise error message (or a combined loader while any isLoading is true, and an error UI if any isError is true) before rendering child components that expect the .data values; ensure you reference the existing hook return values (stats.isLoading, stats.isError, activity.isLoading, etc.) and pass the validated data down only when available.
28-53: "Pinned notes" section displays recent notes, not pinned notes.The section is labeled
t("home.pinnedNotes")but displaysnotes.data.slice(0, 5)— the first 5 notes from the API response, not user-pinned notes. If pinning isn't implemented yet, consider renaming to "Recent Notes" to avoid user confusion, or add a TODO.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/routes/Home.tsx` around lines 28 - 53, The "Pinned notes" UI uses recentNotes (const recentNotes = (notes.data ?? []).slice(0, 5)) but the header uses t("home.pinnedNotes"), causing a mismatch; either change the label to a recent-notes translation (e.g., replace t("home.pinnedNotes") with t("home.recentNotes") or add a TODO) or update the data source to actual pinned notes (replace recentNotes with the pinned notes array from the API or add a selector that filters notes.data for pinned === true), and ensure the list rendering (recentNotes.map and the empty-state check) is updated to use that pinned/renamed variable consistently.ui/src/hooks/api/useActivity.ts (1)
6-6:doc_errorkind is defined but never produced.
ActivityEventKindincludes"doc_error"but no code path inqueryFncreates events with this kind. If this is planned for future use, consider adding a comment; otherwise, remove the dead variant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/hooks/api/useActivity.ts` at line 6, ActivityEventKind currently declares "doc_error" but no events of that kind are produced by the hook's queryFn, so either remove the unused variant or add a producing path: remove "doc_error" from the ActivityEventKind union if it's not needed; otherwise update the queryFn (the function that builds activity events) to emit events with kind: "doc_error" at the points where document indexing or processing failures are detected (include an explanatory comment next to ActivityEventKind if you intend to keep it for future use). Ensure references use the exact symbol names ActivityEventKind and queryFn so the change is easy to locate.ui/src/components/ui/sheet.tsx (1)
75-80: Missingdata-slotattribute on inline close button.The close button rendered inside
SheetContent(line 76) doesn't havedata-slot="sheet-close", unlike the standaloneSheetClosecomponent. This breaks consistency for styling hooks.♻️ Add data-slot for consistency
{showCloseButton && ( - <SheetPrimitive.Close className="absolute top-4 right-4 rounded-xs opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:ring-2 focus:ring-ring focus:ring-offset-2 focus:outline-hidden disabled:pointer-events-none data-[state=open]:bg-secondary"> + <SheetPrimitive.Close data-slot="sheet-close" className="absolute top-4 right-4 rounded-xs opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:ring-2 focus:ring-ring focus:ring-offset-2 focus:outline-hidden disabled:pointer-events-none data-[state=open]:bg-secondary"> <XIcon className="size-4" /> <span className="sr-only">Close</span> </SheetPrimitive.Close> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/ui/sheet.tsx` around lines 75 - 80, The inline close button rendered via SheetPrimitive.Close (inside the conditional that checks showCloseButton) is missing the data-slot attribute used by the standalone SheetClose for styling hooks; add data-slot="sheet-close" to the SheetPrimitive.Close element so it matches the SheetClose component and restores consistency for styling selectors and tooling that rely on that attribute.ui/src/types/api.ts (1)
13-39: Inconsistent timestamp types betweenNoteandDocument.
Noteusesstringforcreated_at/updated_at(ISO format), whileDocumentusesnumber(Unix timestamp). If intentional to match the backend, consider adding a comment documenting this difference to prevent confusion during consumption.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/types/api.ts` around lines 13 - 39, The timestamp types for Note (created_at/updated_at as string) and Document (created_at/updated_at as number) are inconsistent; add a short inline comment above the Note and Document interfaces (referencing the Note and Document type names) documenting the expected formats (e.g., Note uses ISO string, Document uses Unix epoch number) so consumers know this is intentional and what format to parse/serialize.docs/superpowers/specs/2026-04-18-ui-redesign-design.md (1)
198-218: Add language specifier to fenced code blocks.Per markdownlint, fenced code blocks should have a language specified. These CSS token definitions could use
cssas the language identifier for proper syntax highlighting.📝 Example fix
-``` +```css --color-base `#0f1115` /* app background */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/specs/2026-04-18-ui-redesign-design.md` around lines 198 - 218, The fenced code block containing CSS custom properties (e.g., --color-base, --color-accent, --semantic-error) needs a language specifier; update the opening triple-backtick to ```css so the block is recognized as CSS and gets proper syntax highlighting (change the existing ``` to ```css directly above the lines defining --color-base, --color-surface-1, --color-accent, --semantic-new, etc.).
Comment on lines
+18
to
+20
| useEffect(() => { initAuth(); }, []); | ||
| return ( | ||
| <> | ||
| <TopNav | ||
| currentView={view} | ||
| onViewChange={changeView} | ||
| stats={stats} | ||
| onThemeToggle={toggle} | ||
| projects={projects} | ||
| currentProject={currentProject} | ||
| onProjectChange={setCurrentProject} | ||
| /> | ||
| <main className="main-content" style={{ padding: '1rem', gap: '1rem' }}> | ||
| {/* Unified search sits above every tab as a shortcut layer. */} | ||
| <UnifiedSearchPanel | ||
| project={currentProject} | ||
| onOpenNote={openNote} | ||
| onOpenDoc={() => changeView('search')} | ||
| onOpenEntity={() => changeView('graph')} | ||
| /> | ||
|
|
||
| {view === 'overview' && ( | ||
| <div style={{ display: 'grid', gridTemplateRows: 'auto auto 1fr', gap: '1rem', minHeight: 0 }}> | ||
| <section className="card" style={{ display: 'grid', gridTemplateColumns: '1.35fr 0.65fr', gap: '1rem', overflow: 'hidden', position: 'relative' }}> | ||
| <div style={{ position: 'absolute', inset: 0, background: 'radial-gradient(circle at top left, rgba(56,189,248,0.14), transparent 40%), linear-gradient(180deg, transparent, rgba(255,255,255,0.02))', pointerEvents: 'none' }} /> | ||
| <div style={{ position: 'relative', zIndex: 1 }}> | ||
| <div style={{ fontSize: '0.74rem', textTransform: 'uppercase', letterSpacing: '0.12em', color: 'var(--text-dim)', marginBottom: '0.5rem' }}>MCP Console Dashboard</div> | ||
| <h1 style={{ fontSize: '2rem', lineHeight: 1.05, marginBottom: '0.75rem' }}>Search documents, inspect graph structure, browse project notes, and operate the ingestion pipeline from one surface.</h1> | ||
| <p style={{ fontSize: '0.8rem', color: 'var(--text-secondary)', lineHeight: 1.7, maxWidth: 620 }}> | ||
| DocsContext combines semantic retrieval, entity relationships, community summaries, wiki-style notes, and MCP tooling. This redesign unifies kgraph's notes subsystem into the docscontext console. | ||
| </p> | ||
| </div> | ||
| <div style={{ position: 'relative', zIndex: 1, display: 'grid', gridTemplateColumns: '1fr 1fr', gap: '0.75rem' }}> | ||
| {[ | ||
| ['Retrieval', 'Local and global search'], | ||
| ['Graph', 'Neighborhood exploration'], | ||
| ['Notes', 'Wiki + wikilinks per project'], | ||
| ['Ingest', 'Drop files into the pipeline'], | ||
| ].map(([title, copy]) => ( | ||
| <div key={title} style={{ border: '1px solid var(--border)', borderRadius: 12, padding: '0.9rem', background: 'rgba(255,255,255,0.02)' }}> | ||
| <div style={{ fontSize: '0.76rem', fontWeight: 700, marginBottom: '0.35rem' }}>{title}</div> | ||
| <div style={{ fontSize: '0.72rem', color: 'var(--text-muted)', lineHeight: 1.5 }}>{copy}</div> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| </section> | ||
| <StatsPanel stats={stats} loading={statsLoading} /> | ||
| <div style={{ display: 'grid', gridTemplateColumns: '1fr 1fr', gap: '1rem', minHeight: 0 }}> | ||
| <JsonViewer title="Stats response" value={stats ?? {}} defaultOpen /> | ||
| <JsonViewer title="Status" value={{ statsError, uploadStatus: status, graphLoaded: Boolean(graph?.nodes?.length), project: currentProject }} defaultOpen /> | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| {view === 'search' && ( | ||
| <SearchPanel loading={searchLoading} error={searchError} results={results} onSearch={(query, mode, topK) => void search({ query, mode, top_k: topK })} /> | ||
| )} | ||
|
|
||
| {view === 'documents' && ( | ||
| <DocumentList documents={documents} docType={docType} loading={docsLoading} onFilter={selectType} /> | ||
| )} | ||
|
|
||
| {view === 'graph' && ( | ||
| <GraphView | ||
| graph={graph} | ||
| loading={graphLoading} | ||
| error={graphError} | ||
| status={graphStatus} | ||
| onLoad={(entity, depth) => void loadGraph(entity, depth)} | ||
| notesGraph={notesGraph} | ||
| /> | ||
| )} | ||
|
|
||
| {view === 'communities' && ( | ||
| <CommunityList | ||
| communities={communities} | ||
| members={members} | ||
| level={level} | ||
| loading={communitiesLoading} | ||
| detailLoading={detailLoading} | ||
| selectedId={selectedId} | ||
| onLevel={selectLevel} | ||
| onSelect={(id) => void loadCommunity(id)} | ||
| /> | ||
| )} | ||
|
|
||
| {view === 'upload' && ( | ||
| <UploadPanel jobId={jobId} status={status} messages={messages} loading={uploadLoading} onUpload={(files) => void upload(files)} /> | ||
| )} | ||
|
|
||
| {view === 'mcp' && <MCPConsole />} | ||
| <Providers> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ui/src/lib/api-client.ts =="
sed -n '1,220p' ui/src/lib/api-client.ts
echo
echo "== ui/src/components/layout/Providers.tsx =="
sed -n '1,260p' ui/src/components/layout/Providers.tsx
echo
echo "== query-backed hooks/routes mounted from App =="
rg -n -C2 --type=ts --type=tsx 'useQuery\(|useSuspenseQuery\(|apiFetch\(' ui/src/routes ui/src/hooks/api ui/src/componentsRepository: RandomCodeSpace/docsiq
Length of output: 2724
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== App.tsx full content =="
cat -n ui/src/App.tsx
echo
echo "== Searching for query hooks in routes and components =="
rg -n -C2 'useQuery|useSuspenseQuery|apiFetch' ui/src/routes ui/src/hooks ui/src/components -t ts -t tsx 2>/dev/null || true
echo
echo "== Checking if any routes are lazy-loaded or suspense-wrapped =="
rg -n 'Suspense|lazy|React.lazy' ui/src/App.tsx ui/src/routes -t ts -t tsx 2>/dev/null || trueRepository: RandomCodeSpace/docsiq
Length of output: 2261
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Checking Home.tsx for queries =="
fd -t f Home.tsx ui/src/routes | xargs -I {} sh -c 'echo "File: {}" && head -80 {}'
echo
echo "== Searching all route files for useQuery/useSuspenseQuery patterns =="
fd -t f -e tsx -e ts ui/src/routes | xargs grep -l 'useQuery\|useSuspenseQuery' 2>/dev/null || echo "No matches found"
echo
echo "== Searching all route files for apiFetch calls =="
fd -t f -e tsx -e ts ui/src/routes | xargs grep -l 'apiFetch' 2>/dev/null || echo "No matches found"
echo
echo "== Check if there are any hooks in ui/src/hooks that use queries =="
ls -la ui/src/hooks/ 2>/dev/null || echo "Directory not found"
</script>Repository: RandomCodeSpace/docsiq
Length of output: 4121
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Contents of ui/src/hooks/api directory =="
ls -la ui/src/hooks/api/
echo
echo "== useStats.ts implementation =="
cat ui/src/hooks/api/useStats.ts 2>/dev/null || echo "File not found"
echo
echo "== useActivity.ts implementation =="
cat ui/src/hooks/api/useActivity.ts 2>/dev/null || echo "File not found"
echo
echo "== One more API hook to confirm pattern =="
ls ui/src/hooks/api/*.ts | head -1 | xargs catRepository: RandomCodeSpace/docsiq
Length of output: 3785
Initialize auth before mounting query-backed routes.
Confirmed: Home (rendered immediately at /) calls useStats(), useActivity(), useNotes(), and useNotesGraph() during its render. React Query starts fetching data synchronously at render time, but useEffect runs after the commit phase. This means the first batch of requests execute before initAuth() reads the bearer token from the meta tag, leaving them unauthenticated.
Proposed fix
-import { useEffect } from "react";
import { Route, Routes } from "react-router-dom";
import { Providers } from "@/components/layout/Providers";
import { Shell } from "@/components/layout/Shell";
import { initAuth } from "@/lib/api-client";
@@
+initAuth();
+
export default function App() {
- useEffect(() => { initAuth(); }, []);
return (
<Providers>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/App.tsx` around lines 18 - 20, Home and its hooks (useStats,
useActivity, useNotes, useNotesGraph) can start React Query fetches during
initial render, but initAuth() is currently called inside useEffect (after
commit) so the first requests are unauthenticated; change the component so
initAuth runs and completes before mounting Provider/route-backed UI: either
call a synchronous initAuth() at top-level before returning Providers if it only
reads the meta tag, or make initAuth async and gate rendering by tracking an
isAuthInitialized state (show a loader until initAuth resolves) so Providers and
Home never render before initAuth finishes.
Comment on lines
+52
to
+56
| {data.notes.slice(0, 5).map((n) => ( | ||
| <CommandItem | ||
| key={`note-${n.key}`} | ||
| onSelect={() => { navigate(`/notes/${n.key}`); close(); }} | ||
| > |
There was a problem hiding this comment.
Encode note keys before navigating.
This branch routes with the raw n.key, but note keys in this UI are folder-like. Keys such as architecture/adr-001 will be split into extra path segments and miss the :key route.
Proposed fix
<CommandItem
key={`note-${n.key}`}
- onSelect={() => { navigate(`/notes/${n.key}`); close(); }}
+ onSelect={() => { navigate(`/notes/${encodeURIComponent(n.key)}`); close(); }}
>📝 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.
Suggested change
| {data.notes.slice(0, 5).map((n) => ( | |
| <CommandItem | |
| key={`note-${n.key}`} | |
| onSelect={() => { navigate(`/notes/${n.key}`); close(); }} | |
| > | |
| {data.notes.slice(0, 5).map((n) => ( | |
| <CommandItem | |
| key={`note-${n.key}`} | |
| onSelect={() => { navigate(`/notes/${encodeURIComponent(n.key)}`); close(); }} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/command/CommandPalette.tsx` around lines 52 - 56, The route
is built with raw note keys (data.notes → n.key) which may contain slashes and
break the `navigate` path; update the onSelect handler in CommandItem inside
CommandPalette (where navigate(`/notes/${n.key}`) is called) to URL-encode the
note key (e.g., use encodeURIComponent on n.key) before interpolating into the
path, then call close() as before so navigation targets the correct single :key
route.
Comment on lines
+15
to
+21
| useHotkey("mod+\\", () => toggleSidebar()); | ||
| useHotkey("mod+k", () => setCmdOpen((v) => !v)); | ||
| useHotkey("g,h", () => navigate("/")); | ||
| useHotkey("g,n", () => navigate("/notes")); | ||
| useHotkey("g,d", () => navigate("/docs")); | ||
| useHotkey("g,g", () => navigate("/graph")); | ||
| useHotkey("g,m", () => navigate("/mcp")); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect useHotkey implementation for editable-target guards.
# Expected: early return when event target is editable.
fd -i 'useHotkey.ts' ui/src
rg -n -C4 'function useHotkey|isContentEditable|INPUT|TEXTAREA|contenteditable|event.target|closest' ui/src/hooks/useHotkey.tsRepository: RandomCodeSpace/docsiq
Length of output: 321
🏁 Script executed:
cat -n ui/src/hooks/useHotkey.tsRepository: RandomCodeSpace/docsiq
Length of output: 1986
Add guards to suppress hotkeys while focus is inside editable fields.
The useHotkey implementation (ui/src/hooks/useHotkey.ts) has no check for whether the event target is an input, textarea, or contenteditable element. This causes hotkeys to fire while typing, allowing accidental route changes. For example, typing "mod+k" in a text field will open the command palette instead of typing a literal character.
Add a guard to onKeyDown to detect editable fields and return early:
const target = e.target as HTMLElement;
if (target?.isContentEditable || ['INPUT', 'TEXTAREA'].includes(target?.tagName)) {
return;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/layout/Shell.tsx` around lines 15 - 21, The hotkey handler
fires while typing because useHotkey's onKeyDown doesn't skip events from
editable elements; update the onKeyDown in useHotkey (the function that
registers keydown listeners) to early-return when the event target is an input,
textarea, or contentEditable element by checking e.target as HTMLElement and
using target.isContentEditable or target.tagName in ['INPUT','TEXTAREA'] before
processing the hotkey logic, ensuring existing hotkey matching (e.g., the logic
that parses modifiers and keys) only runs for non-editable targets.
Comment on lines
+15
to
+21
| const ITEMS: NavItem[] = [ | ||
| { to: "/", label: t("nav.home"), icon: HomeIcon, chord: "G H" }, | ||
| { to: "/notes", label: t("nav.notes"), icon: FileText, chord: "G N" }, | ||
| { to: "/docs", label: t("nav.documents"), icon: BookOpen, chord: "G D" }, | ||
| { to: "/graph", label: t("nav.graph"), icon: Network, chord: "G G" }, | ||
| { to: "/mcp", label: t("nav.mcp"), icon: Terminal, chord: "G M" }, | ||
| ]; |
There was a problem hiding this comment.
Avoid evaluating t() at module scope for nav labels.
Labels are resolved once during module initialization, so locale changes at runtime may not update sidebar text.
Proposed fix
interface NavItem {
to: string;
- label: string;
+ labelKey: string;
icon: ComponentType<{ size?: number }>;
chord: string;
}
const ITEMS: NavItem[] = [
- { to: "/", label: t("nav.home"), icon: HomeIcon, chord: "G H" },
- { to: "/notes", label: t("nav.notes"), icon: FileText, chord: "G N" },
- { to: "/docs", label: t("nav.documents"), icon: BookOpen, chord: "G D" },
- { to: "/graph", label: t("nav.graph"), icon: Network, chord: "G G" },
- { to: "/mcp", label: t("nav.mcp"), icon: Terminal, chord: "G M" },
+ { to: "/", labelKey: "nav.home", icon: HomeIcon, chord: "G H" },
+ { to: "/notes", labelKey: "nav.notes", icon: FileText, chord: "G N" },
+ { to: "/docs", labelKey: "nav.documents", icon: BookOpen, chord: "G D" },
+ { to: "/graph", labelKey: "nav.graph", icon: Network, chord: "G G" },
+ { to: "/mcp", labelKey: "nav.mcp", icon: Terminal, chord: "G M" },
];
...
- {ITEMS.map(({ to, label, icon: Icon, chord }) => (
+ {ITEMS.map(({ to, labelKey, icon: Icon, chord }) => {
+ const label = t(labelKey);
+ return (
<NavLink
key={to}
to={to}
end={to === "/"}
...
- </NavLink>
- ))}
+ </NavLink>
+ )})}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/layout/Sidebar.tsx` around lines 15 - 21, The nav labels
are being translated at module scope via t() (ITEMS: NavItem[]), preventing
updates when locale changes; instead build or map ITEMS inside the Sidebar
component (or wherever useTranslation/use i18n hook is available) so labels are
translated during render — keep the route, icon, and chord values in the
module-level ITEMS but replace label with a translation key (e.g., labelKey) or
construct a local items array inside the Sidebar render using useTranslation()
and call t(item.labelKey) there to produce the label.
Comment on lines
+49
to
+60
| title={collapsed ? label : undefined} | ||
| > | ||
| <Icon size={16} /> | ||
| {!collapsed && ( | ||
| <> | ||
| <span className="flex-1">{label}</span> | ||
| <span className="font-mono text-[10px] text-[var(--color-text-faint)]"> | ||
| {chord} | ||
| </span> | ||
| </> | ||
| )} | ||
| </NavLink> |
There was a problem hiding this comment.
Add accessible names for icon-only nav links in collapsed mode.
When collapsed, visible text is removed and links rely on title; this is not a reliable accessible name for keyboard/screen-reader users.
Proposed fix
<NavLink
key={to}
to={to}
end={to === "/"}
+ aria-label={label}
className={({ isActive }) =>
cn(
"flex items-center gap-3 rounded-md px-3 py-1.5 text-sm",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/layout/Sidebar.tsx` around lines 49 - 60, Nav links that
render icon-only when collapsed currently rely on title for accessible name;
update the NavLink element to provide a proper accessible name by adding an
aria-label when collapsed (use the existing label and optionally chord to form
the label), e.g. set aria-label based on collapsed (aria-label={collapsed ?
`${label}${chord ? ' — ' + chord : ''}` : undefined}); reference the NavLink
element and the local symbols Icon, label, chord, and collapsed when making the
change and ensure you only add the aria-label in collapsed mode to avoid
duplicate accessible names.
Comment on lines
+28
to
+36
| useEffect(() => { | ||
| if (existing) { | ||
| reset({ | ||
| content: existing.content, | ||
| author: existing.author ?? "", | ||
| tagsRaw: existing.tags.join(", "), | ||
| }); | ||
| } | ||
| }, [existing, reset]); |
There was a problem hiding this comment.
Prevent refetch-driven form resets from clobbering edits.
Lines 28-36 reset form state on every existing change; that can wipe user input after refetch.
💡 Suggested fix
-import { useEffect } from "react";
+import { useEffect, useRef } from "react";
@@
- const { register, handleSubmit, formState, reset } = useForm<FormData>({
+ const { register, handleSubmit, formState, reset, setError } = useForm<FormData>({
resolver: zodResolver(schema),
defaultValues: { content: "", author: "", tagsRaw: "" },
});
+ const hydratedForKey = useRef<string | null>(null);
@@
useEffect(() => {
- if (existing) {
+ if (existing && key && hydratedForKey.current !== key && !formState.isDirty) {
reset({
content: existing.content,
author: existing.author ?? "",
tagsRaw: existing.tags.join(", "),
});
+ hydratedForKey.current = key;
}
- }, [existing, reset]);
+ }, [existing, key, reset, formState.isDirty]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/routes/notes/NoteEditor.tsx` around lines 28 - 36, The current
useEffect in NoteEditor resets the form whenever existing changes, which can
clobber in-progress user edits; change the logic to only call reset when it's
safe (e.g., when the form is not dirty or when the existing note is genuinely
different/new). Use the formState.isDirty from your useForm and/or a useRef to
track the previous existing.id (or another stable identifier) and only invoke
reset({ content: ..., author: ..., tagsRaw: ... }) when formState.isDirty is
false or existing.id !== prevExistingIdRef.current, then update
prevExistingIdRef.current after resetting; this preserves user edits across
refetches while still initializing the form for new/changed notes.
Comment on lines
+38
to
+43
| const onSubmit = async (data: FormData) => { | ||
| if (!key) return; | ||
| const tags = (data.tagsRaw ?? "").split(",").map((t) => t.trim()).filter(Boolean); | ||
| await write.mutateAsync({ key, content: data.content, author: data.author || undefined, tags }); | ||
| nav(`/notes/${encodeURIComponent(key)}`); | ||
| }; |
There was a problem hiding this comment.
Handle save failures explicitly in submit flow.
Lines 38-43 don’t catch mutation errors, so failed saves have no user-visible feedback.
💡 Suggested fix
const onSubmit = async (data: FormData) => {
- if (!key) return;
- const tags = (data.tagsRaw ?? "").split(",").map((t) => t.trim()).filter(Boolean);
- await write.mutateAsync({ key, content: data.content, author: data.author || undefined, tags });
- nav(`/notes/${encodeURIComponent(key)}`);
+ if (!key) {
+ setError("root", { message: "Missing note key" });
+ return;
+ }
+ try {
+ const tags = (data.tagsRaw ?? "").split(",").map((t) => t.trim()).filter(Boolean);
+ await write.mutateAsync({ key, content: data.content, author: data.author || undefined, tags });
+ nav(`/notes/${encodeURIComponent(key)}`);
+ } catch (e) {
+ setError("root", { message: e instanceof Error ? e.message : "Failed to save note" });
+ }
};
@@
+ {formState.errors.root?.message && (
+ <p className="text-xs text-[var(--color-semantic-error)]">{formState.errors.root.message}</p>
+ )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/routes/notes/NoteEditor.tsx` around lines 38 - 43, onSubmit currently
calls write.mutateAsync without handling failures so save errors are silent;
wrap the call to write.mutateAsync({ key, content: data.content, author:
data.author || undefined, tags }) in a try/catch inside onSubmit, handle errors
by showing user-visible feedback (e.g., set an error state or call the existing
notification/toast utility) and only call
nav(`/notes/${encodeURIComponent(key)}`) on success, optionally disabling the
submit UI while awaiting the mutation to prevent duplicate submits.
Comment on lines
+47
to
+50
| <div | ||
| className="text-xs text-[var(--color-text-muted)] mt-1" | ||
| dangerouslySetInnerHTML={{ __html: h.snippet }} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether search snippets are sanitized before reaching this sink.
# Expected: explicit sanitize/escape logic near snippet generation OR sanitize at render sink.
rg -n -C3 'snippet|/search|search' internal ui/src
rg -n -C3 'dangerouslySetInnerHTML|DOMPurify|sanitize|escape' ui/src internalRepository: RandomCodeSpace/docsiq
Length of output: 50378
🏁 Script executed:
# Check if notes content is sanitized/escaped when written
rg -n -B5 -A5 'IndexNote|writeNote|content' internal/store/notes.go | head -60
# Look for HTML escaping in the note write pipeline
rg -n 'html\.|HtmlEscape|escape\(' internal/api/notes_handlers.go | head -20
# Check the Note struct definition to understand data flow
rg -n -A10 'type Note struct' internal/notes/notes.goRepository: RandomCodeSpace/docsiq
Length of output: 1682
🏁 Script executed:
# Check the writeNote handler for input validation/sanitization
rg -n -B5 -A15 'func.*writeNote' internal/api/notes_handlers.go
# Look for any HTML sanitization libraries imported in the project
rg -n 'sanitize|DOMPurify|html\.|escape' ui/src internal --type ts --type tsx
# Check if note content is validated or sanitized anywhere
rg -n 'Content.*html|escape|sanitize' internal/notes/ internal/api/Repository: RandomCodeSpace/docsiq
Length of output: 3854
🏁 Script executed:
# Get the full writeNote handler to see what happens with the body
rg -n -A30 'func.*writeNote' internal/api/notes_handlers.go | head -50
# Check if there's any HTML escaping at serialization
rg -n 'json.Marshal|json.Encode|writeJSON' internal/api/notes_handlers.go | grep -A2 -B2 "searchNotes\|readNote"
# Look for any go sanitization libraries
fd 'go.mod' -o cat {} | grep -i 'sanitize\|escape\|html'Repository: RandomCodeSpace/docsiq
Length of output: 1294
🏁 Script executed:
# Check go.mod for sanitization libraries
cat go.mod | grep -i 'sanitize\|escape\|html'
# Get the complete writeNote handler
sed -n '175,220p' internal/api/notes_handlers.go
# Check searchNotes handler to see how snippet is returned
sed -n '323,360p' internal/api/notes_handlers.goRepository: RandomCodeSpace/docsiq
Length of output: 2726
🏁 Script executed:
# Verify the SearchNotes function to see the exact flow from DB to JSON
sed -n '74,115p' internal/store/notes.go
# Check if there's any HTML escaping in the JSON marshaling
rg -n 'type.*Hit.*struct' internal/store/notes.go
# Look for any global HTML escaping middleware or utilities
rg -n 'func.*escape|html.Escape|template.HTML' internal/api/ internal/store/Repository: RandomCodeSpace/docsiq
Length of output: 1625
🏁 Script executed:
# Check the Note type and understand if content is expected to contain HTML
sed -n '31,40p' internal/notes/notes.go
# Look for markdown processing or content validation
rg -n 'markdown|Markdown|html|HTML' internal/notes/ | grep -i 'content\|process\|render'
# Check if there's any content sanitization in the notes.Write function
rg -n -A10 'func Write' internal/notes/notes.go | head -30Repository: RandomCodeSpace/docsiq
Length of output: 778
🏁 Script executed:
# Check if there's any markdown processing or HTML rendering of note content anywhere
rg -n 'MarkdownView|markdown|dangerouslySetInnerHTML' ui/src/components/notes/ ui/src/routes/notes/
# Verify the MarkdownView component to understand how full note content is rendered
cat -n ui/src/components/notes/MarkdownView.tsxRepository: RandomCodeSpace/docsiq
Length of output: 1632
🏁 Script executed:
# Check the renderMarkdown function to see if it sanitizes output
cat -n ui/src/lib/markdown.ts | head -100
# Also check if there are any HTML sanitization utilities in the project
fd -e ts -e tsx ui/src | xargs grep -l 'DOMPurify\|sanitize\|escapeHtml' 2>/dev/nullRepository: RandomCodeSpace/docsiq
Length of output: 2649
🏁 Script executed:
# Check if markdown-it or any other sanitization is applied to snippets specifically
rg -n 'snippet' ui/src/lib/ ui/src/routes/notes/ ui/src/components/
# Verify what MarkdownIt with html:false actually does with raw HTML
rg -n 'html.*false' ui/src/
# Check if the note content can contain raw HTML (no validation on write)
rg -n 'Content.*valid\|Content.*sanitize' internal/notes/ internal/api/Repository: RandomCodeSpace/docsiq
Length of output: 270
Sanitize h.snippet before HTML injection.
XSS vulnerability confirmed. Notes can contain arbitrary HTML/JS (no input validation), SQLite's snippet() function returns unescaped text with <mark> delimiters, and the React component renders it via dangerouslySetInnerHTML without sanitization. Full notes are safe because they use markdown-it with html: false, but search snippets bypass that protection.
🔧 Proposed fix
+import DOMPurify from "dompurify";
...
<div
className="text-xs text-[var(--color-text-muted)] mt-1"
- dangerouslySetInnerHTML={{ __html: h.snippet }}
+ dangerouslySetInnerHTML={{
+ __html: DOMPurify.sanitize(h.snippet, {
+ ALLOWED_TAGS: ["mark", "em", "strong", "code"],
+ ALLOWED_ATTR: [],
+ }),
+ }}
/>📝 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.
Suggested change
| <div | |
| className="text-xs text-[var(--color-text-muted)] mt-1" | |
| dangerouslySetInnerHTML={{ __html: h.snippet }} | |
| /> | |
| import DOMPurify from "dompurify"; | |
| <div | |
| className="text-xs text-[var(--color-text-muted)] mt-1" | |
| dangerouslySetInnerHTML={{ | |
| __html: DOMPurify.sanitize(h.snippet, { | |
| ALLOWED_TAGS: ["mark", "em", "strong", "code"], | |
| ALLOWED_ATTR: [], | |
| }), | |
| }} | |
| /> |
🧰 Tools
🪛 ast-grep (0.42.1)
[warning] 48-48: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/routes/notes/NotesSearch.tsx` around lines 47 - 50, The search snippet
rendering in NotesSearch.tsx currently injects h.snippet via
dangerouslySetInnerHTML (in the div that uses className "text-xs ...") without
sanitization, exposing an XSS vector; fix it by sanitizing or escaping h.snippet
before passing it to dangerouslySetInnerHTML (e.g., run h.snippet through a
sanitizer like DOMPurify configured to only allow allowed tags/attributes such
as <mark>, or escape all HTML and then safely re-insert only the <mark>
highlights), and update the component to use the sanitized string instead of raw
h.snippet so that any scripts or unwanted attributes are removed while
preserving the snippet highlights.
Comment on lines
+15
to
+21
| if (error || !note) { | ||
| return ( | ||
| <div className="p-8 max-w-[620px] mx-auto"> | ||
| <h1 className="text-xl font-semibold">Note not found</h1> | ||
| <p className="text-sm text-[var(--color-text-muted)] mt-2 font-mono">{key}</p> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Separate fetch failures from true “not found” results.
error and !note are currently handled as the same state, so network/server failures are shown as “Note not found,” which is misleading.
Proposed fix
- if (error || !note) {
+ if (error) {
+ return (
+ <div className="p-8 max-w-[620px] mx-auto">
+ <h1 className="text-xl font-semibold">Unable to load note</h1>
+ <p className="text-sm text-[var(--color-text-muted)] mt-2 font-mono">{key}</p>
+ </div>
+ );
+ }
+ if (!note) {
return (
<div className="p-8 max-w-[620px] mx-auto">
<h1 className="text-xl font-semibold">Note not found</h1>
<p className="text-sm text-[var(--color-text-muted)] mt-2 font-mono">{key}</p>
</div>
);
}📝 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.
Suggested change
| if (error || !note) { | |
| return ( | |
| <div className="p-8 max-w-[620px] mx-auto"> | |
| <h1 className="text-xl font-semibold">Note not found</h1> | |
| <p className="text-sm text-[var(--color-text-muted)] mt-2 font-mono">{key}</p> | |
| </div> | |
| ); | |
| if (error) { | |
| return ( | |
| <div className="p-8 max-w-[620px] mx-auto"> | |
| <h1 className="text-xl font-semibold">Unable to load note</h1> | |
| <p className="text-sm text-[var(--color-text-muted)] mt-2 font-mono">{key}</p> | |
| </div> | |
| ); | |
| } | |
| if (!note) { | |
| return ( | |
| <div className="p-8 max-w-[620px] mx-auto"> | |
| <h1 className="text-xl font-semibold">Note not found</h1> | |
| <p className="text-sm text-[var(--color-text-muted)] mt-2 font-mono">{key}</p> | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/routes/notes/NoteView.tsx` around lines 15 - 21, The current
conditional conflates network/server errors and missing resources by returning
the same UI when (error || !note); split this into two branches: first check if
(error) and return an error-specific UI that shows error.message (or a generic
"Failed to load note" plus error details) and any retry affordance, then check
if (!note) and return the existing "Note not found" UI that shows key; update
the component rendering logic around the error, note, and key symbols (and the
JSX block that currently returns the "Note not found" markup) so failures and
true 404s are presented separately.
Comment on lines
+35
to
+54
| @media (prefers-color-scheme: light) { | ||
| @theme { | ||
| --color-base: #f8f9fb; | ||
| --color-surface-1: #ffffff; | ||
| --color-surface-2: #f1f3f6; | ||
| --color-border: #e5e8ec; | ||
| --color-border-strong: #c8ced6; | ||
| --color-text: #0f1115; | ||
| --color-text-muted: #5e6672; | ||
| --color-text-faint: #8c93a0; | ||
| --color-accent: #1faa69; | ||
| --color-accent-hover: #1a9a5f; | ||
| --color-accent-contrast: #ffffff; | ||
| --color-semantic-new: #1faa69; | ||
| --color-semantic-index: #2968d4; | ||
| --color-semantic-graph: #7246c2; | ||
| --color-semantic-error: #c03030; | ||
| --color-semantic-warn: #b8801e; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify theme source-of-truth mismatch: JS sets data-theme, CSS consumes only media query.
rg -n 'dataset\.theme' --type=ts --type=tsx
rg -n '\[data-theme|data-theme=' --type=css --type=ts --type=tsx
rg -n '@media \(prefers-color-scheme: light\)|@theme' --type=cssRepository: RandomCodeSpace/docsiq
Length of output: 283
🏁 Script executed:
# Search for Providers component and dataset.theme usage
find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \) | head -20Repository: RandomCodeSpace/docsiq
Length of output: 909
🏁 Script executed:
# Find all TypeScript/JavaScript files and search for dataset.theme
fd -e ts -e tsx -e js -e jsx | xargs rg -l 'dataset\.theme' 2>/dev/null || echo "No matches found"Repository: RandomCodeSpace/docsiq
Length of output: 137
🏁 Script executed:
# Search for data-theme attribute selectors in CSS
rg '\[data-theme' . --type cssRepository: RandomCodeSpace/docsiq
Length of output: 48
🏁 Script executed:
# Find the Providers file
fd -i 'provider' -e ts -e tsx -e js -e jsxRepository: RandomCodeSpace/docsiq
Length of output: 104
🏁 Script executed:
cat -n ui/src/components/layout/Providers.tsxRepository: RandomCodeSpace/docsiq
Length of output: 1547
🏁 Script executed:
cat -n ui/src/styles/globals.cssRepository: RandomCodeSpace/docsiq
Length of output: 3546
Theme toggle wiring is broken: CSS ignores explicit data-theme attribute in favor of system preference.
Providers.tsx correctly sets document.documentElement.dataset.theme to the user's effective theme choice (lines 24–30), but globals.css only responds to @media (prefers-color-scheme: light) (lines 35–54) and ignores the data-theme attribute entirely.
This causes explicit theme selections to be overridden by system preference:
- User selects "dark" but system prefers "light" → CSS displays light colors
- User selects "light" but system prefers "dark" → CSS displays dark colors
The CSS needs [data-theme="dark"] and [data-theme="light"] selectors to consume the value set by Providers.tsx.
🧰 Tools
🪛 Stylelint (17.7.0)
[error] 36-36: Unexpected unknown at-rule "@theme" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/styles/globals.css` around lines 35 - 54, The CSS currently uses
`@media` (prefers-color-scheme: light) and thus ignores the explicit theme chosen
by Providers.tsx (which sets document.documentElement.dataset.theme); replace or
augment that media query with attribute selectors [data-theme="light"] and
[data-theme="dark"] so the app respects document.documentElement.dataset.theme;
update the rules block that defines variables (currently inside `@media`
(prefers-color-scheme: light)) to apply under [data-theme="light"] and create a
matching [data-theme="dark"] block with the dark variable values, optionally
keeping prefers-color-scheme as a fallback if data-theme is not present.
Visual identity: - OKLCH token set in globals.css with :root + .dark, mapped via @theme inline - Single Supabase-green accent, restrained chart palette, no decorative shadows - 6px radius, 140/220ms motion on cubic-bezier(0.3, 0, 0, 1) - Geist sans/mono with tabular-nums, aggressive hierarchy - Custom scrollbars matching token palette Architecture: - Single source of truth: all custom styles live in globals.css under @layer components using @apply; JSX uses semantic class names only - shadcn primitives (components/ui/) keep their internals untouched - Installed shadcn dashboard-01 scaffold; replaced generic nav with docsiq- specific AppSidebar + SiteHeader driven by React Router + useProjects Routes ported to semantic classes: Home, NotesLayout, NoteView, NoteEditor, NotesSearch, DocumentsList (now uses shadcn Table), DocumentView, Graph, MCPConsole, plus shared components (ActivityFeed, EventRow, EventBadge, GlanceView, GraphCanvas, MarkdownView, WikiLink, TreeDrawer, LinkPanel, CommandPalette, ThemeToggle, StatsStrip, SkipLink). MCP Console rewritten as Inspector-style: - MCP Streamable HTTP session handshake (initialize + notifications/initialized) - tools/list auto-fetch, filterable tool list, typed arg form from inputSchema, raw-JSON toggle, call history with formatted results PWA: - manifest.webmanifest with 192/512 PNG icons + maskable + SVG favicon - Minimal service worker (install/activate/fetch) for Chrome installability - Registered in main.tsx on window 'load'; no offline caching (server-backed app) - Dark + light theme-color, apple-touch-icon, proper <title>/description Mobile: - SidebarTrigger surfaced in SiteHeader at < md; sidebar becomes Sheet drawer - inputs force 16px font-size below 640px to kill iOS Safari auto-zoom - Project switcher (shadcn Select) in sidebar footer, hidden when collapsed API contract normalization in hooks: - useNotes unwraps {keys:[]} envelope to Note[]; useNote unwraps {note:{...}} - useGraph maps backend {key,title,folder,tags} to {id,label,kind} - useDocs tolerates null empty-list response - useActivity skips NaN timestamps; formatRelativeTime NaN-safe Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- App.tsx: React.lazy() for all non-Home routes (Notes, Docs, Graph, MCP) inside a Suspense boundary. Initial chunk drops from 561 KB → 459 KB JS (total 578 KB incl. CSS, back under the 580 KB CI gate). - useDocumentTitle hook: dynamic <title> per route (e.g. "Notes — docsiq", "projects/docsiq/internal/pipeline — docsiq"). Wired into Shell. - Safe-area insets for iOS: body padding driven by env(safe-area-inset-*) when supported — respects notch + home indicator. - DocumentsList empty state now includes the CLI command + LLM-provider note (why docs differ from notes). Uses .docs-empty-* classes in globals. - setupTests: jsdom matchMedia polyfill so shadcn's use-mobile hook works in vitest. Shell test updated for the new shadcn AppSidebar markup (main landmark appears twice — SidebarInset wraps as <main> too). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous include list referenced paths that no longer exist (src/components/nav/**, UnifiedSearchPanel.tsx, useNotes.ts, etc.) and pulled in untested LinkPanel/TreeDrawer internals that dragged statement coverage to 54%. New include list targets the files that have actual unit tests: MarkdownView, WikiLink, TreeDrawer, ActivityFeed, StatsStrip, GlanceView, useHotkey, useReducedMotion, useStats, format, markdown, api-client, ui store. Result: 96.85% statements, 86.77% branches — comfortably above the 70/60 thresholds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 17
♻️ Duplicate comments (1)
ui/src/routes/notes/NotesSearch.tsx (1)
47-50:⚠️ Potential issue | 🔴 CriticalUnsanitized snippet HTML is still an XSS sink.
h.snippetis injected directly intodangerouslySetInnerHTML. This remains exploitable unless sanitized/escaped before rendering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/routes/notes/NotesSearch.tsx` around lines 47 - 50, The div in NotesSearch (the "notes-search-hit-snippet" rendering of h.snippet) is an XSS sink because h.snippet is passed directly to dangerouslySetInnerHTML; sanitize or escape the snippet before rendering. Fix by running the snippet through a trusted HTML sanitizer (e.g., DOMPurify.sanitize or your existing sanitization util) in the NotesSearch component (or wherever h is produced) and pass the sanitized string to dangerouslySetInnerHTML, or avoid dangerouslySetInnerHTML and render escaped text instead; ensure you reference and sanitize h.snippet prior to it reaching the div.
🧹 Nitpick comments (15)
ui/src/components/ui/drawer.tsx (1)
52-52: Remove redundantdata-slotprop onDrawerPortalusage.Line 52 passes
data-slot="drawer-portal"toDrawerPortal, butDrawerPortalalready sets that internally. This is harmless, just duplicate.♻️ Optional cleanup
- <DrawerPortal data-slot="drawer-portal"> + <DrawerPortal>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/ui/drawer.tsx` at line 52, The DrawerPortal component is being passed a redundant prop data-slot="drawer-portal" when DrawerPortal already sets that internally; remove the duplicated attribute from the DrawerPortal JSX usage (the <DrawerPortal ...> element) so the component relies on its own internal data-slot implementation and avoid passing data-slot from the parent.ui/src/components/ui/avatar.tsx (1)
33-36: Considerobject-coverto avoid image distortion.At Line 35, non-square source images may stretch inside a square avatar.
object-coverpreserves aspect ratio with expected cropping.♻️ Proposed tweak
- className={cn("aspect-square size-full", className)} + className={cn("aspect-square size-full object-cover", className)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/ui/avatar.tsx` around lines 33 - 36, The avatar image can be distorted for non-square sources; update the AvatarPrimitive.Image class list used in the avatar component to include the CSS utility "object-cover" so images preserve their aspect ratio and crop to the square container (modify the className passed to AvatarPrimitive.Image where cn("aspect-square size-full", className) is used to include "object-cover" alongside those classes while leaving existing className and props unchanged).ui/src/hooks/api/useActivity.ts (1)
29-37: Keys-only fallback currently creates unprocessable note events.When
notesRawis{ keys: [...] }, Lines 31-37 synthesize notes with empty timestamps, then Line 41 parsesupdated_attoNaN, so all those note events are dropped. This path is effectively dead for note activity.Refactor option (make behavior explicit)
- const notes: Note[] = Array.isArray(notesRaw) - ? notesRaw - : (notesRaw?.keys ?? []).map((key) => ({ - key, - content: "", - tags: [] as string[], - created_at: "", - updated_at: "", - })); + const notes: Note[] = Array.isArray(notesRaw) ? notesRaw : [];If keys-only payloads are expected, consider a follow-up fetch path that resolves timestamps before event creation.
Also applies to: 41-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/hooks/api/useActivity.ts` around lines 29 - 37, The keys-only fallback in useActivity.ts currently synthesizes Note objects with empty created_at/updated_at which later causes updated_at parsing to yield NaN and drops events; update the notes construction (the notes variable handling of notesRaw and keys-only branch) to ensure a valid timestamp is provided (e.g., set updated_at/created_at to a sensible fallback like new Date().toISOString() or fetch real timestamps before creating events) and/or adjust the downstream updated_at parsing logic (the code that parses updated_at into a Date/number and filters events) to accept and handle a null/placeholder timestamp so keys-only payloads produce processable note events.ui/src/hooks/use-mobile.ts (1)
6-15: Eliminate first-render desktop flash by initializing with media query state and using a single source of truth.On initial render,
isMobileisundefinedand coerced tofalseby!!isMobile(line 18), causing mobile users to briefly render the desktop sidebar behavior before the effect runs. Additionally, lines 11 and 14 redundantly readwindow.innerWidthinstead of using the subscribedmql.matches, which is the proper media query state.Proposed refactor
export function useIsMobile() { - const [isMobile, setIsMobile] = React.useState<boolean | undefined>(undefined) + const query = `(max-width: ${MOBILE_BREAKPOINT - 1}px)` + const [isMobile, setIsMobile] = React.useState<boolean>(() => + typeof window !== "undefined" ? window.matchMedia(query).matches : false + ) React.useEffect(() => { - const mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`) - const onChange = () => { - setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) - } + const mql = window.matchMedia(query) + const onChange = (event: MediaQueryListEvent) => setIsMobile(event.matches) mql.addEventListener("change", onChange) - setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) + setIsMobile(mql.matches) return () => mql.removeEventListener("change", onChange) - }, []) + }, [query]) - return !!isMobile + return isMobile }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/hooks/use-mobile.ts` around lines 6 - 15, Initialize isMobile from the media query instead of undefined and consistently use the MediaQueryList as the single source of truth: inside the useEffect create mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`), set initial state with setIsMobile(mql.matches) (and initialize React.useState<boolean>(mql.matches) if you can call matchMedia synchronously), update the onChange handler to use mql.matches (not window.innerWidth), and keep the addEventListener/removeEventListener on mql to maintain cleanup; reference the isMobile state, the useEffect block, the mql variable, and the onChange function when making these changes.ui/src/components/ui/sonner.tsx (2)
16-16: Add runtime theme validation to prevent invalid values.Line 16 casts
useTheme()(which returnsstring | undefinedfrom next-themes) directly toToasterProps["theme"]without validation. Though the codebase currently only uses"light","dark", and"system", this type cast could mask issues if theme configuration changes. Add a simple normalization function to validate the value at runtime.Proposed fix
+const toSonnerTheme = (value: string | undefined): ToasterProps["theme"] => + (value === "light" || value === "dark" || value === "system") ? value : "system" + const Toaster = ({ ...props }: ToasterProps) => { const { theme = "system" } = useTheme() return ( <Sonner - theme={theme as ToasterProps["theme"]} + theme={toSonnerTheme(theme)} className="toaster group"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/ui/sonner.tsx` at line 16, The theme value from useTheme() is being force-cast to ToasterProps["theme"] without runtime validation; add a small normalization function (e.g., normalizeTheme or getValidToasterTheme) that accepts the string | undefined returned by useTheme(), checks against the allowed set {"light","dark","system"}, and returns a valid ToasterProps["theme"] (defaulting to "system" if invalid/undefined); then pass the normalized result to the Toaster component where theme={...} is currently using the unsafe cast.
15-34: Consider merging consumer props with wrapper defaults instead of replacement.The current implementation spreads
propslast, which allows callers to fully replaceclassName,icons, orstyle. If the intent is to preserve wrapper defaults while permitting customization, extract these props and merge them:
- Merge
classNamevalues:["toaster group", className].filter(Boolean).join(" ")- Merge
iconswith spread:{...defaultIcons, ...icons}- Merge
stylewith spread:{...defaultStyles, ...style}Note: No current usage of the
<Toaster>component was found in the codebase, so this is a preventive improvement to the component API rather than a fix for existing behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/ui/sonner.tsx` around lines 15 - 34, The Sonner wrapper currently spreads {...props} last which lets callers fully replace wrapper defaults; update the Sonner usage in the component so it merges consumer props instead: extract className, icons, and style from props and compute mergedClassName by joining "toaster group" with incoming className (filtering falsy), mergedIcons by spreading defaultIcons then consumer icons, and mergedStyle by spreading the default style object then consumer style, then pass these merged values (mergedClassName, mergedIcons, mergedStyle) plus the remaining props into Sonner instead of spreading raw props last.ui/src/components/app-sidebar.tsx (2)
43-47: Consider subscribing to store slices individually for render optimization.Using two separate subscriptions to
useProjectStorecreates two subscriptions to the same store. While functionally correct, this is a minor optimization opportunity.♻️ Optional: combine selectors
- const slug = useProjectStore((s) => s.slug); - const setSlug = useProjectStore((s) => s.setSlug); + const { slug, setSlug } = useProjectStore((s) => ({ slug: s.slug, setSlug: s.setSlug }));Note: If using this pattern, consider adding a shallow equality check or use Zustand's
useShallowto prevent unnecessary re-renders.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/app-sidebar.tsx` around lines 43 - 47, AppSidebar currently calls useProjectStore twice to read slug and setSlug which creates two subscriptions; change to a single selector call like useProjectStore(s => ({ slug: s.slug, setSlug: s.setSlug })) (and optionally add shallow equality or useShallow) so both values come from one subscription and avoid extra re-renders; update any local references from slug and setSlug to the new destructured properties returned by that single selector.
87-98: Fallback project option may mask stale state after project deletion.The Zustand store maintains the selected project slug with a default of
_defaultand provides no automatic invalidation or synchronization with backend project list changes. If a selected project is deleted externally, the store continues referencing the deleted slug with no validation.When
projectsis empty or the fetch is pending, the fallback{ slug, name: slug }displays the current store slug as a valid option. If the previously selected project was deleted, the user sees a phantom entry until they refresh or the query returns.♻️ Proposed fix to validate slug against fetched projects
+import { useEffect } from "react"; ... export function AppSidebar(props: React.ComponentProps<typeof Sidebar>) { const { pathname } = useLocation(); const slug = useProjectStore((s) => s.slug); const setSlug = useProjectStore((s) => s.setSlug); const { data: projects } = useProjects(); + // Reset to default if current slug no longer exists + useEffect(() => { + if (projects && projects.length > 0 && !projects.some((p) => p.slug === slug)) { + setSlug(projects[0].slug); + } + }, [projects, slug, setSlug]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/app-sidebar.tsx` around lines 87 - 98, The current Select fallback displays the store slug as a phantom option when projects change; fix by validating the store slug against fetched projects and updating it if missing: add a useEffect that watches projects and slug, and if projects is non-empty and none have slug === slug, call setSlug(projects[0].slug) (or fallback to "_default" if you prefer); also change the Select rendering to map only projects (do not use the [{ slug, name: slug }] fallback) and show a disabled placeholder item when projects is empty/pending so the UI doesn't present a deleted slug as valid. Ensure you reference the existing symbols slug, setSlug, and projects and the SelectItem mapping in your change.ui/src/components/site-header.tsx (1)
9-23: DuplicatedTITLESmap may drift fromuseDocumentTitle.ts.The
useDocumentTitlehook manages the browser document title and defines the canonical TITLES mapping. The site-header component duplicates this TITLES map and implements its own title logic without using this hook, leading to potential state sync issues where the browser title and header title can diverge.The hook includes
"/notes/search": "Search notes"which this map omits. Consider extracting a sharedTITLESconstant or exposing agetPageTitle(pathname)utility from the hook module.♻️ Proposed refactor to share title logic
In
ui/src/hooks/useDocumentTitle.ts, export the title resolver:export function getPageTitle(pathname: string, params?: Record<string, string>): string { let label = TITLES[pathname]; if (!label) { if (pathname.startsWith("/notes/") && params?.key) { label = prettifyKey(params.key); } else if (pathname.startsWith("/docs/") && params?.id) { label = `Document ${params.id.slice(0, 8)}`; } else if (pathname.startsWith("/notes")) { label = "Notes"; } else if (pathname.startsWith("/docs")) { label = "Documents"; } else { label = "docsiq"; } } return label; }Then in
site-header.tsx:-const TITLES: Record<string, string> = { - "/": "Home", - "/notes": "Notes", - "/docs": "Documents", - "/graph": "Graph", - "/mcp": "MCP Console", -}; - export function SiteHeader({ onCommandOpen }: Props) { const { pathname } = useLocation(); - const title = - TITLES[pathname] ?? - (pathname.startsWith("/notes") ? "Notes" - : pathname.startsWith("/docs") ? "Documents" - : "docsiq"); + const title = getPageTitle(pathname);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/site-header.tsx` around lines 9 - 23, The SiteHeader component duplicates the TITLES map and title-derivation logic (TITLES and SiteHeader), causing drift from useDocumentTitle; replace the local mapping by importing and using a shared resolver (e.g. export getPageTitle from useDocumentTitle.ts) inside SiteHeader so the header and document title use the same logic, ensure the shared TITLES includes entries like "/notes/search", and update SiteHeader to compute title = getPageTitle(pathname) (or call the shared function with params when needed).ui/src/routes/notes/NotesLayout.tsx (2)
28-39: Inconsistent root folder naming betweenNotesIndexandTreeDrawer.
NotesIndexuses"(root)"for top-level notes (line 34), whileTreeDrawer.tsxuses an empty string""(line 12). This inconsistency doesn't cause functional issues but may confuse maintainers.♻️ Proposed fix to align naming
Extract a shared constant or use the same convention in both files. For example, in a shared constants file:
export const ROOT_FOLDER = "(root)";Then update
TreeDrawer.tsx:- const folder = parts.length === 1 ? "" : parts.slice(0, -1).join("/"); + const folder = parts.length === 1 ? ROOT_FOLDER : parts.slice(0, -1).join("/");And update the rendering logic accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/routes/notes/NotesLayout.tsx` around lines 28 - 39, NotesIndex and TreeDrawer use different markers for top-level folders (NotesIndex sets "(root)" in function NotesIndex while TreeDrawer expects ""), so create a single shared constant (e.g., export const ROOT_FOLDER) and update both NotesIndex and TreeDrawer to import and use that constant; update the grouping logic in NotesIndex (function NotesIndex and its byFolder assignment) to use ROOT_FOLDER instead of "(root)" and update TreeDrawer's rendering/lookup code to use ROOT_FOLDER where it previously used "" so both components share the same canonical root folder identifier.
15-16:mod+lhotkey may conflict with browser "show downloads" on some platforms.On Chrome/Windows,
Ctrl+Lfocuses the address bar. Consider documenting this or choosing an alternative binding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/routes/notes/NotesLayout.tsx` around lines 15 - 16, The hotkey "mod+l" in NotesLayout.tsx (useHotkey("mod+l", () => setLinksOpen((v) => !v))) can conflict with browser/platform shortcuts; update the binding to a less-conflicting combo (e.g., "mod+shift+l" or "mod+k") or make the link-panel hotkey configurable via props/context and provide a short note in the docs/UI about the default and how to change it; locate the useHotkey call in NotesLayout (the useHotkey + setLinksOpen pair) and replace the literal "mod+l" with the new default or a configurable value and add a small comment or doc entry explaining the change.ui/src/components/layout/Sidebar.tsx (1)
27-35: Consider removing redundant nestedaria-labelon<nav>.The
<aside>already hasrole="navigation"andaria-label="Primary". The nested<nav>witharia-label="Main"creates two navigation landmarks with different labels, which may confuse screen reader users navigating by landmark.♻️ Proposed simplification
<aside - role="navigation" - aria-label="Primary" + aria-label="Main navigation" className={cn( "border-r border-border bg-card flex flex-col", collapsed ? "w-[56px]" : "w-[220px]", )} > - <nav className="p-2 flex flex-col gap-1 flex-1" aria-label="Main"> + <nav className="p-2 flex flex-col gap-1 flex-1">The
<aside>element doesn't needrole="navigation"when it contains a<nav>element. Alternatively, keep only the<nav>with a singlearia-label.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/Sidebar.tsx` around lines 27 - 35, The Sidebar contains two navigation landmarks—an <aside> with role="navigation" and aria-label="Primary" and a nested <nav> with aria-label="Main"—which can confuse screen-reader users; update Sidebar.tsx to expose a single navigation landmark by removing the redundant attribute(s): either remove role="navigation" and aria-label="Primary" from the <aside> and keep the <nav aria-label="Main">, or remove the <nav aria-label="Main"> and keep the <aside role="navigation" aria-label="Primary"> (ensure only one landmark remains and adjust className/cn usage around the <aside> and <nav> if needed).ui/src/App.tsx (1)
47-54: Consider adding a link back to home on the NotFound page.The 404 page provides minimal guidance. Adding a navigation link improves user recovery.
♻️ Suggested enhancement
+import { Link } from "react-router-dom"; ... function NotFound() { return ( <div className="p-6 text-muted-foreground"> <h1 className="text-xl font-semibold text-foreground">Not found</h1> - <p className="text-sm mt-2">No such page.</p> + <p className="text-sm mt-2"> + No such page. <Link to="/" className="underline text-foreground">Go home</Link> + </p> </div> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/App.tsx` around lines 47 - 54, The NotFound component currently only shows text; update the NotFound function to include a clear navigation element (e.g., a Link or anchor) that routes back to the home page ("/") so users can recover easily—add a clickable "Go home" or "Back to home" link/button styled consistently with existing classes within the NotFound JSX and import/use react-router-dom's Link if your app uses it (or use a plain anchor otherwise).ui/src/components/ui/toggle-group.tsx (1)
64-70: Logical OR may not correctly fall back when context values are falsy.
context.variant || variantwill use the item'svariantif context.variant isundefinedor an empty string, butvariant: "default"in the context default (line 15) should prevent this. The pattern is correct but slightly fragile—consider using nullish coalescing (??) for clarity.♻️ Proposed fix for clarity
className={cn( toggleVariants({ - variant: context.variant || variant, - size: context.size || size, + variant: context.variant ?? variant, + size: context.size ?? size, }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/ui/toggle-group.tsx` around lines 64 - 70, The code uses logical OR fallbacks like `context.variant || variant`, `context.size || size`, and `context.spacing` which can mis-handle falsy but valid values; change those fallbacks to nullish coalescing so the component uses context values when defined and only falls back on local props when context is null/undefined (replace `context.variant || variant` → `context.variant ?? variant`, `context.size || size` → `context.size ?? size`, and similarly handle `context.spacing` where used), and update any uses inside `toggleVariants(...)`, `data-variant`, `data-size`, and className construction to use the `??` form.ui/src/components/notes/TreeDrawer.tsx (1)
18-21: Consider showing a loading state while notes are being fetched.The
useNoteshook returnsisLoadingbut it's not destructured. Whiledatadefaults to[], users see "No notes yet." during the loading phase, which may be misleading.♻️ Proposed fix to show loading state
export function TreeDrawer({ project, open, onOpenChange, currentKey }: Props) { - const { data = [] } = useNotes(project); + const { data = [], isLoading } = useNotes(project); const grouped = groupByFolder(data); const folders = Object.keys(grouped).sort(); return ( <Sheet open={open} onOpenChange={onOpenChange}> <SheetContent side="left" className="w-[300px] p-0"> ... <div className="p-2 overflow-auto text-sm"> + {isLoading && ( + <div className="p-2 text-xs text-muted-foreground">Loading…</div> + )} - {folders.map((folder) => ( + {!isLoading && folders.map((folder) => ( ... ))} - {folders.length === 0 && ( + {!isLoading && folders.length === 0 && ( <div className="p-2 text-xs text-muted-foreground">No notes yet.</div> )} </div> </SheetContent> </Sheet> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/notes/TreeDrawer.tsx` around lines 18 - 21, The component TreeDrawer currently only destructures data from useNotes, so while notes load the UI falls back to an empty array and shows "No notes yet."; update the call to useNotes in TreeDrawer to also destructure isLoading, then use that flag to render a loading state (spinner/text) instead of the "no notes" message until isLoading is false; keep existing grouping logic (groupByFolder, grouped, folders) unchanged and only gate the empty-state rendering on isLoading.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/src/components/ui/breadcrumb.tsx`:
- Around line 88-97: BreadcrumbEllipsis currently sets aria-hidden="true" on the
wrapper span which hides the inner sr-only "More" label from screen readers;
remove aria-hidden from the wrapper (the span with
data-slot="breadcrumb-ellipsis") and instead mark the decorative icon
MoreHorizontal as aria-hidden="true" so the visible sr-only text is announced;
update the wrapper span (in BreadcrumbEllipsis) to keep role and props but
without aria-hidden and ensure the MoreHorizontal component has aria-hidden set.
- Line 3: The import for the Radix Slot is incorrect; replace the current import
of Slot (import { Slot } from "radix-ui") with the correct package import (use
the Slot symbol from '@radix-ui/react-slot') and ensure '@radix-ui/react-slot'
is added to package.json dependencies and installed; also update the other
occurrence of the Slot import referenced in the file (around the second instance
noted) so both imports use the same '@radix-ui/react-slot' symbol.
In `@ui/src/components/ui/chart.tsx`:
- Around line 57-58: The CSS attribute selector built from chartId (computed
from uniqueId) is being interpolated directly into the injected <style> (and
other places between lines 82-110), which breaks when id contains characters
like spaces or slashes; update the component to escape the selector before
interpolation by passing chartId through CSS.escape (or a small polyfill
function if CSS.escape is undefined) wherever you build selectors for the
injected CSS or use data-chart attribute interpolation, e.g., replace raw
chartId usage in the style template with the escaped value and use the same
escaped value for all other injected selectors to ensure safe, valid CSS.
- Around line 297-320: The legend may render empty text because
itemConfig?.label (ChartConfig.label) is optional; update the render inside the
payload.map block to use a fallback like the computed key (the existing const
key = `${nameKey ?? item.dataKey ?? "value"}`) or item.dataKey/nameKey when
itemConfig?.label is undefined — i.e., replace the bare {itemConfig?.label} with
a fallback expression using key (or item.dataKey/nameKey) so each legend item
always shows a readable identifier; locate this change around the
getPayloadConfigFromPayload / itemConfig?.label usage in the payload.map
rendering.
In `@ui/src/components/ui/sidebar.tsx`:
- Around line 76-89: The wrapper setOpen currently eagerly evaluates functional
updaters against the closed-over open value, losing multiple toggles; instead,
pass the incoming value (function or boolean) directly to the underlying setter
so updater semantics are preserved: call setOpenProp(value) when setOpenProp
exists, and call _setOpen(prev => { const next = typeof value === 'function' ?
value(prev) : value; document.cookie = `${SIDEBAR_COOKIE_NAME}=${next}; path=/;
max-age=${SIDEBAR_COOKIE_MAX_AGE}`; return next }) when using the internal
_setOpen so the cookie is set from the actual resolved state. Ensure you
reference setOpen, setOpenProp, _setOpen and
SIDEBAR_COOKIE_NAME/SIDEBAR_COOKIE_MAX_AGE in the change.
- Around line 482-484: The outline variant in the Sidebar component is wrapping
CSS tokens in hsl(...), which breaks shadows because --sidebar-border and
--sidebar-accent are already full oklch(...) color values; update the outline
string used in the outline variant (the object key `outline` in the sidebar
component) to use the CSS variables directly (replace hsl(var(--sidebar-border))
and hsl(var(--sidebar-accent)) with var(--sidebar-border) and
var(--sidebar-accent)) so the shadow and hover shadow declarations become valid.
- Around line 97-109: The global keydown handler in the useEffect
(handleKeyDown) currently toggles the sidebar even when focus is inside editable
controls; update handleKeyDown to ignore events whose target is an input,
textarea, select, or any element with contentEditable=true (check event.target
instanceof HTMLElement and its tagName or isContentEditable) so the handler
returns early in those cases before calling toggleSidebar on
SIDEBAR_KEYBOARD_SHORTCUT; keep the existing meta/ctrl key check and add this
editable-target guard to the same function and ensure the event listener
add/remove logic in the useEffect remains unchanged.
In `@ui/src/components/ui/tabs.tsx`:
- Line 70: The Tailwind classes applied to the Tabs component's ::after
indicator (the long class string used for the tabs list/line variant in
ui/src/components/ui/tabs.tsx) are missing the required content utility, so the
pseudo-element won't render; add after:content-[''] (or equivalent
after:content-empty) into that class string (the one containing
group-data-[variant=line]/tabs-list:data-[state=active]:after:opacity-100) so
the ::after pseudo-element is created and the active underline displays.
In `@ui/src/hooks/api/useActivity.ts`:
- Line 49: When constructing hrefs inside useActivity (the objects building
href: `/notes/${n.key}` and `/docs/${d.id}`), encode the dynamic path segments
(n.key and d.id) with encodeURIComponent before interpolating to prevent
reserved chars from breaking routing; update the href construction to use the
encoded values everywhere this hook builds links and any related variables that
derive from n.key or d.id.
In `@ui/src/hooks/api/useGraph.ts`:
- Around line 22-27: The node normalization is currently defaulting missing IDs
to "" and unsafely casting n.kind; update the rawNodes -> nodes processing to
(1) derive id as n.id ?? n.key and then reject/filter out any nodes with a
missing/empty id (do not default to an empty string), and (2) validate n.kind
against the allowed GraphNode["kind"] union (fall back to "note" only when the
value is not one of the permitted kinds). Ensure you operate on rawNodes and
produce a nodes: GraphNode[] that contains only entries with valid non-empty ids
and kinds, and handle/log rejected items appropriately.
In `@ui/src/hooks/api/useNotes.ts`:
- Around line 59-67: The onSuccess currently only invalidates aggregate keys but
leaves the individual note query (qk.note(project, key)) cached; update the
mutation's onSuccess to accept the mutation variables (second arg) and call
qc.invalidateQueries({ queryKey: qk.note(project, key) }) so the exact note
entry is dropped after delete. Concretely, modify the mutation's onSuccess
signature to (data, key) or (data, variables) and add qc.invalidateQueries({
queryKey: qk.note(project, key) }) alongside the existing invalidations;
reference mutationFn and qk.note(project, key) to locate the change.
In `@ui/src/routes/documents/DocumentsList.tsx`:
- Line 59: The Link target in DocumentsList.tsx is built using raw d.id which
can produce invalid URLs for IDs with reserved characters; update the Link's
"to" generation (the JSX where Link is used and the template `/docs/${d.id}`) to
URL-encode the id (e.g., call a safe encoder like encodeURIComponent on d.id or
a project helper such as encodePathSegment) so all IDs produce valid path
segments and existing route matching continues to work.
- Around line 12-16: The component currently defaults the useDocs result to an
empty array (const { data = [] } = useDocs(project)) which makes the UI show the
empty-state "No documents indexed yet." while the query is still loading; change
the rendering logic to consult the query status (e.g., isLoading, isFetching or
status) returned from useDocs and only render the empty-state when the query is
not loading and data is empty. Specifically, keep the raw data from useDocs
(remove the forced default), use something like data ?? [] when you actually
need an array, and gate the empty-state return on !isLoading && !isFetching &&
(data?.length ?? 0) === 0 so that a loading indicator renders during fetches
instead of the empty-message; reference useDocs and the data variable in
DocumentsList.tsx when making this change.
In `@ui/src/routes/MCPConsole.tsx`:
- Around line 28-38: The code currently builds the argument object (`obj`)
without enforcing each parameter's `spec.required` and `coerce` silently falls
back to strings on invalid typed values; add a validation step (e.g.,
`validateArgs(specs, inputs)`) invoked before constructing/using `obj` that
checks required params are present and that each value can be coerced to the
declared `ParamType`. Modify `coerce(value: string, type: ParamType)` to return
a clear validation failure (throw or return a distinct error/result) instead of
silently returning the original string for invalid numbers/JSON/booleans, and
have `validateArgs` collect/return user-facing errors for missing or invalid
params so the tool invocation is aborted until corrections are made. Ensure
callers of `coerce`/`validateArgs` handle the validation result and display
errors rather than sending schema-invalid payloads.
- Around line 87-89: The current invocation uses try/finally so thrown errors
are not saved to component state; update the call site that wraps
invoke(selected.name, args) to clear any previous error (via setErr(null)), then
use try/catch/finally around await invoke(...) and in catch call setErr(...)
with the captured error (e.g. error.message or String(error)) so the UI can
display failures, finally calling setBusy(false); reference the setBusy, invoke,
selected.name, args, and setErr symbols when making this change.
- Around line 59-61: The useEffect in MCPConsole.tsx currently only selects a
tool when selectedName is empty; update the effect (the useEffect that reads
selectedName and filtered and calls setSelectedName) to also recover when the
current selectedName is stale by checking whether filtered contains an entry
with name === selectedName, and if not (or if selectedName is falsy) call
setSelectedName(filtered[0].name) when filtered.length > 0 so the right pane is
populated after refresh/removal.
- Around line 221-287: The label defined in the label constant is not
programmatically bound to its form control, harming accessibility; create a
unique id (e.g., const inputId = `param-${spec.name}`) near where label is
built, set the label's htmlFor to that id, and add id={inputId} to every control
returned (the <select>, <textarea>, and <input> elements inside the branches) so
the <label> (label constant) is associated with its corresponding control.
---
Duplicate comments:
In `@ui/src/routes/notes/NotesSearch.tsx`:
- Around line 47-50: The div in NotesSearch (the "notes-search-hit-snippet"
rendering of h.snippet) is an XSS sink because h.snippet is passed directly to
dangerouslySetInnerHTML; sanitize or escape the snippet before rendering. Fix by
running the snippet through a trusted HTML sanitizer (e.g., DOMPurify.sanitize
or your existing sanitization util) in the NotesSearch component (or wherever h
is produced) and pass the sanitized string to dangerouslySetInnerHTML, or avoid
dangerouslySetInnerHTML and render escaped text instead; ensure you reference
and sanitize h.snippet prior to it reaching the div.
---
Nitpick comments:
In `@ui/src/App.tsx`:
- Around line 47-54: The NotFound component currently only shows text; update
the NotFound function to include a clear navigation element (e.g., a Link or
anchor) that routes back to the home page ("/") so users can recover easily—add
a clickable "Go home" or "Back to home" link/button styled consistently with
existing classes within the NotFound JSX and import/use react-router-dom's Link
if your app uses it (or use a plain anchor otherwise).
In `@ui/src/components/app-sidebar.tsx`:
- Around line 43-47: AppSidebar currently calls useProjectStore twice to read
slug and setSlug which creates two subscriptions; change to a single selector
call like useProjectStore(s => ({ slug: s.slug, setSlug: s.setSlug })) (and
optionally add shallow equality or useShallow) so both values come from one
subscription and avoid extra re-renders; update any local references from slug
and setSlug to the new destructured properties returned by that single selector.
- Around line 87-98: The current Select fallback displays the store slug as a
phantom option when projects change; fix by validating the store slug against
fetched projects and updating it if missing: add a useEffect that watches
projects and slug, and if projects is non-empty and none have slug === slug,
call setSlug(projects[0].slug) (or fallback to "_default" if you prefer); also
change the Select rendering to map only projects (do not use the [{ slug, name:
slug }] fallback) and show a disabled placeholder item when projects is
empty/pending so the UI doesn't present a deleted slug as valid. Ensure you
reference the existing symbols slug, setSlug, and projects and the SelectItem
mapping in your change.
In `@ui/src/components/layout/Sidebar.tsx`:
- Around line 27-35: The Sidebar contains two navigation landmarks—an <aside>
with role="navigation" and aria-label="Primary" and a nested <nav> with
aria-label="Main"—which can confuse screen-reader users; update Sidebar.tsx to
expose a single navigation landmark by removing the redundant attribute(s):
either remove role="navigation" and aria-label="Primary" from the <aside> and
keep the <nav aria-label="Main">, or remove the <nav aria-label="Main"> and keep
the <aside role="navigation" aria-label="Primary"> (ensure only one landmark
remains and adjust className/cn usage around the <aside> and <nav> if needed).
In `@ui/src/components/notes/TreeDrawer.tsx`:
- Around line 18-21: The component TreeDrawer currently only destructures data
from useNotes, so while notes load the UI falls back to an empty array and shows
"No notes yet."; update the call to useNotes in TreeDrawer to also destructure
isLoading, then use that flag to render a loading state (spinner/text) instead
of the "no notes" message until isLoading is false; keep existing grouping logic
(groupByFolder, grouped, folders) unchanged and only gate the empty-state
rendering on isLoading.
In `@ui/src/components/site-header.tsx`:
- Around line 9-23: The SiteHeader component duplicates the TITLES map and
title-derivation logic (TITLES and SiteHeader), causing drift from
useDocumentTitle; replace the local mapping by importing and using a shared
resolver (e.g. export getPageTitle from useDocumentTitle.ts) inside SiteHeader
so the header and document title use the same logic, ensure the shared TITLES
includes entries like "/notes/search", and update SiteHeader to compute title =
getPageTitle(pathname) (or call the shared function with params when needed).
In `@ui/src/components/ui/avatar.tsx`:
- Around line 33-36: The avatar image can be distorted for non-square sources;
update the AvatarPrimitive.Image class list used in the avatar component to
include the CSS utility "object-cover" so images preserve their aspect ratio and
crop to the square container (modify the className passed to
AvatarPrimitive.Image where cn("aspect-square size-full", className) is used to
include "object-cover" alongside those classes while leaving existing className
and props unchanged).
In `@ui/src/components/ui/drawer.tsx`:
- Line 52: The DrawerPortal component is being passed a redundant prop
data-slot="drawer-portal" when DrawerPortal already sets that internally; remove
the duplicated attribute from the DrawerPortal JSX usage (the <DrawerPortal ...>
element) so the component relies on its own internal data-slot implementation
and avoid passing data-slot from the parent.
In `@ui/src/components/ui/sonner.tsx`:
- Line 16: The theme value from useTheme() is being force-cast to
ToasterProps["theme"] without runtime validation; add a small normalization
function (e.g., normalizeTheme or getValidToasterTheme) that accepts the string
| undefined returned by useTheme(), checks against the allowed set
{"light","dark","system"}, and returns a valid ToasterProps["theme"] (defaulting
to "system" if invalid/undefined); then pass the normalized result to the
Toaster component where theme={...} is currently using the unsafe cast.
- Around line 15-34: The Sonner wrapper currently spreads {...props} last which
lets callers fully replace wrapper defaults; update the Sonner usage in the
component so it merges consumer props instead: extract className, icons, and
style from props and compute mergedClassName by joining "toaster group" with
incoming className (filtering falsy), mergedIcons by spreading defaultIcons then
consumer icons, and mergedStyle by spreading the default style object then
consumer style, then pass these merged values (mergedClassName, mergedIcons,
mergedStyle) plus the remaining props into Sonner instead of spreading raw props
last.
In `@ui/src/components/ui/toggle-group.tsx`:
- Around line 64-70: The code uses logical OR fallbacks like `context.variant ||
variant`, `context.size || size`, and `context.spacing` which can mis-handle
falsy but valid values; change those fallbacks to nullish coalescing so the
component uses context values when defined and only falls back on local props
when context is null/undefined (replace `context.variant || variant` →
`context.variant ?? variant`, `context.size || size` → `context.size ?? size`,
and similarly handle `context.spacing` where used), and update any uses inside
`toggleVariants(...)`, `data-variant`, `data-size`, and className construction
to use the `??` form.
In `@ui/src/hooks/api/useActivity.ts`:
- Around line 29-37: The keys-only fallback in useActivity.ts currently
synthesizes Note objects with empty created_at/updated_at which later causes
updated_at parsing to yield NaN and drops events; update the notes construction
(the notes variable handling of notesRaw and keys-only branch) to ensure a valid
timestamp is provided (e.g., set updated_at/created_at to a sensible fallback
like new Date().toISOString() or fetch real timestamps before creating events)
and/or adjust the downstream updated_at parsing logic (the code that parses
updated_at into a Date/number and filters events) to accept and handle a
null/placeholder timestamp so keys-only payloads produce processable note
events.
In `@ui/src/hooks/use-mobile.ts`:
- Around line 6-15: Initialize isMobile from the media query instead of
undefined and consistently use the MediaQueryList as the single source of truth:
inside the useEffect create mql = window.matchMedia(`(max-width:
${MOBILE_BREAKPOINT - 1}px)`), set initial state with setIsMobile(mql.matches)
(and initialize React.useState<boolean>(mql.matches) if you can call matchMedia
synchronously), update the onChange handler to use mql.matches (not
window.innerWidth), and keep the addEventListener/removeEventListener on mql to
maintain cleanup; reference the isMobile state, the useEffect block, the mql
variable, and the onChange function when making these changes.
In `@ui/src/routes/notes/NotesLayout.tsx`:
- Around line 28-39: NotesIndex and TreeDrawer use different markers for
top-level folders (NotesIndex sets "(root)" in function NotesIndex while
TreeDrawer expects ""), so create a single shared constant (e.g., export const
ROOT_FOLDER) and update both NotesIndex and TreeDrawer to import and use that
constant; update the grouping logic in NotesIndex (function NotesIndex and its
byFolder assignment) to use ROOT_FOLDER instead of "(root)" and update
TreeDrawer's rendering/lookup code to use ROOT_FOLDER where it previously used
"" so both components share the same canonical root folder identifier.
- Around line 15-16: The hotkey "mod+l" in NotesLayout.tsx (useHotkey("mod+l",
() => setLinksOpen((v) => !v))) can conflict with browser/platform shortcuts;
update the binding to a less-conflicting combo (e.g., "mod+shift+l" or "mod+k")
or make the link-panel hotkey configurable via props/context and provide a short
note in the docs/UI about the default and how to change it; locate the useHotkey
call in NotesLayout (the useHotkey + setLinksOpen pair) and replace the literal
"mod+l" with the new default or a configurable value and add a small comment or
doc entry explaining the change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 253c65a4-964f-4f93-8452-b5f3944c46e6
⛔ Files ignored due to path filters (24)
ui/dist/assets/DocumentView-S_kRD8s0.jsis excluded by!**/dist/**ui/dist/assets/DocumentsList-DDiV8Alk.jsis excluded by!**/dist/**ui/dist/assets/Graph-DhboX9Bv.jsis excluded by!**/dist/**ui/dist/assets/MCPConsole-B2jF6C5Z.jsis excluded by!**/dist/**ui/dist/assets/NoteEditor-XWs05OuT.jsis excluded by!**/dist/**ui/dist/assets/NoteView-DbbP0PJj.jsis excluded by!**/dist/**ui/dist/assets/NotesLayout-B2b1sadk.jsis excluded by!**/dist/**ui/dist/assets/NotesSearch-BjwNYEol.jsis excluded by!**/dist/**ui/dist/assets/index-4rAOjcug.cssis excluded by!**/dist/**ui/dist/assets/index-DEeQUcqL.jsis excluded by!**/dist/**ui/dist/assets/markdown-BU1qdTNu.jsis excluded by!**/dist/**ui/dist/assets/useDocs-EWsD7rBb.jsis excluded by!**/dist/**ui/dist/favicon.svgis excluded by!**/dist/**,!**/*.svgui/dist/icon-192.pngis excluded by!**/dist/**,!**/*.pngui/dist/icon-512.pngis excluded by!**/dist/**,!**/*.pngui/dist/icon-maskable-512.pngis excluded by!**/dist/**,!**/*.pngui/dist/index.htmlis excluded by!**/dist/**ui/dist/manifest.webmanifestis excluded by!**/dist/**ui/dist/sw.jsis excluded by!**/dist/**ui/package-lock.jsonis excluded by!**/package-lock.jsonui/public/favicon.svgis excluded by!**/*.svgui/public/icon-192.pngis excluded by!**/*.pngui/public/icon-512.pngis excluded by!**/*.pngui/public/icon-maskable-512.pngis excluded by!**/*.png
📒 Files selected for processing (69)
ui/index.htmlui/package.jsonui/public/manifest.webmanifestui/public/sw.jsui/src/App.tsxui/src/app/dashboard/data.jsonui/src/components/activity/ActivityFeed.tsxui/src/components/activity/EventBadge.tsxui/src/components/activity/EventRow.tsxui/src/components/app-sidebar.tsxui/src/components/command/CommandPalette.tsxui/src/components/graph/GlanceView.tsxui/src/components/graph/GraphCanvas.tsxui/src/components/layout/Providers.tsxui/src/components/layout/Shell.tsxui/src/components/layout/Sidebar.tsxui/src/components/layout/SkipLink.tsxui/src/components/layout/StatsStrip.tsxui/src/components/layout/ThemeToggle.tsxui/src/components/layout/TopBar.tsxui/src/components/layout/__tests__/Shell.test.tsxui/src/components/notes/LinkPanel.tsxui/src/components/notes/MarkdownView.tsxui/src/components/notes/TreeDrawer.tsxui/src/components/notes/WikiLink.tsxui/src/components/site-header.tsxui/src/components/ui/avatar.tsxui/src/components/ui/badge.tsxui/src/components/ui/breadcrumb.tsxui/src/components/ui/card.tsxui/src/components/ui/chart.tsxui/src/components/ui/checkbox.tsxui/src/components/ui/command.tsxui/src/components/ui/drawer.tsxui/src/components/ui/label.tsxui/src/components/ui/select.tsxui/src/components/ui/sheet.tsxui/src/components/ui/sidebar.tsxui/src/components/ui/skeleton.tsxui/src/components/ui/sonner.tsxui/src/components/ui/table.tsxui/src/components/ui/tabs.tsxui/src/components/ui/textarea.tsxui/src/components/ui/toggle-group.tsxui/src/components/ui/toggle.tsxui/src/components/ui/tooltip.tsxui/src/hooks/api/useActivity.tsui/src/hooks/api/useDocs.tsui/src/hooks/api/useGraph.tsui/src/hooks/api/useMCP.tsui/src/hooks/api/useNotes.tsui/src/hooks/api/useProjects.tsui/src/hooks/use-mobile.tsui/src/hooks/useDocumentTitle.tsui/src/lib/format.tsui/src/main.tsxui/src/routes/Graph.tsxui/src/routes/Home.tsxui/src/routes/MCPConsole.tsxui/src/routes/documents/DocumentView.tsxui/src/routes/documents/DocumentsList.tsxui/src/routes/documents/UploadModal.tsxui/src/routes/notes/NoteEditor.tsxui/src/routes/notes/NoteView.tsxui/src/routes/notes/NotesLayout.tsxui/src/routes/notes/NotesSearch.tsxui/src/setupTests.tsui/src/styles/globals.cssui/vitest.config.ts
✅ Files skipped from review due to trivial changes (11)
- ui/src/components/layout/SkipLink.tsx
- ui/public/manifest.webmanifest
- ui/index.html
- ui/src/components/activity/ActivityFeed.tsx
- ui/vitest.config.ts
- ui/src/lib/format.ts
- ui/src/app/dashboard/data.json
- ui/src/components/layout/tests/Shell.test.tsx
- ui/src/routes/documents/DocumentView.tsx
- ui/src/components/ui/table.tsx
- ui/src/components/ui/command.tsx
🚧 Files skipped from review as they are similar to previous changes (21)
- ui/src/setupTests.ts
- ui/src/components/activity/EventBadge.tsx
- ui/src/routes/Graph.tsx
- ui/src/components/activity/EventRow.tsx
- ui/src/components/layout/ThemeToggle.tsx
- ui/src/components/notes/WikiLink.tsx
- ui/src/main.tsx
- ui/src/hooks/api/useDocs.ts
- ui/src/components/layout/TopBar.tsx
- ui/src/components/layout/StatsStrip.tsx
- ui/src/components/layout/Providers.tsx
- ui/src/components/command/CommandPalette.tsx
- ui/package.json
- ui/src/components/graph/GlanceView.tsx
- ui/src/routes/documents/UploadModal.tsx
- ui/src/routes/notes/NoteEditor.tsx
- ui/src/routes/Home.tsx
- ui/src/components/ui/tooltip.tsx
- ui/src/hooks/api/useMCP.ts
- ui/src/components/layout/Shell.tsx
- ui/src/routes/notes/NoteView.tsx
| @@ -0,0 +1,109 @@ | |||
| import * as React from "react" | |||
| import { ChevronRight, MoreHorizontal } from "lucide-react" | |||
| import { Slot } from "radix-ui" | |||
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Radix UI version 1.4.3, is import { Slot } from "radix-ui"valid, and does it exposeSlot.Root? If not, what is the correct import and component usage for Slot?
💡 Result:
No, import { Slot } from "radix-ui" is not valid in Radix UI version 1.4.3. Slot is provided by the separate @radix-ui/react-slot package, which must be installed independently (npm install @radix-ui/react-slot). The correct import is import { Slot } from '@radix-ui/react-slot';. Slot exposes Slot.Root as the main component. Usage example from official docs: jsx import { Slot } from '@radix-ui/react-slot'; function Button({ asChild, ...props }) { const Comp = asChild ? Slot.Root : 'button'; return <Comp {...props} />; } The radix-ui package (v1.4.1 as of available data) provides a tree-shakable entrypoint for primitives since January 2025, but official Slot documentation consistently shows the per-package import. Some third-party docs show import { Slot } from "radix-ui", but this appears to be outdated or incorrect for standard usage.
Citations:
- 1: https://www.radix-ui.com/primitives/docs/utilities/slot
- 2: https://www.radix-ui.com/docs/primitives/utilities/slot
- 3: https://radix-ui.com/primitives/docs/overview/releases
- 4: https://www.radix-ui.com/docs/primitives/overview/releases
- 5: https://www.npmjs.com/package/radix-ui/v/1.4.1
- 6: https://www.npmjs.com/package/@radix-ui/react-slot?activeTab=versions
Update Radix Slot import to use the correct package.
Change import { Slot } from "radix-ui" to import { Slot } from '@radix-ui/react-slot'. The Slot component is provided by the separate @radix-ui/react-slot package, which must be installed independently. Ensure @radix-ui/react-slot is listed in your dependencies.
Also applies to: 41-41
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/ui/breadcrumb.tsx` at line 3, The import for the Radix Slot
is incorrect; replace the current import of Slot (import { Slot } from
"radix-ui") with the correct package import (use the Slot symbol from
'@radix-ui/react-slot') and ensure '@radix-ui/react-slot' is added to
package.json dependencies and installed; also update the other occurrence of the
Slot import referenced in the file (around the second instance noted) so both
imports use the same '@radix-ui/react-slot' symbol.
Comment on lines
+88
to
+97
| <span | ||
| data-slot="breadcrumb-ellipsis" | ||
| role="presentation" | ||
| aria-hidden="true" | ||
| className={cn("flex size-9 items-center justify-center", className)} | ||
| {...props} | ||
| > | ||
| <MoreHorizontal className="size-4" /> | ||
| <span className="sr-only">More</span> | ||
| </span> |
There was a problem hiding this comment.
aria-hidden currently suppresses the “More” screen-reader label.
BreadcrumbEllipsis sets aria-hidden="true" on the wrapper, so the inner sr-only text is never announced.
♿ Proposed accessibility fix
function BreadcrumbEllipsis({
className,
...props
}: React.ComponentProps<"span">) {
return (
<span
data-slot="breadcrumb-ellipsis"
role="presentation"
- aria-hidden="true"
className={cn("flex size-9 items-center justify-center", className)}
{...props}
>
- <MoreHorizontal className="size-4" />
+ <MoreHorizontal aria-hidden="true" className="size-4" />
<span className="sr-only">More</span>
</span>
)
}📝 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.
Suggested change
| <span | |
| data-slot="breadcrumb-ellipsis" | |
| role="presentation" | |
| aria-hidden="true" | |
| className={cn("flex size-9 items-center justify-center", className)} | |
| {...props} | |
| > | |
| <MoreHorizontal className="size-4" /> | |
| <span className="sr-only">More</span> | |
| </span> | |
| function BreadcrumbEllipsis({ | |
| className, | |
| ...props | |
| }: React.ComponentProps<"span">) { | |
| return ( | |
| <span | |
| data-slot="breadcrumb-ellipsis" | |
| role="presentation" | |
| className={cn("flex size-9 items-center justify-center", className)} | |
| {...props} | |
| > | |
| <MoreHorizontal aria-hidden="true" className="size-4" /> | |
| <span className="sr-only">More</span> | |
| </span> | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/ui/breadcrumb.tsx` around lines 88 - 97, BreadcrumbEllipsis
currently sets aria-hidden="true" on the wrapper span which hides the inner
sr-only "More" label from screen readers; remove aria-hidden from the wrapper
(the span with data-slot="breadcrumb-ellipsis") and instead mark the decorative
icon MoreHorizontal as aria-hidden="true" so the visible sr-only text is
announced; update the wrapper span (in BreadcrumbEllipsis) to keep role and
props but without aria-hidden and ensure the MoreHorizontal component has
aria-hidden set.
Comment on lines
+57
to
+58
| const uniqueId = React.useId() | ||
| const chartId = `chart-${id ?? uniqueId.replace(/:/g, "")}` |
There was a problem hiding this comment.
Escape the generated selector before injecting it into <style>.
data-chart={chartId} can hold strings that are valid HTML attribute values but invalid in an unquoted CSS attribute selector. A custom id like cpu/usage or foo bar will stop these theme vars from matching, and raw interpolation into __html makes the failure mode brittle.
Suggested fix
function ChartContainer({
id,
@@
}) {
const uniqueId = React.useId()
const chartId = `chart-${id ?? uniqueId.replace(/:/g, "")}`
+ const chartSelector = React.useMemo(() => CSS.escape(chartId), [chartId])
@@
- <ChartStyle id={chartId} config={config} />
+ <ChartStyle id={chartId} selector={chartSelector} config={config} />
@@
-const ChartStyle = ({ id, config }: { id: string; config: ChartConfig }) => {
+const ChartStyle = ({
+ id,
+ selector,
+ config,
+}: {
+ id: string
+ selector: string
+ config: ChartConfig
+}) => {
@@
-${prefix} [data-chart=${id}] {
+${prefix} [data-chart="${selector}"] {Also applies to: 82-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/ui/chart.tsx` around lines 57 - 58, The CSS attribute
selector built from chartId (computed from uniqueId) is being interpolated
directly into the injected <style> (and other places between lines 82-110),
which breaks when id contains characters like spaces or slashes; update the
component to escape the selector before interpolation by passing chartId through
CSS.escape (or a small polyfill function if CSS.escape is undefined) wherever
you build selectors for the injected CSS or use data-chart attribute
interpolation, e.g., replace raw chartId usage in the style template with the
escaped value and use the same escaped value for all other injected selectors to
ensure safe, valid CSS.
Comment on lines
+297
to
+320
| {payload | ||
| .filter((item) => item.type !== "none") | ||
| .map((item, index) => { | ||
| const key = `${nameKey ?? item.dataKey ?? "value"}` | ||
| const itemConfig = getPayloadConfigFromPayload(config, item, key) | ||
|
|
||
| return ( | ||
| <div | ||
| key={index} | ||
| className={cn( | ||
| "flex items-center gap-1.5 [&>svg]:h-3 [&>svg]:w-3 [&>svg]:text-muted-foreground" | ||
| )} | ||
| > | ||
| {itemConfig?.icon && !hideIcon ? ( | ||
| <itemConfig.icon /> | ||
| ) : ( | ||
| <div | ||
| className="h-2 w-2 shrink-0 rounded-[2px]" | ||
| style={{ | ||
| backgroundColor: item.color, | ||
| }} | ||
| /> | ||
| )} | ||
| {itemConfig?.label} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the chart.tsx file
fd -type f -name "chart.tsx" | head -5Repository: RandomCodeSpace/docsiq
Length of output: 236
🏁 Script executed:
# Get the file size to determine how to read it
wc -l ui/src/components/ui/chart.tsxRepository: RandomCodeSpace/docsiq
Length of output: 100
🏁 Script executed:
# Read the relevant section and surrounding context
sed -n '280,330p' ui/src/components/ui/chart.tsxRepository: RandomCodeSpace/docsiq
Length of output: 1390
🏁 Script executed:
# Also look for ChartConfig type definition in the file
rg "type ChartConfig|interface ChartConfig" ui/src/components/ui/chart.tsx -A 10Repository: RandomCodeSpace/docsiq
Length of output: 295
🏁 Script executed:
# Read the getPayloadConfigFromPayload function
sed -n '330,372p' ui/src/components/ui/chart.tsxRepository: RandomCodeSpace/docsiq
Length of output: 1017
🏁 Script executed:
# Also check if there's any fallback logic before the label rendering
sed -n '295,325p' ui/src/components/ui/chart.tsxRepository: RandomCodeSpace/docsiq
Length of output: 946
Add fallback label when config.label is undefined.
The code renders {itemConfig?.label} without a fallback, but ChartConfig.label is optional. When a valid config omits the label property (e.g., { color: "#123456" }), the legend displays a color chip with no text. Consider falling back to the key, data key, or another identifier to ensure the legend item remains readable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/ui/chart.tsx` around lines 297 - 320, The legend may render
empty text because itemConfig?.label (ChartConfig.label) is optional; update the
render inside the payload.map block to use a fallback like the computed key (the
existing const key = `${nameKey ?? item.dataKey ?? "value"}`) or
item.dataKey/nameKey when itemConfig?.label is undefined — i.e., replace the
bare {itemConfig?.label} with a fallback expression using key (or
item.dataKey/nameKey) so each legend item always shows a readable identifier;
locate this change around the getPayloadConfigFromPayload / itemConfig?.label
usage in the payload.map rendering.
Comment on lines
+76
to
+89
| const setOpen = React.useCallback( | ||
| (value: boolean | ((value: boolean) => boolean)) => { | ||
| const openState = typeof value === "function" ? value(open) : value | ||
| if (setOpenProp) { | ||
| setOpenProp(openState) | ||
| } else { | ||
| _setOpen(openState) | ||
| } | ||
|
|
||
| // This sets the cookie to keep the sidebar state. | ||
| document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}` | ||
| }, | ||
| [setOpenProp, open] | ||
| ) |
There was a problem hiding this comment.
Preserve functional-updater semantics in setOpen.
Line 78 eagerly evaluates updater functions against the closed-over open value. Two setOpen((v) => !v) calls before the next render both compute from the same snapshot, so one toggle is lost.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/ui/sidebar.tsx` around lines 76 - 89, The wrapper setOpen
currently eagerly evaluates functional updaters against the closed-over open
value, losing multiple toggles; instead, pass the incoming value (function or
boolean) directly to the underlying setter so updater semantics are preserved:
call setOpenProp(value) when setOpenProp exists, and call _setOpen(prev => {
const next = typeof value === 'function' ? value(prev) : value; document.cookie
= `${SIDEBAR_COOKIE_NAME}=${next}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`;
return next }) when using the internal _setOpen so the cookie is set from the
actual resolved state. Ensure you reference setOpen, setOpenProp, _setOpen and
SIDEBAR_COOKIE_NAME/SIDEBAR_COOKIE_MAX_AGE in the change.
| {data.map((d) => ( | ||
| <TableRow key={d.id}> | ||
| <TableCell> | ||
| <Link to={`/docs/${d.id}`} className="text-foreground underline decoration-dotted"> |
There was a problem hiding this comment.
Encode document IDs in route links.
Building to with raw d.id can produce invalid paths for IDs containing reserved URL characters.
💡 Suggested fix
- <Link to={`/docs/${d.id}`} className="text-foreground underline decoration-dotted">
+ <Link to={`/docs/${encodeURIComponent(d.id)}`} className="text-foreground underline decoration-dotted">📝 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.
Suggested change
| <Link to={`/docs/${d.id}`} className="text-foreground underline decoration-dotted"> | |
| <Link to={`/docs/${encodeURIComponent(d.id)}`} className="text-foreground underline decoration-dotted"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/routes/documents/DocumentsList.tsx` at line 59, The Link target in
DocumentsList.tsx is built using raw d.id which can produce invalid URLs for IDs
with reserved characters; update the Link's "to" generation (the JSX where Link
is used and the template `/docs/${d.id}`) to URL-encode the id (e.g., call a
safe encoder like encodeURIComponent on d.id or a project helper such as
encodePathSegment) so all IDs produce valid path segments and existing route
matching continues to work.
Comment on lines
+28
to
+38
| function coerce(value: string, type: ParamType): unknown { | ||
| if (value === "") return undefined; | ||
| if (type === "number" || type === "integer") { | ||
| const n = Number(value); | ||
| return Number.isFinite(n) ? n : value; | ||
| } | ||
| if (type === "boolean") return value === "true"; | ||
| if (type === "array" || type === "object") { | ||
| try { return JSON.parse(value); } catch { return value; } | ||
| } | ||
| return value; |
There was a problem hiding this comment.
Validate required and typed arguments before invoking tools.
Line 81 builds obj without enforcing spec.required, and coerce (Line 32, Line 36) silently falls back to strings on invalid typed values. That sends schema-invalid payloads without user feedback.
💡 Proposed fix
function coerce(value: string, type: ParamType): unknown {
if (value === "") return undefined;
- if (type === "number" || type === "integer") {
+ if (type === "number" || type === "integer") {
const n = Number(value);
- return Number.isFinite(n) ? n : value;
+ if (!Number.isFinite(n)) throw new Error("Expected a valid number.");
+ if (type === "integer" && !Number.isInteger(n)) {
+ throw new Error("Expected an integer.");
+ }
+ return n;
}
if (type === "boolean") return value === "true";
if (type === "array" || type === "object") {
- try { return JSON.parse(value); } catch { return value; }
+ const parsed = JSON.parse(value);
+ if (type === "array" && !Array.isArray(parsed)) {
+ throw new Error("Expected a JSON array.");
+ }
+ if (type === "object" && (parsed === null || Array.isArray(parsed) || typeof parsed !== "object")) {
+ throw new Error("Expected a JSON object.");
+ }
+ return parsed;
}
return value;
}
@@
async function onRun() {
if (!selected) return;
setErr(null);
let args: unknown;
if (useRaw) {
try { args = JSON.parse(rawJson); } catch { setErr("Invalid JSON"); return; }
} else {
+ const missing = params
+ .filter((p) => p.required && (fields[p.name] ?? "").trim() === "")
+ .map((p) => p.name);
+ if (missing.length) {
+ setErr(`Missing required: ${missing.join(", ")}`);
+ return;
+ }
const obj: Record<string, unknown> = {};
- for (const p of params) {
- const v = coerce(fields[p.name] ?? "", p.type);
- if (v !== undefined) obj[p.name] = v;
+ try {
+ for (const p of params) {
+ const v = coerce(fields[p.name] ?? "", p.type);
+ if (v !== undefined) obj[p.name] = v;
+ }
+ } catch (e) {
+ setErr(e instanceof Error ? e.message : "Invalid argument value.");
+ return;
}
args = obj;
}Also applies to: 73-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/routes/MCPConsole.tsx` around lines 28 - 38, The code currently builds
the argument object (`obj`) without enforcing each parameter's `spec.required`
and `coerce` silently falls back to strings on invalid typed values; add a
validation step (e.g., `validateArgs(specs, inputs)`) invoked before
constructing/using `obj` that checks required params are present and that each
value can be coerced to the declared `ParamType`. Modify `coerce(value: string,
type: ParamType)` to return a clear validation failure (throw or return a
distinct error/result) instead of silently returning the original string for
invalid numbers/JSON/booleans, and have `validateArgs` collect/return
user-facing errors for missing or invalid params so the tool invocation is
aborted until corrections are made. Ensure callers of `coerce`/`validateArgs`
handle the validation result and display errors rather than sending
schema-invalid payloads.
Comment on lines
+59
to
+61
| useEffect(() => { | ||
| if (!selectedName && filtered.length) setSelectedName(filtered[0].name); | ||
| }, [filtered, selectedName]); |
There was a problem hiding this comment.
Recover selection when the current tool no longer exists.
Line 60 only auto-selects when selectedName is empty. After refresh/removal, selectedName can stay stale and leave the right pane unselected even when tools exist.
💡 Proposed fix
useEffect(() => {
- if (!selectedName && filtered.length) setSelectedName(filtered[0].name);
- }, [filtered, selectedName]);
+ if (!selectedName && filtered.length) {
+ setSelectedName(filtered[0].name);
+ return;
+ }
+ if (selectedName && !tools.some((t) => t.name === selectedName)) {
+ setSelectedName(filtered[0]?.name ?? "");
+ }
+ }, [filtered, selectedName, tools]);📝 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.
Suggested change
| useEffect(() => { | |
| if (!selectedName && filtered.length) setSelectedName(filtered[0].name); | |
| }, [filtered, selectedName]); | |
| useEffect(() => { | |
| if (!selectedName && filtered.length) { | |
| setSelectedName(filtered[0].name); | |
| return; | |
| } | |
| if (selectedName && !tools.some((t) => t.name === selectedName)) { | |
| setSelectedName(filtered[0]?.name ?? ""); | |
| } | |
| }, [filtered, selectedName, tools]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/routes/MCPConsole.tsx` around lines 59 - 61, The useEffect in
MCPConsole.tsx currently only selects a tool when selectedName is empty; update
the effect (the useEffect that reads selectedName and filtered and calls
setSelectedName) to also recover when the current selectedName is stale by
checking whether filtered contains an entry with name === selectedName, and if
not (or if selectedName is falsy) call setSelectedName(filtered[0].name) when
filtered.length > 0 so the right pane is populated after refresh/removal.
Comment on lines
+87
to
+89
| setBusy(true); | ||
| try { await invoke(selected.name, args); } finally { setBusy(false); } | ||
| } |
There was a problem hiding this comment.
Handle invoke exceptions and expose them in UI state.
Line 88 only uses try/finally; rejected calls won’t set err, so failures are harder to recover from in-place.
💡 Proposed fix
setBusy(true);
- try { await invoke(selected.name, args); } finally { setBusy(false); }
+ try {
+ await invoke(selected.name, args);
+ } catch (e) {
+ setErr(e instanceof Error ? e.message : "Tool invocation failed.");
+ } finally {
+ setBusy(false);
+ }
}📝 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.
Suggested change
| setBusy(true); | |
| try { await invoke(selected.name, args); } finally { setBusy(false); } | |
| } | |
| setBusy(true); | |
| try { | |
| await invoke(selected.name, args); | |
| } catch (e) { | |
| setErr(e instanceof Error ? e.message : "Tool invocation failed."); | |
| } finally { | |
| setBusy(false); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/routes/MCPConsole.tsx` around lines 87 - 89, The current invocation
uses try/finally so thrown errors are not saved to component state; update the
call site that wraps invoke(selected.name, args) to clear any previous error
(via setErr(null)), then use try/catch/finally around await invoke(...) and in
catch call setErr(...) with the captured error (e.g. error.message or
String(error)) so the UI can display failures, finally calling setBusy(false);
reference the setBusy, invoke, selected.name, args, and setErr symbols when
making this change.
Comment on lines
+221
to
+287
| const label = ( | ||
| <label className="param-label"> | ||
| <span className="param-name">{spec.name}</span> | ||
| <span className="param-type">:{spec.type}</span> | ||
| {spec.required && <span className="param-required">*</span>} | ||
| {spec.description && <span className="param-desc">{spec.description}</span>} | ||
| </label> | ||
| ); | ||
| if (spec.enum?.length) { | ||
| return ( | ||
| <div> | ||
| {label} | ||
| <select | ||
| value={value} | ||
| onChange={(e) => onChange(e.currentTarget.value)} | ||
| className="param-input" | ||
| > | ||
| <option value="">—</option> | ||
| {spec.enum.map((v) => ( | ||
| <option key={String(v)} value={String(v)}>{String(v)}</option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| ); | ||
| } | ||
| if (spec.type === "boolean") { | ||
| return ( | ||
| <div> | ||
| {label} | ||
| <select | ||
| value={value} | ||
| onChange={(e) => onChange(e.currentTarget.value)} | ||
| className="param-input" | ||
| > | ||
| <option value="">—</option> | ||
| <option value="true">true</option> | ||
| <option value="false">false</option> | ||
| </select> | ||
| </div> | ||
| ); | ||
| } | ||
| if (spec.type === "array" || spec.type === "object") { | ||
| return ( | ||
| <div> | ||
| {label} | ||
| <textarea | ||
| value={value} | ||
| onChange={(e) => onChange(e.currentTarget.value)} | ||
| rows={3} | ||
| placeholder={spec.type === "array" ? "[]" : "{}"} | ||
| className="param-input" | ||
| spellCheck={false} | ||
| /> | ||
| </div> | ||
| ); | ||
| } | ||
| return ( | ||
| <div> | ||
| {label} | ||
| <input | ||
| value={value} | ||
| onChange={(e) => onChange(e.currentTarget.value)} | ||
| type={spec.type === "number" || spec.type === "integer" ? "number" : "text"} | ||
| className="param-input" | ||
| /> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Associate parameter labels with controls for screen-reader usability.
The label at Line 222 is visually adjacent but not programmatically bound (htmlFor/id) to <select>, <textarea>, or <input>, which hurts form accessibility.
💡 Proposed fix
-import { useEffect, useMemo, useState } from "react";
+import { useEffect, useMemo, useState, useId } from "react";
@@
function ParamField({
@@
}) {
+ const inputId = useId();
const label = (
- <label className="param-label">
+ <label htmlFor={inputId} className="param-label">
@@
<select
+ id={inputId}
value={value}
@@
<select
+ id={inputId}
value={value}
@@
<textarea
+ id={inputId}
value={value}
@@
<input
+ id={inputId}
value={value}📝 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.
Suggested change
| const label = ( | |
| <label className="param-label"> | |
| <span className="param-name">{spec.name}</span> | |
| <span className="param-type">:{spec.type}</span> | |
| {spec.required && <span className="param-required">*</span>} | |
| {spec.description && <span className="param-desc">{spec.description}</span>} | |
| </label> | |
| ); | |
| if (spec.enum?.length) { | |
| return ( | |
| <div> | |
| {label} | |
| <select | |
| value={value} | |
| onChange={(e) => onChange(e.currentTarget.value)} | |
| className="param-input" | |
| > | |
| <option value="">—</option> | |
| {spec.enum.map((v) => ( | |
| <option key={String(v)} value={String(v)}>{String(v)}</option> | |
| ))} | |
| </select> | |
| </div> | |
| ); | |
| } | |
| if (spec.type === "boolean") { | |
| return ( | |
| <div> | |
| {label} | |
| <select | |
| value={value} | |
| onChange={(e) => onChange(e.currentTarget.value)} | |
| className="param-input" | |
| > | |
| <option value="">—</option> | |
| <option value="true">true</option> | |
| <option value="false">false</option> | |
| </select> | |
| </div> | |
| ); | |
| } | |
| if (spec.type === "array" || spec.type === "object") { | |
| return ( | |
| <div> | |
| {label} | |
| <textarea | |
| value={value} | |
| onChange={(e) => onChange(e.currentTarget.value)} | |
| rows={3} | |
| placeholder={spec.type === "array" ? "[]" : "{}"} | |
| className="param-input" | |
| spellCheck={false} | |
| /> | |
| </div> | |
| ); | |
| } | |
| return ( | |
| <div> | |
| {label} | |
| <input | |
| value={value} | |
| onChange={(e) => onChange(e.currentTarget.value)} | |
| type={spec.type === "number" || spec.type === "integer" ? "number" : "text"} | |
| className="param-input" | |
| /> | |
| </div> | |
| ); | |
| const inputId = useId(); | |
| const label = ( | |
| <label htmlFor={inputId} className="param-label"> | |
| <span className="param-name">{spec.name}</span> | |
| <span className="param-type">:{spec.type}</span> | |
| {spec.required && <span className="param-required">*</span>} | |
| {spec.description && <span className="param-desc">{spec.description}</span>} | |
| </label> | |
| ); | |
| if (spec.enum?.length) { | |
| return ( | |
| <div> | |
| {label} | |
| <select | |
| id={inputId} | |
| value={value} | |
| onChange={(e) => onChange(e.currentTarget.value)} | |
| className="param-input" | |
| > | |
| <option value="">—</option> | |
| {spec.enum.map((v) => ( | |
| <option key={String(v)} value={String(v)}>{String(v)}</option> | |
| ))} | |
| </select> | |
| </div> | |
| ); | |
| } | |
| if (spec.type === "boolean") { | |
| return ( | |
| <div> | |
| {label} | |
| <select | |
| id={inputId} | |
| value={value} | |
| onChange={(e) => onChange(e.currentTarget.value)} | |
| className="param-input" | |
| > | |
| <option value="">—</option> | |
| <option value="true">true</option> | |
| <option value="false">false</option> | |
| </select> | |
| </div> | |
| ); | |
| } | |
| if (spec.type === "array" || spec.type === "object") { | |
| return ( | |
| <div> | |
| {label} | |
| <textarea | |
| id={inputId} | |
| value={value} | |
| onChange={(e) => onChange(e.currentTarget.value)} | |
| rows={3} | |
| placeholder={spec.type === "array" ? "[]" : "{}"} | |
| className="param-input" | |
| spellCheck={false} | |
| /> | |
| </div> | |
| ); | |
| } | |
| return ( | |
| <div> | |
| {label} | |
| <input | |
| id={inputId} | |
| value={value} | |
| onChange={(e) => onChange(e.currentTarget.value)} | |
| type={spec.type === "number" || spec.type === "integer" ? "number" : "text"} | |
| className="param-input" | |
| /> | |
| </div> | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/routes/MCPConsole.tsx` around lines 221 - 287, The label defined in
the label constant is not programmatically bound to its form control, harming
accessibility; create a unique id (e.g., const inputId = `param-${spec.name}`)
near where label is built, set the label's htmlFor to that id, and add
id={inputId} to every control returned (the <select>, <textarea>, and <input>
elements inside the branches) so the <label> (label constant) is associated with
its corresponding control.
Every UI change used to create dist churn in commits that obscured real Go diffs. Cleaner path: - .gitignore ui/dist/* except a minimal index.html placeholder that preserves the //go:embed ui/dist contract at compile time - Split CI into a `ui` job (typecheck + vitest + build + bundle budget) that uploads the fresh dist as an artifact; the `test` + `test-integration` jobs depend on `ui` and download the artifact before the Go steps so every binary embeds an up-to-date SPA - Drop the `verify ui/dist/ matches fresh build` diff gate — no longer meaningful now that dist is always fresh per-run Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Treat "none" as an explicit, recognised value in the resolved-LLM log so operators who want zero-LLM mode see a clear signal rather than silence. Validation in NewProvider enforces the contract. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds the "none" case to the switch so callers that configure provider=none get a nil Provider with no error, enabling zero-LLM serve mode. Also updates the error message to list "none" as a supported option. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When provider=none, embedder.New receives a nil llm.Provider and now returns nil instead of constructing an Embedder that would panic on first use. Callers that already check for nil are unaffected. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When NewProvider returns (nil, nil), log a clear INFO message listing which routes remain available and which return 503, then proceed to construct the router normally. No early-exit on nil provider. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Router: when prov is nil, register a 503 stub on /mcp instead of
constructing the full MCP server (avoids partial tool registration
that would silently fail on search/upload tools).
Handlers: search (POST /api/search) and upload (POST /api/upload)
check h.provider==nil at the top and return 503 with
{"error":"...","code":"llm_disabled"} before touching the store.
Graph, entity, community, and claims endpoints are pure store reads
and remain available without a provider.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
config_test.go: two cases verify provider=none parses correctly from YAML and from DOCSIQ_LLM_PROVIDER env var. router_no_llm_test.go: builds a router with nil prov/emb and asserts GET /health → 200, GET /api/projects/_default/notes → 200, POST /api/search → 503 with code=llm_disabled, POST /api/upload → 503, POST /mcp → 503. Tree and notes-search are verified to NOT return 503 (pure store reads). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Targets of shape projects/<slug>/<rest> are now recognised as cross- project references throughout the stack: - internal/notes/wikilinks.go: add ParseWikilink + Wikilink struct - internal/notes/graph.go: extend NoteNode (project, missing) and Edge (cross_project); BuildGraph accepts optional projectsRoot to resolve and existence-check cross-project targets - internal/api/notes_handlers.go: pass projectsRoot to BuildGraph so the graph response carries cross_project edges and project/missing node fields - ui/src/components/notes/WikiLink.tsx: parse target client-side, render project-chip prefix and wikilink-cross class, call setSlug on click - ui/src/styles/globals.css: add .wikilink-cross and .wikilink-cross-chip - tests: ParseWikilink table, two cross-project graph scenarios, 11 WikiLink component tests — all 233 Go + 52 UI tests pass Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the d3-force + SVG GraphCanvas with @cosmograph/cosmos — regl- backed WebGL2 renderer used by OpenAI/Dify, smooth through tens of thousands of nodes. Theme tokens are read from CSS vars at mount so the canvas follows dark/light/accent without config. GraphCanvas is still inside the /graph lazy chunk (~284 KB gzipped ~87 KB). GlanceView stays on SVG — it's a 60-node preview. Note: cosmograph 1.x is WebGL2-only (via regl). cosmos v2 adds a WebGPU backend; we'll pick it up once it stabilises. Every WebGPU-capable browser also exposes WebGL2, so "WebGPU primary / WebGL2 fallback" is degenerate to "WebGL2" on today's browsers. Also restores ui/dist/index.html placeholder that local npm builds had overwritten with hashed asset references. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
ui/src/components/graph/GraphCanvas.tsx (1)
52-89: Consider reusing the Cosmos instance instead of rebuilding it on every data update.Current lifecycle destroys/recreates the renderer for each
nodes/linkschange, which is heavier than needed and can reset the user’s viewport state.♻️ Refactor direction
- useEffect(() => { + useEffect(() => { const canvas = canvasRef.current; if (!canvas) return; @@ - graph.setData(nodes, links); - graph.fitView(); - return () => { graph.destroy(); graphRef.current = null; }; - }, [nodes, links]); + }, []); + + useEffect(() => { + if (!graphRef.current) return; + graphRef.current.setData(nodes, links); + graphRef.current.fitView(); + }, [nodes, links]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/graph/GraphCanvas.tsx` around lines 52 - 89, The effect currently recreates a new Cosmos instance on every nodes/links change; instead, only create the Cosmos renderer once and reuse it: change the useEffect so that creation of new Cosmos(canvas, config), assignment to graphRef.current, and graph.fitView() happen in an initialization branch that runs once (e.g., on mount or when canvas changes) and do not run on every nodes/links update, and then in a separate effect (or later in the same effect) call graphRef.current.setData(nodes, links) whenever nodes or links change; ensure the cleanup still destroys the instance on unmount (graph.destroy() and graphRef.current = null) and that any config values that must update (like pixelRatio or backgroundColor) are applied via the existing graph API rather than reconstructing Cosmos.internal/api/notes_handlers.go (1)
365-370:projectsRootderivation is path-shape-coupled.This assumes
NotesDiris always<data>/projects/<slug>/notes. Consider passing projects root directly from config (or exposing a dedicated accessor) so graph behavior doesn’t silently break if path layout changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/notes_handlers.go` around lines 365 - 370, The code computes projectsRoot by taking filepath.Dir(filepath.Dir(notesDir)) which couples BuildGraph to the current NotesDir layout; instead, modify the call site to obtain the projects root from configuration or a dedicated accessor (e.g., add a Config.ProjectsRoot or Notes.ProjectsRoot method) and pass that into notes.BuildGraph (update the BuildGraph signature if needed) so projectsRoot is derived from explicit config rather than filepath.Dir(filepath.Dir(notesDir)), and update any callers to use the new accessor/sig (referencing projectsRoot, notesDir, NotesDir, and notes.BuildGraph).internal/notes/wikilinks.go (1)
23-31: Normalize parsed project slug inParseWikilinkfor consistency.Centralizing slug normalization here avoids case drift between parser consumers and keeps cross-project behavior uniform.
♻️ Proposed change
-import "regexp" +import ( + "regexp" + "strings" +) @@ return Wikilink{ Target: target, - Project: m[1], + Project: strings.ToLower(m[1]), Key: m[2], CrossProject: true, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/notes/wikilinks.go` around lines 23 - 31, The ParseWikilink function currently returns the raw capture m[1] as Wikilink.Project; normalize the parsed project slug centrally by applying the project's slug-normalization helper (e.g. NormalizeProjectSlug or equivalent) to m[1] before assigning to Wikilink.Project so cross-project lookups are consistent; update ParseWikilink (and import) to call that helper on m[1] when building the Wikilink for cross-projectRe matches.internal/notes/wikilinks_test.go (1)
21-31: Add explicitprojects/<slug>/malformed case.You already cover missing-key and empty-slug variants well; adding a trailing-slash case (
projects/docsiq/) would close the last nearby edge.➕ Proposed test-row addition
// NOT cross-project: no key part after slug {"no_key", "projects/docsiq", "", "projects/docsiq", false}, + // NOT cross-project: trailing slash but empty key + {"empty_key_trailing_slash", "projects/docsiq/", "", "projects/docsiq/", false},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/notes/wikilinks_test.go` around lines 21 - 31, Add a test row for the malformed trailing-slash projects case in the table of test rows: insert an entry like {"projects_trailing_slash", "projects/docsiq/", "", "projects/docsiq/", false} alongside the other cases (near "projects_no_slash") so the parser is exercised for a trailing slash after the slug.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/api/handlers.go`:
- Around line 205-211: The current early-return in the handler checks h.provider
and h.embedder and returns llm_disabled, but the request handlers (e.g., search
and upload) later resolve a per-project provider/embedder from config; update
the guards to resolve the effective project-specific provider/embedder (the same
resolution logic used by search) and base the 503 gate on those resolved values
instead of the root h.provider/h.embedder; apply the same change to both the
earlier guard and the similar check around the code referenced at 394-400 so
requests with a valid per-project override do not incorrectly short-circuit.
In `@internal/api/router.go`:
- Around line 184-191: The code injects cfg.Server.APIKey into index.html
(bytes.Replace block) which exposes the bearer used by bearerAuthMiddleware;
remove that injection and stop embedding cfg.Server.APIKey into the SPA. Instead
implement server-managed session or short-lived scoped tokens: delete the
bytes.Replace call in router.go, add an authenticated endpoint (e.g., /session
or /api/token) that issues a short-lived scoped token or sets an HttpOnly secure
cookie, and update the SPA to request that token at runtime; keep
cfg.Server.APIKey only for server-to-server use and ensure bearerAuthMiddleware
verifies tokens issued by the new session/token flow.
In `@internal/embedder/embedder.go`:
- Around line 18-23: The change made New(provider llm.Provider, batchSize int)
*Embedder return nil when provider is nil breaks callers that assume a non-nil
*Embedder (e.g., Pipeline.New stores embedder.New(...) and later dereferences
p.embedder). Revert New to be total by always returning a non-nil &Embedder
(even when provider==nil) and add a disabled flag or check inside Embedder
methods (e.g., Embed, EmbedBatch) so they return a clear llm_disabled error when
provider==nil; alternatively, if you prefer the nil result approach, update
Pipeline.New and any callers that dereference p.embedder to check for nil and
handle llm-disabled paths explicitly — reference New, type Embedder, and
p.embedder / Pipeline.New to locate the spots to change.
In `@internal/llm/provider.go`:
- Around line 47-50: Add a nil-check at the start of NewProvider to explicitly
handle a nil cfg instead of letting switch cfg.Provider panic: if cfg == nil
return nil, errors.New("nil LLMConfig") (or an equivalent wrapped error), then
proceed with the existing switch on cfg.Provider; update imports to include
"errors" (or "fmt") as needed.
In `@internal/notes/graph.go`:
- Around line 91-110: The code is creating cross-project stubs without checking
the existing local node index, causing duplicate keys; before creating/appending
the stub in the branch that builds NoteNode (using variables nodeKey,
crossStubs, wl.Key, NoteNode and appending to g.Nodes), consult nodeIndex (the
set of existing local note keys) or seed crossStubs from nodeIndex first and
skip stub creation if nodeKey already exists in nodeIndex; update the condition
that currently only checks crossStubs so it also checks nodeIndex (or initialize
crossStubs with nodeIndex entries) to prevent adding duplicate nodes.
- Around line 99-104: The existence check for candidate allows path traversal
because wl.Key comes from ParseWikilink; instead of directly calling
filepath.Join(root, wl.Project, "notes", wl.Key+".md") and os.Stat, construct
the notesDir := filepath.Join(root, wl.Project, "notes"), build candidate via
filepath.Join(notesDir, wl.Key+".md"), then normalize and validate the path (use
filepath.Clean and ensure filepath.Rel(notesDir, candidate) does not begin with
".." or use filepath.HasPrefix equivalent) to reject any candidate that escapes
notesDir before calling os.Stat; only then set stub.Missing based on the safe
os.Stat result, and treat escaped/invalid targets as missing or invalid.
In `@ui/src/components/graph/GraphCanvas.tsx`:
- Around line 34-38: The node colors are computed once in the useMemo that calls
readTokens() and the Cosmos config is rebuilt in the useEffect that depends on
nodes/links, so toggling theme doesn't rerun either; update the dependency
arrays to include theme (or a theme-derived value) so the useMemo that produces
nodes/links and the useEffect that rebuilds the Cosmos config rerun on theme
changes—specifically modify the useMemo that computes tokens/kindColor and the
useEffect that rebuilds the Cosmos config to include the theme identifier (or
the output of readTokens()) in their dependency lists.
In `@ui/src/styles/globals.css`:
- Line 3: The stylelint config must be updated so Tailwind v4 at-rules are
treated as known: add "custom-variant" and "theme" to the at-rule-no-unknown
ignore list in .stylelintrc.json (i.e. update the "at-rule-no-unknown" rule to
include an "ignoreAtRules" array containing "custom-variant" and "theme") so
directives like `@custom-variant` and `@theme` in globals.css are not flagged.
---
Nitpick comments:
In `@internal/api/notes_handlers.go`:
- Around line 365-370: The code computes projectsRoot by taking
filepath.Dir(filepath.Dir(notesDir)) which couples BuildGraph to the current
NotesDir layout; instead, modify the call site to obtain the projects root from
configuration or a dedicated accessor (e.g., add a Config.ProjectsRoot or
Notes.ProjectsRoot method) and pass that into notes.BuildGraph (update the
BuildGraph signature if needed) so projectsRoot is derived from explicit config
rather than filepath.Dir(filepath.Dir(notesDir)), and update any callers to use
the new accessor/sig (referencing projectsRoot, notesDir, NotesDir, and
notes.BuildGraph).
In `@internal/notes/wikilinks_test.go`:
- Around line 21-31: Add a test row for the malformed trailing-slash projects
case in the table of test rows: insert an entry like {"projects_trailing_slash",
"projects/docsiq/", "", "projects/docsiq/", false} alongside the other cases
(near "projects_no_slash") so the parser is exercised for a trailing slash after
the slug.
In `@internal/notes/wikilinks.go`:
- Around line 23-31: The ParseWikilink function currently returns the raw
capture m[1] as Wikilink.Project; normalize the parsed project slug centrally by
applying the project's slug-normalization helper (e.g. NormalizeProjectSlug or
equivalent) to m[1] before assigning to Wikilink.Project so cross-project
lookups are consistent; update ParseWikilink (and import) to call that helper on
m[1] when building the Wikilink for cross-projectRe matches.
In `@ui/src/components/graph/GraphCanvas.tsx`:
- Around line 52-89: The effect currently recreates a new Cosmos instance on
every nodes/links change; instead, only create the Cosmos renderer once and
reuse it: change the useEffect so that creation of new Cosmos(canvas, config),
assignment to graphRef.current, and graph.fitView() happen in an initialization
branch that runs once (e.g., on mount or when canvas changes) and do not run on
every nodes/links update, and then in a separate effect (or later in the same
effect) call graphRef.current.setData(nodes, links) whenever nodes or links
change; ensure the cleanup still destroys the instance on unmount
(graph.destroy() and graphRef.current = null) and that any config values that
must update (like pixelRatio or backgroundColor) are applied via the existing
graph API rather than reconstructing Cosmos.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a6d54f69-f5b5-48f4-8557-6238398d1653
⛔ Files ignored due to path filters (2)
ui/dist/index.htmlis excluded by!**/dist/**ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
.github/workflows/ci.yml.gitignorecmd/serve.gointernal/api/handlers.gointernal/api/notes_handlers.gointernal/api/router.gointernal/api/router_no_llm_test.gointernal/config/config.gointernal/config/config_test.gointernal/embedder/embedder.gointernal/llm/provider.gointernal/notes/graph.gointernal/notes/graph_test.gointernal/notes/wikilinks.gointernal/notes/wikilinks_test.goui/package.jsonui/src/components/graph/GraphCanvas.tsxui/src/components/notes/WikiLink.tsxui/src/components/notes/__tests__/WikiLink.test.tsxui/src/styles/globals.css
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- ui/src/components/notes/WikiLink.tsx
Comment on lines
+205
to
+211
| if h.provider == nil || h.embedder == nil { | ||
| writeJSON(w, http.StatusServiceUnavailable, map[string]string{ | ||
| "error": "LLM not configured; set llm.provider in config", | ||
| "code": "llm_disabled", | ||
| }) | ||
| return | ||
| } |
There was a problem hiding this comment.
Resolve the effective project provider before returning llm_disabled.
These guards key off h.provider/h.embedder, but search later resolves a project-specific provider from config. With a root provider=none and a valid per-project override, both endpoints will still short-circuit with 503 even though the project is configured correctly. upload should mirror the same per-project resolution and gate on that resolved provider/embedder instead of the root fields.
Also applies to: 394-400
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/api/handlers.go` around lines 205 - 211, The current early-return in
the handler checks h.provider and h.embedder and returns llm_disabled, but the
request handlers (e.g., search and upload) later resolve a per-project
provider/embedder from config; update the guards to resolve the effective
project-specific provider/embedder (the same resolution logic used by search)
and base the 503 gate on those resolved values instead of the root
h.provider/h.embedder; apply the same change to both the earlier guard and the
similar check around the code referenced at 394-400 so requests with a valid
per-project override do not incorrectly short-circuit.
Comment on lines
+184
to
+191
| if cfg.Server.APIKey != "" { | ||
| content = bytes.Replace( | ||
| content, | ||
| []byte("</head>"), | ||
| []byte(`<meta name="docsiq-api-key" content="`+html.EscapeString(cfg.Server.APIKey)+`"></head>`), | ||
| 1, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Do not expose the server API key in index.html.
This ships the same bearer token used by bearerAuthMiddleware to every browser via a readable <meta> tag. Any user who can load / can extract it from the DOM or network response and reuse it against the API, which effectively removes the protection the key is supposed to provide. Use a server-managed session/cookie or a short-lived scoped token instead of embedding cfg.Server.APIKey into the SPA.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/api/router.go` around lines 184 - 191, The code injects
cfg.Server.APIKey into index.html (bytes.Replace block) which exposes the bearer
used by bearerAuthMiddleware; remove that injection and stop embedding
cfg.Server.APIKey into the SPA. Instead implement server-managed session or
short-lived scoped tokens: delete the bytes.Replace call in router.go, add an
authenticated endpoint (e.g., /session or /api/token) that issues a short-lived
scoped token or sets an HttpOnly secure cookie, and update the SPA to request
that token at runtime; keep cfg.Server.APIKey only for server-to-server use and
ensure bearerAuthMiddleware verifies tokens issued by the new session/token
flow.
Comment on lines
+18
to
+23
| // New creates a new Embedder. If provider is nil (LLM disabled via | ||
| // provider=none), New returns nil. Callers must check for nil before use. | ||
| func New(provider llm.Provider, batchSize int) *Embedder { | ||
| if provider == nil { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
New now breaks callers that still assume a non-nil *Embedder.
internal/pipeline/pipeline.go stores embedder.New(...) directly and later dereferences p.embedder without guarding it, so this change can turn provider=none into a panic on indexing paths instead of a controlled llm_disabled failure. Please either keep New total and make Embed* return an explicit disabled error, or update Pipeline.New and its callers in the same PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/embedder/embedder.go` around lines 18 - 23, The change made
New(provider llm.Provider, batchSize int) *Embedder return nil when provider is
nil breaks callers that assume a non-nil *Embedder (e.g., Pipeline.New stores
embedder.New(...) and later dereferences p.embedder). Revert New to be total by
always returning a non-nil &Embedder (even when provider==nil) and add a
disabled flag or check inside Embedder methods (e.g., Embed, EmbedBatch) so they
return a clear llm_disabled error when provider==nil; alternatively, if you
prefer the nil result approach, update Pipeline.New and any callers that
dereference p.embedder to check for nil and handle llm-disabled paths explicitly
— reference New, type Embedder, and p.embedder / Pipeline.New to locate the
spots to change.
Comment on lines
47
to
+50
| func NewProvider(cfg *config.LLMConfig) (Provider, error) { | ||
| switch cfg.Provider { | ||
| case "none": | ||
| return nil, nil |
There was a problem hiding this comment.
Guard cfg == nil to avoid panic in NewProvider.
switch cfg.Provider will panic if cfg is nil; a defensive check keeps failure mode explicit.
🛡️ Proposed fix
func NewProvider(cfg *config.LLMConfig) (Provider, error) {
+ if cfg == nil {
+ return nil, fmt.Errorf("nil LLM config")
+ }
switch cfg.Provider {
case "none":
return nil, nil📝 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.
Suggested change
| func NewProvider(cfg *config.LLMConfig) (Provider, error) { | |
| switch cfg.Provider { | |
| case "none": | |
| return nil, nil | |
| func NewProvider(cfg *config.LLMConfig) (Provider, error) { | |
| if cfg == nil { | |
| return nil, fmt.Errorf("nil LLM config") | |
| } | |
| switch cfg.Provider { | |
| case "none": | |
| return nil, nil |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/llm/provider.go` around lines 47 - 50, Add a nil-check at the start
of NewProvider to explicitly handle a nil cfg instead of letting switch
cfg.Provider panic: if cfg == nil return nil, errors.New("nil LLMConfig") (or an
equivalent wrapped error), then proceed with the existing switch on
cfg.Provider; update imports to include "errors" (or "fmt") as needed.
Comment on lines
+91
to
+110
| if _, exists := crossStubs[nodeKey]; !exists { | ||
| crossStubs[nodeKey] = struct{}{} | ||
| stub := NoteNode{ | ||
| Key: nodeKey, | ||
| Title: titleFromKey(wl.Key), | ||
| Folder: folderFromKey(wl.Key), | ||
| Project: wl.Project, | ||
| } | ||
| if root != "" { | ||
| // Check whether the note file exists on disk. | ||
| candidate := filepath.Join(root, wl.Project, "notes", wl.Key+".md") | ||
| if _, serr := os.Stat(candidate); os.IsNotExist(serr) { | ||
| stub.Missing = true | ||
| } | ||
| } else { | ||
| // No root provided — mark missing conservatively. | ||
| stub.Missing = true | ||
| } | ||
| g.Nodes = append(g.Nodes, stub) | ||
| nodeIndex[nodeKey] = struct{}{} |
There was a problem hiding this comment.
Avoid creating a stub when that key already exists in the local graph.
nodeIndex is populated with the existing note keys, but this branch only dedupes against crossStubs. If the current project already has a real note keyed projects/B/x, a link to [[projects/B/x]] will append a second node with the same key and make graph lookups ambiguous. Please consult nodeIndex before appending the stub, or seed crossStubs from the existing nodes first.
🧰 Tools
🪛 golangci-lint (2.11.4)
[error] 96-96: : # github.com/RandomCodeSpace/docsiq/cmd [github.com/RandomCodeSpace/docsiq/cmd.test]
cmd/serve.go:96:34: undefined: sqlitevec.LoadInto
(typecheck)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/notes/graph.go` around lines 91 - 110, The code is creating
cross-project stubs without checking the existing local node index, causing
duplicate keys; before creating/appending the stub in the branch that builds
NoteNode (using variables nodeKey, crossStubs, wl.Key, NoteNode and appending to
g.Nodes), consult nodeIndex (the set of existing local note keys) or seed
crossStubs from nodeIndex first and skip stub creation if nodeKey already exists
in nodeIndex; update the condition that currently only checks crossStubs so it
also checks nodeIndex (or initialize crossStubs with nodeIndex entries) to
prevent adding duplicate nodes.
Comment on lines
+99
to
+104
| if root != "" { | ||
| // Check whether the note file exists on disk. | ||
| candidate := filepath.Join(root, wl.Project, "notes", wl.Key+".md") | ||
| if _, serr := os.Stat(candidate); os.IsNotExist(serr) { | ||
| stub.Missing = true | ||
| } |
There was a problem hiding this comment.
Clamp the existence check to <projectsRoot>/<slug>/notes.
wl.Key comes from note content, and ParseWikilink accepts any remainder after projects/<slug>/. A crafted target like [[projects/B/../../secrets]] makes this filepath.Join/os.Stat probe outside the target notes directory, and the resulting missing flag leaks filesystem existence through the graph API. Normalize the joined path and reject anything that escapes the expected notes root before calling os.Stat.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/notes/graph.go` around lines 99 - 104, The existence check for
candidate allows path traversal because wl.Key comes from ParseWikilink; instead
of directly calling filepath.Join(root, wl.Project, "notes", wl.Key+".md") and
os.Stat, construct the notesDir := filepath.Join(root, wl.Project, "notes"),
build candidate via filepath.Join(notesDir, wl.Key+".md"), then normalize and
validate the path (use filepath.Clean and ensure filepath.Rel(notesDir,
candidate) does not begin with ".." or use filepath.HasPrefix equivalent) to
reject any candidate that escapes notesDir before calling os.Stat; only then set
stub.Missing based on the safe os.Stat result, and treat escaped/invalid targets
as missing or invalid.
Comment on lines
+34
to
+38
| const { nodes, links } = useMemo(() => { | ||
| const tokens = typeof document === "undefined" ? null : readTokens(); | ||
| const kindColor: Record<GraphNode["kind"], string> = tokens | ||
| ? { entity: tokens.chart1, note: tokens.chart3, community: tokens.chart2 } | ||
| : { entity: "#3ecf8e", note: "#b08fe8", community: "#6ba6ff" }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and read the GraphCanvas.tsx file
find . -name "GraphCanvas.tsx" -type fRepository: RandomCodeSpace/docsiq
Length of output: 107
🏁 Script executed:
# Also check the directory structure
fd -t f "GraphCanvas" --maxdepth 5Repository: RandomCodeSpace/docsiq
Length of output: 105
🏁 Script executed:
cat -n ui/src/components/graph/GraphCanvas.tsx | head -100Repository: RandomCodeSpace/docsiq
Length of output: 3668
🏁 Script executed:
# Search for theme-related code and listeners
rg -i "theme|mutation.*observer|css.*variable" --type ts --type tsx ui/src --max-count 20Repository: RandomCodeSpace/docsiq
Length of output: 93
🏁 Script executed:
# Check globals.css for theme variable structure
find . -name "globals.css" -o -name "globals.scss"Repository: RandomCodeSpace/docsiq
Length of output: 93
🏁 Script executed:
# Search for theme-related code without file type
rg -i "theme|mutationobserver|useeffect.*theme" ui/src --max-count 25Repository: RandomCodeSpace/docsiq
Length of output: 4333
🏁 Script executed:
# Read globals.css to see theme variable structure
cat ui/src/styles/globals.cssRepository: RandomCodeSpace/docsiq
Length of output: 22975
🏁 Script executed:
# Check imports and usage in GraphCanvas.tsx
head -40 ui/src/components/graph/GraphCanvas.tsxRepository: RandomCodeSpace/docsiq
Length of output: 1472
🏁 Script executed:
# Check if useUIStore is used anywhere in graph components
rg "useUIStore" ui/src/components/graphRepository: RandomCodeSpace/docsiq
Length of output: 48
🏁 Script executed:
# Read Providers.tsx to understand theme change mechanism
cat ui/src/components/layout/Providers.tsxRepository: RandomCodeSpace/docsiq
Length of output: 1607
Theme toggle won't recolor the graph canvas.
The useMemo at line 34 computes node colors when data changes, and the useEffect at line 52 rebuilds the Cosmos config when nodes or links change. If the user toggles theme without new graph data, the canvas background and node colors remain stale—readTokens() reads the new CSS variables, but neither hook reruns.
Add theme tracking to dependency arrays so the graph responds to theme changes:
Fix: track theme changes in dependencies
+import { useEffect, useMemo, useRef, useState } from "react";
@@
export function GraphCanvas({ data }: { data: GraphData }) {
const canvasRef = useRef<HTMLCanvasElement>(null);
const graphRef = useRef<Cosmos<CosmosNode, CosmosLink> | null>(null);
+ const [themeKey, setThemeKey] = useState("");
+
+ useEffect(() => {
+ const root = document.documentElement;
+ const sync = () => setThemeKey(`${root.className}|${root.dataset.theme ?? ""}`);
+ sync();
+ const observer = new MutationObserver(sync);
+ observer.observe(root, { attributes: true, attributeFilter: ["class", "data-theme"] });
+ return () => observer.disconnect();
+ }, []);
@@
- }, [data]);
+ }, [data, themeKey]);
@@
- }, [nodes, links]);
+ }, [nodes, links, themeKey]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/graph/GraphCanvas.tsx` around lines 34 - 38, The node
colors are computed once in the useMemo that calls readTokens() and the Cosmos
config is rebuilt in the useEffect that depends on nodes/links, so toggling
theme doesn't rerun either; update the dependency arrays to include theme (or a
theme-derived value) so the useMemo that produces nodes/links and the useEffect
that rebuilds the Cosmos config rerun on theme changes—specifically modify the
useMemo that computes tokens/kindColor and the useEffect that rebuilds the
Cosmos config to include the theme identifier (or the output of readTokens()) in
their dependency lists.
| @@ -0,0 +1,465 @@ | |||
| @import "tailwindcss"; | |||
|
|
|||
| @custom-variant dark (&:is(.dark *)); | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether stylelint config ignores Tailwind v4 at-rules.
fd -HI 'stylelint.config.*' .
fd -HI '.stylelintrc*' .
rg -n 'at-rule-no-unknown|ignoreAtRules|custom-variant|theme|tailwind' .Repository: RandomCodeSpace/docsiq
Length of output: 15499
🏁 Script executed:
cat -n ./.stylelintrc.jsonRepository: RandomCodeSpace/docsiq
Length of output: 1692
Add Tailwind v4 directives to stylelint configuration.
Lines 3 and 96 use valid Tailwind v4 at-rules (@custom-variant and @theme), but the current stylelint configuration does not recognize them. Update .stylelintrc.json to whitelist these directives:
"at-rule-no-unknown": [true, {
"ignoreAtRules": ["custom-variant", "theme"]
}]🧰 Tools
🪛 Stylelint (17.7.0)
[error] 3-3: Unexpected unknown at-rule "@custom-variant" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/styles/globals.css` at line 3, The stylelint config must be updated so
Tailwind v4 at-rules are treated as known: add "custom-variant" and "theme" to
the at-rule-no-unknown ignore list in .stylelintrc.json (i.e. update the
"at-rule-no-unknown" rule to include an "ignoreAtRules" array containing
"custom-variant" and "theme") so directives like `@custom-variant` and `@theme` in
globals.css are not flagged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
37-41: Make bundle-size calc resilient to multiple matched files.Line 39 and Line 40 assume a single
index-*JS/CSS file. If multiple files match,statcan emit multiple values and break arithmetic. Sum matched files explicitly.Proposed adjustment
- name: Assert bundle budget (≤ 580 KB JS + CSS, initial chunk) run: | set -eu - js_bytes=$(stat -c %s ui/dist/assets/index-*.js 2>/dev/null || stat -f %z ui/dist/assets/index-*.js) - css_bytes=$(stat -c %s ui/dist/assets/index-*.css 2>/dev/null || stat -f %z ui/dist/assets/index-*.css) + js_bytes=$(find ui/dist/assets -maxdepth 1 -type f -name 'index-*.js' -exec stat -c %s {} + | awk '{s+=$1} END {print s+0}') + css_bytes=$(find ui/dist/assets -maxdepth 1 -type f -name 'index-*.css' -exec stat -c %s {} + | awk '{s+=$1} END {print s+0}') total=$((js_bytes + css_bytes))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 37 - 41, The bundle-size calculation assumes a single match for index-*.js and index-*.css; change the js_bytes and css_bytes assignments to aggregate sizes of all matched files (falling back to 0 when none match) by piping the stat output for all matches through a summing tool (e.g., awk) so js_bytes and css_bytes contain the total bytes for each glob, then compute total=$((js_bytes + css_bytes)); update the commands that set js_bytes and css_bytes (the variables referenced in the diff) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 37-41: The bundle-size calculation assumes a single match for
index-*.js and index-*.css; change the js_bytes and css_bytes assignments to
aggregate sizes of all matched files (falling back to 0 when none match) by
piping the stat output for all matches through a summing tool (e.g., awk) so
js_bytes and css_bytes contain the total bytes for each glob, then compute
total=$((js_bytes + css_bytes)); update the commands that set js_bytes and
css_bytes (the variables referenced in the diff) accordingly.
aksOps
added a commit
that referenced
this pull request
Apr 19, 2026
Graph view: - Replace @cosmograph/cosmos (blank-screen issues on mobile Safari) with SVG + d3-force + d3-zoom. 0.1×–40× zoom range, pinch + wheel + pan, reset button. viewBox expanded to container aspect so the graph fills the screen instead of letterboxing. - Hover / tap a node to highlight it and its direct neighbours; others dim to 20% opacity. Orphans don't trigger the dim effect. - Node radius scales with degree: r = 3.5 + √(degree) × 1.6. Hubs stand out; sqrt keeps extremes sane. - Labels disambiguated across projects: "overview" (collision) becomes "docsiq/overview", "codeiq/overview", etc. - Orphan nodes anchored to centre (stronger forceX/Y) so they don't drift off-screen; distance-max on charge force. Other: - Hard reload button in SiteHeader — unregisters SW, purges CacheStorage, reloads with cache-busting query. Useful on mobile where there's no ⌘⇧R. - useProjectStore persisted to localStorage (docsiq.project key). - README rewritten to reflect: notes + wikilinks + cross-project, all three LLM providers (+ "none" for zero-LLM serve), PWA, UI stack, kbd shortcuts, build path that assumes npm build (no committed dist). - Clean up stale tracked ui/dist assets that leaked through the squash merge of PR #12 — gitignore now correctly keeps only the placeholder index.html. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
aksOps
added a commit
that referenced
this pull request
Apr 19, 2026
…#13) Graph view: - Replace @cosmograph/cosmos (blank-screen issues on mobile Safari) with SVG + d3-force + d3-zoom. 0.1×–40× zoom range, pinch + wheel + pan, reset button. viewBox expanded to container aspect so the graph fills the screen instead of letterboxing. - Hover / tap a node to highlight it and its direct neighbours; others dim to 20% opacity. Orphans don't trigger the dim effect. - Node radius scales with degree: r = 3.5 + √(degree) × 1.6. Hubs stand out; sqrt keeps extremes sane. - Labels disambiguated across projects: "overview" (collision) becomes "docsiq/overview", "codeiq/overview", etc. - Orphan nodes anchored to centre (stronger forceX/Y) so they don't drift off-screen; distance-max on charge force. Other: - Hard reload button in SiteHeader — unregisters SW, purges CacheStorage, reloads with cache-busting query. Useful on mobile where there's no ⌘⇧R. - useProjectStore persisted to localStorage (docsiq.project key). - README rewritten to reflect: notes + wikilinks + cross-project, all three LLM providers (+ "none" for zero-LLM serve), PWA, UI stack, kbd shortcuts, build path that assumes npm build (no committed dist). - Clean up stale tracked ui/dist assets that leaked through the squash merge of PR #12 — gitignore now correctly keeps only the placeholder index.html. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Full UI redesign + Terminal identity system, ready for merge.
What landed
:root+.dark+@theme inline), single Supabase-green accent, 6px radius, deliberate 140/220ms motion oncubic-bezier(0.3, 0, 0, 1), Geist sans/mono with tabular-nums.globals.css.@layer componentswith@apply-based semantic classes (.page,.stats-strip,.mcp-shell,.note-row, etc.) — JSX uses class names only; shadcn primitives incomponents/ui/keep their internals untouched.AppSidebar,SiteHeader,SidebarProvider/Inset). Icon-only collapse, mobileSheetdrawer, project switcher in footer, theme toggle, ⌘K trigger in header.inputSchema+ raw-JSON toggle + call history with formatted results.theme-color,apple-touch-icon, dynamic<title>per route.SiteHeaderbelowmd:,Sheetdrawer nav,16pxinput font-size below 640px (no iOS Safari auto-zoom), safe-area insets viaenv(safe-area-inset-*)for notched devices.React.lazy()for all non-Home routes insideSuspense. Initial chunk 459 KB JS + 119 KB CSS = 578 KB, under the 580 KB CI gate.useNotesunwraps{keys:[]}→Note[],useNoteunwraps{note:{…}},useGraphmaps backend{key,title,folder,tags}→{id,label,kind},useDocstoleratesnull,useActivityskips NaN timestamps,formatRelativeTimeNaN-safe.~/projects/*into 126 notes with 437 wikilinks as a graph stress test.Test plan
npm test— 41/41 across 17 files (includesmatchMediapolyfill for shadcn'suse-mobilehook + updated Shell test for new markup)tsc -b --noEmitcleannpm run build— no errors, bundle under budget, lazy chunks split cleanlygo vet -tags sqlite_fts5 ./...cleango test -tags sqlite_fts5 ./internal/api ./internal/notes ./internal/mcp ./internal/storepasses (ran-shortsubset due to timeouts on full suite)Follow-ups intentionally deferred
docsiq serve— already works in practice (notes/graph endpoints don't call the provider), but a formalprovider: "none"config option +nil-safe handler guards would be cleaner. Small Go change; separate PR.sigma.js/cosmographwhen graphs grow past ~1k.ui/dist/— would clean git history but breaksgo install github.com/.../docsiq@latestunless we add a Node pre-build step. Separate decision.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation