[codex] Optimize thread loading and browser panels#3
Merged
Eccentric-jamaican merged 2 commits intoJun 11, 2026
Conversation
|
Warning Review limit reached
More reviews will be available in 36 minutes and 22 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR implements a multi-layer thread detail hydration and synchronization system. It adds a store-level preservation mechanism to retain previously-loaded thread details during lightweight snapshots, deduplicates snapshot requests, updates root hydration with options-aware syncing, adds chat-route focused snapshots for split-view support, improves UI loading states and scroll behavior, implements sidebar hover-based thread prefetching, and updates bootstrap and disposal flows to preserve thread details consistently. ChangesThread Detail Hydration and Synchronization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/ChatView.tsx (1)
5260-5280:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve non-authoritative thread bodies on the focused post-create sync.
This success path still applies a focused snapshot with
syncServerReadModel(snapshot)only. Focused snapshots are exactly the payloads that can omit details for other threads, so this can still blank out already-loaded messages/plans/activities right before navigation.Suggested fix
.then(() => api.orchestration.getSnapshot({ mode: "focused", threadId: nextThreadId })) .then((snapshot) => { - syncServerReadModel(snapshot); + syncServerReadModel(snapshot, { + preserveThreadDetails: true, + authoritativeThreadIds: [nextThreadId], + }); return navigate({ to: "/$threadId", params: { threadId: nextThreadId }, }); })🤖 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/web/src/components/ChatView.tsx` around lines 5260 - 5280, The focused-snapshot success path currently calls syncServerReadModel(snapshot) which may omit other threads' bodies and wipe loaded content; update the success branch to call syncServerReadModel(snapshot, { preserveThreadDetails: true }) (the same option used in the bootstrap fallback) before calling navigate so non-authoritative thread bodies are preserved; locate the promise chain where .then((snapshot) => { syncServerReadModel(snapshot); return navigate({ to: "/$threadId", params: { threadId: nextThreadId } }); }) and add the preserveThreadDetails option to that syncServerReadModel call.
🧹 Nitpick comments (1)
apps/web/src/splitViewStore.test.ts (1)
4-27: ⚡ Quick winConsider adding type annotation for compile-time verification.
The
createMemoryStorageimplementation is correct, but adding asatisfies Storageannotation would provide compile-time verification that all Storage interface methods are properly implemented, following the pattern established inbrowserPaneStore.test.ts(lines 3-26).✨ Suggested type annotation
-function createMemoryStorage(): Storage { +function createMemoryStorage() { const values = new Map<string, string>(); - return { + return { get length() { return values.size; }, clear() { values.clear(); }, getItem(key: string) { return values.get(key) ?? null; }, key(index: number) { return [...values.keys()][index] ?? null; }, removeItem(key: string) { values.delete(key); }, setItem(key: string, value: string) { values.set(key, value); }, - }; + } satisfies Storage; }🤖 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/web/src/splitViewStore.test.ts` around lines 4 - 27, The createMemoryStorage function should be annotated to ensure it conforms to the DOM Storage interface; update the return expression so the object literal satisfies the Storage type (use the same "satisfies Storage" pattern used in browserPaneStore.test.ts) to get compile-time verification that getItem, setItem, removeItem, key, clear and length match the Storage signature for the createMemoryStorage function.
🤖 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/web/src/components/chat/MessagesTimeline.tsx`:
- Around line 3694-3705: The bottom-threshold logic in
rowVirtualizer.shouldAdjustScrollPositionOnItemSizeChange ignores the incoming
delta and uses instance.getTotalSize() (which is post-resize), causing
misclassification; change the signature to use the received delta (remove the
underscore) and compute the pre-resize total as previousTotal =
instance.getTotalSize() - delta, then compute remainingDistance = previousTotal
- (scrollOffset + viewportHeight) and use that in isReadingOlderHistory (still
comparing to AUTO_SCROLL_BOTTOM_THRESHOLD_PX), keeping the other checks
(changedItemIsAboveViewport and instance.scrollDirection) unchanged.
In `@apps/web/src/components/ChatView.tsx`:
- Around line 3583-3591: The code is treating any pointer down as a manual
scroll intent because isPointerScrollActiveRef.current is set on pointer down;
update the logic so mere pointer presses don’t disable auto-follow: stop setting
isPointerScrollActiveRef in the pointerdown handler and instead set it only when
a pointermove/pan occurs (or when a pointer-move delta exceeds a small
threshold), then keep the existing branch that checks shouldAutoScrollRef,
pendingUserScrollUpIntentRef and isPointerScrollActiveRef (and call
cancelPendingStickToBottom as before) so only actual pointer-driven scrolling
(not clicks/selection) disables stick-to-bottom; adjust the pointer handlers
where isPointerScrollActiveRef and pendingUserScrollUpIntentRef are mutated to
implement this change.
In `@apps/web/src/components/Sidebar.tsx`:
- Around line 2667-2681: Pinned threads are missing the hover/focus prefetch
handlers, so update renderPinnedThreadRow to mirror renderThreadRow's wiring:
add onMouseEnter to call scheduleThreadDetailPrefetch(thread), onMouseLeave to
call cancelThreadDetailPrefetch(thread.id) and clearArchiveConfirm(thread.id),
onFocusCapture to scheduleThreadDetailPrefetch(thread), and onBlurCapture to
check relatedTarget same as the current logic and then
cancelThreadDetailPrefetch(thread.id) and clearArchiveConfirm(thread.id); ensure
you reference the same helper functions scheduleThreadDetailPrefetch,
cancelThreadDetailPrefetch, and clearArchiveConfirm and use the same
event.relatedTarget containment check as in renderThreadRow.
In `@apps/web/src/routes/_chat`.$threadId.tsx:
- Around line 1601-1620: The current guard uses threadExists and
focusedSnapshotNeedsThreadDetails which can skip loading sibling server-thread
details when the routed thread is draft-only; instead, change the condition to
check whether any focused thread exists in store.threads (e.g., inspect
store.threads for at least one focused id) and only return if no focused threads
at all, and when calling api.orchestration.getSnapshot use a server-backed
threadId (resolve a server thread id from focusedSnapshotThreadIds or omit
threadId so the server fetch targets all focusedThreadIds) rather than the
routed draft-only threadId; update the conditional around
threadsHydrated/hydrationStatus to use this new existence check and adjust the
getSnapshot call arguments (threadId vs threadIds) so server pane messages are
fetched.
- Around line 1072-1083: The pane state reset currently only triggers when
projectChanged is true, but it should also reset when the current pane is empty
(currentPaneThreadId === null); update the condition used in setPanePanelState
so that it resets panel and filesOpen when either projectChanged is true OR
currentPaneThreadId is null. Locate the computation of
currentPaneProjectId/nextPaneProjectId and the projectChanged boolean, then
change the setPanePanelState call (adjacent to replacePaneThread) to use
(projectChanged || currentPaneThreadId === null) to decide whether to spread {
panel: null, filesOpen: false }.
- Around line 992-998: When serializing the focused pane state, preserve the
files rail by carrying over focusedPanelState.filesOpen instead of dropping it;
update the branches that return { panel: "browser" } and { panel: "diff", diff:
"1" } to also include filesOpen: focusedPanelState.filesOpen, and in the
fallback return ensure filesOpen is preserved (e.g., return { filesOpen:
focusedPanelState.filesOpen } when present) so filesOpen isn't lost when
maximizing/minimizing.
---
Outside diff comments:
In `@apps/web/src/components/ChatView.tsx`:
- Around line 5260-5280: The focused-snapshot success path currently calls
syncServerReadModel(snapshot) which may omit other threads' bodies and wipe
loaded content; update the success branch to call syncServerReadModel(snapshot,
{ preserveThreadDetails: true }) (the same option used in the bootstrap
fallback) before calling navigate so non-authoritative thread bodies are
preserved; locate the promise chain where .then((snapshot) => {
syncServerReadModel(snapshot); return navigate({ to: "/$threadId", params: {
threadId: nextThreadId } }); }) and add the preserveThreadDetails option to that
syncServerReadModel call.
---
Nitpick comments:
In `@apps/web/src/splitViewStore.test.ts`:
- Around line 4-27: The createMemoryStorage function should be annotated to
ensure it conforms to the DOM Storage interface; update the return expression so
the object literal satisfies the Storage type (use the same "satisfies Storage"
pattern used in browserPaneStore.test.ts) to get compile-time verification that
getItem, setItem, removeItem, key, clear and length match the Storage signature
for the createMemoryStorage function.
🪄 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
Run ID: 02435800-5dc5-4a2d-b623-a98787f74d19
📒 Files selected for processing (12)
apps/web/src/components/ChatView.tsxapps/web/src/components/Sidebar.tsxapps/web/src/components/chat/MessagesTimeline.tsxapps/web/src/hooks/useDisposableThreadLifecycle.tsapps/web/src/index.cssapps/web/src/routes/__root.tsxapps/web/src/routes/_chat.$threadId.tsxapps/web/src/splitViewStore.test.tsapps/web/src/splitViewStore.tsapps/web/src/store.test.tsapps/web/src/store.tsapps/web/src/wsNativeApi.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.
Problem statement
The app had several user-visible reliability and latency issues in busy workspaces and long conversations:
A deeper issue also surfaced during investigation: the app did not need full thread details for every thread to make the sidebar useful, but some client paths treated omitted details as authoritative empty data.
What changed
Validation
bun run lintpassed. Existing repo warnings remain, no errors.bun run typecheckpassed.bun run --cwd apps/web test src/store.test.ts src/splitViewStore.test.ts src/wsNativeApi.test.tspassed.Notes
OpenCode draft prewarm was intentionally not added. Its session startup emits real provider/session events, so using it as casual prewarm before the user sends a message could create confusing session state. The PR keeps that path conservative.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style
Tests