Skip to content

fix(desktop): honor upward wheel scroll in long threads - #37831

Closed
ferminquant wants to merge 1 commit into
NousResearch:mainfrom
ferminquant:fix/desktop-thread-wheel-scroll
Closed

fix(desktop): honor upward wheel scroll in long threads#37831
ferminquant wants to merge 1 commit into
NousResearch:mainfrom
ferminquant:fix/desktop-thread-wheel-scroll

Conversation

@ferminquant

@ferminquant ferminquant commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fixes Desktop long-thread scrollback snapping back to the bottom after the first upward mouse-wheel scroll.
  • Clears stale bottom-pin bookkeeping when explicit user scroll intent disarms sticky-bottom anchoring.
  • Adds a focused renderer regression test for the exact stale-pending-programmatic-scroll path.

Root Cause

pinToBottom() increments programmaticScrollPendingRef before writing el.scrollTop so the resulting scroll event is treated as programmatic, not user-driven. If the browser is already clamped at the bottom, the expected programmatic scroll event may not arrive, leaving the pending count stale.

The next real upward wheel scroll could then be consumed as if it were programmatic, re-arm sticky-bottom behavior, and let the next resize/content-growth pin pull the viewport back down.

Test Plan

  • RED / negative control on current main with only the regression test added:
    • npm run test:ui -- src/components/assistant-ui/streaming.test.tsx -t "honors the first upward wheel scroll"
    • Failed as expected: AssertionError: expected 1200 to be 420
  • Patched focused regression:
    • npm run test:ui -- src/components/assistant-ui/streaming.test.tsx -t "honors the first upward wheel scroll"
    • Passed: 1 test passed, 9 skipped.
  • Patched focused file:
    • npm run test:ui -- src/components/assistant-ui/streaming.test.tsx
    • Passed: 10 tests passed.
  • TypeScript:
    • npm run type-check
    • Passed.
  • Lint on changed files:
    • npx eslint src/components/assistant-ui/thread-virtualizer.tsx src/components/assistant-ui/streaming.test.tsx
    • 0 errors; 6 existing warnings in thread-virtualizer.tsx.
  • Whitespace:
    • git diff --check
    • Passed.
  • Full Desktop UI test command, scoped honestly:
    • npm run test:ui
    • Still fails with the same pre-existing baseline failures observed before this patch: four Electron .test.cjs files reported by Vitest as no-suite files, plus src/components/pane-shell/pane-shell.test.tsx / uses widthOverride from the store when set.
    • Baseline before this patch: 5 failed files, 39 passed, 1 failed test, 228 passed.
    • Patched branch: 5 failed files, 39 passed, 1 failed test, 229 passed. The extra passing test is this PR's new regression.
  • Independent diff review:
    • Passed: no security concerns or logic errors reported.

Manual / Real Behavior Proof

  • Real source-level proof: the regression test drives the Desktop assistant-ui scroll anchor path with the same data shape as the bug: a pending programmatic bottom-pin event exists, the user wheels upward, scroll position moves to 420, content grows, and the viewport must remain at 420 instead of being pinned to 1200.
  • Real negative-control proof: the same focused regression failed on current main before the production fix was applied.
  • Real headless Electron/CDP proof performed after PR creation:
    • Started the Desktop renderer with Vite no-HMR mode and launched Electron against it with --remote-debugging-port=9222 --ozone-platform=headless --disable-gpu --no-sandbox using temporary HERMES_HOME and HERMES_DESKTOP_USER_DATA_DIR.
    • CDP verified the real Electron renderer mounted the Hermes Desktop UI: URL http://127.0.0.1:5174/#/, title Hermes, composer present, thread viewport present, and window.__PERF_DRIVE__ available.
    • CDP then drove a tall live thread through the real React/assistant-ui message store, pinned the actual thread viewport to bottom, started a second synthetic stream to trigger programmatic bottom pins/content growth, dispatched an upward Input.dispatchMouseEvent wheel event over the viewport, and sampled real DOM scrollTop/scrollHeight/clientHeight events.
    • Result: before wheel scrollTop=1735, scrollHeight=2463, clientHeight=728, distFromBottom=0; after the upward wheel event scrollTop=1354, distFromBottom=478; after continued stream growth final scrollTop=1354, scrollHeight=3613, distFromBottom=1531. The viewport moved up on the first wheel and was not pinned back to bottom while content continued growing.
    • Local evidence artifact captured at /tmp/desktop-wheel-proof.json; summary output reported passed: true.
  • Not claimed: no hand-driven/manual Desktop GUI session or full end-to-end production app proof was performed for this PR. The added real-app proof is headless Electron via CDP with synthetic message-store stream data.

Security / Privacy Impact

  • No credentials, tokens, chat IDs, message content, database files, local session handles, or screenshots are added.
  • Added-line security-pattern scan found no matches.
  • The code change only clears local scroll-anchor bookkeeping in the Desktop renderer.

Risks

  • Low implementation risk: the production change is one line in the existing disarm path.
  • The main behavioral risk is accidental loss of sticky-bottom behavior during genuine programmatic pins; focused tests still cover existing “stay at user scroll position after scroll-up” behavior and the new stale-pending case.
  • Full npm run test:ui is not clean on current main, so broad-suite proof is limited to baseline comparison rather than a green full suite.

Scope Boundaries

Linked Issue

Fixes #37527
Related to #37549

Commits

  • Base commit: b28dd3417
  • Patched commit: 154ea3b03

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have javascript labels Jun 3, 2026
@ferminquant
ferminquant marked this pull request as draft June 3, 2026 03:23
@ferminquant
ferminquant marked this pull request as ready for review June 3, 2026 03:32
@ferminquant
ferminquant marked this pull request as draft June 3, 2026 03:37
@ferminquant
ferminquant marked this pull request as ready for review June 3, 2026 03:40
@ferminquant

Copy link
Copy Markdown
Contributor Author

Fix already merged via #38224 (commit c930a49 on upstream/main). Closing as superseded.

@ferminquant ferminquant closed this Jun 3, 2026
@ferminquant
ferminquant deleted the fix/desktop-thread-wheel-scroll branch June 3, 2026 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Flickering on the Hermes Desktop app for chat transcripts [Bug]: Desktop chat mouse-wheel scroll-up snaps back down in long threads

2 participants