refactor(chat): rename useTranscriptScroll → useDeprecatedTranscriptScroll, add dev flag to disable it by vellum-apollo-bot[bot] · Pull Request #32030 · vellum-ai/vellum-assistant

vellum-apollo-bot · 2026-05-25T20:14:45Z

Why

Signals intent to delete the existing transcript scroll-coordination logic without introducing a replacement in this PR. Sets up a runtime flag (window._vellumDebug.flags.toggleTranscriptScrollController()) that turns the deprecated hook off. Flipping it on leaves the transcript with zero JavaScript scroll coordination — that's the baseline against which the eventual TranscriptScrollController will be built and landed (behind this same flag) in a follow-up PR.

The flag name reflects the eventual destination, not the current intermediate state.

What's in this PR

Renames (the deprecation signal)

use-transcript-scroll.ts → use-deprecated-transcript-scroll.ts
useTranscriptScroll → useDeprecatedTranscriptScroll
UseTranscriptScrollArgs → UseDeprecatedTranscriptScrollArgs
UseTranscriptScrollReturn → UseDeprecatedTranscriptScrollReturn
Corresponding .test.ts file renamed.

Behavior changes inside the renamed hook

The hook now owns its own scroll-event listener via useEffect. The orchestrator no longer wires the scroll listener externally.
handleScroll is no longer part of the return surface — it's an internal-only callback.
A module-load constant TRANSCRIPT_SCROLL_CONTROLLER_ENABLED (read once from localStorage) gates the entire hook body. When the flag is on, the hook early-returns { isPinnedToLatest: true, showScrollToLatest: false, scrollToLatest: () => {} } and runs no effects, no state, no listeners. Because the constant is stable across the page lifetime (toggling reloads), the early return is taken consistently every render → React hook-order rules are satisfied.

New files

transcript-scroll-flag.ts — localStorage flag, default off, persists across reloads, toggling reloads the page.

Changes to the orchestrator

chat-route-content.tsx imports useDeprecatedTranscriptScroll directly (no dispatcher abstraction).
The scroll-listener useEffect moved into the hook.
No mode narrowing — when the flag is on the hook simply returns defaults that produce a no-op pill and a no-op scrollToLatest.

Debug API

debug-api.ts + tests — adds toggleTranscriptScrollController(flag?: boolean) to window._vellumDebug.flags.

How to test

// In DevTools console, with chat open:
window._vellumDebug.flags.toggleTranscriptScrollController() // → true, reloads page
// Transcript now has zero JS scroll coordination. Use chat normally.
window._vellumDebug.flags.toggleTranscriptScrollController(false) // → back to deprecated hook, reloads

Test coverage

All 77 existing use-deprecated-transcript-scroll.test.ts cases still pass.
All 4 existing debug-api.test.ts cases still pass (extended for the new flag toggler).

Follow-ups (not in this PR)

Introduce TranscriptScrollController and a new hook that drives it. Land behind the same flag — flipping it on routes to the new path. The deprecated hook stays as-is on the other side of the toggle until parity.
Once the new path reaches parity, flip the default and delete use-deprecated-transcript-scroll.ts outright.

dvargasfuertes · 2026-05-25T21:43:01Z

+  // Imperative scroll controller — owns ResizeObservers, user-input
+  // listeners, the auto-pin window, and the saved anchor snapshot. The
+  // hook keeps a stable reference for the controller's lifetime.
+  const [controller] = useState(() => new TranscriptScrollController());


This all feels too risky to do at once. Let's do this:

create this as a parallel implementation path instead of changing useTranscriptScroll

add a window._vellumDebug.flags.toggleTranscriptScrollController(flag?: boolean)

we can develop old vs new so we can just nuke use-transcript-scroll.ts when we're done

Reshaped per your direction — pushed force-with-lease on the same branch.

use-transcript-scroll.ts is now bit-for-bit identical to origin/main. Zero diff against legacy behavior.

New parallel hook lives at use-transcript-scroll-controller.ts.

window._vellumDebug.flags.toggleTranscriptScrollController(flag?: boolean) is wired up — persists to localStorage, reloads page, default off.

useTranscriptScrollDispatch (new file) resolves to one of the two hooks at module-load time so React sees a stable hook identity across renders.

PR 2 will evolve the new file to drive the controller via imperative methods and delete its kitchen-sink items effect. PR 3 splits chain-load into a pagination-owned hook. When the new path reaches parity, we flip the flag default and nuke use-transcript-scroll.ts in a final cleanup PR.

Updated PR description + title above.

dvargasfuertes · 2026-05-26T11:03:12Z

+// PR 1 (this PR) ships the parallel implementation at near-feature
+// parity with the legacy hook so the toggle is a fair comparison: the
+// imperative DOM bookkeeping lives in the controller, but the
+// kitchen-sink items `useLayoutEffect` is preserved (now talking to
+// controller methods instead of local refs) so the path behaves
+// identically to `use-transcript-scroll.ts` for every existing
+// scenario.


Delete all comments referencing PR numbers

Done. Stripped PR-number references from all five new files (transcript-scroll-controller.ts, use-transcript-scroll-controller.ts, use-transcript-scroll-dispatch.ts, transcript-scroll-flag.ts, and the test file). Pre-existing ATL-* references in debug-api.ts left alone — those are not from this PR.

dvargasfuertes · 2026-05-26T11:03:43Z

+// PR plan (3 PRs total — see scratch/scroll-imperative-spec.md):
+//   • PR 1 (this PR): extract the controller, keep the existing hook
+//     surface and behavior. Net: zero behavior change.
+//   • PR 2: flip orchestrator call sites to imperative method calls
+//     around the data events (`onConversationSwitched`,
+//     `onMessageSubmitted`, `beforeOlderPageFetch`,
+//     `afterOlderPageApplied`). Delete the items `useLayoutEffect`.
+//   • PR 3: split the underfilled-viewport chain-load into a hook
+//     owned by the pagination layer.


Same as this remove

Done — see reply above for the file list.

dvargasfuertes · 2026-05-26T11:22:59Z

I would expect all three of these to be disabled if we toggled our scroll controller. And the new scroll controller should not need to necessarily align with the api given by our hook

Reshaped the new path to be event-driven instead of API-mirrored. The new hook no longer matches the legacy { handleScroll, scrollToLatest({behavior}) } surface — it returns { mode: "controller", isPinnedToLatest, showScrollToLatest, onMessageSubmitted, onScrollToLatestClicked } instead.

All three orchestrator wirings in this file now narrow on scrollCoordinator.mode and short-circuit in controller mode:

Scroll listener useEffect (above) — early-returns when mode !== "legacy". The controller attaches its own scroll listener inside attachScrollElement so we don't double-attach.

handleScrollToLatest callback — routes to scrollCoordinator.onScrollToLatestClicked() on controller mode, falls back to the legacy scrollToLatest({behavior:"smooth"}) on legacy mode.

handleSubmit scroll call (line ~932) — routes to scrollCoordinator.onMessageSubmitted() on controller mode, falls back to the legacy scrollToLatest({behavior:"auto"}) on legacy mode.

The controller now owns the scroll-event listener, the classified state (isPinnedToLatest / showScrollToLatest exposed via subscribe + snapshot for useSyncExternalStore), and the imperative scroll-to-latest. The orchestrator in controller mode just instantiates the hook and informs it of user events; the controller decides what to do.

Dispatcher return is now a discriminated union (UseTranscriptScrollLegacyReturn | UseTranscriptScrollControllerReturn) so the narrowing is type-safe.

vellum-apollo-bot · 2026-05-26T15:28:18Z

Reshaped per the latest direction from FE.

This PR no longer introduces the controller. It only:

Renames useTranscriptScroll → useDeprecatedTranscriptScroll (file + function + types + test file). Signals the deletion intent without changing behavior — git diff --stat shows 99% similarity vs origin/main.
Adds window._vellumDebug.flags.toggleTranscriptScrollController(flag?: boolean). Enabling does nothing except turn off the old way. The dispatcher returns { mode: "disabled", isPinnedToLatest: true, showScrollToLatest: false } and the orchestrator's three legacy wirings short-circuit. Transcript scrolls natively with zero JS coordination.
Leaves the TranscriptScrollController work entirely for follow-up PRs (it'll land behind this same flag).

Pushed force-with-lease at 2c75640310. Down from +1521 lines on round 2 to +234 / −26. All 77 existing tests still pass.

The three earlier review comments are now stale (the files they pointed to no longer exist), so I won't try to resolve them in-thread. Happy to walk through the new shape on review.

dvargasfuertes · 2026-05-26T16:49:44Z

  // -------------------------------------------------------------------------

-  const scrollCoordinator = useTranscriptScroll({
+  const scrollCoordinator = useTranscriptScrollDispatch({


Don't make this useTranscriptScrollDispatch, just import useDeprecatedTranscriptScroll directly

Done — orchestrator now imports useDeprecatedTranscriptScroll directly. d1a752f

dvargasfuertes · 2026-05-26T16:49:55Z

@@ -846,6 +846,10 @@ export function ChatRouteContent({
  });

  useEffect(() => {


move this useEffect into useDeprecatedTranscriptScroll

Done — useEffect moved into useDeprecatedTranscriptScroll. The orchestrator no longer wires the scroll listener. d1a752f

dvargasfuertes · 2026-05-26T16:51:02Z

+function useTranscriptScrollDisabled(
+  _args: TranscriptScrollDispatchArgs,
+): TranscriptScrollDisabledReturn {
+  return DISABLED_RESULT;
+}


delete this - we don't need any logic at all for TRANSCRIPT_SCROLL_CONTROLLER_ENABLED

Done — deleted use-transcript-scroll-dispatch.ts entirely. d1a752f

dvargasfuertes · 2026-05-26T16:52:34Z

+  | TranscriptScrollDisabledReturn;
+
+const DISABLED_RESULT: TranscriptScrollDisabledReturn = {
+  mode: "disabled",


the other mode is going to be "controller", not "deprecated", but we should not introduce that other mode in this PR at all. for now our toggle will just on and off the deprecated scroll

Done — no dispatcher / no controller mode in this PR. The flag (when ON) just early-returns no-op defaults from useDeprecatedTranscriptScroll itself. Module-load constant so the early return is consistent across renders → React hook rules satisfied. d1a752f

…croll, add dev flag to disable it Signals intent to delete the existing transcript scroll-coordination logic without introducing a replacement in this PR. Sets up a runtime flag (`window._vellumDebug.flags.toggleTranscriptScrollController()`) that turns the deprecated hook OFF — flipping it on leaves the transcript with no JavaScript scroll coordination at all. This is the baseline against which the eventual controller will be built and landed (behind this same flag) in a follow-up PR. Renames: - `use-transcript-scroll.ts` → `use-deprecated-transcript-scroll.ts` - `useTranscriptScroll` → `useDeprecatedTranscriptScroll` - `UseTranscriptScrollArgs` → `UseDeprecatedTranscriptScrollArgs` - `UseTranscriptScrollReturn` → `UseDeprecatedTranscriptScrollReturn` - Corresponding `.test.ts` file renamed. New files: - `transcript-scroll-flag.ts` — localStorage flag (default off, persists across reloads, toggling reloads the page so React hook ordering stays stable). - `use-transcript-scroll-dispatch.ts` — module-load picker that returns a discriminated union: `{ mode: "deprecated", ...full deprecated-hook surface }` when the flag is off; `{ mode: "disabled", isPinnedToLatest: true, showScrollToLatest: false }` when on. Changes to existing files: - `chat-route-content.tsx` — switched from `useTranscriptScroll` to `useTranscriptScrollDispatch`. Three legacy wirings (scroll listener `useEffect`, scroll-to-latest button callback, submit-time scroll call) now narrow on `scrollCoordinator.mode === "deprecated"` and short-circuit in disabled mode. - `debug-api.ts` + tests — adds `toggleTranscriptScrollController()` to `_vellumDebug.flags`. Import path updated for the rename. Invariants: - Deprecated-hook file is byte-for-byte identical to origin/main modulo the four rename substitutions above (verified via `git diff --stat` showing 99% similarity). - Flag default off → production behavior unchanged. Test coverage: - All 77 existing `use-deprecated-transcript-scroll.test.ts` cases still pass. - All 4 existing `debug-api.test.ts` cases still pass (extended for the new flag toggler).

dvargasfuertes reviewed May 25, 2026

View reviewed changes

vellum-apollo-bot Bot force-pushed the apollo/transcript-scroll-controller-extract branch from 5088da4 to 415842c Compare May 25, 2026 21:53

vellum-apollo-bot Bot changed the title ~~refactor(web/transcript): extract TranscriptScrollController (PR 1 of 3)~~ refactor(web/transcript): ship parallel TranscriptScrollController behind a dev flag (PR 1 of 3) May 25, 2026

dvargasfuertes requested changes May 26, 2026

View reviewed changes

vellum-apollo-bot Bot force-pushed the apollo/transcript-scroll-controller-extract branch 2 times, most recently from e5052db to 2c75640 Compare May 26, 2026 15:27

vellum-apollo-bot Bot changed the title ~~refactor(web/transcript): ship parallel TranscriptScrollController behind a dev flag (PR 1 of 3)~~ refactor(chat): rename useTranscriptScroll → useDeprecatedTranscriptScroll, add dev flag to disable it May 26, 2026

dvargasfuertes requested changes May 26, 2026

View reviewed changes

vellum-apollo-bot Bot force-pushed the apollo/transcript-scroll-controller-extract branch from 2c75640 to d1a752f Compare May 26, 2026 17:02

dvargasfuertes previously approved these changes May 26, 2026

View reviewed changes

vellum-apollo-bot Bot dismissed dvargasfuertes’s stale review via 69305a8 May 26, 2026 18:07

vellum-apollo-bot Bot force-pushed the apollo/transcript-scroll-controller-extract branch from d1a752f to 69305a8 Compare May 26, 2026 18:07

dvargasfuertes previously approved these changes May 26, 2026

View reviewed changes

vellum-apollo-bot Bot dismissed dvargasfuertes’s stale review via dbb6084 May 26, 2026 18:18

vellum-apollo-bot Bot force-pushed the apollo/transcript-scroll-controller-extract branch from 69305a8 to dbb6084 Compare May 26, 2026 18:18

dvargasfuertes approved these changes May 26, 2026

View reviewed changes

dvargasfuertes merged commit 45b2f5d into main May 26, 2026
7 checks passed

dvargasfuertes deleted the apollo/transcript-scroll-controller-extract branch May 26, 2026 18:39

vellum-apollo-bot Bot mentioned this pull request May 26, 2026

refactor(chat): drop isPinnedToLatest from useDeprecatedTranscriptScroll return #32139

Merged

		@@ -846,6 +846,10 @@ export function ChatRouteContent({
		});

		useEffect(() => {

Conversation

vellum-apollo-bot Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What's in this PR

Renames (the deprecation signal)

Behavior changes inside the renamed hook

New files

Changes to the orchestrator

Debug API

How to test

Test coverage

Follow-ups (not in this PR)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vellum-apollo-bot Bot May 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vellum-apollo-bot Bot May 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vellum-apollo-bot Bot May 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vellum-apollo-bot Bot May 26, 2026

Choose a reason for hiding this comment

Uh oh!

vellum-apollo-bot Bot commented May 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vellum-apollo-bot Bot May 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vellum-apollo-bot Bot May 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vellum-apollo-bot Bot May 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vellum-apollo-bot Bot May 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vellum-apollo-bot Bot commented May 25, 2026 •

edited

Loading