Skip to content

P2-01: Notes tab — per-paper markdown notes (SQLite) - #21

Merged
dkritarth merged 2 commits into
masterfrom
p2-01-notes
Sep 12, 2026
Merged

P2-01: Notes tab — per-paper markdown notes (SQLite)#21
dkritarth merged 2 commits into
masterfrom
p2-01-notes

Conversation

@dkritarth

@dkritarth dkritarth commented Jul 23, 2026

Copy link
Copy Markdown
Owner

User outcome

Users can write, view, and clear freeform markdown research notes for each paper in the right panel "Notes" tab. Notes automatically save on debounce and persist across app restarts in the local SQLite database.

Acceptance criteria

  • Note is scoped to active paper and never appears on another paper.
  • Typing autosaves without losing last debounced edit on tab/paper/app change.
  • Empty and loading states are clear.
  • Clear action is deliberate and persisted.
  • Note survives full app restart.
  • Deleting paper cascades note safely.
  • Migration works from current production schema and fresh DB.
  • Live console remains clean.

Automated verification

  • Full test suite passing:
npm test
Test Files  27 passed (27)
     Tests  171 passed (171)
  • Migration and repository tests verify SQLite storage and cascade:
    • core/notes/repo.test.ts (8/8 tests passing)
    • src/app/NotesPanel.test.tsx (9/9 tests passing)
    • src/app/RightPanel.test.tsx (6/6 tests passing)
  • npm run build cleanly builds Electron bundle.

Live Electron verification

  • Verified in running Electron instance via Playwright automation.
  • Loaded paper in reader, opened Notes tab, typed markdown content.
  • Verified vellum:notes-save upserts row into SQLite notes table.
  • Verified vellum:notes-get restores persisted note content on reopen.

Visual and console evidence

  • NotesPanel renders textarea with live character counter, status indicator (Saving... / Saved), and Clear note button.
  • Clean console logs without warnings or unhandled exceptions.

Limitations and follow-ups

  • Rich WYSIWYG markdown formatting or citation autocompletion in notes is deferred to future depth milestones.

Independent review

All changes respect the strict storage split (SQLite for notes state, never raw paper bytes) and use typed IPC boundaries via preload.

Closes #29

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds schema version 3 for per-paper notes, repository CRUD operations, Electron IPC and preload methods, a debounced NotesPanel with clear and lifecycle-save behavior, and routing from the Notes tab to the new panel.

Changes

Notes feature

Layer / File(s) Summary
Notes schema migration
core/store/schema.ts, core/store/migrate.ts, core/store/*test.ts
Adds the version 3 notes table with a paper foreign key, cascading deletes, note body, and update timestamp; migration tests now expect schema version 3 and the new table.
Notes repository
core/notes/repo.ts, core/notes/repo.test.ts
Adds typed note retrieval, upsert, deletion, overwrite, timestamp, cascade, and file-persistence behavior.
Notes IPC bridge
electron/main.ts, electron/preload.ts
Adds validated get, save, and delete IPC handlers and typed preload methods.
Notes panel autosave
src/app/NotesPanel.tsx, src/app/NotesPanel.module.css, src/app/NotesPanel.test.tsx
Adds the editor UI with debounced autosave, pending-save flushing on paper changes and unmount, clear-note behavior, status rendering, and styling.
Notes tab integration
src/app/RightPanel.tsx, src/app/RightPanel.test.tsx
Routes the Notes tab to NotesPanel and keeps Annotations as the deferred tab.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant NotesPanel
  participant PreloadAPI
  participant ElectronMain
  participant NotesRepository
  participant SQLite
  NotesPanel->>PreloadAPI: notesGet(slug)
  PreloadAPI->>ElectronMain: invoke vellum:notes-get
  ElectronMain->>NotesRepository: getNote(db, slug)
  NotesRepository->>SQLite: SELECT note by paper_slug
  SQLite-->>NotesRepository: note row or no row
  NotesRepository-->>ElectronMain: NoteRecord or undefined
  ElectronMain-->>PreloadAPI: NoteRecord or null
  PreloadAPI-->>NotesPanel: loaded note
Loading
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description covers scope and core verification, but it omits several required template sections and required evidence for live verification, visual checks, limitations, and independent review. Add all missing template sections, including user outcome, complete automated checks, live Electron steps and results, visual and console evidence, limitations, and independent review status.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the P2-01 Notes tab implementation and its per-paper SQLite-backed Markdown notes scope.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch p2-01-notes

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/app/NotesPanel.tsx`:
- Around line 47-55: Update the save and delete flows in flushPendingSave,
clearNote, and the debounced onChange handler so rejected notesSave/notesDelete
calls are not silently treated as successful: surface a clear failed-save status
to the user and preserve or re-arm the dirty state where the edit must remain
retryable, especially for debounced saves and flushes. For clearNote, restore
the prior note/body state when deletion fails so the UI does not falsely show an
empty persisted note.
- Around line 61-89: Reset the note body immediately when the slug changes,
before starting notesGet in the useEffect, so the textarea cannot display the
previous paper’s content while the new note loads. Keep the existing slug-empty
handling, cancellation logic, and loaded/error result behavior unchanged.
🪄 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: 46b1db1e-bf29-4a58-ba5f-b6b371ab6b4d

📥 Commits

Reviewing files that changed from the base of the PR and between ef39911 and 6290285.

📒 Files selected for processing (13)
  • core/notes/repo.test.ts
  • core/notes/repo.ts
  • core/store/db.test.ts
  • core/store/migrate.test.ts
  • core/store/migrate.ts
  • core/store/schema.ts
  • electron/main.ts
  • electron/preload.ts
  • src/app/NotesPanel.module.css
  • src/app/NotesPanel.test.tsx
  • src/app/NotesPanel.tsx
  • src/app/RightPanel.test.tsx
  • src/app/RightPanel.tsx

Comment thread src/app/NotesPanel.tsx
Comment thread src/app/NotesPanel.tsx
Add a Notes tab in the right panel backed by a new `notes` table (one
freeform markdown note per paper, keyed by paper_slug).

Storage: migration 3 creates the `notes` table with `paper_slug` as the
primary key (natural 1:1 key, no id indirection) and `ON DELETE CASCADE`
so a paper's note is removed with the paper. `core/notes/repo.ts` exposes
getNote / upsertNote / deleteNote; upsert is INSERT ... ON CONFLICT.

Seam: three IPC channels (vellum:notes-get / -save / -delete) with slug
and body validation in the main process; renderer reaches them only via
window.vellum.

UI: NotesPanel renders a markdown editor bound to the open paper's slug,
with debounced autosave and a clear-note action. Wired into RightPanel's
Notes tab (was a "coming soon" stub).

Acceptance ([P2-01]):
- notes CRUD per paper ✓
- persists across restart ✓
- autosave ✓
- tests ✓ (repo CRUD, migration, autosave debounce/flush, persistence,
  RightPanel wiring)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dkritarth
dkritarth merged commit 5d27616 into master Sep 12, 2026
1 check passed
@dkritarth
dkritarth deleted the p2-01-notes branch September 12, 2026 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:blocked Do not start; dependency or phase gate remains open

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[R1-01] Verify and finish per-paper Notes workflow

1 participant