Skip to content

fix(desktop): flush transcript synchronously when switching onto a busy session - #72151

Open
adurham wants to merge 1 commit into
NousResearch:mainfrom
adurham:upstream-pr/desktop-raf-throttle-blank-transcript
Open

fix(desktop): flush transcript synchronously when switching onto a busy session#72151
adurham wants to merge 1 commit into
NousResearch:mainfrom
adurham:upstream-pr/desktop-raf-throttle-blank-transcript

Conversation

@adurham

@adurham adurham commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a blank-transcript bug: clicking back into a still-running (busy)
session shows nothing until the turn finishes, even though the backend
keeps streaming.

Root Cause

useSessionStateCache's syncSessionStateToView() RAF-batches the first
paint after switching onto a still-running session the same as a routine
busy heartbeat. If that RAF tick lands on a throttled frame (window
backgrounded/occluded — Chromium clamps requestAnimationFrame in that
state), the transcript stays permanently blank because nothing else forces
a flush until the turn finishes.

Changes Made

apps/desktop/src/app/session/hooks/use-session-state-cache.ts: force a
synchronous flush whenever the session being synced differs from
viewSessionIdRef (the session currently painted into $messages) — not
just on busy -> idle/needsInput transitions. A repeat update to a
session already on screen still gets RAF coalescing to avoid scroll jank.

Related Issue

No existing issue found (searched gh search issues for "session switch
blank transcript", "clicked back into running session blank",
"backgrounded window transcript stall", "RAF throttle desktop transcript" —
no hits).

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

How to Test

  1. Start a long-running turn in session A.
  2. Switch to session B, then switch back to session A while it's still
    busy, with the window backgrounded/occluded.
  3. Before this fix: the transcript can stay blank until the turn ends.
  4. After this fix: the cached transcript appears immediately on switch.
  5. vitest run src/app/session/hooks/use-session-state-cache.test.tsx
    14 passed (12 pre-existing + 2 new).
  6. Full src/app/session suite: 320 passed, 0 failed.

Added 2 regression tests: one stubs requestAnimationFrame to never
invoke its callback (simulating a throttled/backgrounded window) and
confirms a session switch onto a busy session still flushes synchronously;
the other confirms a repeat heartbeat to a session already on screen still
gets RAF-coalesced (unaffected). The first test fails against the pre-fix
code (verified via a scripted revert).

Checklist

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs/issues (none found)
  • My PR contains only changes related to this fix
  • I've run the relevant test suite and all tests pass
  • I've added tests for my changes
  • Tested on macOS

…sy session

syncSessionStateToView() RAF-batched the first paint after switching onto a
still-running session the same as a routine busy heartbeat. If that RAF
tick landed on a throttled frame (window backgrounded/occluded), the
transcript stayed permanently blank even though the backend kept streaming
-- clicking back into a running session showed nothing until the turn
finished.

Force a synchronous flush whenever the session being synced differs from
viewSessionIdRef (the session currently painted into $messages), not just
on busy->idle/needsInput transitions. Repeat updates to a session already
on screen still get RAF coalescing to avoid scroll jank.

Added 2 regression tests: one confirms the first paint after a session
switch flushes
synchronously even when requestAnimationFrame never fires (simulating a
throttled/backgrounded window), and one confirms a repeat heartbeat to a
session already on screen still gets RAF coalescing. The first test fails
against the pre-fix code (confirmed via a scripted revert).

Full tests/app/session suite: 320 passed, 0 failed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 26, 2026
adurham added a commit to adurham/hermes-agent that referenced this pull request Jul 26, 2026
…tems

Documents outcomes for the 6 desktop-category Bucket A items:

Submitted (4):
- PR NousResearch#72151 -- RAF-throttle blank transcript on session switch
- PR NousResearch#72152 -- profile deletion zombie backend + cross-window rail staleness
- PR NousResearch#72153 -- Nerd Font terminal fallback
- PR NousResearch#72155 -- desktop model picker hiding Anthropic

Deprioritized, needs hand-reconciliation (3): drag-to-reorder, workspace
tab close button, queued composer wrong-session delivery. All three
rejected 1+ files on git apply --check with real semantic drift (not
line-offset noise) against the fast-churning desktop session/composer
code. Flagged for revisit rather than force-applying a stale patch.

The profile-deletion and model-picker items both got real scrutiny beyond
"does it apply cleanly": profile-deletion required two rounds of external
consult after search-first turned up 2 already-merged PRs on the same
headline symptom -- verified directly (not assumed) that this fix's two
pieces are genuinely non-overlapping gaps, not stale duplicates. Model
picker's fix was confirmed to extend an already-established upstream
credential-detection pattern rather than introduce new CC-mimicry
plumbing, addressing the specific caution flagged in the original audit.

Saved verified patches to .upstream-candidates/ for reference.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
adurham added a commit to adurham/hermes-agent that referenced this pull request Jul 26, 2026
Ran all 7 upstream PRs filed this session back through external review
with the real diffs. 2 came back with genuine, actionable findings, both
fixed and pushed as follow-up commits to the existing PRs:

- NousResearch#72054 (MCP orphan reap): silent exception swallow in the cleanup path
  now logs; added a 4th test exercising the real shutdown()/park
  machinery end-to-end rather than only faked versions.
- NousResearch#72152 (profile deletion): tightened a false-positive-prone script-name
  match to the actual known console-script entry points.

2 more findings were checked against the real code and resolved as
non-issues (not accepted at face value, not dismissed either) -- one on
NousResearch#72087 (content/blocks divergence risk -- verified architecturally safe
since blocks are what's actually replayed regardless of content's state;
image-strip gap -- verified blocks can never contain images given how
they're populated) and one on NousResearch#72151 (ref-update ordering -- verified
correct by reading the real flush function).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression fix. Current main still RAF-defers a busy state after a session switch: apps/desktop/src/app/session/hooks/use-session-state-cache.ts:238-264 only flushes idle/needs-input states synchronously, while the cached resume path calls syncSessionStateToView after resetting the view at apps/desktop/src/app/session/hooks/use-session-actions/index.ts:558,687-690.

The isSessionSwitch condition in 4082ede36694 makes that first cached busy paint synchronous while retaining RAF coalescing after viewSessionIdRef has been established. The two added regression tests cover both sides of that distinction. Main changes since the PR base do not alter this target branch, so this is mechanically salvageable.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/sessions Session lifecycle, resume, persistence, history labels Jul 30, 2026
adurham added a commit to adurham/hermes-agent that referenced this pull request Jul 31, 2026
…s on all 7 PRs

Documents the real external engagement on the 7 upstream PRs filed
2026-07-26, and the 3 substantive follow-up fixes pushed in response:

- NousResearch#72054 closed as superseded, but merged anyway via NousResearch#74139 (contributor
  CrowLoki's reconciliation with NousResearch#62026, credited via Co-authored-by).
- NousResearch#72087, NousResearch#72151, NousResearch#72152, NousResearch#72153, NousResearch#72155, NousResearch#72164 all reviewed by the
  repo's automated sweeper -- keep_open/high on all 6.
- Fixed NousResearch#72087 (payload-proportional test assertions, catching a future
  allowlist-regression risk flagged by both the sweeper and an
  independent contributor who measured it precisely on their own fork).
- Fixed NousResearch#72152 (extracted ProfileRail's focus/visibilitychange wiring
  into a tested hook, matching the directory's own established
  use-profile-prewarm.ts pattern).
- Rebased NousResearch#72155 past a real merge conflict (an unrelated upstream
  test-pruning pass removed 3 tests my diff's context touched).

All fixes verified by simulating the exact regression each review was
warning about and confirming the new tests catch it, then restoring the
real fix.

Also noted a real environment issue found this session: the `upstream`
remote's SSH URL intermittently fails to connect from this network; a
one-off HTTPS fetch into a separate ref works around it without touching
the configured remote.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants