feat: stream real-time updates into Director's Notes#508
Open
pedramamini wants to merge 2 commits intomainfrom
Open
feat: stream real-time updates into Director's Notes#508pedramamini wants to merge 2 commits intomainfrom
pedramamini wants to merge 2 commits intomainfrom
Conversation
…e activity indicators Broadcast new history entries via IPC when they are added, subscribe in the UnifiedHistoryTab with RAF batching and deduplication, and extend the HistoryStatsBar with spinning Active agent count and Queued message count indicators derived from the Zustand session store.
📝 WalkthroughWalkthroughThis PR adds real-time history streaming: the main process now broadcasts new history entries via IPC, preload exposes an onHistoryEntryAdded subscription, IPC handler registration accepts a safeSend dependency, and renderer UI components subscribe and display live history and activity counts. Changes
Sequence Diagram(s)sequenceDiagram
participant Main as Main Process
participant Handlers as IPC Handlers
participant Preload as Preload Layer
participant Renderer as Renderer (UnifiedHistoryTab)
participant SessionStore as Session Store
Main->>Handlers: add history entry
Handlers->>Handlers: persist entry
Handlers->>Main: call deps.safeSend(entry, sourceSessionId)
Main->>Preload: IPC message 'history:entryAdded' (entry, sourceSessionId)
Preload->>Renderer: invoke onHistoryEntryAdded callback
Renderer->>SessionStore: lookup agent/session names
SessionStore-->>Renderer: return sessionNameMap
Renderer->>Renderer: buffer entry (pendingEntriesRef)
Renderer->>Renderer: requestAnimationFrame flush -> dedupe & enrich
Renderer->>Renderer: merge into entries & update stats
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR adds real-time streaming of history entries into Director's Notes by broadcasting Key observations:
Confidence Score: 2/5
Sequence DiagramsequenceDiagram
participant Agent as Agent (Main Process)
participant HistoryHandler as history:add IPC Handler
participant HistoryManager as HistoryManager
participant SafeSend as safeSend
participant Renderer as Renderer (BrowserWindow)
participant Preload as directorNotes preload
participant Component as UnifiedHistoryTab
Agent->>HistoryHandler: ipcRenderer.invoke('history:add', entry)
HistoryHandler->>HistoryManager: addEntry(sessionId, projectPath, entry)
HistoryManager-->>HistoryHandler: stored
HistoryHandler->>SafeSend: safeSend('history:entryAdded', entry, sessionId)
SafeSend->>Renderer: webContents.send('history:entryAdded', entry, sessionId)
Renderer->>Preload: ipcRenderer.on('history:entryAdded', handler)
Preload->>Component: callback(entry, sessionId)
Component->>Component: push to pendingEntriesRef
Component->>Component: requestAnimationFrame(flushPending)
Component->>Component: flushPending() — deduplicate, sort, setState
Component->>Component: setEntries / setGraphEntries / setHistoryStats (batched)
Last reviewed commit: a9952f4 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+113
to
+119
| const sessionNameMap = useSessionStore((s) => { | ||
| const map = new Map<string, string>(); | ||
| for (const sess of s.sessions) { | ||
| map.set(sess.id, sess.name); | ||
| } | ||
| return map; | ||
| }); |
There was a problem hiding this comment.
The sessionNameMap Zustand selector constructs a new Map on every invocation. Since Zustand uses strict reference equality checks for selector memoization, this fresh Map reference is never equal to the previous one, causing the selector result to change on every render.
This makes sessionNameMap an unstable dependency for the streaming useEffect below (line 121), which tears down and re-subscribes to the IPC listener on every render cycle. Under active agent load, this creates constant re-subscription churn, making the streaming connection effectively unstable and wasting CPU cycles.
Use a stable ref pattern instead to read session data without making it a reactive dependency:
Suggested change
| const sessionNameMap = useSessionStore((s) => { | |
| const map = new Map<string, string>(); | |
| for (const sess of s.sessions) { | |
| map.set(sess.id, sess.name); | |
| } | |
| return map; | |
| }); | |
| const sessionsRef = useRef(useSessionStore.getState().sessions); | |
| useEffect(() => { | |
| // Subscribe to future changes but keep current value in a ref | |
| return useSessionStore.subscribe((s) => { | |
| sessionsRef.current = s.sessions; | |
| }); | |
| }, []); | |
| // Then in the onHistoryEntryAdded callback below, read from sessionsRef: | |
| const enriched = { | |
| ...rawEntry, | |
| sourceSessionId, | |
| agentName: sessionsRef.current.find((s) => s.id === sourceSessionId)?.name, | |
| } as UnifiedHistoryEntry; |
This way, the streaming effect has no reactive dependencies on session data and remains stable.
| return merged; | ||
| }); | ||
|
|
||
| setTotalEntries((prev) => prev + batch.length); |
There was a problem hiding this comment.
setEntries correctly deduplicates the batch—only adding entries whose IDs aren't already in the list—but setTotalEntries increments by the full batch.length, counting duplicate IDs that were silently dropped. Over time, especially if IPC redelivers the same entry due to the re-subscription churn described above, the entry count will drift upward while the actual entry list does not grow, breaking the badge display.
The count should mirror the number of entries actually added:
Suggested change
| setTotalEntries((prev) => prev + batch.length); | |
| setEntries((prev) => { | |
| const existingIds = new Set(prev.map((e) => e.id)); | |
| const newEntries = batch.filter((e) => !existingIds.has(e.id)); | |
| if (newEntries.length === 0) return prev; | |
| // Track exactly how many new entries were added | |
| setTotalEntries((t) => t + newEntries.length); | |
| const merged = [...newEntries, ...prev]; | |
| merged.sort((a, b) => b.timestamp - a.timestamp); | |
| return merged; | |
| }); |
(Nesting setTotalEntries inside the setEntries updater is safe in React 18's automatic batching and ensures the delta always matches the deduplicated count.)
src/main/preload/directorNotes.ts
Outdated
| onHistoryEntryAdded: ( | ||
| callback: (entry: UnifiedHistoryEntry, sourceSessionId: string) => void | ||
| ): (() => void) => { | ||
| const handler = (_event: unknown, entry: UnifiedHistoryEntry, sessionId: string) => { |
There was a problem hiding this comment.
The IPC handler types the entry argument as UnifiedHistoryEntry, which declares sourceSessionId as a required field. However, the main process broadcasts a plain HistoryEntry (which lacks sourceSessionId); the sourceSessionId is passed separately as the sessionId parameter. This type mismatch means TypeScript will not catch downstream code that accidentally reads entry.sourceSessionId directly, believing it to be populated from the wire when it's actually absent.
Use the correct source type:
Suggested change
| const handler = (_event: unknown, entry: UnifiedHistoryEntry, sessionId: string) => { | |
| const handler = (_event: unknown, entry: HistoryEntry, sessionId: string) => { | |
| callback(entry, sessionId); | |
| }; |
(You may need to import HistoryEntry from ../../shared/types if not already imported.)
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/preload/directorNotes.ts`:
- Around line 114-118: The callback currently types its first arg as
UnifiedHistoryEntry but the actual payload lacks sourceSessionId (it's passed as
the second arg), so update the callback and handler signatures to reflect that
(e.g. change the first parameter type to Omit<UnifiedHistoryEntry,
'sourceSessionId'>) in onHistoryEntryAdded and the inner handler variable so
callers can't assume sourceSessionId is already present; keep passing sessionId
as the second argument when invoking callback.
In `@src/renderer/components/DirectorNotes/UnifiedHistoryTab.tsx`:
- Around line 121-165: flushPending currently dedupes only against prior state
and updates totals/stats/graph from the raw batch, and pendingEntriesRef is
never cleared on cleanup which can replay stale items; modify flushPending (and
the similar handler around lines 190-195) to 1) internally dedupe the incoming
batch itself (remove duplicates within batch) and then dedupe that result
against existing IDs from entries/graph before merging, 2) compute
setTotalEntries and setHistoryStats counts from the final newEntries (not the
raw batch) so counters reflect only actually-added items, and 3) ensure on
unmount/resubscribe you clear pendingEntriesRef.current (and cancel rafIdRef) to
avoid flushing stale queued entries.
In `@src/renderer/global.d.ts`:
- Around line 2715-2721: The realtime usageStats shape in the callback currently
uses cacheReadTokens/cacheWriteTokens which diverges from the canonical
UsageStats interface; update the usageStats object to match the exact property
names and types defined on the existing UsageStats interface (e.g., rename
cacheReadTokens/cacheWriteTokens to the canonical property names used by
UsageStats or import/extend UsageStats directly), and ensure the usageStats
declaration (symbol: usageStats) matches the canonical UsageStats type so
streamed entries are correctly typed.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/__tests__/main/ipc/handlers/history.test.tssrc/main/ipc/handlers/history.tssrc/main/ipc/handlers/index.tssrc/main/preload/directorNotes.tssrc/renderer/components/DirectorNotes/UnifiedHistoryTab.tsxsrc/renderer/components/History/HistoryStatsBar.tsxsrc/renderer/global.d.ts
Comment on lines
+114
to
+118
| onHistoryEntryAdded: ( | ||
| callback: (entry: UnifiedHistoryEntry, sourceSessionId: string) => void | ||
| ): (() => void) => { | ||
| const handler = (_event: unknown, entry: UnifiedHistoryEntry, sessionId: string) => { | ||
| callback(entry, sessionId); |
There was a problem hiding this comment.
Fix onHistoryEntryAdded callback payload typing.
On Line 115, entry is typed as UnifiedHistoryEntry (which requires sourceSessionId), but sourceSessionId is delivered as the second callback arg. This contract is misleading and can cause incorrect assumptions in callers.
Suggested fix
-import type { ToolType } from '../../shared/types';
+import type { ToolType, HistoryEntry } from '../../shared/types';
@@
onHistoryEntryAdded: (
- callback: (entry: UnifiedHistoryEntry, sourceSessionId: string) => void
+ callback: (entry: HistoryEntry, sourceSessionId: string) => void
): (() => void) => {
- const handler = (_event: unknown, entry: UnifiedHistoryEntry, sessionId: string) => {
+ const handler = (_event: unknown, entry: HistoryEntry, sessionId: string) => {
callback(entry, sessionId);
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/preload/directorNotes.ts` around lines 114 - 118, The callback
currently types its first arg as UnifiedHistoryEntry but the actual payload
lacks sourceSessionId (it's passed as the second arg), so update the callback
and handler signatures to reflect that (e.g. change the first parameter type to
Omit<UnifiedHistoryEntry, 'sourceSessionId'>) in onHistoryEntryAdded and the
inner handler variable so callers can't assume sourceSessionId is already
present; keep passing sessionId as the second argument when invoking callback.
…orrect types - Replace unstable sessionNameMap Zustand selector (new Map per render) with a stable ref + subscribe pattern to avoid streaming effect re-subscription - Dedupe within batch before merging; compute setTotalEntries and setHistoryStats from deduplicated entries only (not raw batch) - Clear pendingEntriesRef on cleanup to prevent stale replay after resubscribe - Use HistoryEntry (not UnifiedHistoryEntry) in preload callback type since the wire payload lacks sourceSessionId - Use canonical UsageStats interface in global.d.ts (fixes pre-existing cacheReadTokens/cacheWriteTokens field name mismatch)
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/renderer/components/DirectorNotes/UnifiedHistoryTab.tsx (1)
137-160:⚠️ Potential issue | 🟠 MajorAvoid side effects inside the
setEntriesupdater to prevent counter drift.
setTotalEntriesandsetHistoryStatsare invoked from inside thesetEntriesupdater at lines 143 and 146. With React StrictMode enabled in the renderer (src/renderer/main.tsx:88), this can double-apply counter updates even when entry merges are correct, causing drift intotalCount,autoCount, anduserCount.Move both state updates outside the updater function to prevent replay issues:
Proposed fix
+ const knownEntryIdsRef = useRef<Set<string>>(new Set()); + useEffect(() => { + knownEntryIdsRef.current = new Set(entries.map((e) => e.id)); + }, [entries]); useEffect(() => { const flushPending = () => { rafIdRef.current = null; const batch = pendingEntriesRef.current; if (batch.length === 0) return; pendingEntriesRef.current = []; // Dedupe within the batch itself const seen = new Set<string>(); const uniqueBatch: UnifiedHistoryEntry[] = []; for (const entry of batch) { if (!seen.has(entry.id)) { seen.add(entry.id); uniqueBatch.push(entry); } } + + const newEntries = uniqueBatch.filter((e) => !knownEntryIdsRef.current.has(e.id)); + if (newEntries.length === 0) return; + for (const entry of newEntries) knownEntryIdsRef.current.add(entry.id); setEntries((prev) => { - const existingIds = new Set(prev.map((e) => e.id)); - const newEntries = uniqueBatch.filter((e) => !existingIds.has(e.id)); - if (newEntries.length === 0) return prev; - - // Update total count to match actual additions - setTotalEntries((t) => t + newEntries.length); - - // Incrementally update stats counters from deduplicated entries - setHistoryStats((prevStats) => { - if (!prevStats) return prevStats; - let newAuto = 0; - let newUser = 0; - for (const entry of newEntries) { - if (entry.type === 'AUTO') newAuto++; - else if (entry.type === 'USER') newUser++; - } - return { - ...prevStats, - autoCount: prevStats.autoCount + newAuto, - userCount: prevStats.userCount + newUser, - totalCount: prevStats.totalCount + newAuto + newUser, - }; - }); - const merged = [...newEntries, ...prev]; merged.sort((a, b) => b.timestamp - a.timestamp); return merged; }); + + setTotalEntries((t) => t + newEntries.length); + setHistoryStats((prevStats) => { + if (!prevStats) return prevStats; + let newAuto = 0; + let newUser = 0; + for (const entry of newEntries) { + if (entry.type === 'AUTO') newAuto++; + else if (entry.type === 'USER') newUser++; + } + return { + ...prevStats, + autoCount: prevStats.autoCount + newAuto, + userCount: prevStats.userCount + newUser, + totalCount: prevStats.totalCount + newAuto + newUser, + }; + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/DirectorNotes/UnifiedHistoryTab.tsx` around lines 137 - 160, The setEntries updater currently performs side effects by calling setTotalEntries and setHistoryStats inside its functional update (see setEntries, setTotalEntries, setHistoryStats, uniqueBatch, existingIds, newEntries); move those state updates out of the setEntries updater to avoid double application under React StrictMode: inside the updater only return the new entries array (compute newEntries by filtering uniqueBatch against existingIds), then after setEntries completes compute the increments (newAuto/newUser and newEntries.length) and call setTotalEntries and setHistoryStats once with those computed deltas (use the functional form for setHistoryStats to add deltas to prevStats) so counters are updated exactly once per batch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/renderer/components/DirectorNotes/UnifiedHistoryTab.tsx`:
- Around line 137-160: The setEntries updater currently performs side effects by
calling setTotalEntries and setHistoryStats inside its functional update (see
setEntries, setTotalEntries, setHistoryStats, uniqueBatch, existingIds,
newEntries); move those state updates out of the setEntries updater to avoid
double application under React StrictMode: inside the updater only return the
new entries array (compute newEntries by filtering uniqueBatch against
existingIds), then after setEntries completes compute the increments
(newAuto/newUser and newEntries.length) and call setTotalEntries and
setHistoryStats once with those computed deltas (use the functional form for
setHistoryStats to add deltas to prevStats) so counters are updated exactly once
per batch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 068890e1-97a0-4333-a726-a0828a04751c
📒 Files selected for processing (3)
src/main/preload/directorNotes.tssrc/renderer/components/DirectorNotes/UnifiedHistoryTab.tsxsrc/renderer/global.d.ts
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
history:entryAdded) afterhistoryManager.addEntry()so the Director's Notes modal receives live updates while openHistoryStatsBarwith spinning Active agent count (warning color) and Queued message count indicators, backward-compatible with the per-session History panelUnifiedHistoryTabon mount withrequestAnimationFramebatching, Set-based deduplication, lookback filtering, and incremental stats counter updates — live counts derived from two primitive Zustand selectors against the session storeChanged files
src/main/ipc/handlers/history.tsHistoryHandlerDependencieswithsafeSend, broadcast afteraddEntry()src/main/ipc/handlers/index.tscreateSafeSend(deps.getMainWindow)to history handlerssrc/main/preload/directorNotes.tsonHistoryEntryAdded()IPC subscriptionsrc/renderer/global.d.tssrc/renderer/components/History/HistoryStatsBar.tsxactiveAgentCount+totalQueuedItemswith spinning/queued indicatorssrc/renderer/components/DirectorNotes/UnifiedHistoryTab.tsxsrc/__tests__/main/ipc/handlers/history.test.tsTest plan
Summary by CodeRabbit