UI: graph polish + persistence + mobile reload + README#13
Merged
Conversation
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>
📝 WalkthroughWalkthroughThis PR refactors the UI graph visualization from WebGL-based (Cosmos) to SVG-based rendering with D3, introduces a fallback component for missing WebGL2, improves graph layout physics, adds cache-busting reload functionality, and extensively updates documentation to reflect architectural changes. Changes
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Review the following changes in direct dependencies. 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: 2
🧹 Nitpick comments (4)
ui/src/components/graph/SVGGraphFallback.tsx (1)
44-45: Consider expanding viewBox to match container aspect ratio.Unlike
GraphCanvas, this fallback uses a fixedviewBoxmatching container dimensions without aspect-ratio expansion. This may cause letterboxing when container aspect differs from content bounds. The main renderer inGraphCanvas.tsx(lines 58-70) handles this.This is acceptable for a fallback renderer, but worth noting if visual consistency matters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/graph/SVGGraphFallback.tsx` around lines 44 - 45, SVGGraphFallback currently sets the SVG viewBox directly to the container size (viewBox={`0 0 ${size.w} ${size.h}`}) which can cause letterboxing when the container aspect ratio differs; update SVGGraphFallback to expand or adjust its viewBox to match the container aspect ratio similar to GraphCanvas (reference GraphCanvas resizing logic around its viewBox handling) so the fallback uses an aspect-ratio-aware viewBox calculation (based on size.w/size.h and desired content bounds) to avoid letterboxing and maintain visual consistency with GraphCanvas.ui/package.json (1)
37-38: Move@types/*packages todevDependencies.Type definition packages are only needed at compile time, not runtime. They should be in
devDependenciesto avoid bloating production installs.♻️ Proposed fix
Move these two entries from
dependenciestodevDependencies:"dependencies": { ... - "@types/d3-selection": "^3.0.11", - "@types/d3-zoom": "^3.0.8", ... }, "devDependencies": { ... "@types/d3-force": "^3.0.10", + "@types/d3-selection": "^3.0.11", + "@types/d3-zoom": "^3.0.8", "@types/markdown-it": "^14.1.2", ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/package.json` around lines 37 - 38, Move the type packages out of runtime dependencies: remove "@types/d3-selection" and "@types/d3-zoom" from dependencies and add them under devDependencies in package.json so type-only packages are installed only for development/compile time; ensure version strings remain the same and run npm/yarn install or update package-lock to reflect the change.ui/src/components/graph/GraphCanvas.tsx (1)
7-11: Consider extracting sharedCOLORconstant.This
COLORmap is duplicated inSVGGraphFallback.tsx(lines 7-11). Extract to a shared module to maintain consistency.♻️ Extract to shared constant
Create
ui/src/lib/graph-colors.ts:export const NODE_COLOR: Record<string, string> = { entity: "var(--chart-1)", note: "var(--chart-3)", community: "var(--chart-2)", };Then import in both components.
🤖 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 7 - 11, Extract the duplicated COLOR map into a shared constant module and import it from both components: create a new module (e.g., graph-colors.ts) that exports NODE_COLOR (Record<string,string>) with the same entries currently in COLOR, then replace the local COLOR definitions in GraphCanvas (COLOR) and SVGGraphFallback (duplicate) with imports of NODE_COLOR and update any usages accordingly (e.g., where GraphCanvas references COLOR, switch to NODE_COLOR). Ensure exports/imports use the same identifier in both components so the color mapping remains consistent across GraphCanvas and SVGGraphFallback.ui/src/components/site-header.tsx (1)
47-56: Consider using theButtoncomponent for consistency.The Search button above uses the
Buttoncomponent withvariant="outline"andsize="sm", while this uses a plain<button>. UsingButtonwould maintain visual consistency.♻️ Optional: Use Button component
- <button - type="button" - aria-label="Hard reload" - title="Hard reload (clears caches + service worker)" - onClick={() => { setReloading(true); void hardReload(); }} - disabled={reloading} - className="site-header-reload" - > - <RefreshCw className={reloading ? "size-4 animate-spin" : "size-4"} /> - </button> + <Button + variant="ghost" + size="icon" + aria-label="Hard reload" + title="Hard reload (clears caches + service worker)" + onClick={() => { setReloading(true); void hardReload(); }} + disabled={reloading} + className="site-header-reload" + > + <RefreshCw className={reloading ? "size-4 animate-spin" : "size-4"} /> + </Button>🤖 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 47 - 56, Replace the plain <button> in site-header.tsx with the existing Button component to match the Search button styling: keep the same aria-label, title, onClick handler (setReloading(true); void hardReload()), and disabled prop (reloading), pass variant="outline" and size="sm", and render the RefreshCw icon as the Button child with the same conditional className (reloading ? "size-4 animate-spin" : "size-4"); also add an import for Button if it's not already imported so Button is available in the module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 101: The fenced code block showing the project architecture (the
directory tree starting with "cmd/ CLI commands (cobra): index,
serve, search, projects, init, hooks, vec" and ending with "ui/
React 19 + Vite 6 SPA, embedded at compile time") is missing a language
identifier; update the opening fence from ``` to ```text so markdownlint MD040
is satisfied and the block is explicitly marked as plain text.
In `@ui/src/styles/globals.css`:
- Line 419: The h-[calc(100svh-var(--header-height))] utility in the .graph-page
rule is invalid because calc() needs spaces around the minus operator; update
the utility to h-[calc(100svh - var(--header-height))] (i.e., change .graph-page
{ `@apply` ... h-[calc(100svh-var(--header-height))]; } to use h-[calc(100svh -
var(--header-height))]) so the height is parsed correctly.
---
Nitpick comments:
In `@ui/package.json`:
- Around line 37-38: Move the type packages out of runtime dependencies: remove
"@types/d3-selection" and "@types/d3-zoom" from dependencies and add them under
devDependencies in package.json so type-only packages are installed only for
development/compile time; ensure version strings remain the same and run
npm/yarn install or update package-lock to reflect the change.
In `@ui/src/components/graph/GraphCanvas.tsx`:
- Around line 7-11: Extract the duplicated COLOR map into a shared constant
module and import it from both components: create a new module (e.g.,
graph-colors.ts) that exports NODE_COLOR (Record<string,string>) with the same
entries currently in COLOR, then replace the local COLOR definitions in
GraphCanvas (COLOR) and SVGGraphFallback (duplicate) with imports of NODE_COLOR
and update any usages accordingly (e.g., where GraphCanvas references COLOR,
switch to NODE_COLOR). Ensure exports/imports use the same identifier in both
components so the color mapping remains consistent across GraphCanvas and
SVGGraphFallback.
In `@ui/src/components/graph/SVGGraphFallback.tsx`:
- Around line 44-45: SVGGraphFallback currently sets the SVG viewBox directly to
the container size (viewBox={`0 0 ${size.w} ${size.h}`}) which can cause
letterboxing when the container aspect ratio differs; update SVGGraphFallback to
expand or adjust its viewBox to match the container aspect ratio similar to
GraphCanvas (reference GraphCanvas resizing logic around its viewBox handling)
so the fallback uses an aspect-ratio-aware viewBox calculation (based on
size.w/size.h and desired content bounds) to avoid letterboxing and maintain
visual consistency with GraphCanvas.
In `@ui/src/components/site-header.tsx`:
- Around line 47-56: Replace the plain <button> in site-header.tsx with the
existing Button component to match the Search button styling: keep the same
aria-label, title, onClick handler (setReloading(true); void hardReload()), and
disabled prop (reloading), pass variant="outline" and size="sm", and render the
RefreshCw icon as the Button child with the same conditional className
(reloading ? "size-4 animate-spin" : "size-4"); also add an import for Button if
it's not already imported so Button is available in the module.
🪄 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: eb85b042-1075-4bd1-8732-d6c66accbb45
⛔ Files ignored due to path filters (26)
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/editor-l0sNRNKZ.jsis excluded by!**/dist/**ui/dist/assets/graph-mfGjgYhn.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/fonts/Geist-400.woff2is excluded by!**/dist/**,!**/*.woff2ui/dist/fonts/Geist-500.woff2is excluded by!**/dist/**,!**/*.woff2ui/dist/fonts/Geist-600.woff2is excluded by!**/dist/**,!**/*.woff2ui/dist/fonts/GeistMono-400.woff2is excluded by!**/dist/**,!**/*.woff2ui/dist/fonts/GeistMono-500.woff2is excluded by!**/dist/**,!**/*.woff2ui/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/manifest.webmanifestis excluded by!**/dist/**ui/dist/sw.jsis excluded by!**/dist/**ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
README.mdui/package.jsonui/src/components/graph/GraphCanvas.tsxui/src/components/graph/SVGGraphFallback.tsxui/src/components/site-header.tsxui/src/lib/graph-layout.tsui/src/lib/hard-reload.tsui/src/routes/Graph.tsxui/src/stores/project.tsui/src/styles/globals.css
|
|
||
| # 5. Start server | ||
| docsiq serve --port 8080 | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the architecture code fence.
Line 101 uses a fenced block without a language, which triggers markdownlint MD040.
Suggested fix
-```
+```text
cmd/ CLI commands (cobra): index, serve, search, projects, init, hooks, vec
internal/
api/ REST API + /mcp handler
@@
ui/ React 19 + Vite 6 SPA, embedded at compile time</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 101-101: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 101, The fenced code block showing the project
architecture (the directory tree starting with "cmd/ CLI commands
(cobra): index, serve, search, projects, init, hooks, vec" and ending with "ui/
React 19 + Vite 6 SPA, embedded at compile time") is missing a language
identifier; update the opening fence from ``` to ```text so markdownlint MD040
is satisfied and the block is explicitly marked as plain text.
| .graph-page-head { @apply flex items-baseline justify-between px-6 py-4 border-b border-border; } | ||
| .graph-canvas-wrap { @apply flex-1 min-h-0 relative; } | ||
| .graph-canvas { @apply absolute inset-0 w-full h-full block; } | ||
| .graph-page { @apply flex flex-col flex-1 min-h-0 h-[calc(100svh-var(--header-height))]; } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In CSS, is calc(100svh-var(--header-height))valid syntax, or must subtraction be written ascalc(100svh - var(--header-height))with spaces around-?
💡 Result:
No, calc(100svh-var(--header-height)) is not valid CSS syntax. Subtraction must be written as calc(100svh - var(--header-height)) with spaces around the - operator. The CSS specification requires whitespace around the + and - operators in calc expressions to distinguish them from unary minus signs in dimensions (e.g., negative lengths like -8px). Without spaces, 100svh-var(--header-height) is parsed as a dimension 100svh followed by an identifier or invalid token starting with -var(...), making it invalid. This rule applies regardless of using viewport units like svh or CSS custom properties like var(--header-height). All examples in MDN documentation and the CSS Values and Units spec use spaces around these operators.
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/CSS/calc%28%29
- 2: https://developer.mozilla.org/en-US/docs/Web/CSS/calc
- 3: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/calc-sum
- 4: https://stackoverflow.com/questions/30412580/why-is-css-calc100-250px-not-working
- 5: https://stackoverflow.com/questions/33925997/css-calc-invalid-property-value
- 6: https://developer.mozilla.org/en-US/docs/Web/CSS/calc-sum
- 7: http://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/calc
- 8: https://johnkavanagh.co.uk/articles/dynamic-calculations-in-css-using-calc
- 9: https://johnkavanagh.co.uk/articles/dynamic-calculations-in-css-using-calc/
- 10: https://calmops.com/programming/web/viewport-units-svh-dvh-lvh/
Fix invalid calc() syntax in graph page height.
At Line 419, h-[calc(100svh-var(--header-height))] produces invalid CSS. The CSS calc() function requires spaces around the - operator; without them, 100svh-var(--header-height) is parsed as a dimension followed by an invalid token, causing the height rule to be ignored.
✅ Proposed fix
- .graph-page { `@apply` flex flex-col flex-1 min-h-0 h-[calc(100svh-var(--header-height))]; }
+ .graph-page { `@apply` flex flex-col flex-1 min-h-0; height: calc(100svh - var(--header-height)); }📝 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
| .graph-page { @apply flex flex-col flex-1 min-h-0 h-[calc(100svh-var(--header-height))]; } | |
| .graph-page { `@apply` flex flex-col flex-1 min-h-0; height: calc(100svh - var(--header-height)); } |
🤖 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 419, The
h-[calc(100svh-var(--header-height))] utility in the .graph-page rule is invalid
because calc() needs spaces around the minus operator; update the utility to
h-[calc(100svh - var(--header-height))] (i.e., change .graph-page { `@apply` ...
h-[calc(100svh-var(--header-height))]; } to use h-[calc(100svh -
var(--header-height))]) so the height is parsed correctly.
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
Iterative polish on top of the merged UI redesign (#12).
Graph view
@cosmograph/cosmos— rendered blank on mobile Safari. Replaced with SVG +d3-force+d3-zoom.r = 3.5 + √(degree) × 1.6. Hubs read clearly; extremes stay sane.docsiq/overviewvscodeiq/overviewetc.distanceMax(260).Misc
SiteHeader— unregisters SW, purges CacheStorage, reloads with?_r=<ts>. Mobile-friendly⌘⇧Rsubstitute.useProjectStore.slugnow sticks inlocalStorage["docsiq.project"]. Theme/sidebar/drawer pins already persisted.none, PWA, kbd shortcuts, new build path (npm build before go build;ui/dist/is no longer committed).ui/dist/assets/*and fonts that leaked through the squash merge of UI redesign — v1 (React 19 + Tailwind 4 + shadcn/ui) #12.ui/dist/index.htmlplaceholder stays.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Style