Conversation
Resolve the target thread in the router's beforeLoad and throw redirect() instead of mounting a component that runs the resolution in a useEffect. Eliminates the one-frame blank render between /playground and the chat sub-route.
Introduce chatThreadQueryOptions and consume it from the /playground/chat/$threadId loader via ensureQueryData + pendingComponent. useChatStreaming reads messages from the cache with useQuery and hydrates the useChat instance once per thread via a ref guard, so switching threads no longer flashes a loading state. usePlaygroundThreads invalidates the new query key on streaming-complete to refresh titles and messages. Remove the now-unused loadMessages from useThreadManagement.
Enable router-level scrollRestoration keyed by location.pathname and register the chat messages container with data-scroll-restoration-id so TSR persists and restores its scroll position across navigations. Apply overflow-anchor: auto on the scroll container and each MCP app iframe wrapper so async height changes don't shove the viewport. useAutoScroll is rewritten around the new contract: its responsibilities shrink to following the bottom while streaming, the scroll-to-bottom button, and an initial 5s settling window that re-applies the saved scroll target as MCP iframes finish their size handshake. Distinguishes overflow-anchor-driven scroll events from genuine user input (wheel / touch / key / pointer) so the settling window isn't cancelled by the first iframe resize. The in-component loading overlay is removed — the route's pendingComponent now owns that state.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR moves chat thread hydration and scroll restoration to route-level primitives: thread data is preloaded in the TanStack Router loader and cached via React Query, while scroll position is delegated to TanStack Router’s element scroll restoration (with a revised useAutoScroll focusing on follow-bottom + a “settling” window for async iframe growth).
Changes:
- Preload chat thread metadata + messages in
/playground/chat/$threadIdroute loader via React QueryensureQueryData. - Update
useChatStreamingto hydrateuseChatfrom the query cache (removing the in-component “Loading chat history…” overlay behavior). - Replace custom sessionStorage-based scroll persistence with TanStack Router element scroll restoration and a reworked
useAutoScroll(plusoverflow-anchortweaks for MCP iframes).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| renderer/src/routes/playground.index.tsx | Moves initial thread resolution/redirect into beforeLoad to avoid mount-then-redirect blank/flash. |
| renderer/src/routes/playground.chat.$threadId.tsx | Adds loader + pendingComponent for loader-driven chat history warmup and pending UI. |
| renderer/src/routes/tests/playground.index.test.tsx | Updates index-route tests to execute beforeLoad in a test router. |
| renderer/src/renderer.tsx | Enables TanStack Router scroll restoration and keys restoration by pathname. |
| renderer/src/features/chat/lib/thread-query.ts | Introduces chatThreadQueryOptions(threadId) queryOptions factory for thread preload/consumption. |
| renderer/src/features/chat/hooks/use-thread-management.ts | Removes message-loading responsibility; keeps only “current thread id” resolution + clear messages. |
| renderer/src/features/chat/hooks/use-playground-threads.ts | Invalidates the loader-primed thread query key on streaming-complete signals. |
| renderer/src/features/chat/hooks/use-chat-streaming.ts | Consumes thread cache via useQuery and hydrates useChat once per thread id. |
| renderer/src/features/chat/hooks/use-auto-scroll.ts | Rewrites auto-scroll around TSR element scroll restoration + a placement/settling window. |
| renderer/src/features/chat/hooks/tests/use-chat-streaming.test.ts | Updates mocks for new thread query path (adds getThread mocking). |
| renderer/src/features/chat/hooks/tests/use-auto-scroll.test.ts | Reworks tests to align with TSR scroll restoration + ResizeObserver-driven settling logic. |
| renderer/src/features/chat/components/mcp-app-view.tsx | Adds overflowAnchor: auto to stabilize viewport around async iframe resizing. |
| renderer/src/features/chat/components/chat-interface.tsx | Removes in-component loading overlay; registers scroll container for TSR restoration + overflow anchoring. |
| renderer/src/features/chat/components/tests/chat-interface.test.tsx | Drops loading-overlay assertions; stubs TSR restoration hook and updates mocks accordingly. |
The placement effect gated on \`savedScroll.scrollY > 0\`, which treated a valid restored \`scrollY: 0\` (user was scrolled to the very top of a thread before leaving) the same as the "no saved entry" case and force-scrolled to the bottom instead of restoring the top. Only the absence of an entry should fall back to 'bottom'; a concrete entry of 0 is a legitimate restore target.
After moving thread preload to the route loader + React Query, IPC or query failures from \`chatThreadQueryOptions\` were silently swallowed: \`useQuery\` only exposed \`data\`/\`isPending\` and the error never made it into \`processedError\`, leaving the UI with empty messages and no indication that loading had failed. Destructure \`error\` from the query and fold it into \`persistentError\` (which already flows into \`processedError\`), matching the pre-refactor \`loadMessages\` try/catch behaviour.
- Swap the \`globalThis.ResizeObserver\` overwrite for \`vi.stubGlobal\` + \`vi.unstubAllGlobals()\` in afterEach so the stub doesn't leak into other test files sharing the worker. - Update the scrollY=0 placement test to assert the corrected restore-to-top behaviour (was previously encoding the bug). - Fix a stale comment that referenced a module-level "first-mount-per-thread" Set; the hook uses a per-instance \`placedForThreadRef\` now.
peppescg
reviewed
Apr 22, 2026
\`overflow-anchor: auto\` is the CSS default for every element and nothing in the tree (or Tailwind preflight) sets it to \`none\`, so re-asserting it inline on the chat scroll container and the MCP app wrapper was a no-op. Remove the inline \`style\` props, move the \`width: 100%\` on the MCP app wrapper to a \`w-full\` utility, and tighten the scroll-container comment to reflect that we rely on native scroll anchoring rather than opting into it.
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
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.
Two issues compounded whenever the user opened a chat thread:
useChatStreamingfetched the message history in auseEffectafter mount, so every thread switch bounced through an in-component "Loading chat history…" overlay even when the data was trivially small.useAutoScrollowned its own save/restore viasessionStorageand chased the bottom on every re-render. It lost track across back/forward navigation, collided with MCP iframes (bridge.onsizechangegrows the iframe async, which shoved the viewport), and animated smoothly through long threads on restore.This PR moves both concerns onto primitives we already have. Thread data is preloaded in the TanStack Router route loader and cached in React Query; scroll position is owned by TanStack Router's native element-level scroll restoration.
useAutoScrollshrinks to the things only the chat actually needs (follow-the-bottom while streaming, the scroll-to-bottom button, and a brief "settling window" so MCP iframes finishing their size handshake don't strand the user on the wrong content).Kapture.2026-04-22.at.10.54.04.mp4
Route layer
renderer/src/features/chat/lib/thread-query.tsexposingchatThreadQueryOptions(threadId)— aqueryOptionsfactory that loadsgetThread+getThreadMessagesForTransportin parallel withstaleTime: 0so invalidations fire cleanly on stream completion./playground/chat/$threadId:loader: ({ context, params }) => context.queryClient.ensureQueryData(chatThreadQueryOptions(params.threadId))+pendingComponent: ChatLoadingDots. The in-component "Loading chat history…" overlay goes away; the route owns the pending state now./playground/(index): redirect logic moves intobeforeLoadusingthrow redirect(...), so the bounce from/playgroundto the active/most-recent thread never renders a blank frame.Consuming the cache
useChatStreaminggains auseQuery(chatThreadQueryOptions(currentThreadId))and hydrates theuseChatinstance exactly once per thread id via a ref guard (hydratedThreadRef). Streaming-in-progress token updates insideuseChatare never overwritten by a cache refetch of the same thread.useThreadManagementloses itsloadMessagesIPC path — it now only owns the "resolve current thread id" responsibility.usePlaygroundThreadsinvalidates['chat', 'thread', threadId]in addition to the existing per-thread refresh when the streaming-complete / thread-started cache signal fires, so the loader sees fresh data on the next navigation.Scroll restoration
routergetsscrollRestoration: true,scrollRestorationBehavior: 'instant', andgetScrollRestorationKey: (location) => location.pathname. Keying by pathname (not by TSR's default per-location state id) means the same/playground/chat/<id>URL restores to the same position regardless of whether the user arrived via back/forward, sidebar click, or deep link.data-scroll-restoration-id="chat-messages"andoverflow-anchor: auto. The MCP app iframe wrapper inmcp-app-view.tsxalso getsoverflow-anchor: autosobridge.onsizechangegrowingiframeHeightdoesn't push surrounding messages downward.useAutoScrollis rewritten against that contract:useElementScrollRestoration({ id: 'chat-messages', getKey: location.pathname }).useChat-drivenhasContentflip, which happens on thread switch), records a placement target (savedscrollYor'bottom') and runs a 5s settling window: aResizeObserveron the inner content re-applies the target as MCP iframes report their final size. Long-thread restores landbehavior: 'instant', not smooth, so there's no long animation.overflow-anchor-driven scroll events from genuine user input by tracking the timestamp of the lastwheel/touchstart/keydown/pointerdownand requiring it to be within 500 ms. Without this, the first iframe resize fires a non-programmatic scroll event that would otherwise look like a user scroll and cancel placement.ResizeObservers into one effect that branches onisStreaming.How to validate
/playgrounddirectly (deep link, new window, orhistory.pushState). The redirect resolves inbeforeLoadand the page never paints a blank frame.pnpm test:nonInteractive— the full suite (1942 tests) is green.Not changed in this PR (deliberate)
getThread/getThreadMessagesForTransport/updateThreadMessagesall behave as before; the migration is renderer-side only.staleTime: 0guarantees fresh loads on re-entry; real cache-size tuning can be a follow-up if memory pressure from very long threads shows up.isPersistentLoadingflag fromuseChatStreaming's return — other callers still read it viauseChatto gate the composer, and it cleanly derives from the new query state now.usePlaygroundThreadsstill owns the flat thread list viauseStaterather than React Query. A similar migration is straightforward but out of scope here.