canary

GGitHub♻️ refactor(conversation): unify scroll-to-user + spacer hooks (#14132)

b55cf6b9创建于 29 天前历史提交

文件	最后提交记录	最后更新时间
community	♻️ test: rename discover to community in e2e (#11274) rename discover to community	4 个月前
llm	♻️ refactor(conversation): unify scroll-to-user + spacer hooks (#14132) * ♻️ refactor(conversation): unify spacer + scroll-to-user hooks Merge `useConversationSpacer` and `useScrollToUserMessage` into a single `useConversationScroll` hook to eliminate the races that caused occasional "send message but viewport doesn't pin to the new user message" regressions. Race fixes: - Single `prevLengthRef` and a single send-detection effect, replacing two hooks with independent length tracking that could disagree across renders. - `virtuaRef` is passed in and dereferenced at call time instead of reading `virtuaRef.current?.scrollToIndex` during render — removes the window where the ref hadn't been attached yet when a send fired. - Pin state is an explicit `{ index, seenActive }` ref with three clear transitions (send / layout-bump / user-scroll-up) instead of several cooperating refs + derived flags. - Retries are layout-driven: each `spacerLayoutVersion` bump re-fires `scrollToIndex` exactly once. The old 0/32/96ms timer fan-out is gone. Also bumps `AT_BOTTOM_THRESHOLD` 100 → 300 so `atBottom` stays stable while the spacer is settling. * ♻️ refactor(conversation): extract sub-hooks from useConversationScroll Split the unified conversation scroll hook into four cooperating sub-hooks in the same file so each layer has one clear concern: - useSpacerLayoutSignal — ResizeObserver on the spacer node → version bumps - useSpacerHeight — natural height / mount lifecycle / shrink state - usePinController — pin state machine + virtua-aware scroll dispatch - useScrollShrink — scrollOffset delta → cancel pin / shrink spacer The main hook now owns just the send-detection effect, the pin re-fire on layout settle, and derived output. Behavior is unchanged — same 15 tests pass — but each piece is now readable in isolation. * ⚡️ perf(conversation): narrow VirtualizedList subscription to a boolean VirtualizedList only needs to know whether the second-to-last message is the user's — the full displayMessages array was never used. Move the derivation into `dataSelectors.isSecondLastMessageFromUser` so the component re-renders on role transitions, not on every assistant token. * ✅ test(e2e): cover conversation scroll behavior across the auto-scroll setting Adds three scenarios under `@AGENT-SCROLL-*` that exercise the merged `useConversationScroll` hook end-to-end through the real chat UI: - AGENT-SCROLL-001 — with auto-scroll ON, the viewport ends up near the bottom once a long response has finished streaming. - AGENT-SCROLL-002 — with auto-scroll OFF, the user's message stays pinned to the top and the viewport does not chase the assistant. - AGENT-SCROLL-003 — with auto-scroll ON, scrolling up mid-stream cancels the pin and the viewport is not yanked back to the bottom afterwards. Also extends the LLM mock with `setConfig` / `resetConfig` so scenario 3 can slow the response down enough for the mid-stream manual scroll, and adds `presetResponses.longScrollArticle` (long enough to overflow the viewport so scroll assertions are meaningful). * ✅ test(e2e): cover send-time pin-to-top as its own scenario AGENT-SCROLL-004 exercises the core pin behavior of `useConversationScroll` independent of the auto-scroll setting: after sending a message, the user's turn must be anchored to the top of the scrollport. Uses the slow-response mock so the assertion runs while the spacer is still mounted. * ✅ test(e2e): tune scroll scenarios after runtime validation Run outcomes against a cold Next dev server (paradedb + next dev -p 3006): - AGENT-SCROLL-001 (enabled → viewport stays near bottom) — passing - AGENT-SCROLL-002 (disabled → user msg pinned to top) — passing - AGENT-SCROLL-004 (send pins user msg to top) — passing - AGENT-SCROLL-003 (mid-stream scroll-up cancels pin) — skipped Scenario 3 is marked `@skip` until the LLM mock supports truly chunked SSE streaming. The current mock fulfils the whole body at once, which collapses the "mid-stream" window to a handful of ms and makes the manual-scroll timing race-prone. The cancel-pin path is already covered at the unit level in `useConversationScroll.test.ts`, so the e2e placeholder just keeps the scenario on the radar. Other tweaks for dev-mode reliability: - Bumped setting-toggle step timeout to 90 s (turbopack cold compile of `/settings/chat-appearance` can exceed the default 30 s on first hit) - Relaxed the inner `networkidle` / `toBeVisible` waits there to match - Added a matching negative-path Then ("not pinned") that would power the skipped scenario once the mock is upgraded * 🐛 fix(conversation): rebind pin tracking on every new turn The message index refs that drive `latestAssistantSignature` and the messages `ResizeObserver` were plain `useRef`s updated inside the send- detection effect. On the render triggered by spacer state updates right after a send, `[dataSource, displayMessages]` could be unchanged, so the signature memo returned its cached value and the observer effect never rebound to the new turn's user/assistant DOM nodes. Under certain commit orderings this left spacer height tracking the previous turn and let the pin-to-user anchor drift. Turn the indices into state, include `assistantMessageIndex` in the signature memo's deps, and forward the state (not a ref) to `useSpacerHeight`. The observer now reliably rebinds to the fresh nodes on the very next render. Adds a unit regression covering the observer-rebind path and an e2e scenario (`AGENT-SCROLL-005`) that sends two consecutive turns and checks that the second user message still pins to the top.	29 天前
index.ts	♻️ test: rename discover to community in e2e (#11274) rename discover to community	4 个月前