fix(web/chat): scope items-effect deps to actual items-change triggers

[vellum-apollo-bot]

Summary

Fixes a load-older multi-fire bug where scrolling to the top of the transcript triggered older-page fetches two-at-a-time, despite PR #32386's synchronous in-flight lock.

Root Cause

The items-useLayoutEffect in use-transcript-scroll.ts listed isLoadingOlder (plus 5 other mutable values) in its dep array. When the parent's isLoadingOlder mirror useEffect in use-conversation-history.ts commits at urgent priority before the prepend's setMessages (which runs inside startTransition), the items-effect re-fires on the same items reference. In that intermediate commit the body:

1. Releases the in-flight lock (isLoadingOlder true→false transition).
2. Calls decideItemsChangeAction which returns "anchor-correct" because the saved anchor key is still present in the unchanged items — the apply branch consumes savedAnchorRef on a heightDelta = 0 no-op, defeating scroll preservation when the real prepend lands.
3. Re-classifies scroll position, sees the user still near the top + the just-released lock, and fires onLoadOlder() again.

The deeper issue: the items-effect's dep array was over-specified by exhaustive-deps to include values the body reads but whose changes are not semantically "items-change events." The dep list was lying about the effect's purpose, and that mismatch was the bug surface.

Fix (Structural, Not Defensive)

1. Lock release moves to its own useLayoutEffect keyed on [isLoadingOlder], declared before the items-effect. When both change in the same commit (underfilled-viewport chain-load), it runs first and the items-effect sees the released lock. When only isLoadingOlder transitions, only this effect fires — the items-effect doesn't run at all.

2. latestRef sync moves from useEffect (post-paint) to useLayoutEffect (commit phase), declared before the items-effect so the mirrored values are fresh by the time the items-effect reads them.

3. Items-effect deps trimmed to [items, conversationId, transcriptRef, engageAutoPin] — only what semantically determines "this is an items-change event." hasMore, isLoadingOlder, onLoadOlder, isPinnedToLatest, showScrollToLatest move to latestRef reads. As a side benefit, this eliminates the isPinnedToLatest / showScrollToLatest self-trigger loop (state the effect itself sets via guard-compare).

Regression Test

isLoadingOlder transition without items change does not auto-fire onLoadOlder in use-transcript-scroll.burst.test.tsx rerenders the hook with isLoadingOlder=true then false WITHOUT changing items and asserts onLoadOlder is not called. Fails on the old code, passes on the fix.

All 45 scroll-hook tests pass — including the underfilled-viewport chain-load: false→true→false sequence kicks again when still underfilled case which exercises the legitimate path where lock-release and chain-load must coordinate within a single commit.

Lesson Codified

When exhaustive-deps forces you to list a value in a dep array, ask whether the effect's stated purpose actually wants to re-run for changes to that value. If the effect is named "items change handler" but re-runs on isLoadingOlder changes, the dep list is lying about the effect's purpose — and that mismatch produces bugs. The structural answer is usually to split the effect by responsibility or route stable closure reads through a latestRef mirror, not to add if (a === b) return guards.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 65cc1bdfb1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Clear stale anchors when older-page loads finish empty

When an older-page request fails or returns no new items, isLoadingOlder still transitions true→false with the same items reference. This new early return releases the in-flight lock but leaves savedAnchorRef intact; the next unrelated items change (for example a new message/streaming append while the user is still scrolled up) will run decideItemsChangeAction, treat that stale anchor as a prepend anchor, and adjust scrollTop by the later height delta, causing an unexpected viewport jump. The previous path consumed/cleared the anchor on the no-item settle, so the bailout needs to avoid preserving anchors for genuine failed/empty loads while still handling the intermediate transition case.

Useful? React with 👍 / 👎.