fix: migrate enhanced_md to enhanced_notes table for markdown saving #1956
Conversation
- Remove enhanced_md field from packages/db/src/schema.ts sessions table - Remove enhanced_md from schema-external.ts sessions schema - Update localPersister2.ts to iterate over enhanced_notes table instead of sessions - Update main.ts to save enhanced notes as separate md files in session folder - Update search indexing to get enhanced content from enhanced_notes table - Update title-transform.ts to read from enhanced_notes - Update search.tsx to display enhanced_notes content - Update test files and devtool seed data to remove enhanced_md references Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughRemoves session-level Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas requiring extra attention:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
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/contexts/search/engine/listeners.ts (1)
13-48: Missing listener:enhanced_noteschanges don't trigger session re-indexing.When an enhanced note is added, updated, or deleted, the associated session's search index content will become stale. Consider adding a listener for the
enhanced_notestable that triggers re-indexing of the affected session:export function createEnhancedNoteListener( index: Index, sessionListener: ReturnType<typeof createSessionListener>, ): RowListener<Schemas, "enhanced_notes", null, PersistedStore> { return (store, _, rowId) => { const sessionId = store.getCell("enhanced_notes", rowId, "session_id"); if (typeof sessionId === "string") { // Trigger session re-indexing sessionListener(store, "sessions", sessionId, () => {}); } }; }
♻️ Duplicate comments (1)
apps/desktop/src/contexts/search/engine/listeners.ts (1)
51-66: Duplicate code already flagged inindexing.ts.Same helper duplicated here. See the refactor suggestion in
indexing.tsto extract this to a shared utility.
🧹 Nitpick comments (4)
apps/desktop/src/store/zustand/ai-task/task-configs/title-transform.ts (1)
17-27: Aggregation logic fromenhanced_noteslooks correct; consider perf/semantics checksThe iteration and filtering are straightforward and correctly aggregate all non‑empty
contentcells for the givensessionIdand join them with"\n\n". Two small things to keep in mind:
- This does a full scan of the
enhanced_notestable on every call. If the table can grow large and the store defines an index or helper forsession_id‑scoped access, it may be worth switching to that to avoid O(N) scans per transform.readEnhancedMarkdownnow always returns a string (possibly""). If callers previously distinguished between “no enhanced content” (e.g.,undefined/null) and “empty string”, it’s worth double‑checking that this behavior change is acceptable for all consumers ofTaskArgsMapTransformed["title"].enhancedMd.apps/desktop/src/contexts/search/engine/indexing.ts (1)
38-53: Performance concern: O(n) scan for each session lookup.
getEnhancedContentForSessioniterates through allenhanced_notesrows to find notes matching the session. DuringindexSessions, this results in O(n×m) complexity where n = sessions and m = enhanced_notes.Consider using an index lookup if TinyBase supports it (similar to how
INDEXES.enhancedNotesBySessionis used in the UI component), or pre-build a map of session_id → contents before the indexing loop.export function indexSessions(db: Index, store: PersistedStore): void { + // Pre-build enhanced content map for O(1) lookups + const enhancedContentMap = buildEnhancedContentMap(store); + const fields = [ "user_id", "created_at", ... ]; store.forEachRow("sessions", (rowId: string, _forEachCell) => { const row = collectCells(store, "sessions", rowId, fields); const title = toTrimmedString(row.title) || "Untitled"; - const enhancedContent = getEnhancedContentForSession(store, rowId); + const enhancedContent = enhancedContentMap.get(rowId) ?? ""; void insert(db, { ... }); }); } + +function buildEnhancedContentMap(store: PersistedStore): Map<string, string> { + const map = new Map<string, string>(); + store.forEachRow("enhanced_notes", (rowId: string, _forEachCell) => { + const sessionId = store.getCell("enhanced_notes", rowId, "session_id"); + const content = store.getCell("enhanced_notes", rowId, "content"); + if (typeof sessionId === "string" && typeof content === "string" && content) { + const existing = map.get(sessionId) ?? ""; + map.set(sessionId, existing ? `${existing} ${content}` : content); + } + }); + return map; +}apps/desktop/src/store/tinybase/localPersister2.ts (1)
4-25: EnhancedNote-based persister wiring looks correct; avoid mutating Tinybase rows /ts-ignoreSwitching
handlePersistto(enhancedNote: EnhancedNote & { id: string }) => Promise<void>and iteratingtables.enhanced_noteswithPromise.allis consistent with the new schema and async file writes.You can drop the
// @ts-ignoreand avoid mutating the Tinybase snapshot row by cloning it instead:- const promises: Promise<void>[] = []; - Object.entries(tables?.enhanced_notes ?? {}).forEach(([id, row]) => { - // @ts-ignore - row.id = id; - promises.push(handlePersist(row as EnhancedNote & { id: string })); - }); - await Promise.all(promises); + const promises: Promise<void>[] = []; + Object.entries(tables?.enhanced_notes ?? {}).forEach(([id, row]) => { + const enhancedNote = { + ...(row as EnhancedNote), + id, + } as EnhancedNote & { id: string }; + + promises.push(handlePersist(enhancedNote)); + }); + await Promise.all(promises);This keeps Tinybase content snapshots immutable and removes the need for the
ts-ignore.apps/desktop/src/store/tinybase/main.ts (1)
179-215: Consider reducing repeated parsing/writes and handling permanently invalid content more gracefullyBecause
createLocalPersister2’s save is scheduled viasetInterval(listener, 1000), the handler will:
- Re-parse
enhancedNote.contentJSON for every row on every tick.- On any permanently invalid
content(parse error), log"Failed to save enhanced note markdown"every second for that row.- Re-write markdown files for unchanged notes on every tick.
Not blockers, but you may want to:
- Use the
_changesargument from the persister callback to only process enhanced notes that actually changed in the last transaction.- Optionally add a more specific log/flag when
isValidTiptapContent(parsed)fails, so you can distinguish “bad tiptap content” from FS errors.- (If needed later) short-circuit permanently broken rows after the first failure to avoid noisy repeated logs.
These would make the persister cheaper and easier to debug without changing its external behavior.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/desktop/src/components/chat/message/tool/search.tsx(2 hunks)apps/desktop/src/components/devtool/seed/data/curated.json(0 hunks)apps/desktop/src/components/devtool/seed/data/loader.ts(0 hunks)apps/desktop/src/components/devtool/seed/data/schema.ts(0 hunks)apps/desktop/src/components/devtool/seed/shared/session.ts(0 hunks)apps/desktop/src/contexts/search/engine/content.ts(1 hunks)apps/desktop/src/contexts/search/engine/indexing.ts(1 hunks)apps/desktop/src/contexts/search/engine/listeners.ts(2 hunks)apps/desktop/src/store/tinybase/localPersister2.ts(2 hunks)apps/desktop/src/store/tinybase/main.ts(2 hunks)apps/desktop/src/store/tinybase/schema-external.ts(1 hunks)apps/desktop/src/store/zustand/ai-task/task-configs/title-transform.ts(1 hunks)apps/desktop/src/utils/timeline.test.ts(0 hunks)packages/db/src/schema.ts(0 hunks)
💤 Files with no reviewable changes (6)
- apps/desktop/src/components/devtool/seed/data/loader.ts
- apps/desktop/src/utils/timeline.test.ts
- apps/desktop/src/components/devtool/seed/shared/session.ts
- packages/db/src/schema.ts
- apps/desktop/src/components/devtool/seed/data/schema.ts
- apps/desktop/src/components/devtool/seed/data/curated.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/desktop/src/store/zustand/ai-task/task-configs/title-transform.tsapps/desktop/src/contexts/search/engine/indexing.tsapps/desktop/src/contexts/search/engine/content.tsapps/desktop/src/contexts/search/engine/listeners.tsapps/desktop/src/store/tinybase/localPersister2.tsapps/desktop/src/store/tinybase/schema-external.tsapps/desktop/src/store/tinybase/main.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/store/zustand/ai-task/task-configs/title-transform.tsapps/desktop/src/contexts/search/engine/indexing.tsapps/desktop/src/contexts/search/engine/content.tsapps/desktop/src/contexts/search/engine/listeners.tsapps/desktop/src/store/tinybase/localPersister2.tsapps/desktop/src/components/chat/message/tool/search.tsxapps/desktop/src/store/tinybase/schema-external.tsapps/desktop/src/store/tinybase/main.ts
🧬 Code graph analysis (4)
apps/desktop/src/contexts/search/engine/indexing.ts (1)
apps/desktop/src/contexts/search/engine/content.ts (1)
createSessionSearchableContent(3-12)
apps/desktop/src/contexts/search/engine/listeners.ts (1)
apps/desktop/src/contexts/search/engine/content.ts (1)
createSessionSearchableContent(3-12)
apps/desktop/src/store/tinybase/localPersister2.ts (1)
apps/desktop/src/store/tinybase/schema-external.ts (1)
EnhancedNote(170-170)
apps/desktop/src/store/tinybase/main.ts (1)
packages/tiptap/src/shared/utils.ts (2)
isValidTiptapContent(13-20)json2md(78-83)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: ci (macos, depot-macos-14)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: fmt
🔇 Additional comments (7)
apps/desktop/src/store/tinybase/schema-external.ts (1)
62-68: I'll help you verify the review comment. Let me search for any remaining references toenhanced_mdand validate the migration completeness.
<function_calls>
shell
#!/bin/bashFind remaining references to enhanced_md in TypeScript/TSX files
echo "=== Searching for enhanced_md references ==="
rg -nP --type=ts --type=tsx 'enhanced_md' | head -50
</function_calls>apps/desktop/src/components/chat/message/tool/search.tsx (2)
101-112: Inconsistent content aggregation: only first enhanced note used here vs. all notes joined elsewhere.In
indexing.tsandlisteners.ts,getEnhancedContentForSessionjoins all enhanced notes for a session. Here, only the first note's content is displayed. If a session has multiple enhanced notes, the search preview will show partial content compared to what's indexed.Consider whether this is intentional (show first note as preview) or if you want to aggregate all notes for display consistency.
127-137: LGTM!The fallback logic correctly handles the case where enhanced content is unavailable, preferring enhanced note content when available and falling back to
raw_md.apps/desktop/src/contexts/search/engine/content.ts (1)
3-12: LGTM!Clean refactor to accept
enhancedContentas an optional parameter. This properly decouples content creation from the data source, making the function more flexible.apps/desktop/src/contexts/search/engine/indexing.ts (1)
26-32: LGTM!The integration of enhanced content into session indexing is correct. The enhanced content is properly fetched and passed to the content creation function.
apps/desktop/src/contexts/search/engine/listeners.ts (1)
33-39: LGTM!The session listener correctly fetches enhanced content before updating the search index.
apps/desktop/src/store/tinybase/main.ts (1)
25-25: Enhanced-note → markdown export path is well-guarded and matches the new modelThe new
createLocalPersister2handler correctly:
- Requires both
contentandsession_idbefore doing work.- Parses JSON, validates it as Tiptap content via
isValidTiptapContent, and only then callsjson2md.- Ensures the per-session directory under
BaseDirectory.Data/hyprnote/sessions/{session_id}exists before writing.- Wraps the whole pipeline in a
try/catchand logs failures with the enhanced note id, which is important given this runs on an interval and during window close/blur.This aligns with the migration away from
session.enhanced_mdtoenhanced_notesand looks correct end-to-end.Also applies to: 179-215
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/contexts/search/engine/utils.ts (1)
116-131: Consider performance implications of full table scan.The function performs a full table scan of the
enhanced_notestable for each session during indexing. When indexing multiple sessions, this results in O(n×m) complexity where n is the number of sessions and m is the number of enhanced notes. If you anticipate a large number of enhanced notes, consider building an index or restructuring the iteration to group notes by session upfront.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/contexts/search/engine/indexing.ts(2 hunks)apps/desktop/src/contexts/search/engine/listeners.ts(2 hunks)apps/desktop/src/contexts/search/engine/utils.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/desktop/src/contexts/search/engine/utils.tsapps/desktop/src/contexts/search/engine/indexing.tsapps/desktop/src/contexts/search/engine/listeners.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/contexts/search/engine/utils.tsapps/desktop/src/contexts/search/engine/indexing.tsapps/desktop/src/contexts/search/engine/listeners.ts
🧬 Code graph analysis (2)
apps/desktop/src/contexts/search/engine/indexing.ts (2)
apps/desktop/src/contexts/search/engine/utils.ts (1)
getEnhancedContentForSession(116-131)apps/desktop/src/contexts/search/engine/content.ts (1)
createSessionSearchableContent(3-12)
apps/desktop/src/contexts/search/engine/listeners.ts (2)
apps/desktop/src/contexts/search/engine/utils.ts (1)
getEnhancedContentForSession(116-131)apps/desktop/src/contexts/search/engine/content.ts (1)
createSessionSearchableContent(3-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: ci (macos, depot-macos-14)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (5)
apps/desktop/src/contexts/search/engine/listeners.ts (2)
4-18: LGTM! Clean import organization.The imports are properly updated to include the new helper function
getEnhancedContentForSessionand maintain good organization by grouping related imports together.
30-46: No data preservation issue found –enhanced_mdis a legacy, unused column in the database schema.The verification reveals that
enhanced_mdwas defined in the initial Drizzle migration but is explicitly omitted from the session schema (.omit({ id: true, enhanced_md: true })) and never actually read from or written to in the codebase. The application already uses theenhanced_notestable exclusively for retrieving enhanced content viagetEnhancedContentForSession(). Sinceenhanced_mdwas not actively used by the current application, there is no existing data to migrate or preserve.apps/desktop/src/contexts/search/engine/utils.ts (1)
1-2: LGTM! Proper type import.The import of
PersistedStoretype is correctly added to support the new helper function.apps/desktop/src/contexts/search/engine/indexing.ts (2)
9-14: LGTM! Proper import of the shared helper.The import correctly includes
getEnhancedContentForSessionfrom the shared utils module, resolving the previous code duplication concern that was flagged in earlier reviews.
17-37: Migration implementation looks correct.The removal of
enhanced_mdfrom the fields list and the integration ofgetEnhancedContentForSessionproperly implements the migration to theenhanced_notestable. The pattern is consistent with the listener implementation.
| export function getEnhancedContentForSession( | ||
| store: PersistedStore, | ||
| sessionId: string, | ||
| ): string { | ||
| const contents: string[] = []; | ||
| store.forEachRow("enhanced_notes", (rowId: string, _forEachCell) => { | ||
| const noteSessionId = store.getCell("enhanced_notes", rowId, "session_id"); | ||
| if (noteSessionId === sessionId) { | ||
| const content = store.getCell("enhanced_notes", rowId, "content"); | ||
| if (typeof content === "string" && content) { | ||
| contents.push(content); | ||
| } | ||
| } | ||
| }); | ||
| return contents.join(" "); | ||
| } |
There was a problem hiding this comment.
Add type checking for session_id comparison.
The function correctly validates that content is a non-empty string before adding it to the array. However, the session_id comparison on line 122 doesn't include a type check. If noteSessionId is undefined or a different type, the comparison may not work as expected.
Consider adding a type check:
export function getEnhancedContentForSession(
store: PersistedStore,
sessionId: string,
): string {
const contents: string[] = [];
store.forEachRow("enhanced_notes", (rowId: string, _forEachCell) => {
const noteSessionId = store.getCell("enhanced_notes", rowId, "session_id");
- if (noteSessionId === sessionId) {
+ if (typeof noteSessionId === "string" && noteSessionId === sessionId) {
const content = store.getCell("enhanced_notes", rowId, "content");
if (typeof content === "string" && content) {
contents.push(content);
}
}
});
return contents.join(" ");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function getEnhancedContentForSession( | |
| store: PersistedStore, | |
| sessionId: string, | |
| ): string { | |
| const contents: string[] = []; | |
| store.forEachRow("enhanced_notes", (rowId: string, _forEachCell) => { | |
| const noteSessionId = store.getCell("enhanced_notes", rowId, "session_id"); | |
| if (noteSessionId === sessionId) { | |
| const content = store.getCell("enhanced_notes", rowId, "content"); | |
| if (typeof content === "string" && content) { | |
| contents.push(content); | |
| } | |
| } | |
| }); | |
| return contents.join(" "); | |
| } | |
| export function getEnhancedContentForSession( | |
| store: PersistedStore, | |
| sessionId: string, | |
| ): string { | |
| const contents: string[] = []; | |
| store.forEachRow("enhanced_notes", (rowId: string, _forEachCell) => { | |
| const noteSessionId = store.getCell("enhanced_notes", rowId, "session_id"); | |
| if (typeof noteSessionId === "string" && noteSessionId === sessionId) { | |
| const content = store.getCell("enhanced_notes", rowId, "content"); | |
| if (typeof content === "string" && content) { | |
| contents.push(content); | |
| } | |
| } | |
| }); | |
| return contents.join(" "); | |
| } |
🤖 Prompt for AI Agents
In apps/desktop/src/contexts/search/engine/utils.ts around lines 116 to 131, the
code compares noteSessionId to sessionId without confirming noteSessionId is a
string; add a type check before the comparison to avoid unexpected matches or
runtime issues (e.g., if noteSessionId is undefined or a number). Update the
loop so you retrieve noteSessionId, verify typeof noteSessionId === "string" (or
otherwise coerce safely) and only then compare with sessionId using strict
equality; keep the existing content type check and push logic unchanged.
yujonglee
force-pushed
the
main
branch
4 times, most recently
from
cd99174 to
cb527d5
Compare
fix: migrate enhanced_md to enhanced_notes table for markdown saving
Summary
This PR completes the migration from storing enhanced markdown in
sessions.enhanced_mdto using the separateenhanced_notestable. The codebase had already introduced theenhanced_notestable but the persistence layer and various consumers were still referencing the oldenhanced_mdfield.Key changes:
enhanced_mdfield from session schemas (both DB and Tinybase)localPersister2.tsto iterate overenhanced_notesinstead ofsessionshyprnote/sessions/{session_id}/{enhanced_note_id}.mdenhanced_notesUpdates since last revision
getEnhancedContentForSessionhelper into shared utility (utils.ts) to eliminate duplication betweenindexing.tsandlisteners.tsReview & Testing Checklist for Human
enhanced_mdfrom the schema but does NOT include a database migration. Verify this is intentional and existing data won't be lost, or add a migration if needed.enhanced_noteschanges. Verify that adding/editing enhanced notes properly updates the search index.ONBOARDING=0 pnpm -F desktop tauri dev) and verify:Notes
Link to Devin run: https://app.devin.ai/sessions/3fb32e6b691e49588eb56483c4bcfbe0
Requested by: yujonglee (@yujonglee)