Repository navigation
v2.1.9 - UX Tweaks. - #59
Conversation
…fix (v2.1.9) - Fix date picker portal clicks dismissing dialogs (pointer-events + onInteractOutside) - Render kanban card descriptions as markdown with expand/collapse - Move card tags to bottom, add xs badge size variant - Add wikilinks + semantic links sections to graph detail panel - Add linked tasks / linked notes collapsible sections to graph detail panel - Deep-link 'Open in Board' to scroll-to-column + open-card-detail - Switch 5 components to shared NoteMarkdownPreview (MDPreviewPanel, AboutSection, board archive, card-detail, AiSummaryNode) - Increase graph snippet length 120 → 600 chars - Add changelog v2.1.9
…cape hatch, tooltips (v2.1.9)
…pty state, edge count, CairnEvents
…, dbl-click position, ai_summary edit, swatch colors
…rs, cross-platform copy
… edge legend, persistence, tag links, breadcrumb
…hitespace, overflow fix
…es, escape close, type-to-confirm, export error
…te, a11y for icon buttons
- Add carry-buffer SSE line parser (electron/lib/sse.ts) so data: records
straddling reader.read() chunk boundaries are no longer dropped — the
root cause of truncated note titles and malformed tool-call JSON.
- Both chat loop (electron/ipc/chat.ts) and coding agent loop
(electron/lib/pi-agent-loop.ts) replaced their split('\n') anti-pattern
with iterSseData(); both now surface JSON.parse errors to the model
instead of silently substituting args={} and executing the tool.
- ensure_note title matching now uses normalizeNoteTitle() (trim +
collapse internal whitespace) at both lookup sites (notes.ts and
chat-executor.ts wrapper), so whitespace-variant titles dedup. Wrapper
and canonical path no longer risk disagreeing on the match key.
- Dev-gated trace logging (electron/lib/tool-trace.ts) at sse-args,
parse, and lookup stages — active only when CAIRN_TOOL_TRACE=1 or
NODE_ENV=development; logs lengths + short SHA-256 + head/tail only.
- Regression tests (electron/pipeline-regression.test.ts) cover
chunk-boundary reassembly and title normalization.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughv2.1.9 introduces a shared ChangesSSE Tool-Call Pipeline Fix
Frontend UI: Graph, Kanban, Flow, Notes, Settings
Sequence Diagram(s)sequenceDiagram
participant Provider as AI Provider (SSE)
participant iterSseData as iterSseData
participant chat_ts as chat.ts / pi-agent-loop.ts
participant traceTool as traceTool
participant executeTool as executeTool / executeSingleTool
Provider-->>iterSseData: ReadableStream Uint8Array chunks
loop SSE frames (carry-buffered)
iterSseData-->>chat_ts: yield data: payload string
chat_ts->>chat_ts: accumulate contentBuffer / toolCallBuffers[index]
end
chat_ts->>traceTool: traceTool("sse-args", {name, args})
chat_ts->>chat_ts: JSON.parse(tc.function.arguments)
alt parse success
chat_ts->>traceTool: traceTool("parse", {title, content, raw})
chat_ts->>executeTool: executeTool(name, args)
executeTool-->>chat_ts: result
else parse failure
chat_ts->>chat_ts: emitToolCall + emitToolCallDone (UI events)
chat_ts->>chat_ts: push tool role message {error: parseError}
chat_ts-->>Provider: continue to next tool call (no execution)
end
sequenceDiagram
participant User
participant GraphDetailPanel
participant CairnEvents
participant notesView as notes-view.tsx
participant board as board / KanbanColumn
User->>GraphDetailPanel: click tag node "Browse tagged notes"
GraphDetailPanel->>CairnEvents: dispatch filterByTag(tagId) via rAF
GraphDetailPanel->>GraphDetailPanel: setView("notes"), close panel
CairnEvents-->>notesView: cairn:filter-by-tag event
notesView->>notesView: setActiveTagId(tagId)
User->>GraphDetailPanel: click card node "Open in Board"
GraphDetailPanel->>GraphDetailPanel: setView("board"), rAF
GraphDetailPanel->>CairnEvents: dispatch scrollToColumn(columnId)
GraphDetailPanel->>CairnEvents: dispatch openCard(cardId)
CairnEvents-->>board: scroll column + open card detail modal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed due to a network error. Comment |
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (1)
src/components/layout/sidebar.tsx (1)
219-223: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winPrecompute project card counts once instead of filtering inside each row.
Line 222 recalculates a full
cards.filter(...)for every project render. Pre-aggregating counts once per render avoids repeated scans.⚡ Suggested refactor
const projects = React.useMemo( () => activeWorkspaceId ? getWorkspaceProjects(activeWorkspaceId) : [], // eslint-disable-next-line react-hooks/exhaustive-deps [activeWorkspaceId, workspaces], ); + const openCardCountByProject = React.useMemo(() => { + const counts = new Map<string, number>(); + for (const card of cards) { + if (card.archivedAt) continue; + counts.set(card.projectId, (counts.get(card.projectId) ?? 0) + 1); + } + return counts; + }, [cards]); @@ - openCardCount={cards.filter((c) => c.projectId === project.id && !c.archivedAt).length} + openCardCount={openCardCountByProject.get(project.id) ?? 0}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/sidebar.tsx` around lines 219 - 223, The openCardCount calculation on line 222 performs a full cards.filter scan for every project in the render loop, causing redundant iterations over the cards array. Create a precomputed map object before rendering the project list that aggregates card counts by projectId (filtering for non-archived cards), then replace the inline cards.filter expression with a simple lookup into this map to retrieve the count for each project.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/graph/GraphDetailPanel.tsx`:
- Line 383: The hover text color class at line 383 uses the raw Tailwind color
name `hover:text-white`, which violates the repository's color-token rule
requiring all colors to use CSS variables. Replace `hover:text-white` with an
appropriate CSS variable token such as `hover:text-[var(--text-primary)]` or
another semantic token that matches your design system to ensure consistency
with the styling guidelines.
- Around line 77-90: The semantic array is sorted by weight but does not
deduplicate entries by noteId, while the keyed rows use key={link.noteId}. This
can result in duplicate keys and unstable React rendering when multiple edges
reference the same note. After sorting the semantic array by weight, add
deduplication logic to ensure each noteId appears only once, keeping the
highest-weighted entry for each note. Apply the same deduplication pattern to
the other locations mentioned (also applies to lines 325-328).
- Around line 127-139: The current code uses a single requestAnimationFrame to
defer the dispatch of CairnEvents.scrollToColumn and CairnEvents.openCard
events, but this can execute before the destination useEffect listeners have
attached, causing events to be dropped. Wrap the entire existing
requestAnimationFrame callback (which contains the calls to window.dispatchEvent
for both CairnEvents.scrollToColumn and CairnEvents.openCard) inside another
requestAnimationFrame call to defer the event dispatch to a later animation
frame, ensuring the listeners are mounted and ready to receive the events.
In `@src/components/graph/KnowledgeGraphView.tsx`:
- Around line 66-73: The initialization of the spacing and semanticThreshold
states can receive NaN values when parseFloat processes invalid data, which will
corrupt the slider values and display. Add validation using isFinite() to both
state initializers to check that the parsed float values are valid finite
numbers. If a parsed value from localStorage is not finite, return the
corresponding default value instead (1.2 for spacing state and 1.0 for
semanticThreshold state). Apply this check after calling parseFloat but before
setting the state value.
- Around line 19-24: The EDGE_LEGEND constant initializes color values once
using resolveCssVar(), which resolves CSS variables to static values at startup.
When the theme changes at runtime, these cached color values become stale and
don't update. To fix this, either store the CSS variable strings directly in the
EDGE_LEGEND array (such as "var(--accent)" instead of the resolved color value)
and let the CSS system resolve them dynamically, or move the resolveCssVar()
calls from the EDGE_LEGEND initialization into the render or component logic
where they can be re-evaluated on each render when the theme changes.
- Around line 122-137: The current search matching logic in the filter operation
for matchingIds is inefficiently scanning tagMemberEdges for every node being
checked, creating O(nodes × edges) complexity. Pre-compute a Map that associates
each node ID with its array of connected tag IDs before the filter operation
begins (build it once from tagMemberEdges where e.source maps to node IDs), then
replace the tagMemberEdges.filter call inside the filter function with a simple
lookup in this pre-computed map to directly get the tag IDs for each node.
In `@src/components/kanban/card.tsx`:
- Around line 145-149: The status badge styling in the span className uses
slash-alpha syntax (bg-[var(--danger)]/10 and bg-[var(--warning)]/10) which
deviates from the repository standard. Replace these slash-alpha background
classes with the color-mix pattern following the guideline: use color-mix(in
srgb, var(--token) X%, transparent) for alpha variants instead. Apply this
change to both the "overdue" status styling (currently bg-[var(--danger)]/10)
and the "today" status styling (currently bg-[var(--warning)]/10) to maintain
consistency with the codebase conventions.
In `@src/components/kanban/column.tsx`:
- Line 347: The pluralization ternary operator in the archived cards display
text is ineffective because it returns an empty string in both branches. In the
section displaying the archived cards count using archivedCards.length, fix the
ternary conditional to return "s" when the length is not equal to 1 (for
plural), and an empty string when it equals 1 (for singular). This will properly
pluralize the word "archived" to "archiveds" when there are multiple archived
cards.
In `@src/components/notes/notes-view.tsx`:
- Around line 307-313: The button element that clears the filter (with
onClick={() => setFilter("")}) is missing an accessible name, which prevents
assistive technologies from announcing its purpose. Add an aria-label attribute
to this button with a descriptive label such as "Clear filter" or "Clear search"
to ensure screen readers can properly communicate the button's function to
users.
In `@src/components/settings/DataSettings.tsx`:
- Around line 99-108: The text input field in the delete confirmation section
lacks a programmatic label, making it inaccessible to screen reader users. Add
an explicit label for the input element by either creating a label element with
an id and associating it to the input via htmlFor attribute, or by adding an
aria-labelledby attribute to the input that references the id of the existing
instruction paragraph above it, or by adding an aria-label attribute directly to
the input with descriptive text such as "Type DELETE to confirm deletion".
In `@src/components/settings/MobileSettings.tsx`:
- Around line 126-133: The span element containing "Scan to connect" text uses
text-[10px] for font sizing, which violates the rem-based Tailwind class
requirement and prevents scaling with user font settings. Replace the
text-[10px] class on the span with an appropriate rem-based Tailwind text size
class (such as text-xs or text-sm) that will scale properly with the user's font
size preferences.
In `@src/components/settings/shared.tsx`:
- Around line 50-52: The id prop is being injected into the children element in
lines 50-52 but is not being forwarded to the actual switch/toggle element
rendered in lines 63-78, breaking the label htmlFor binding. Modify the code in
the lines 63-78 section (likely in the Toggle component or switch rendering) to
ensure the id prop is passed through and applied to the actual input element so
that the label can properly reference it via htmlFor.
In `@src/components/settings/TagsSettings.tsx`:
- Around line 10-23: The color palette array in TagsSettings.tsx contains
hardcoded hex color values that violate the coding guideline requiring all
colors to use CSS variables. Replace each hardcoded hex value in the color
property of the palette objects (the entries with "Indigo", "Violet", "Purple",
"Pink", "Rose", "Red", "Orange", "Yellow", "Green", "Teal", "Cyan", "Blue",
"Slate", and "Stone" names) with appropriate CSS variable references using the
var() function instead of the literal hex codes like "`#6366f1`", "`#8b5cf6`", etc.
- Around line 66-71: The color-swatch trigger button in TagsSettings.tsx lacks
proper accessibility for screen readers. Add an aria-label attribute to the
button element that contains the onClick handler with setColorPickerId. The
aria-label should provide a clear, descriptive action text (such as "Change
color" or "Change tag color") that screen readers will announce, ensuring the
button's purpose is accessible to all users. This should accompany or replace
the existing title attribute for proper screen reader support.
---
Nitpick comments:
In `@src/components/layout/sidebar.tsx`:
- Around line 219-223: The openCardCount calculation on line 222 performs a full
cards.filter scan for every project in the render loop, causing redundant
iterations over the cards array. Create a precomputed map object before
rendering the project list that aggregates card counts by projectId (filtering
for non-archived cards), then replace the inline cards.filter expression with a
simple lookup into this map to retrieve the count for each project.
🪄 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 Plus
Run ID: 12e3e9db-5d83-4ffb-9b99-2f1e4acdc3a8
📒 Files selected for processing (49)
changelogs/v2.1.9.mdelectron/db/graph-queries.tselectron/ipc/chat-executor.tselectron/ipc/chat.tselectron/lib/pi-agent-loop.tselectron/lib/sse.tselectron/lib/tool-trace.tselectron/mcp/tools/notes.tselectron/pipeline-regression.test.tselectron/shared/text-utils.tssrc/app/globals.csssrc/components/flow/NodeEditModal.tsxsrc/components/flow/edges/FlowEdge.tsxsrc/components/flow/flow-view.tsxsrc/components/flow/nodes/AiSummaryNode.tsxsrc/components/flow/nodes/GroupNode.tsxsrc/components/flow/nodes/IdeaNode.tsxsrc/components/flow/nodes/NoteRefNode.tsxsrc/components/flow/nodes/TaskRefNode.tsxsrc/components/flow/nodes/UrlNode.tsxsrc/components/graph/ForceGraphCanvas.tsxsrc/components/graph/GraphDetailPanel.tsxsrc/components/graph/KnowledgeGraphView.tsxsrc/components/graph/RadialTreeCanvas.tsxsrc/components/kanban/board.tsxsrc/components/kanban/card-detail.tsxsrc/components/kanban/card.tsxsrc/components/kanban/column.tsxsrc/components/layout/project-overview/useProjectMetrics.tssrc/components/layout/sidebar.tsxsrc/components/notes/MDPreviewPanel.tsxsrc/components/notes/WikilinkPicker.tsxsrc/components/notes/notes-view.tsxsrc/components/notes/notes-view/NoteListItem.tsxsrc/components/settings/AISettings.tsxsrc/components/settings/AboutSection.tsxsrc/components/settings/AgentSettings.tsxsrc/components/settings/DataSettings.tsxsrc/components/settings/EmbeddingsSettings.tsxsrc/components/settings/GeneralSettings.tsxsrc/components/settings/MobileSettings.tsxsrc/components/settings/ShortcutsSettings.tsxsrc/components/settings/TagsSettings.tsxsrc/components/settings/ViewVisibilitySettings.tsxsrc/components/settings/shared.tsxsrc/components/ui/badge.tsxsrc/components/ui/date-picker.tsxsrc/components/ui/dialog.tsxsrc/lib/events.ts
- Remove MIN_SPLASH_MS=3000 artificial floor and splashOpenedAt timing from main.ts closeSplash(). Splash now closes as soon as the main window finishes loading + 200ms CSS transition. - Remove the ambient cycle from bootsplash.ts (6 fake messages, 250ms interval, creep-toward-95% bar). Progress bar now only moves on real boot events. - Add real progress events for pre-boot work in main.ts: Opening
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
electron/main.ts (1)
232-242: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueMinor timing mismatch with CSS transition duration.
The 200ms delay is slightly shorter than the 300ms CSS transition defined in bootsplash.ts line 167 (
transition: width 300ms ease). In practice, this is unlikely to be noticeable since the progress bar is probably already at 95%+ when "Ready" is sent, so the actual transition duration is shorter than 300ms.If you want perfect timing alignment, consider using 300-350ms to account for the full transition plus any frame timing variations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/main.ts` around lines 232 - 242, The closeSplash function uses a 200ms setTimeout delay before showing the main window, but this is shorter than the 300ms CSS transition duration defined in bootsplash.ts. To achieve perfect timing alignment with the CSS transition and account for frame timing variations, increase the setTimeout delay from 200ms to 300-350ms.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@electron/main.ts`:
- Around line 202-214: The pre-boot progress events in main.ts are emitting
specific percentages (3% and 6%) that conflict with the boot-sequence.ts
progress events, causing the progress bar to reset to 0% when actual migrations
begin. Remove the pct property from both splash.progress() calls (the one with
label "Opening database…" and the one with label "Registering handlers…") to use
only the step and label properties, allowing boot-sequence.ts to own the full
0-100% progress range and prevent the backwards jump. Keep the step and label
values to provide user feedback without numerical progress values.
---
Nitpick comments:
In `@electron/main.ts`:
- Around line 232-242: The closeSplash function uses a 200ms setTimeout delay
before showing the main window, but this is shorter than the 300ms CSS
transition duration defined in bootsplash.ts. To achieve perfect timing
alignment with the CSS transition and account for frame timing variations,
increase the setTimeout delay from 200ms to 300-350ms.
🪄 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 Plus
Run ID: ee5d55bd-a041-4a6b-a6ca-9efd24a06b2b
📒 Files selected for processing (13)
electron/main.tselectron/splash/boot-sequence.tselectron/splash/bootsplash.tssrc/components/graph/GraphDetailPanel.tsxsrc/components/graph/KnowledgeGraphView.tsxsrc/components/kanban/card.tsxsrc/components/kanban/column.tsxsrc/components/layout/sidebar.tsxsrc/components/notes/notes-view.tsxsrc/components/settings/DataSettings.tsxsrc/components/settings/MobileSettings.tsxsrc/components/settings/TagsSettings.tsxsrc/components/settings/shared.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/settings/MobileSettings.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- src/components/settings/TagsSettings.tsx
- src/components/layout/sidebar.tsx
- src/components/settings/shared.tsx
- src/components/notes/notes-view.tsx
- src/components/settings/DataSettings.tsx
- src/components/graph/KnowledgeGraphView.tsx
- src/components/graph/GraphDetailPanel.tsx
- src/components/kanban/card.tsx
Pre-boot progress events in main.ts (Opening database…, Registering handlers…) were sending pct: 3 and pct: 6, but boot-sequence.ts starts migrations at pct: 0 — causing the progress bar to jump backwards. Removed pct from both pre-boot events so they only update the label. Made SplashProgress.pct optional and updated the splash:progress handler to only set the bar width when pct is explicitly provided, keeping the current position otherwise.
What does this PR do?
v2.1.9 — a broad UX/UI quality release spanning the Knowledge Graph, Idea Flow, Settings, Kanban board, and the AI chat tool-call pipeline. The headline fix is a root-cause repair for SSE tool-call argument assembly (which was silently dropping streamed fragments at
reader.read()boundaries, causing truncated note titles and malformed tool calls). Beyond that, the release adds markdown rendering on board cards, a restructured graph detail panel with links/linked-tasks/linked-notes sections, comprehensive accessibility and hardcoded-colour audits across Settings/Idea Flow/Knowledge Graph, and numerous polish fixes across all views.Type of change
Screenshots / recording
Checklist
npm run type-check:allpassesnpm run lintpassesnpm testpasses (569 tests, 31 files — includeselectron/pipeline-regression.test.ts)npm run test:e2epasses (run before merging UI changes or cutting a release)var(--accent),var(--text-primary), etc.)text-[Npx]pixel font classes — rem equivalents only (text-[0.714rem],text-xs, etc.)handle()and returnIpcResult<T>(no new IPC handlers)schema.ts(no new migrations)electron/db/queries.ts— single source of truth (no new SQL)dependenciesordevDependenciesadded toROLE_MAPinscripts/generate-licenses.jsandlicenses.jsonregenerated (no new deps)electron/mcp/tools/index.tsdispatch +electron/lib/tool-schemas.tsZod schema (no new MCP tools)--external:<pkg>flag in thecompilescript: updated the appropriate allowlist group (no compile changes)Notes for reviewer
Critical fix — SSE assembly pipeline:
The most important change is the SSE tool-call assembly fix (
electron/lib/sse.ts+ changes inelectron/ipc/chat.tsandelectron/lib/pi-agent-loop.ts). This was diagnosed after a proxy-team investigation confirmed the OpenAI proxy was a clean passthrough — the corruption was in Cairn's SSE line-splitting. The newiterSseData()async generator uses a carry buffer to survive chunk boundaries. Regression tests inelectron/pipeline-regression.test.tscover 12 arbitrary boundary positions.Dev-only trace logging:
electron/lib/tool-trace.tslogs SHA-256/length/head-tail atsse-args,parse, andlookupstages. Active only whenCAIRN_TOOL_TRACE=1orNODE_ENV=development. Zero output in packaged builds — safe to leave in.Key areas to review:
electron/lib/sse.ts,electron/ipc/chat.ts:171-217,electron/lib/pi-agent-loop.ts:546-602) — verify the carry-buffer logic handles[DONE]termination and final unterminated lines correctly.electron/ipc/chat.ts:247-276,electron/lib/pi-agent-loop.ts:654-685) — verify the error message is pushed back to the model as atool-role message so the model can recover.ensure_notenormalization (electron/shared/text-utils.ts:34-37,electron/mcp/tools/notes.ts:46-48,electron/ipc/chat-executor.ts:147-150) — verify both lookup sites use the samenormalizeNoteTitle()predicate (this was a previous source of duplicate-note bugs when they disagreed).src/components/graph/GraphDetailPanel.tsx) — flex-column layout with pinned collapsible sections. Verify the link/task/note navigation works and sections expand/collapse correctly.src/components/ui/DialogContent.tsx,src/globals.css) — verify the date picker no longer dismisses the parent dialog when clicked, and thatonInteractOutsidecorrectly identifies portaled children.Summary by CodeRabbit