Repository navigation
feat(notes,board): cross-project drag + fix live project-rename folder move - #90
Conversation
… toggle + live trace Adds an optional subagent architecture to the in-app AI chat: a thin dispatcher delegates read-only research and write work to focused sub-agents, each with a reduced tool set. On capable models this cut prompt tokens ~43% on research-heavy tasks at equal output quality (benchmarks + LLM-as-judge scorer included). - Extract runToolLoop into shared electron/lib/chat-loop.ts (behaviour unchanged), add optional tool-array override + test-only arg hook. - electron/lib/chat-subagent-loop.ts: runDispatchLoop with research/write sub-agents, forced final-synthesis turn to rescue empty output on weak models, streaming SubagentEvents (start/done/token/thought/tool/usage). - Context accounting: main ring shows the DISPATCHER's context (system + briefs); each sub-agent reports its own usage for a dedicated ring. - Per-thread toggle persisted via schema v29 (chat_threads.use_subagents, chat_messages.subagents) + idempotent ensureColumns guard for version drift. - Renderer: useChatStream accumulates subagents; ChatSubagentBlock renders an expandable live/persisted trace; toolbar toggle (hidden on on-device Llama). - Tests: token/latency benchmark, fault-injection recovery, quality scorer, streaming smoke.
…r move - Fix: renaming a project now relocates its notes folder on disk immediately (no restart). A racing autosave could pre-create the new-slug folder, forcing renameProjectNotesDir into a merge whose collision-skip left a stale duplicate + the old folder behind until the next launch. mergeDirInto now removes same-note-id duplicates and clears the old dir (incl. OS cruft); db-handlers reconciles as a fallback when the primary move no-ops. Regression test added. - Feat: drag a folder in the notes sidebar onto another folder (or "Move to root") to reparent the whole subtree (moveFolder). - Feat: drag a folder, a single note, or a task card onto a project row in the leftmost sidebar to move it cross-project. Cards land in the target project's same-type column (fallback backlog/first). Shared cross-project-dnd payload bridges native DnD (notes) and dnd-kit (board) via a data-project-drop-id hit-test.
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR adds cross-project drag-and-drop for notes, folders, and cards; introduces optional dispatcher-based subagent chat with streamed and persisted traces; extracts shared chat-loop logic; hardens project rename cleanup; and adds benchmarks and quality-scoring tests. ChangesCross-project drag-and-drop
Subagent chat
Project rename cleanup
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Remove unnecessary eslint-disable no-console directives and an unused afterEach import from the subagent test harnesses.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
electron/lib/chat-agent-benchmark.test.ts (1)
73-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffExtract the duplicated live-endpoint harness into a shared test helper. All four new benchmark files re-declare the same
BASE_URL/MODEL/API_KEYenv parsing (vianormaliseBaseUrl) plus an identicalendpointUp()reachability probe. Consolidating into one helper (e.g.electron/lib/__testutils__/live-endpoint.tsexporting the config +endpointUp) removes drift risk when the probe or endpoint convention changes.
electron/lib/chat-agent-benchmark.test.ts#L73-L83: replaceendpointUp(and reuse sharedBASE_URL/MODEL/API_KEYfrom L52-56) with the shared helper.electron/lib/chat-quality-scorer.test.ts#L133-L138: replace the parameterizedendpointUp(url, key)and config L43-45 with the shared helper (keep the judge-endpoint override wiring).electron/lib/chat-subagent-stream.test.ts#L28-L33: replaceendpointUpand config L24-26 with the shared helper.electron/lib/chat-write-recovery-benchmark.test.ts#L92-L100: replaceendpointUpand config L48-50 with the shared helper.🤖 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/lib/chat-agent-benchmark.test.ts` around lines 73 - 83, Extract the duplicated live-endpoint configuration and reachability probe into a shared helper exporting BASE_URL, MODEL, API_KEY, and endpointUp. Update electron/lib/chat-agent-benchmark.test.ts:73-83, electron/lib/chat-quality-scorer.test.ts:133-138, electron/lib/chat-subagent-stream.test.ts:28-33, and electron/lib/chat-write-recovery-benchmark.test.ts:92-100 to consume it; preserve the quality scorer’s judge-endpoint override wiring while removing local config parsing and endpointUp implementations.src/store/slices/notes.ts (1)
207-277: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate "collect affected notes in a folder subtree" logic between
moveFolderandmoveFolderToProject.The
srcLower/affectedfilter block is identical in both methods (only theprojectIdfield name differs). Extract a small helper, e.g.notesInFolderSubtree(notes, projectId, source), to avoid divergent fixes later (e.g. if the case-insensitive matching rule ever changes, it'd need updating in two places).♻️ Proposed extraction
+function notesInFolderSubtree(notes: Note[], projectId: ID, source: string): Note[] { + const srcLower = source.toLowerCase(); + return notes.filter((n) => { + if (n.projectId !== projectId) return false; + const f = normalizeFolderPath(n.folder).toLowerCase(); + return f === srcLower || f.startsWith(`${srcLower}/`); + }); +}Then call
notesInFolderSubtree(get().notes, projectId, source)/notesInFolderSubtree(get().notes, sourceProjectId, source)in place of the two duplicated blocks.🤖 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/store/slices/notes.ts` around lines 207 - 277, Extract the duplicated subtree-filtering logic from moveFolder and moveFolderToProject into a shared notesInFolderSubtree helper accepting notes, project ID, and normalized source path. Preserve the existing case-insensitive folder matching and descendant inclusion, then call the helper from both methods with their respective project IDs.src/components/kanban/board.tsx (1)
452-473: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueMinor: sidebar highlight is updated from two places during the same drag.
updateSidebarDropHighlightis called both here (on every rawpointermove/touchmove) and again inhandleDragMove(line 306) using the samelivePointervalue that this effect just set. The second call is redundant — by the timehandleDragMoveruns, this effect's listener has already updated the highlight for that pointer position.🤖 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/kanban/board.tsx` around lines 452 - 473, Remove the redundant updateSidebarDropHighlight call from handleDragMove, while preserving its use of livePointer for other drag-move behavior. Keep the pointermove and touchmove listeners in the activeCard effect as the sole source of sidebar highlight updates, including existing cleanup via clearSidebarDropHighlight.src/store/slices/notes.test.ts (1)
1-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood coverage for
moveFolder;moveFolderToProjecthas none.This file thoroughly covers
moveFolder(reparent, root move, self/descendant no-ops, cross-project isolation), but the newmoveFolderToProjectaction — which also mutatesprojectId/workspaceIdfor a whole subtree — has no dedicated test here. Given it's a new bulk-mutating action, worth mirroring themoveCardToProjecttest pattern fromboard.test.ts(project validation, subtree matching, no-op when source===target).🤖 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/store/slices/notes.test.ts` around lines 1 - 89, Add dedicated tests in the moveFolder suite for moveFolderToProject, covering destination-project validation, updating projectId and workspaceId for the entire folder subtree while leaving unrelated notes unchanged, and the no-op behavior when source and target projects are identical. Reuse setup, note, and folderOf, and mirror the existing moveCardToProject test pattern from board.test.ts.src/lib/cross-project-dnd.ts (1)
38-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused drag helpers
writeCrossProjectDragandreadCrossProjectDragaren’t imported outsidesrc/lib/cross-project-dnd.ts; the current consumers still callsetActiveCrossProjectDrag/getActiveCrossProjectDragdirectly. Either delete these exports or switch the callers over so theDataTransferpath is actually used.🤖 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/lib/cross-project-dnd.ts` around lines 38 - 63, Remove the unused exported helpers writeCrossProjectDrag and readCrossProjectDrag from the cross-project drag module, since consumers directly use setActiveCrossProjectDrag and getActiveCrossProjectDrag. Keep the directly used mirror functions and related behavior unchanged.
🤖 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/ipc/chat.ts`:
- Around line 217-233: Thread abortCtrl.signal from the chat IPC handler through
runDispatchLoop, runSubagent, forceFinalAnswer, and the nested runToolLoop call
so every model/tool fetch observes user cancellation. Wrap the runDispatchLoop
await in error-safe cleanup that always removes the abort controller and sends
chat:done, including when cancellation or fetch errors throw.
In `@electron/lib/chat-agent-benchmark.test.ts`:
- Line 31: Remove the unused afterEach import from the Vitest import declaration
in chat-agent-benchmark.test.ts, while retaining describe, it, beforeEach, and
expect.
In `@electron/lib/chat-subagent-loop.ts`:
- Around line 422-444: Update the fetch request inside runDispatchLoop to catch
network or other request failures and return the same graceful error-result
shape used by the single-agent loop, preserving the existing HTTP error
handling. Do not address AbortSignal propagation here; keep the change scoped to
preventing fetch rejection from escaping the dispatch loop.
In `@electron/shared/notes-io.ts`:
- Around line 315-377: Update removeDirIfNoNotesLeft and dirContainsNote so
directory cleanup preserves every leftover file except explicitly recognized OS
cruft, rather than treating only .md files as data. Replace the note-extension
check with a helper such as dirContainsRealData that recursively considers
non-cruft files as blocking deletion, while allowing .DS_Store and other
explicitly listed OS-cruft names; have removeDirIfNoNotesLeft call this helper.
In `@src/hooks/useChatStream.ts`:
- Around line 245-247: Update the hydrate/refresh trigger in useChatStream to
include tool calls from finalSubagents when determining hasPersistedWrite, not
just finalToolCalls. Reuse the existing subagent tool-call data and preserve the
current refresh behavior for top-level tool calls.
---
Nitpick comments:
In `@electron/lib/chat-agent-benchmark.test.ts`:
- Around line 73-83: Extract the duplicated live-endpoint configuration and
reachability probe into a shared helper exporting BASE_URL, MODEL, API_KEY, and
endpointUp. Update electron/lib/chat-agent-benchmark.test.ts:73-83,
electron/lib/chat-quality-scorer.test.ts:133-138,
electron/lib/chat-subagent-stream.test.ts:28-33, and
electron/lib/chat-write-recovery-benchmark.test.ts:92-100 to consume it;
preserve the quality scorer’s judge-endpoint override wiring while removing
local config parsing and endpointUp implementations.
In `@src/components/kanban/board.tsx`:
- Around line 452-473: Remove the redundant updateSidebarDropHighlight call from
handleDragMove, while preserving its use of livePointer for other drag-move
behavior. Keep the pointermove and touchmove listeners in the activeCard effect
as the sole source of sidebar highlight updates, including existing cleanup via
clearSidebarDropHighlight.
In `@src/lib/cross-project-dnd.ts`:
- Around line 38-63: Remove the unused exported helpers writeCrossProjectDrag
and readCrossProjectDrag from the cross-project drag module, since consumers
directly use setActiveCrossProjectDrag and getActiveCrossProjectDrag. Keep the
directly used mirror functions and related behavior unchanged.
In `@src/store/slices/notes.test.ts`:
- Around line 1-89: Add dedicated tests in the moveFolder suite for
moveFolderToProject, covering destination-project validation, updating projectId
and workspaceId for the entire folder subtree while leaving unrelated notes
unchanged, and the no-op behavior when source and target projects are identical.
Reuse setup, note, and folderOf, and mirror the existing moveCardToProject test
pattern from board.test.ts.
In `@src/store/slices/notes.ts`:
- Around line 207-277: Extract the duplicated subtree-filtering logic from
moveFolder and moveFolderToProject into a shared notesInFolderSubtree helper
accepting notes, project ID, and normalized source path. Preserve the existing
case-insensitive folder matching and descendant inclusion, then call the helper
from both methods with their respective project IDs.
🪄 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: 0d90c231-65ba-4562-815c-ef8f439a4567
📒 Files selected for processing (32)
changelogs/v2.5.4.mdelectron/db/queries.tselectron/db/schema.tselectron/ipc/chat.tselectron/ipc/db-handlers.tselectron/lib/chat-agent-benchmark.test.tselectron/lib/chat-loop.tselectron/lib/chat-quality-scorer.test.tselectron/lib/chat-subagent-loop.tselectron/lib/chat-subagent-stream.test.tselectron/lib/chat-write-recovery-benchmark.test.tselectron/lib/tools.tselectron/notes-files.test.tselectron/preload.tselectron/shared/db-mappers.tselectron/shared/notes-io.tssrc/app/globals.csssrc/components/chat/chat-panel/ChatMessageBubble.tsxsrc/components/chat/chat-panel/ChatSubagentBlock.tsxsrc/components/chat/chat-panel/index.tsxsrc/components/kanban/board.tsxsrc/components/layout/sidebar.tsxsrc/components/notes/notes-view.tsxsrc/components/notes/notes-view/FolderTreeNode.tsxsrc/hooks/useChatStream.tssrc/lib/cross-project-dnd.tssrc/store/slices/board.test.tssrc/store/slices/board.tssrc/store/slices/chat.tssrc/store/slices/notes.test.tssrc/store/slices/notes.tssrc/types/index.ts
Subagent chat: - Thread abortCtrl.signal through runDispatchLoop → runSubagent → forceFinalAnswer and the nested runToolLoop so user cancellation stops every model/tool fetch; wrap the dispatch call in try/catch/finally that always removes the abort controller and emits chat:done (cancel or error). - Catch network failures in the dispatch fetch and return the same graceful error-result shape as the single-agent loop instead of throwing. - useChatStream: include write sub-agent tool calls when deciding hasPersistedWrite so subagent-mode writes still refresh the board/notes. Cross-project drag / notes: - notes-io: replace .md-only dir check with dirContainsRealData so directory cleanup preserves any non-cruft file (attachments, .canvas, images) and only deletes recognised OS cruft (.DS_Store, Thumbs.db, …). - notes slice: extract shared notesInFolderSubtree helper used by moveFolder and moveFolderToProject; add moveFolderToProject unit tests. - board: drop redundant sidebar-highlight call in handleDragMove (listeners are the sole source). - cross-project-dnd: remove unused write/readCrossProjectDrag exports. Tests: - Extract shared electron/lib/bench-endpoint.ts (BASE_URL/MODEL/API_KEY/ endpointUp) consumed by the four benchmark tests; preserve the quality scorer's judge-endpoint override.
1 participant
What does this PR do?
Two groups of changes:
Bug fix — project rename now relocates the notes folder on disk immediately (no restart). When a project was renamed while a note was open, the note's autosave wrote the
.mdunder the new-slug folder before the rename ran, forcingrenameProjectNotesDirinto its merge path. The same-name collision-skip left a stale duplicate (and the now-empty old folder) on disk until the next launch.mergeDirIntonow removes same-note-id duplicates and clears the old directory (including OS cruft like.DS_Store); the update handler falls back toreconcileProjectFolderswhen the primary move no-ops (empty project / vault-root).New drag-and-drop reorganisation:
A shared
cross-project-dndpayload module bridges the notes sidebar's native HTML5 DnD and the board's dnd-kit drag (which has nooveroutside its context) via adata-project-drop-idpointer hit-test.Type of change
Screenshots / recording
Checklist
npm run type-check:allpassesnpm run lintpasses (0 errors; only pre-existing warnings)npm testpasses (ran the affected suites: notes/board slice + notes-files — 79 passing; fullnpm testnot run here)npm run test:e2epasses (run before merging UI changes or cutting a release)text-[Npx]pixel font classes — rem equivalents onlyhandle()and returnIpcResult<T>(no new IPC handlers added; reused existingnote.moveToFolder/note.moveToProject/card.update)schema.ts(none needed)electron/db/queries.ts(none added; reused existing queries)dependencies/devDependencies(none added)--external:<pkg>flags changedNotes for reviewer
DndContext), the board publishes the drag payload to a module singleton and hit-tests the live pointer againstdata-project-drop-idrows on drag end. Worth a look atsrc/lib/cross-project-dnd.ts, thehandleDragEndcross-project branch inboard.tsx, and the sidebarhandleCrossProjectDrop.moveFolderre-prefixes descendant note paths and rejects nesting a folder inside itself/a descendant (unit-tested).electron/notes-files.test.tsreproducing the racing-autosave merge case.Summary by CodeRabbit