feat(planning): Planning Kanban board (data + UI) - #444
Open
tomymaritano wants to merge 19 commits into
Open
Conversation
Foundation for the Planning Kanban board. Introduces a `boardStage`
Kanban column field on notes and a reserved "Planning" notebook that
backs the board — persisted as DB metadata so moving a card never
rewrites the user's markdown ("markdown is sacred").
Core (packages/core):
- BoardStage type + BOARD_STAGES + DEFAULT_BOARD_STAGE, PLANNING_NOTEBOOK_ID
- Note.boardStage field; setBoardStage() (metadata-only, mirrors setNoteStatus)
- createNote defaults boardStage to 'backlog' in the Planning notebook, else null
- createPlanningNotebook() + isPlanning(); Planning notebook is undeletable
- boardStage added to NoteSnapshot/NoteSummary + toSnapshot/toSummary
Storage (packages/storage-sqlite):
- Migration 018: board_stage column + index, idempotent Planning notebook seed
- board_stage mapped in rowToNote and persisted in the upsert + all SELECTs
IPC + preload (apps/desktop):
- notes:setBoardStage handler (mirrors notes:setStatus)
- window.dripnex.notes.setBoardStage + BoardStage in preload types
- noteToSnapshot carries boardStage; SyncService new-note literal sets null
Tests: core boardStage/planning-notebook suite; storage round-trip + seed.
Note: 2 pre-existing FTS search-by-title tests fail on develop too; untouched.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR adds board-stage tracking to notes, introduces a reserved Planning notebook, persists the new field in SQLite, wires IPC/preload and renderer navigation for planning mode, and adds planning-board UI, drag-and-drop stage updates, and tests. ChangesBoard Stage Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/tests/boardStage.test.ts (1)
1-79: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSolid coverage; consider adding the missing edge case.
Tests thoroughly cover defaulting,
setBoardStage, snapshot serialization, and Planning notebook invariants, satisfying the "Core domain changes require tests" guideline. Consider adding a case forcreateNote({ notebookId: PLANNING_NOTEBOOK_ID, boardStage: null })to catch the nullish-coalescing regression flagged innote.ts.As per coding guidelines, "Core domain changes require tests."
🤖 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 `@packages/core/tests/boardStage.test.ts` around lines 1 - 79, The board-stage tests in boardStage.test.ts miss the regression case where a Planning note is created with an explicit null boardStage, so add a test around createNote with notebookId PLANNING_NOTEBOOK_ID and boardStage null to verify it still defaults to DEFAULT_BOARD_STAGE; this will protect the createNote logic in note.ts from a nullish-coalescing bug and should be placed alongside the existing createNote board stage default cases.Source: Coding guidelines
🤖 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 `@apps/desktop/src/main/handlers/noteHandlers.ts`:
- Around line 45-47: The BoardStageSchema in noteHandlers should not hardcode
the stage strings; it duplicates the source of truth from `@dripnex/core`. Update
the schema to reuse BOARD_STAGES from core with z.enum(BOARD_STAGES) and keep
the nullable handling, so the schema stays aligned with the BoardStage type and
any future core changes.
- Around line 184-195: The notes:setBoardStage IPC handler currently allows
setBoardStage() to persist board metadata on any note, including non-Planning
notebooks. Update the handler in noteHandlers so it checks the fetched note’s
notebook before calling setBoardStage; only Planning notes should accept a
non-null boardStage, while Inbox/other notebooks should reject non-null stages
(or only allow clearing). Use the existing notes:setBoardStage handler,
setBoardStage(), and noteToSnapshot() as the main points to locate and adjust
the logic.
In `@packages/core/src/domain/note.ts`:
- Around line 101-112: The `createNote` logic in `note.ts` is overriding an
explicit `boardStage: null` because `??` treats null as missing, so Planning
notes default to `DEFAULT_BOARD_STAGE` unexpectedly. Update the `boardStage`
fallback to only apply when `options.boardStage` is undefined, while preserving
an explicit null value as-is. Use the `createNote` return object and
`PLANNING_NOTEBOOK_ID` / `DEFAULT_BOARD_STAGE` condition to locate the change.
---
Outside diff comments:
In `@packages/core/tests/boardStage.test.ts`:
- Around line 1-79: The board-stage tests in boardStage.test.ts miss the
regression case where a Planning note is created with an explicit null
boardStage, so add a test around createNote with notebookId PLANNING_NOTEBOOK_ID
and boardStage null to verify it still defaults to DEFAULT_BOARD_STAGE; this
will protect the createNote logic in note.ts from a nullish-coalescing bug and
should be placed alongside the existing createNote board stage default cases.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6d086f08-fc08-4217-a809-a2c194e65a07
📒 Files selected for processing (17)
apps/desktop/src/main/handlers/localServerHandlers.tsapps/desktop/src/main/handlers/noteHandlers.tsapps/desktop/src/main/handlers/types.tsapps/desktop/src/main/index.tsapps/desktop/src/main/services/sync/SyncService.tsapps/desktop/src/preload/api/notes.tsapps/desktop/src/preload/api/types.tspackages/core/src/contracts/NoteSnapshot.tspackages/core/src/domain/note.tspackages/core/src/domain/notebook.tspackages/core/src/domain/types.tspackages/core/tests/boardStage.test.tspackages/storage-sqlite/src/migrations/018_board_stage.tspackages/storage-sqlite/src/migrations/index.tspackages/storage-sqlite/src/repositories/SQLiteNoteRepository.tspackages/storage-sqlite/src/repositories/noteMapping.tspackages/storage-sqlite/tests/repository.test.ts
## Summary The UI half of the Planning Kanban feature. **Stacked on #444** (the data layer) — this PR's base is `feature/planning-kanban-data` and will auto-retarget to `develop` once #444 merges. Review #444 first. Adds the **"Planning" sidebar item** and the **Kanban board** it opens. Columns are the 5 Linear-style stages; dragging a card persists via `notes.setBoardStage` — **DB metadata only, the note's markdown is never rewritten** ("markdown is sacred"). ## What's included **Navigation** - `NavigationState` gains a `planning` kind + `goToPlanning` action + `selectIsPlanningContext` - Sidebar **Planning** quick-filter item (`KanbanSquare`), active-state aware - `App.tsx` renders `<PlanningBoard>` in the editor pane when `navigation.kind === 'planning'` **Board** (`components/planning/`) - `PlanningBoard` — groups Planning-notebook notes into Backlog/Todo/In Progress/In Review/In Staging; **Board ↔ Graph** toggle - `PlanningColumn` — droppable column (native HTML5 DnD, custom `application/x-dripnex-note` MIME — the app's DnD convention, no new dep) - `PlanningCard` — draggable card: title, excerpt, tags, task progress via `countMarkdownTasks`; click opens the note - Graph mode reuses `GraphView` via a new optional `filterNotebookId` prop that restricts nodes/edges to the Planning notebook **Hooks**: `useNoteMutations.setBoardStage`; `BoardStage` re-exported from preload. ## Testing - `pnpm typecheck` (all desktop projects) — green - `pnpm test` — green; `pnpm lint` — green - `pnpm --filter @dripnex/desktop build` — bundles successfully - Manual smoke test pending on `pnpm dev` (Planning item → board → drag persists → markdown unchanged → graph toggle) ## How it connects to the graph No new graph infra — the existing `GraphView` / `useGraphData` / `window.dripnex.links` are reused; `filterNotebookId` scopes them to the board's notes (nodes already carry `notebookId`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tomymaritano
disabled auto-merge
July 3, 2026 03:39
tomymaritano
marked this pull request as draft
July 3, 2026 03:39
tomymaritano
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/hooks/useNavigation.ts (1)
119-169: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winAdd a skip flag to
useNotesin planning mode.
useFilteredNotesstill fetches the full note list onnavigation.kind === 'planning', even though that branch always returns[].PlanningBoardalso runs its ownuseNotes({ archived: 'active' })query, so the planning screen pays for two separate note-list fetches. Plumb anenabled/skip option throughuseNotesand disable this hook in planning mode.🤖 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 `@apps/desktop/src/renderer/hooks/useNavigation.ts` around lines 119 - 169, `useFilteredNotes` always calls `useNotes` even when `navigation.kind` is `planning`, but that branch later returns an empty list, so the full note query is wasted. Add an `enabled`/skip option to the `useNotes` call inside `useFilteredNotes`, and set it to false for planning mode so the `useNotes` query does not run there. Keep the existing filtering logic in `useFilteredNotes` and ensure the new flag is threaded through the `useNotes` API without affecting other callers like `PlanningBoard`.
🤖 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 `@apps/desktop/src/renderer/App.tsx`:
- Around line 586-593: The PlanningBoard onOpenNote handler in App should also
clear graph mode before selecting the note, because
goToNotebook(PLANNING_NOTEBOOK_ID) alone leaves isGraphOpen active and can keep
rendering GraphView. Update the onOpenNote flow to reset the graph state first,
then call handleSelectNote(noteId), using the existing isGraphOpen logic and
PlanningBoard handler as the key locations to change.
In `@apps/desktop/src/renderer/components/planning/PlanningBoard.tsx`:
- Around line 56-97: The Planning board stage change flow in PlanningBoard and
its child card/column components is drag-and-drop only, so add a
keyboard-operable alternative for moving notes between stages. Update the
PlanningCard/PlanningColumn interaction so users can change stage without HTML5
drag events, such as a per-card “Move to…” control or shortcut that calls the
existing handleDropNote/setBoardStage path. Make sure the new affordance works
alongside the current onDropNote behavior and is reachable from the card UI.
In `@apps/desktop/src/renderer/components/planning/PlanningCard.tsx`:
- Around line 35-37: The keyboard handler in PlanningCard only activates on
Enter, so update the onKeyDown logic to also treat Space as an activation key
for the role="button" element. In the PlanningCard component, handle both e.key
=== 'Enter' and e.key === ' ' (or Space), and call preventDefault for the Space
case before invoking onOpen(note.id) so the page does not scroll.
In `@apps/desktop/src/renderer/components/sidebar/SidebarQuickFilters.tsx`:
- Around line 89-102: The Planning button is duplicating the same
sidebar-quick-filter selected/aria-pressed markup already handled by
QuickFilterItem. Update QuickFilterItem to support an optional count prop so it
can omit the count span when not provided, then replace the inline Planning
button in SidebarQuickFilters with QuickFilterItem using the existing
isPlanningSelected and onOpenPlanning props. Keep the Planning icon/title/label
behavior the same, but centralize the shared button structure in
QuickFilterItem.
---
Outside diff comments:
In `@apps/desktop/src/renderer/hooks/useNavigation.ts`:
- Around line 119-169: `useFilteredNotes` always calls `useNotes` even when
`navigation.kind` is `planning`, but that branch later returns an empty list, so
the full note query is wasted. Add an `enabled`/skip option to the `useNotes`
call inside `useFilteredNotes`, and set it to false for planning mode so the
`useNotes` query does not run there. Keep the existing filtering logic in
`useFilteredNotes` and ensure the new flag is threaded through the `useNotes`
API without affecting other callers like `PlanningBoard`.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1d17ae84-8678-4f76-8723-783bc5b96f64
📒 Files selected for processing (13)
apps/desktop/src/preload/index.tsapps/desktop/src/renderer/App.tsxapps/desktop/src/renderer/components/GraphView.tsxapps/desktop/src/renderer/components/planning/PlanningBoard.cssapps/desktop/src/renderer/components/planning/PlanningBoard.tsxapps/desktop/src/renderer/components/planning/PlanningCard.tsxapps/desktop/src/renderer/components/planning/PlanningColumn.tsxapps/desktop/src/renderer/components/planning/constants.tsapps/desktop/src/renderer/components/sidebar/Sidebar.tsxapps/desktop/src/renderer/components/sidebar/SidebarQuickFilters.tsxapps/desktop/src/renderer/hooks/useNavigation.tsapps/desktop/src/renderer/hooks/useNotes.tsapps/desktop/src/renderer/stores/navigationStore.ts
Makes the Kanban board actually usable and richer: - "+" button per column (and empty-column CTA) creates a note in the Planning notebook at that stage and opens it for editing - Per-card "…" menu: Open, Move to <stage>, Remove from board, Delete - Tag chips now show their real colors (from the tag-colors store) - Cards show their created date Also addresses CodeRabbit on #444: BoardStage is now derived from the BOARD_STAGES const tuple (single source of truth), and the IPC BoardStageSchema reuses z.enum(BOARD_STAGES) instead of duplicating the stage literals. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drag-to-move between columns silently failed: onDragOver only called preventDefault() when dataTransfer.types included our custom MIME, but some Chromium/Electron builds don't enumerate custom types during dragover — so preventDefault() never ran and the drop event never fired. Now preventDefault() runs unconditionally on dragenter/dragover (drop is always allowed); the drop handler still reads only our MIME via getData (reliable on drop), so notebook drags — which also set text/plain — are ignored. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a `priority` field (none/low/medium/high/urgent) end-to-end, mirroring the status/boardStage pattern — DB metadata, never touches markdown. - core: NotePriority (derived from NOTE_PRIORITIES tuple), Note.priority, setNotePriority(), createNote default 'none', snapshot support + tests - storage: migration 019 (priority column + index), mapping, upsert, all note SELECTs - IPC/preload: notes:setPriority, window.dripnex.notes.setPriority, NotePriority type, noteToSnapshot + SyncService wiring - UI: colored priority dot on cards + "Priority…" submenu in the card menu Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cards can now be reordered within (and moved across) columns by dragging onto another card — an above/below indicator shows the drop position. - core: Note.boardOrder + setBoardPosition(stage, order); snapshot support - storage: migration 020 (board_order column + composite index), mapping, upsert, all note SELECTs - IPC/preload: notes:reorderColumn(stage, orderedIds) reindexes a column in one pass (avoids fractional-order collisions); window.dripnex.notes.reorderColumn - UI: cards are drop targets with above/below detection; column background drop appends; board sorts each column by boardOrder then newest-first; "Move to <stage>" appends to the target column Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a toolbar (board mode) to filter the Kanban: - text search across title/content/tags - filter by tag (dropdown of tags present on the board) - filter by priority - Clear button when any filter is active Filtering is applied before grouping, so column counts reflect the filtered set. Purely client-side over the already-loaded notes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PlanningCard read PRIORITY_CONFIG[note.priority].color directly, which threw "Cannot read properties of undefined (reading 'color')" whenever a snapshot arrived without a priority — e.g. a stale main process during dev HMR, or notes synced before the field existed. Now the card falls back to 'none' for a missing/unknown priority, and the board's column sort tolerates a missing boardOrder. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The middle note-list pane is redundant in Planning mode — the board owns its own notes, so the list just showed "No notes yet" and felt disconnected. Hide the list (and its resize handle) when navigation.kind === 'planning' so the board fills the pane; it returns for every other view. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The header "+" wasn't discoverable. Every column now shows a dashed "+ Add card" button at the bottom of its list (not just when empty), so adding a card to Backlog/any column is obvious. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, a11y, atomicity Applies the audit's must-fix set plus the security/atomicity fixes: 1. Board 50-note cap: add a notebookId filter to the list query (ListNotesOptions + notes:list schema + repo SQL) and have PlanningBoard fetch the Planning notebook scoped with an explicit high limit, so aged/high-count boards no longer silently drop cards. 2. "Remove from board" now moves the note out of the Planning notebook (board membership is notebook-based) instead of nulling the stage, which just re-grouped it into Backlog. 3. Reorder while a filter/search is active now reindexes the FULL column (unfilteredByStage), so hidden cards keep contiguous board_order. 4. Unknown/legacy board_stage falls back to Backlog instead of throwing (mirrors the priority guard). 5. Membership guard: reorderColumn (via SQL WHERE notebook_id) and setBoardStage only ever write board metadata onto Planning-notebook notes. 6. reorderColumn is now atomic: a single repo.reorderBoard() transaction doing direct board_stage/board_order UPDATEs (no N+1 save(), no tag re-extraction). 7. Tests: pure computeReorderedIds helper (extracted + unit-tested, incl. off-by-one and drop-on-self edges) and a storage reorderBoard integration test (atomic reindex, membership guard, markdown untouched). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BoardStage / NotePriority / NoteStatus were hand-duplicated in preload/api/types.ts, so adding a value meant editing two files with no compile error if they drifted. Preload now imports+re-exports them from @dripnex/core (type-only, erased at build); the renderer still imports from the preload barrel unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The notes column list was restated across 5 SELECTs and a positional INSERT, so adding a column meant hand-editing each in lockstep (the exact error-prone pattern the audit flagged). Now a single NOTE_COLUMNS array drives every SELECT (via noteColumns(alias)) and the upsert (columns/values/SET derived from it, bound by @name so positions can't misalign). Test: list() carries board fields + honors the notebookId filter (guards the SELECT parity across the refactor). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The previous audit-fix commit plumbed the `notebookId` list option through core/IPC/preload but never wired PlanningBoard to use it, so the board was still hitting the unscoped default (limit 50). Pass notebookId + limit:100000 so the 50-note cap fix is real. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Dragging a card previously invalidated noteKeys.all and full-refetched the board, causing a visible snap/flicker. reorderColumn now applies an optimistic cache update (applyBoardReorder patches every cached note list in place), rolls back on error, and resyncs on settle by invalidating only noteKeys.lists(). Test: applyBoardReorder pure helper (stage+contiguous order to reordered ids, others untouched, no input mutation). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Card a11y (audit medium): the card was role="button" while wrapping an interactive menu button (invalid ARIA) and its Enter handler double-fired when the menu had focus. Dropped role="button"; mouse-open stays (the menu stops propagation) and Enter is gated to the card itself; added aria-label. FTS: migration 021 recreates the notes_fts_update trigger with a WHEN guard so metadata-only writes (board_stage/board_order/priority/status/pin) no longer churn the FTS index — reordering a column previously rebuilt FTS per card. Recreating the trigger also clears the two pre-existing FTS search-doubling failures, so the storage suite is now fully green (24/24). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
noteToSnapshot was hand-written in index.ts and its shape re-declared in handlers/types.ts (NoteToSnapshotFn), so a new note field meant editing both plus core. Both now derive from @dripnex/core's toSnapshot (noteToSnapshot = toSnapshot; NoteToSnapshotFn = typeof toSnapshot). localServer's getNote copies the readonly tags into a mutable array. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tomymaritano
marked this pull request as ready for review
July 3, 2026 13:47
tomymaritano
disabled auto-merge
July 3, 2026 13:48
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/tests/boardStage.test.ts (1)
12-29: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMissing coverage for explicit
boardStage: nullin the Planning notebook.Existing tests cover "no option → null outside Planning" and "no option → 'backlog' inside Planning" and "explicit non-null override", but none cover
createNote({ notebookId: PLANNING_NOTEBOOK_ID, boardStage: null }), which is exactly the scenario affected by the??fallback bug innote.ts(Lines 132-133). Adding this case would have caught the regression.🤖 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 `@packages/core/tests/boardStage.test.ts` around lines 12 - 29, Add a test in the Board stage (Kanban) suite for createNote with notebookId set to PLANNING_NOTEBOOK_ID and boardStage explicitly null, and assert it stays null instead of falling back to DEFAULT_BOARD_STAGE. This covers the note.ts boardStage assignment path that uses nullish coalescing, so update the createNote expectations alongside the existing default and explicit override cases.Source: Coding guidelines
♻️ Duplicate comments (2)
packages/core/src/domain/note.ts (1)
122-136: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExplicit
boardStage: nullstill gets overridden (unresolved from prior review).
options.boardStage ?? (notebookId === PLANNING_NOTEBOOK_ID ? DEFAULT_BOARD_STAGE : null)treats an explicitnullthe same as "not provided" since??only short-circuits onnull/undefined... wait,??does short-circuit onnull, sooptions.boardStagebeingnullwill still trigger the fallback becausenullitself is nullish. A caller passingboardStage: nullexplicitly for a note created in the Planning notebook will unexpectedly get'backlog'instead ofnull. This is the same issue raised in a previous review on this file.🐛 Proposed fix distinguishing "unset" from explicit null
- boardStage: - options.boardStage ?? (notebookId === PLANNING_NOTEBOOK_ID ? DEFAULT_BOARD_STAGE : null), + boardStage: + options.boardStage !== undefined + ? options.boardStage + : notebookId === PLANNING_NOTEBOOK_ID + ? DEFAULT_BOARD_STAGE + : null,As per coding guidelines, "Core domain changes require tests" — please also add a regression test for
createNote({ notebookId: PLANNING_NOTEBOOK_ID, boardStage: null })retainingnull.🤖 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 `@packages/core/src/domain/note.ts` around lines 122 - 136, The createNote builder in note.ts is still treating an explicit null boardStage as unset because of the nullish fallback on options.boardStage; update the boardStage assignment so only an omitted value falls back to the Planning default, while an explicit null is preserved. Use the createNote function and the boardStage/notebookId logic to locate it, and add a regression test covering createNote({ notebookId: PLANNING_NOTEBOOK_ID, boardStage: null }) returning null.Source: Coding guidelines
apps/desktop/src/renderer/App.tsx (1)
591-599: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGraph mode not cleared when opening a Planning note (still unresolved).
onOpenNotecallsgoToNotebook(PLANNING_NOTEBOOK_ID)which changesnavigation.kindaway from'planning', causing the outer ternary to fall through to theisGraphOpenbranch. IfisGraphOpenwas previouslytrue, opening a card from the Planning board will renderGraphViewinstead of the selected note. This mirrors a previously flagged concern that does not appear to carry an "Addressed" marker for this file.🐛 Proposed fix
<PlanningBoard onOpenNote={noteId => { goToNotebook(PLANNING_NOTEBOOK_ID); + setIsGraphOpen(false); void handleSelectNote(noteId); }} />🤖 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 `@apps/desktop/src/renderer/App.tsx` around lines 591 - 599, The Planning board note-open flow in App should prevent graph mode from taking over when switching notebooks. Update the PlanningBoard onOpenNote handler to clear or disable the graph state before calling goToNotebook(PLANNING_NOTEBOOK_ID) and handleSelectNote(noteId), so the navigation.kind change does not fall through to the isGraphOpen branch. Use the App component’s navigation/isGraphOpen logic and the PlanningBoard onOpenNote callback as the key places to adjust the state transition.
🤖 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 `@apps/desktop/src/main/handlers/noteHandlers.ts`:
- Around line 217-226: The notes:reorderColumn handler in noteHandlers.ts should
sanitize the incoming orderedIds before calling repo.reorderBoard. Deduplicate
the IDs while preserving the caller’s intended order, and filter out
stale/nonexistent IDs using the same board/note identity checks already implied
by reorderBoard. Keep the existing BoardStageValueSchema and IdSchema
validation, then pass only the cleaned list into repo.reorderBoard so the board
order cannot be silently corrupted by duplicate or stale entries.
In `@apps/desktop/src/main/services/sync/SyncService.ts`:
- Around line 899-901: Replace the hardcoded note priority in SyncService’s sync
payload with the shared default from core so it stays aligned with storage
behavior. Update the note object construction in the sync flow to use
DEFAULT_NOTE_PRIORITY instead of the inline 'none' literal, and keep the
existing boardStage and boardOrder fields unchanged. Use the SyncService sync
logic and the note payload mapping as the main location to make this
shared-default swap.
In `@apps/desktop/src/renderer/components/planning/PlanningBoard.tsx`:
- Around line 144-155: `handleAddCard` in `PlanningBoard` can reject because it
is called fire-and-forget from `PlanningColumn`, so add explicit error handling
around the async flow. Update `handleAddCard` (and the `onAddCard={() =>
onAddCard(stage)}` call path) to catch failures from `createNote.mutateAsync`
and `setBoardStage.mutateAsync`, surface a user-visible error, and avoid leaving
an unhandled promise rejection; keep `onOpenNote` only on successful
creation/move.
- Around line 132-138: `handleRemoveFromBoard` currently triggers
`setBoardStage.mutate` and `moveNote.mutate` independently, which can leave the
note in a mixed state if one succeeds and the other fails. Update
`handleRemoveFromBoard` in `PlanningBoard` to treat removal as one operation:
sequence the stage clear and notebook move with failure handling/rollback, or
replace both with a single atomic mutation/IPC call that removes the note from
the Planning board and clears stale stage data together. Ensure any error path
reports failure and does not silently reintroduce the note into Backlog or leave
a stale `boardStage`.
In `@apps/desktop/src/renderer/components/planning/PlanningCard.tsx`:
- Around line 107-126: The PlanningCard menu callback uses a parameter named
priority that shadows the outer priority value, which hurts readability. In
PlanningCard, rename the onSetPriority arrow function parameter to something
distinct like nextPriority and pass that through to onSetPriority(note.id, ...),
keeping the existing note.id wiring intact.
In `@apps/desktop/src/renderer/components/planning/PlanningCardMenu.tsx`:
- Around line 89-97: The PlanningCardMenu submenu toggles are independent, so
opening “Move to…” and “Priority…” can leave both open at once. Update the click
handlers in PlanningCardMenu so that the submenu state is mutually exclusive:
when toggling showMove, also clear showPriority, and when toggling showPriority,
also clear showMove. Apply the same logic anywhere those submenu buttons are
defined so only one submenu can be expanded at a time.
In `@apps/desktop/src/renderer/components/planning/PlanningColumn.tsx`:
- Around line 106-112: The add-card click path in PlanningColumn is calling the
async onAddCard(stage) without any rejection handling, so failures from
createNote or setBoardStage can become unhandled rejections. Update the
handleAddCard flow to catch and handle errors, or attach a .catch at the button
onClick call site, and make sure the error handling is tied to the existing
onAddCard and handleAddCard symbols so the async failure is safely surfaced.
In `@packages/storage-sqlite/src/repositories/SQLiteNoteRepository.ts`:
- Around line 119-134: The `reorderBoard` update in `SQLiteNoteRepository`
changes only `board_stage` and `board_order`, so it bypasses sync tracking and
reordered notes never get pushed. Update the sync mechanism used by
`notes_update_sync_tracking` to include `board_stage` and `board_order`, or
explicitly mark the affected rows as syncable inside `reorderBoard` after the
transactional `update.run(...)` calls. Keep the fix aligned with `reorderBoard`,
`notes_update_sync_tracking`, and the existing `needs_sync` handling.
In `@packages/storage-sqlite/tests/repository.test.ts`:
- Around line 151-164: The `reorderBoard` test only checks that note content
stays unchanged, but it should also verify the core invariant that
`metadata.updatedAt` is preserved. In `repository.test.ts`, capture the note
returned by `repository.save` or `repository.get` before calling
`repository.reorderBoard('in_progress', ['c'])`, then assert the fetched note’s
`metadata.updatedAt` is identical afterward. Use the existing
`repository.reorderBoard` and `repository.get` flow in this test to strengthen
coverage without changing behavior.
---
Outside diff comments:
In `@packages/core/tests/boardStage.test.ts`:
- Around line 12-29: Add a test in the Board stage (Kanban) suite for createNote
with notebookId set to PLANNING_NOTEBOOK_ID and boardStage explicitly null, and
assert it stays null instead of falling back to DEFAULT_BOARD_STAGE. This covers
the note.ts boardStage assignment path that uses nullish coalescing, so update
the createNote expectations alongside the existing default and explicit override
cases.
---
Duplicate comments:
In `@apps/desktop/src/renderer/App.tsx`:
- Around line 591-599: The Planning board note-open flow in App should prevent
graph mode from taking over when switching notebooks. Update the PlanningBoard
onOpenNote handler to clear or disable the graph state before calling
goToNotebook(PLANNING_NOTEBOOK_ID) and handleSelectNote(noteId), so the
navigation.kind change does not fall through to the isGraphOpen branch. Use the
App component’s navigation/isGraphOpen logic and the PlanningBoard onOpenNote
callback as the key places to adjust the state transition.
In `@packages/core/src/domain/note.ts`:
- Around line 122-136: The createNote builder in note.ts is still treating an
explicit null boardStage as unset because of the nullish fallback on
options.boardStage; update the boardStage assignment so only an omitted value
falls back to the Planning default, while an explicit null is preserved. Use the
createNote function and the boardStage/notebookId logic to locate it, and add a
regression test covering createNote({ notebookId: PLANNING_NOTEBOOK_ID,
boardStage: null }) returning null.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3ce48bf4-80a4-4ab0-870a-8aa79f90ee0f
📒 Files selected for processing (31)
apps/desktop/src/main/handlers/localServerHandlers.tsapps/desktop/src/main/handlers/noteHandlers.tsapps/desktop/src/main/handlers/types.tsapps/desktop/src/main/index.tsapps/desktop/src/main/services/sync/SyncService.tsapps/desktop/src/preload/api/notes.tsapps/desktop/src/preload/api/types.tsapps/desktop/src/preload/index.tsapps/desktop/src/renderer/App.tsxapps/desktop/src/renderer/components/planning/PlanningBoard.cssapps/desktop/src/renderer/components/planning/PlanningBoard.tsxapps/desktop/src/renderer/components/planning/PlanningCard.tsxapps/desktop/src/renderer/components/planning/PlanningCardMenu.tsxapps/desktop/src/renderer/components/planning/PlanningColumn.tsxapps/desktop/src/renderer/components/planning/PlanningToolbar.tsxapps/desktop/src/renderer/components/planning/__tests__/reorder.test.tsapps/desktop/src/renderer/components/planning/constants.tsapps/desktop/src/renderer/components/planning/reorder.tsapps/desktop/src/renderer/hooks/useNotes.tspackages/core/src/contracts/NoteSnapshot.tspackages/core/src/domain/note.tspackages/core/src/domain/types.tspackages/core/tests/boardStage.test.tspackages/storage-core/src/types/ListNotesOptions.tspackages/storage-sqlite/src/migrations/019_note_priority.tspackages/storage-sqlite/src/migrations/020_board_order.tspackages/storage-sqlite/src/migrations/021_fts_update_guard.tspackages/storage-sqlite/src/migrations/index.tspackages/storage-sqlite/src/repositories/SQLiteNoteRepository.tspackages/storage-sqlite/src/repositories/noteMapping.tspackages/storage-sqlite/tests/repository.test.ts
Comment on lines
+217
to
+226
| defineIpcHandler({ | ||
| channel: 'notes:reorderColumn', | ||
| args: z.tuple([BoardStageValueSchema, z.array(IdSchema).max(2000)]), | ||
| handler: async (stage, orderedIds) => { | ||
| // Atomic reindex of the column (metadata-only; markdown untouched). The | ||
| // repo guards notebook membership so ids outside Planning are ignored. | ||
| repo.reorderBoard(stage, orderedIds); | ||
| return { ok: true }; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect reorderBoard implementation for notebook-membership and dedup checks
ast-grep run --pattern 'reorderBoard($_, $_) { $$$ }' --lang typescript packages/storage-sqlite/src/repositories/SQLiteNoteRepository.ts
rg -n -A20 'reorderBoard' packages/storage-sqlite/src/repositories/SQLiteNoteRepository.tsRepository: dripnex/readide
Length of output: 1202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## noteHandlers.ts (reorder + sibling handler)\n'
sed -n '190,250p' apps/desktop/src/main/handlers/noteHandlers.ts
printf '\n## SQLiteNoteRepository.ts (reorderBoard)\n'
sed -n '110,140p' packages/storage-sqlite/src/repositories/SQLiteNoteRepository.ts
printf '\n## Search for any dedup/validation around reorderBoard inputs\n'
rg -n -A3 -B3 'dedup|duplicate|unique|membership|PLANNING_NOTEBOOK_ID|reorderBoard' packages/storage-sqlite/src/repositories/SQLiteNoteRepository.ts apps/desktop/src/main/handlers/noteHandlers.tsRepository: dripnex/readide
Length of output: 11077
Deduplicate and validate orderedIds before reindexing. reorderBoard already filters by notebook_id, but it still accepts duplicates or stale IDs and writes the caller’s order as-is, which can silently produce the wrong board order.
🤖 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 `@apps/desktop/src/main/handlers/noteHandlers.ts` around lines 217 - 226, The
notes:reorderColumn handler in noteHandlers.ts should sanitize the incoming
orderedIds before calling repo.reorderBoard. Deduplicate the IDs while
preserving the caller’s intended order, and filter out stale/nonexistent IDs
using the same board/note identity checks already implied by reorderBoard. Keep
the existing BoardStageValueSchema and IdSchema validation, then pass only the
cleaned list into repo.reorderBoard so the board order cannot be silently
corrupted by duplicate or stale entries.
- App.tsx: clear isGraphOpen when opening a card from the board, else the editor pane could render GraphView instead of the selected note (Major). - handleRemoveFromBoard: sequence move-then-clear via mutateAsync with error toast, instead of two unsequenced fire-and-forget mutations (Major). - handleAddCard: wrap in try/catch with an error toast so the async column "+" call sites can't leak unhandled rejections. - reorderBoard: dedupe orderedIds so a duplicate can't take two positions. - SyncService: use DEFAULT_NOTE_PRIORITY instead of a hardcoded 'none'. - PlanningCardMenu: opening one submenu now closes the other. - PlanningCard: Space-key opens the card (with preventDefault); drop the priority param shadowing. - storage test: assert reorderBoard preserves updatedAt (metadata-only). Deferred (intentional): board metadata stays device-local (board sync is tracked as separate future work); createNote null→backlog in the Planning notebook is acceptable; QuickFilterItem reuse is low-value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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
Adds a Planning Kanban board reachable from the sidebar — a Linear-style board over the notes in a reserved "Planning" notebook, with a Board↔Graph toggle. This PR now contains both layers (the UI PR #445 was auto-merged into this branch, so #444 is the single combined PR to
develop).Two product decisions (agreed with the user):
backlog / todo / in_progress / in_review / in_staging) via a newboardStagefield.Markdown is sacred:
boardStageis DB metadata, changed viasetBoardStage(no content/updatedAtchange) — moving a card never rewrites markdown.Data layer (core / storage / IPC / preload)
BoardStage,BOARD_STAGES,DEFAULT_BOARD_STAGE,PLANNING_NOTEBOOK_IDNote.boardStage+setBoardStage();createNotedefaults tobacklogin the Planning notebook elsenullcreatePlanningNotebook()+isPlanning(); Planning notebook undeletableboardStageonNoteSnapshot/NoteSummary018:board_stagecolumn + index, idempotent Planning-notebook seednotes:setBoardStageIPC handler;window.dripnex.notes.setBoardStage;noteToSnapshotcarries it;SyncServicenew-note literal setsnullUI (renderer)
NavigationStateplanningkind +goToPlanning+selectIsPlanningContextKanbanSquare)PlanningBoard(5 columns + Board/Graph toggle),PlanningColumn(droppable),PlanningCard(draggable; title, excerpt, tags,countMarkdownTasksprogress; click opens note)application/x-dripnex-note), no new depsGraphViewvia a new optionalfilterNotebookIdpropuseNoteMutations.setBoardStageTesting
pnpm typecheck/pnpm test/pnpm lint— greenpnpm --filter @dripnex/desktop build— bundles OKsearch-by-titletests fail ondeveloptoo (verified via stash) — untouchedpnpm devpending🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes