fix(staged): show the requested session in SessionModal#768
Merged
Conversation
After the shadcn migration, SessionModal is mounted once and persists, loading via a reactive effect keyed on open + sessionId. The teardown guard `closed` was reset by a separately-declared effect that, on reopen, raced behind the load effect in the same flush — so loadSession() bailed at its `if (closed) return` and the modal rendered the previously-viewed session at full fidelity. Reset `closed` deterministically inside the load effect before loading, drop the now-redundant reset effect, and clear session/messages at the top of loadSession() so any bail-out or in-flight window shows a clean loading state for the requested id rather than a stale transcript. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Clearing session/messages on every load made reopening the same session flash a loading state and re-fetch instead of showing the prior transcript instantly. Only clear when the requested sessionId differs from the currently loaded one, preserving the stale-paint safety for a *different* session while keeping the same-session reopen instant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The session chat dialog reportedly shows a loading spinner for a long time and appears to freeze on reopen. Add temporary console.debug instrumentation across the load lifecycle to pinpoint where time is spent or where a load gets stuck: - A `slog()` helper stamps each line with a monotonic performance.now() timestamp plus the requested vs. currently-loaded session id, the closed/open/loading flags. - The load effect logs when it fires and which bail-out branch it takes. - loadSession() logs entry, the backend await start/resolve with elapsed time and result counts, any error, the closed bail-outs, and finally. - poll() logs skip reasons, status-fetch latency, and errors (which were previously swallowed silently). This makes a slow backend call, an out-of-order/stale load, or a stuck in-flight poll visible in the console for diagnosis. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The diagnostic slog() helper read $state (session, loading) and props (open, sessionId) synchronously. Because Svelte 5 effects track every reactive read across the entire synchronous call stack, calling slog() inside the load effect — directly and via loadSession()'s synchronous prologue — subscribed that effect to session/loading. loadSession() then mutates exactly those (clearing session, toggling loading), so each load invalidated the effect's freshly-tracked deps, re-firing it in a tight infinite loop that re-fetched the session and messages every reactive flush (~15ms) and made the dialog appear frozen. Wrap slog()'s reactive reads in untrack() so the snapshot never registers dependencies on the caller. The instrumentation stays usable for further diagnosis without perturbing reactivity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The previous untrack() fix wrapped slog()'s reactive reads but left loadSession()'s synchronous prologue reading session?.id directly to decide whether to clear the transcript. Because Svelte 5 effects track every reactive read across the synchronous call stack, that read subscribed the load effect to `session` — which loadSession() then mutates (clearing it, assigning the loaded session), re-invalidating the effect and re-firing the load in a tight infinite loop, making the dialog appear frozen. Wrap the session?.id read in untrack() so the prologue's same-session check never registers a dependency on `session`, breaking the loop while preserving the clear-on-different-session behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The infinite-load-loop root cause was fixed (untrack the session?.id read in loadSession's prologue), so the temporary slog() instrumentation added to diagnose the slow/frozen SessionModal load path is no longer needed. Remove the slog() helper, the loadSeq counter, and every slog() call across the load effect, loadSession(), and poll(). This also restores the load timing locals (startedAt/reqId/seq) that only existed to feed the logs and the poll() catch block to a silent ignore. The genuine fixes — the deterministic closed=false reset, the untracked same-session check, and the clear-on-different-session behavior — are preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
matt2e
changed the title
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
Fixes the SessionModal sometimes painting a previously-viewed session instead of the one that was just requested, and resolves a related infinite load loop.
closedteardown guard deterministically inside the load effect (same flush) rather than via a separate reset effect that could race ahead of the load.session/messagesup front so a bail-out or in-flight load can never paint the prior session — a clean loading state shows for the requested id. Reopening the same session keeps the existing transcript visible.untrackin the synchronous load prologue so the load effect doesn't subscribe tosession(which it mutates), avoiding a tight infinite re-trigger loop.🤖 Generated with Claude Code