fix(desktop): preserve per-session chat scroll position
Rework per hermes-sweeper review (keep_open, salvageability=medium): - Capture moves out of the render body into the session-switch layout-effect cleanup, so only committed switches persist state (no uncommitted render can write localStorage). - Live state is fed by both scroll events and a ResizeObserver on the content element, so distance-from-bottom stays fresh under async relayout that changes height without a scroll event (the staleness gap #70478's own review threads flagged). - State is distance-from-bottom (or sticky-bottom), not raw scrollTop: the render-budget backfill prepends older turns and main anchors by distance-from-bottom, so an absolute offset no longer identifies the same reading location after the height change. - Restore integrates with main's hasGroups/settle/anchor lifecycle: the settle loop re-applies the remembered target, defers on a clamped offset (content still arriving), hands back locked only for sticky-bottom, and leaves mid-read sessions escaped at their offset. - anchorBeforePrepend no longer records 0 mid-load, which would clobber a restored offset once the backfill lands; the settle loop owns the target every frame until settled. - Storage is scoped per profile with the session.ts .profile.<encoded> key pattern (no cross-profile bleed, #67709 pattern) and LRU-capped at 120 sessions per profile. - Regression tests: state classification, target math, profile isolation, LRU eviction, corrupt/invalid payload handling. Related to #45562 (partial; no automatic closure) (cherry picked from commit 1d79bef93f63d7b190b471bf652b5109b4288a8e)
This commit is contained in:
David Metcalfe
Teknium
@@ -22,11 +22,18 @@ import { useI18n } from '@/i18n'
|
||||
import { messagePaintWeight } from '@/lib/render-weight'
|
||||
import { cn } from '@/lib/utils'
|
||||
import {
|
||||
getThreadScrollPosition,
|
||||
onScrollToBottomRequest,
|
||||
onThreadEditClose,
|
||||
onThreadEditOpen,
|
||||
publishThreadAtBottom,
|
||||
resetPublishedThreadScroll
|
||||
resetPublishedThreadScroll,
|
||||
planThreadScrollRestore,
|
||||
saveThreadScrollPosition,
|
||||
THREAD_SCROLL_BOTTOM,
|
||||
type ThreadScrollState,
|
||||
threadScrollStateFromMetrics,
|
||||
threadScrollTargetTop
|
||||
} from '@/store/thread-scroll'
|
||||
import { isSecondaryWindow } from '@/store/windows'
|
||||
|
||||
@@ -496,12 +503,20 @@ const ThreadMessageListInner: FC<ThreadMessageListProps> = ({
|
||||
|
||||
// Record where the view should land once a prepend has grown the content,
|
||||
// measured from the BOTTOM so the added height doesn't invalidate it. Only a
|
||||
// settled load has an offset the user chose; mid-load the answer is simply
|
||||
// the bottom.
|
||||
// settled load has an offset the user chose; while the settle loop is still
|
||||
// running, scrollTop is a way-point of a load in progress (or a restored
|
||||
// offset the loop is applying) — never anchor to it. Recording 0 here would
|
||||
// make the restore effect clobber a restored offset with the bottom once the
|
||||
// backfill lands; the settle loop re-writes its own target every frame, so
|
||||
// skipping is safe.
|
||||
const anchorBeforePrepend = useCallback(() => {
|
||||
const el = scrollRef.current
|
||||
|
||||
restoreFromBottomRef.current = el && loadSettledRef.current ? el.scrollHeight - el.scrollTop : 0
|
||||
if (!el || !loadSettledRef.current) {
|
||||
return
|
||||
}
|
||||
|
||||
restoreFromBottomRef.current = el.scrollHeight - el.scrollTop
|
||||
}, [scrollRef])
|
||||
|
||||
// Backfill from FIRST_PAINT_BUDGET to the full budget after the small
|
||||
@@ -653,12 +668,68 @@ const ThreadMessageListInner: FC<ThreadMessageListProps> = ({
|
||||
}
|
||||
})
|
||||
|
||||
// Reset the cap and pin to bottom on mount + every session switch (messages
|
||||
// swap in place on a long-lived runtime, so sessionKey is the only signal).
|
||||
// Live scroll state of the CURRENT session, updated on every scroll event
|
||||
// AND on content height changes (ResizeObserver). The RO leg is what keeps
|
||||
// the recorded distance-from-bottom honest: async relayout (images,
|
||||
// highlight, the budget backfill) changes scrollHeight WITHOUT a scroll
|
||||
// event, so a scroll-only cache records a stale offset (the gap #70478's
|
||||
// review threads flagged). Both legs write stateFromMetrics(el).
|
||||
const liveScrollStateRef = useRef<ThreadScrollState>(THREAD_SCROLL_BOTTOM)
|
||||
// Key the restore loop has already applied to the current transcript — the
|
||||
// record gate: an instance records only the state it actually showed under
|
||||
// its own key (an empty-transcript instance still holds the PREVIOUS
|
||||
// session's live state and must not file it under the new key).
|
||||
const restoredContentKeyRef = useRef<string | null | undefined>(undefined)
|
||||
|
||||
// eslint-disable-next-line no-restricted-syntax -- DOM-event cache (scroll/ResizeObserver callbacks), not an atom mirror
|
||||
useEffect(() => {
|
||||
const el = scrollRef.current
|
||||
const content = contentRef.current
|
||||
|
||||
if (!el || !content) {
|
||||
return
|
||||
}
|
||||
|
||||
const update = () => {
|
||||
liveScrollStateRef.current = threadScrollStateFromMetrics(el)
|
||||
}
|
||||
|
||||
el.addEventListener('scroll', update, { passive: true })
|
||||
const observer = new ResizeObserver(update)
|
||||
observer.observe(content)
|
||||
|
||||
return () => {
|
||||
el.removeEventListener('scroll', update)
|
||||
observer.disconnect()
|
||||
}
|
||||
}, [contentRef, scrollRef])
|
||||
|
||||
// Persist the live position on app close, so a reading position survives a
|
||||
// quit without a session switch (the switch cleanup below only runs on
|
||||
// committed switches). Guarded by the same restored-content gate AND the
|
||||
// settled gate — a close mid-settle must not persist transient clamped
|
||||
// metrics.
|
||||
useEffect(() => {
|
||||
const flush = () => {
|
||||
if (sessionKey && loadSettledRef.current && restoredContentKeyRef.current === sessionKey) {
|
||||
saveThreadScrollPosition(sessionKey, liveScrollStateRef.current)
|
||||
}
|
||||
}
|
||||
|
||||
window.addEventListener('beforeunload', flush)
|
||||
|
||||
return () => window.removeEventListener('beforeunload', flush)
|
||||
}, [sessionKey])
|
||||
|
||||
// Reset the cap and restore the remembered scroll state on mount + every
|
||||
// session switch (messages swap in place on a long-lived runtime, so
|
||||
// sessionKey is the only signal). Sessions the user left mid-read reapply
|
||||
// their exact distance-from-bottom; sticky-bottom sessions pin to the bottom.
|
||||
// The swap is multi-step and lays out over many frames; letting the library
|
||||
// follow re-pins every frame to a moving target — visible as ~10 scroll jumps.
|
||||
// Instead: quiet it, glue to the true bottom until the height holds steady,
|
||||
// then hand back locked. Live streaming afterward uses the normal resize follow.
|
||||
// follow re-pins every frame to a moving target — visible as ~10 scroll
|
||||
// jumps. Instead: quiet it, glue to the remembered target until the height
|
||||
// holds steady, then hand back (locked at the bottom, escaped at an offset).
|
||||
// Live streaming afterward uses the normal resize follow.
|
||||
//
|
||||
// `hasGroups` joins sessionKey as a dep because a COLD load changes the key
|
||||
// while the transcript is still empty and publishes messages hundreds of ms
|
||||
@@ -668,7 +739,11 @@ const ThreadMessageListInner: FC<ThreadMessageListProps> = ({
|
||||
// down once use-stick-to-bottom's ResizeObserver noticed, a full-viewport
|
||||
// lurch on every cold load. The empty→non-empty flip re-arms for the
|
||||
// transcript that actually arrived; being a boolean, it cannot re-fire on a
|
||||
// streaming append.
|
||||
// streaming append. The restore must re-run at first content too — that is
|
||||
// what `restoredContentKeyRef` gates: one restore per key AFTER its
|
||||
// transcript exists. The effect cleanup is the record point: it runs with
|
||||
// the OLD session's closure, synchronously in the commit that swaps
|
||||
// transcripts.
|
||||
useLayoutEffect(() => {
|
||||
const el = scrollRef.current
|
||||
|
||||
@@ -678,19 +753,57 @@ const ThreadMessageListInner: FC<ThreadMessageListProps> = ({
|
||||
|
||||
const sessionSwitched = settleKeyRef.current !== sessionKey
|
||||
|
||||
if (sessionSwitched) {
|
||||
settledNonEmptyRef.current = false
|
||||
const plan = planThreadScrollRestore(restoredContentKeyRef.current, sessionKey, hasGroups, loadSettledRef.current)
|
||||
|
||||
restoredContentKeyRef.current = plan.gate
|
||||
|
||||
// Record only states that were actually shown under this key: the gate
|
||||
// equals this closure's sessionKey exactly when this instance restored
|
||||
// content (cleanups run before the next instance's effect, so a later
|
||||
// cold-switch instance clearing the ref can't spoof it). An
|
||||
// empty-transcript instance still holds the PREVIOUS session's live state,
|
||||
// which must not be filed under this key. And only SETTLED states: mid-
|
||||
// settle the ref holds transient clamped metrics (the loop writing targets
|
||||
// into a still-arriving transcript), and persisting those would corrupt
|
||||
// the session's real reading position.
|
||||
const record = () => {
|
||||
if (sessionKey && loadSettledRef.current && restoredContentKeyRef.current === sessionKey) {
|
||||
saveThreadScrollPosition(sessionKey, liveScrollStateRef.current)
|
||||
}
|
||||
}
|
||||
|
||||
// Same-session refresh (transcript briefly cleared and repopulated) must
|
||||
// keep the reader's position. Run before stopScroll / scrollTop reset so
|
||||
// a refresh neither yanks the view nor clears the settled flag.
|
||||
if (!shouldRePinOnTranscriptReload({ sessionSwitched, settledNonEmpty: settledNonEmptyRef.current })) {
|
||||
return
|
||||
if (plan.cold) {
|
||||
// Cold switch: transcript not landed yet (or emptied for a reload). The
|
||||
// DOM collapse clamps scrollTop to garbage, so forget the restore gate —
|
||||
// when content (re)arrives, reapply from memory. The previous session's
|
||||
// real state was already recorded by its own cleanup just before this.
|
||||
// An anchor captured for the OUTGOING transcript must not be applied to
|
||||
// this one — a switch owns the position outright. The empty→non-empty
|
||||
// re-arm is the SAME load, whose in-flight anchor is still correct.
|
||||
loadSettledRef.current = false
|
||||
|
||||
if (settleKeyRef.current !== sessionKey) {
|
||||
settleKeyRef.current = sessionKey
|
||||
restoreFromBottomRef.current = null
|
||||
}
|
||||
|
||||
return record
|
||||
}
|
||||
|
||||
if (!plan.restore) {
|
||||
// Same key, already settled: the restore is done, keep recording only.
|
||||
return record
|
||||
}
|
||||
|
||||
const remembered = sessionKey ? getThreadScrollPosition(sessionKey) : undefined
|
||||
const target = remembered ?? THREAD_SCROLL_BOTTOM
|
||||
|
||||
// The previous session's parting state must not leak into this one: from
|
||||
// here every scroll/RO event describes the restored session.
|
||||
liveScrollStateRef.current = target
|
||||
|
||||
stopScroll()
|
||||
el.scrollTop = el.scrollHeight
|
||||
el.scrollTop = threadScrollTargetTop(target, el)
|
||||
loadSettledRef.current = false
|
||||
|
||||
// An anchor captured for the OUTGOING transcript must not be applied to
|
||||
@@ -714,17 +827,36 @@ const ThreadMessageListInner: FC<ThreadMessageListProps> = ({
|
||||
|
||||
const height = node.scrollHeight
|
||||
|
||||
stableFrames = height === lastHeight ? stableFrames + 1 : 0
|
||||
// An offset deeper than the current scroll range means content is still
|
||||
// arriving (the budget backfill prepends older turns) — a quiet frame in
|
||||
// that state is not stability, keep waiting for the height.
|
||||
const clamped = target.kind === 'offset' && target.fromBottom > Math.max(0, height - node.clientHeight)
|
||||
|
||||
stableFrames = height === lastHeight && !clamped ? stableFrames + 1 : 0
|
||||
lastHeight = height
|
||||
node.scrollTop = height
|
||||
node.scrollTop = threadScrollTargetTop(target, node)
|
||||
|
||||
// Most session switches are synchronous and stabilize within 2 frames;
|
||||
// the old 90-frame ceiling was for slow async image loads. Cap at 15
|
||||
// frames to minimize the settle-loop racing markdown paint on every switch.
|
||||
if (stableFrames >= 2 || ++frame > 15) {
|
||||
void scrollToBottom('instant')
|
||||
settledNonEmptyRef.current = hasGroups
|
||||
loadSettledRef.current = true
|
||||
if (target.kind === 'bottom') {
|
||||
// Hand back to use-stick-to-bottom locked, so late async growth
|
||||
// (images, highlight) keeps following the bottom.
|
||||
void scrollToBottom('instant')
|
||||
loadSettledRef.current = true
|
||||
} else if (clamped) {
|
||||
// Content hasn't finished arriving (the backfill transition is still
|
||||
// rendering). Park the offset in the anchor so the restore effect
|
||||
// re-applies it the moment the taller tree lands — otherwise the
|
||||
// view is stranded at the clamped position. Keep loadSettled false:
|
||||
// anchorBeforePrepend skips while unsettled, so the parked offset
|
||||
// can't be overwritten by a mid-load anchor measurement. The restore
|
||||
// effect flips settled once it consumes the parked value.
|
||||
restoreFromBottomRef.current = target.fromBottom
|
||||
} else {
|
||||
loadSettledRef.current = true
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
@@ -734,7 +866,10 @@ const ThreadMessageListInner: FC<ThreadMessageListProps> = ({
|
||||
|
||||
let rafId = requestAnimationFrame(settle)
|
||||
|
||||
return () => cancelAnimationFrame(rafId)
|
||||
return () => {
|
||||
cancelAnimationFrame(rafId)
|
||||
record()
|
||||
}
|
||||
}, [hasGroups, scrollRef, scrollToBottom, sessionKey, stopScroll])
|
||||
|
||||
// Prepend an older page while preserving the on-screen position. The user is
|
||||
@@ -765,6 +900,9 @@ const ThreadMessageListInner: FC<ThreadMessageListProps> = ({
|
||||
if (el && restoreFromBottomRef.current != null) {
|
||||
el.scrollTop = el.scrollHeight - restoreFromBottomRef.current
|
||||
restoreFromBottomRef.current = null
|
||||
// Consuming a parked offset (clamped-exit) means the view just landed at
|
||||
// its real reading position — the load is settled from here on.
|
||||
loadSettledRef.current = true
|
||||
}
|
||||
// renderBudget covers DOM pages; groups.length covers store-window expands.
|
||||
}, [scrollRef, renderBudget, groups.length])
|
||||
|
||||
@@ -90,4 +90,4 @@ describe('requestScrollToBottom', () => {
|
||||
expect(second).toHaveBeenCalledOnce()
|
||||
stopSecond()
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -1,5 +1,8 @@
|
||||
import { atom, type WritableAtom } from 'nanostores'
|
||||
|
||||
import { readKey, writeKey } from '@/lib/storage'
|
||||
import { $activeProfile, normalizeProfileKey } from '@/store/profile'
|
||||
|
||||
// "Is the thread parked at the bottom" is owned by use-stick-to-bottom inside
|
||||
// ThreadMessageList (the scroll container). That state lives only in that
|
||||
// subtree, so ThreadMessageList mirrors it into these atoms for the composer,
|
||||
@@ -94,3 +97,172 @@ export const onThreadEditClose = (handler: () => void) => {
|
||||
}
|
||||
|
||||
export const notifyThreadEditClose = () => editCloseHandlers.forEach(handler => handler())
|
||||
|
||||
// ── Per-session scroll position persistence ──────────────────────────────────
|
||||
// When the user scrolls up to read history, their distance-from-bottom is
|
||||
// saved keyed by sessionKey and profile. On return, the session-switch settle
|
||||
// loop restores it instead of pinning to the bottom, so the reading position
|
||||
// survives session switches. Offsets are stored as distance-from-bottom, not
|
||||
// scrollTop: the render-budget backfill prepends older turns and the switch
|
||||
// relayout reshapes content above the on-screen rows, and bottom-anchored math
|
||||
// keeps the restored view steady under that churn — the same reason the
|
||||
// "Show earlier" flow in list.tsx restores from the bottom edge.
|
||||
export type ThreadScrollState = { kind: 'bottom' } | { fromBottom: number; kind: 'offset' }
|
||||
|
||||
export const THREAD_SCROLL_BOTTOM: ThreadScrollState = { kind: 'bottom' }
|
||||
|
||||
// Within this many pixels of the bottom edge counts as "parked at the bottom".
|
||||
// Deliberately tight: use-stick-to-bottom's own near-bottom band re-locks lazy
|
||||
// scrollers anyway, and recording a small real offset as `bottom` would yank a
|
||||
// reader who stopped just shy of the edge.
|
||||
export const THREAD_SCROLL_STICKY_THRESHOLD_PX = 8
|
||||
|
||||
export type ThreadScrollMetrics = {
|
||||
clientHeight: number
|
||||
scrollHeight: number
|
||||
scrollTop: number
|
||||
}
|
||||
|
||||
export function threadScrollDistanceFromBottom(metrics: ThreadScrollMetrics): number {
|
||||
return Math.max(0, metrics.scrollHeight - metrics.scrollTop - metrics.clientHeight)
|
||||
}
|
||||
|
||||
/** Classify live metrics as sticky-bottom or an exact reading offset. */
|
||||
export function threadScrollStateFromMetrics(
|
||||
metrics: ThreadScrollMetrics,
|
||||
threshold = THREAD_SCROLL_STICKY_THRESHOLD_PX
|
||||
): ThreadScrollState {
|
||||
const fromBottom = threadScrollDistanceFromBottom(metrics)
|
||||
|
||||
return fromBottom <= threshold ? THREAD_SCROLL_BOTTOM : { fromBottom, kind: 'offset' }
|
||||
}
|
||||
|
||||
/** The scrollTop that re-applies `state` at the current content height. */
|
||||
export function threadScrollTargetTop(
|
||||
state: ThreadScrollState,
|
||||
metrics: Pick<ThreadScrollMetrics, 'clientHeight' | 'scrollHeight'>
|
||||
): number {
|
||||
const max = Math.max(0, metrics.scrollHeight - metrics.clientHeight)
|
||||
|
||||
return state.kind === 'bottom' ? max : Math.max(0, max - state.fromBottom)
|
||||
}
|
||||
|
||||
// Storage is scoped per profile with the same `.profile.<encoded>` suffix the
|
||||
// app's other persisted session state uses (session.ts profileNavigationKey),
|
||||
// so two profiles can never read or evict each other's reading positions.
|
||||
const SCROLL_POS_KEY_BASE = 'hermes.desktop.threadScroll.v1'
|
||||
|
||||
export function threadScrollStorageKey(profile: string): string {
|
||||
return `${SCROLL_POS_KEY_BASE}.profile.${encodeURIComponent(normalizeProfileKey(profile))}`
|
||||
}
|
||||
|
||||
// Bounded so a marathon runtime that touches hundreds of sessions doesn't grow
|
||||
// the map forever. JS object insertion order gives LRU eviction — saving
|
||||
// delete-and-re-adds the key, so the front is always the least-recently-used.
|
||||
export const THREAD_SCROLL_MEMORY_LIMIT = 120
|
||||
|
||||
function isValidState(value: unknown): value is ThreadScrollState {
|
||||
if (!value || typeof value !== 'object') {
|
||||
return false
|
||||
}
|
||||
|
||||
const record = value as Record<string, unknown>
|
||||
|
||||
if (record.kind === 'bottom') {
|
||||
return true
|
||||
}
|
||||
|
||||
return record.kind === 'offset' && typeof record.fromBottom === 'number' && Number.isFinite(record.fromBottom)
|
||||
}
|
||||
|
||||
function loadPositions(profile: string): Record<string, ThreadScrollState> {
|
||||
const raw = readKey(threadScrollStorageKey(profile))
|
||||
|
||||
if (!raw) {
|
||||
return {}
|
||||
}
|
||||
|
||||
try {
|
||||
const parsed = JSON.parse(raw) as unknown
|
||||
|
||||
if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
|
||||
return {}
|
||||
}
|
||||
|
||||
return Object.fromEntries(
|
||||
Object.entries(parsed as Record<string, unknown>).filter((entry): entry is [string, ThreadScrollState] =>
|
||||
isValidState(entry[1])
|
||||
)
|
||||
)
|
||||
} catch {
|
||||
return {}
|
||||
}
|
||||
}
|
||||
|
||||
function persistPositions(profile: string, positions: Record<string, ThreadScrollState>) {
|
||||
const keys = Object.keys(positions)
|
||||
|
||||
while (keys.length > THREAD_SCROLL_MEMORY_LIMIT) {
|
||||
delete positions[keys[0]!]
|
||||
keys.shift()
|
||||
}
|
||||
|
||||
writeKey(threadScrollStorageKey(profile), keys.length === 0 ? null : JSON.stringify(positions))
|
||||
}
|
||||
|
||||
export function getThreadScrollPosition(sessionKey: string): ThreadScrollState | undefined {
|
||||
return loadPositions($activeProfile.get())[sessionKey]
|
||||
}
|
||||
|
||||
export function saveThreadScrollPosition(sessionKey: string, state: ThreadScrollState) {
|
||||
const profile = $activeProfile.get()
|
||||
const positions = loadPositions(profile)
|
||||
|
||||
// Delete then re-add to track recency (insertion order = LRU anchor).
|
||||
delete positions[sessionKey]
|
||||
positions[sessionKey] = state
|
||||
persistPositions(profile, positions)
|
||||
}
|
||||
|
||||
export function clearThreadScrollPosition(sessionKey: string) {
|
||||
const profile = $activeProfile.get()
|
||||
const positions = loadPositions(profile)
|
||||
|
||||
if (positions[sessionKey] === undefined) {
|
||||
return
|
||||
}
|
||||
|
||||
delete positions[sessionKey]
|
||||
persistPositions(profile, positions)
|
||||
}
|
||||
|
||||
/**
|
||||
* The restore/record gate for the session-switch settle loop. Pure so the
|
||||
* warm/cold switch lifecycle is testable without a DOM:
|
||||
*
|
||||
* - cold (no transcript yet): forget any in-flight restore, do not record —
|
||||
* an empty-transcript instance holds the PREVIOUS session's live state and
|
||||
* must not file it under the new key.
|
||||
* - same key, already settled: the restore is done; keep recording only.
|
||||
* - same key, still settling: a dep identity change re-ran the effect
|
||||
* mid-loop — re-arm the restore instead of stranding the viewport.
|
||||
* - anything else (first content for this key, or a key change): restore.
|
||||
*/
|
||||
export type ThreadScrollRestorePlan = { cold: boolean; gate: string | null | undefined; restore: boolean }
|
||||
|
||||
export function planThreadScrollRestore(
|
||||
prevGate: string | null | undefined,
|
||||
sessionKey: string | null | undefined,
|
||||
hasGroups: boolean,
|
||||
settled: boolean
|
||||
): ThreadScrollRestorePlan {
|
||||
if (!hasGroups) {
|
||||
return { cold: true, gate: null, restore: false }
|
||||
}
|
||||
|
||||
if (prevGate === sessionKey && settled) {
|
||||
return { cold: false, gate: sessionKey, restore: false }
|
||||
}
|
||||
|
||||
return { cold: false, gate: sessionKey, restore: true }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user