feat: implement lazy loading for session content #3329
Conversation
- Load only metadata at startup for faster app launch - Load session content (transcripts, notes) on-demand when session is opened - Add useSessionContentLoader hook for components to trigger content loading - Show loading state while content is being fetched - Mark newly created sessions as loaded to avoid unnecessary loading - Clear content load state when sessions are reloaded 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 canceled.
|
✅ Deploy Preview for howto-fix-macos-audio-selection canceled.
|
✅ Deploy Preview for hyprnote canceled.
|
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (contentLoading) { | ||
| return ( | ||
| <div className="flex items-center justify-center h-full"> | ||
| <div className="text-sm text-muted-foreground">Loading session...</div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔴 React hooks called after conditional early return
The TabContentNote component has an early return at lines 96-102 that occurs before hooks are called, violating React's rules of hooks.
Click to expand
Problem
In TabContentNote, hooks (useEffect at line 104 and useQuery at line 135) are called after a conditional early return:
if (contentLoading) {
return (
<div className="flex items-center justify-center h-full">
<div className="text-sm text-muted-foreground">Loading session...</div>
</div>
);
}
useEffect(() => { // This hook is called conditionally!
...
}, [...]);React requires hooks to be called in the same order on every render. When contentLoading is true, the hooks after the return statement are not called, but when contentLoading becomes false, they suddenly are. This can cause:
- React errors/warnings about hooks being called in a different order
- Unpredictable behavior and potential crashes
- State inconsistencies
Fix
Move all hooks before the early return, or move the loading check into the JSX return.
Was this helpful? React with 👍 or 👎 to provide feedback.
| export async function loadSessionContent( | ||
| dataDir: string, | ||
| sessionId: string, | ||
| ): Promise<LoadResult<LoadedSessionData>> { | ||
| const result = createEmptyLoadedSessionData(); | ||
| const sessionsDir = [dataDir, "sessions"].join(sep()); | ||
|
|
||
| const scanResult = await fsSyncCommands.scanAndRead( | ||
| sessionsDir, | ||
| [SESSION_TRANSCRIPT_FILE, `*${SESSION_NOTE_EXTENSION}`], | ||
| true, | ||
| `/${sessionId}/`, | ||
| ); | ||
|
|
||
| if (scanResult.status === "error") { | ||
| if (isDirectoryNotFoundError(scanResult.error)) { | ||
| return ok(result); | ||
| } | ||
| console.error( | ||
| `[${LABEL}] loadSessionContent scan error:`, | ||
| scanResult.error, | ||
| ); | ||
| return err(scanResult.error); | ||
| } | ||
|
|
||
| for (const [path, content] of Object.entries(scanResult.data.files)) { | ||
| if (!content) continue; | ||
| if (path.endsWith(SESSION_TRANSCRIPT_FILE)) { | ||
| processTranscriptFile(path, content, result); | ||
| } | ||
| } | ||
|
|
||
| const mdPromises: Promise<void>[] = []; | ||
| for (const [path, content] of Object.entries(scanResult.data.files)) { | ||
| if (!content) continue; | ||
| if (path.endsWith(SESSION_NOTE_EXTENSION)) { | ||
| mdPromises.push(processMdFile(path, content, result)); | ||
| } | ||
| } | ||
| await Promise.all(mdPromises); | ||
|
|
||
| return ok(result); | ||
| } |
There was a problem hiding this comment.
🔴 Session raw_md content is never loaded during lazy loading
The loadSessionContent function fails to load raw_md content from _memo.md files because result.sessions is empty when processing note files.
Click to expand
Problem
In loadSessionContent (apps/desktop/src/store/tinybase/persister/session/load/index.ts:132-174):
- It creates an empty result with
result.sessions = {} - It scans for
*.mdfiles including_memo.md - When
processMdFileprocesses_memo.md, it checksif (result.sessions[fm.session_id])atnote.ts:37before settingraw_md - Since no meta file is loaded,
result.sessionsis empty, so the check fails andraw_mdis never set
// In note.ts:36-39
if (path.endsWith(SESSION_MEMO_FILE)) {
if (result.sessions[fm.session_id]) { // Always false since sessions is empty!
result.sessions[fm.session_id].raw_md = tiptapContent;
}
}Then in ops.ts:98-101, the code tries to use this never-populated data:
const session = result.data.sessions[sessionId]; // undefined
if (session?.raw_md) {
store.setCell("sessions", sessionId, "raw_md", session.raw_md);
}Impact
User notes stored in _memo.md files will not be loaded when opening existing sessions. Users will see empty notes even though they have saved content.
Fix
In loadSessionContent, initialize result.sessions[sessionId] before processing note files, or change processMdFile to create the session entry if it doesn't exist for memo files.
Was this helpful? React with 👍 or 👎 to provide feedback.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| if (result.status === "error") { | ||
| console.error( | ||
| `[SessionOps] Failed to load content for session ${sessionId}:`, | ||
| result.error, | ||
| ); | ||
| contentLoadState.loading.delete(sessionId); | ||
| contentLoadState.loaded.add(sessionId); | ||
| return false; |
There was a problem hiding this comment.
Error handling breaks retry logic: When content loading fails, the session is still marked as loaded (line 83). This prevents any future retry attempts, permanently leaving the session without its content.
if (result.status === "error") {
console.error(
`[SessionOps] Failed to load content for session ${sessionId}:`,
result.error,
);
contentLoadState.loading.delete(sessionId);
// Don't mark as loaded on error - allow retry
return false;
}Remove line 83 (contentLoadState.loaded.add(sessionId)) from the error handler to allow retries on subsequent session opens.
| if (result.status === "error") { | |
| console.error( | |
| `[SessionOps] Failed to load content for session ${sessionId}:`, | |
| result.error, | |
| ); | |
| contentLoadState.loading.delete(sessionId); | |
| contentLoadState.loaded.add(sessionId); | |
| return false; | |
| if (result.status === "error") { | |
| console.error( | |
| `[SessionOps] Failed to load content for session ${sessionId}:`, | |
| result.error, | |
| ); | |
| contentLoadState.loading.delete(sessionId); | |
| return false; |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
feat: implement lazy loading for session content
Summary
This PR implements hybrid lazy loading for sessions to improve app startup time. At startup, only session metadata (title, created_at, participants, etc.) is loaded into TinyBase. Session content (transcripts, notes, raw_md) is loaded on-demand when a session is opened.
Key changes:
LoadModetype ("metadata" | "full") to control what gets loaded during startuploadAllSessionDatato use "metadata" mode at startup, skipping transcript and note file readsensureSessionContentLoaded()function that loads content into TinyBase when a session is openeduseSessionContentLoaderhook for components to trigger content loadingThe existing TinyBase persister infrastructure, editing, undo/redo, and auto-save logic remain unchanged - content is still stored in TinyBase once loaded.
Review & Testing Checklist for Human
_meta.jsonfiles are readRecommended test plan:
Notes
There's module-level state (
contentLoadState) tracking which sessions have content loaded. This is intentional to prevent duplicate loads, but worth noting that this state is per-window and resets on hot reload.Link to Devin run: https://app.devin.ai/sessions/7dddac540af2421da06531009e8acdf9
Requested by: yujonglee (@yujonglee)