diff --git a/apps/web/src/domains/chat/components/chat-route-content.tsx b/apps/web/src/domains/chat/components/chat-route-content.tsx index e909f07cde1..76cfcb27e05 100644 --- a/apps/web/src/domains/chat/components/chat-route-content.tsx +++ b/apps/web/src/domains/chat/components/chat-route-content.tsx @@ -34,7 +34,7 @@ import { usePullRefresh } from "@/domains/chat/hooks/use-pull-refresh.js"; import { useRefreshLatestMessages as _useRefreshLatestMessages } from "@/domains/chat/hooks/use-refresh-latest-messages.js"; import { useConversationStarters } from "@/domains/chat/hooks/use-conversation-starters.js"; import type { TranscriptHandle, TranscriptProps } from "@/domains/chat/transcript/transcript.js"; -import { useTranscriptScroll } from "@/domains/chat/transcript/use-transcript-scroll.js"; +import { useDeprecatedTranscriptScroll } from "@/domains/chat/transcript/use-deprecated-transcript-scroll.js"; import { hasPendingAssistantResponse } from "@/domains/chat/utils/chat-utils.js"; import type { ChatError } from "@/domains/chat/types.js"; import type { AssistantState } from "@/domains/chat/hooks/use-assistant-lifecycle.js"; @@ -836,7 +836,7 @@ export function ChatRouteContent({ // Scroll coordination // ------------------------------------------------------------------------- - const scrollCoordinator = useTranscriptScroll({ + const scrollCoordinator = useDeprecatedTranscriptScroll({ transcriptRef: refs.transcriptRef, items: transcriptItems, conversationId: activeConversationId, @@ -845,16 +845,6 @@ export function ChatRouteContent({ onLoadOlder: loadOlder, }); - useEffect(() => { - const el = refs.transcriptRef.current?.getScrollElement(); - if (!el) return; - const handler = (e: Event) => scrollCoordinator.handleScroll(e); - el.addEventListener("scroll", handler, { passive: true }); - return () => { - el.removeEventListener("scroll", handler); - }; - }, [scrollCoordinator, activeConversationId, transcriptItems, refs.transcriptRef]); - const handleScrollToLatest = useCallback(() => { scrollCoordinator.scrollToLatest({ behavior: "smooth" }); }, [scrollCoordinator]); diff --git a/apps/web/src/domains/chat/components/scroll-to-latest-button.tsx b/apps/web/src/domains/chat/components/scroll-to-latest-button.tsx index f6dd6323f2c..0659ddf4596 100644 --- a/apps/web/src/domains/chat/components/scroll-to-latest-button.tsx +++ b/apps/web/src/domains/chat/components/scroll-to-latest-button.tsx @@ -4,7 +4,7 @@ import { ChatPill } from "@/domains/chat/components/chat-pill.js"; /** * Pill-shaped "Go to Newest" affordance shown above the composer when the - * user has scrolled far enough up that `useTranscriptScroll` reports + * user has scrolled far enough up that the scroll coordinator reports * `showScrollToLatest`. Clicking pins the transcript back to the latest * message. * diff --git a/apps/web/src/domains/chat/transcript/transcript-scroll-flag.ts b/apps/web/src/domains/chat/transcript/transcript-scroll-flag.ts new file mode 100644 index 00000000000..a0a4242e745 --- /dev/null +++ b/apps/web/src/domains/chat/transcript/transcript-scroll-flag.ts @@ -0,0 +1,80 @@ +// Runtime flag that turns OFF the deprecated transcript scroll hook. +// +// In the current shape of this work-in-progress: +// +// • Flag OFF (default) — the orchestrator runs the deprecated hook +// `useDeprecatedTranscriptScroll`, which is the existing +// production scroll-coordination logic. Production users land +// here. We keep this in the tree so we don't regress shipping +// behavior while we redesign the replacement. +// • Flag ON — the orchestrator runs neither the deprecated hook +// nor a replacement. The transcript scrolls natively with no +// JavaScript coordination at all. No auto-pin, no anchor +// correction, no chain-load, no "Go to Newest" pill. This is the +// baseline against which the eventual controller will be built. +// +// The eventual `TranscriptScrollController` (not yet introduced) will +// land behind this same flag — flipping it on will route to the new +// path. The name `toggleTranscriptScrollController` reflects the +// destination, not the current intermediate state. +// +// Why module-load read + reload-on-toggle: +// +// • React forbids conditionally calling different hooks across +// renders. By making the dispatch decision once at module-import +// time (before any component mounts), the dispatcher resolves to a +// single function identity for the entire page lifetime. +// • Toggling without a reload would leave the DOM in an inconsistent +// intermediate state (scroll listeners attached but no longer +// handled, in-flight auto-pin timers orphaned). A page reload is +// cheap and dev-only. +// +// Surface (exposed under `window._vellumDebug.flags`): +// +// toggleTranscriptScrollController() — flip current value +// toggleTranscriptScrollController(true) — force on +// toggleTranscriptScrollController(false) — force off + +const STORAGE_KEY = "vellumDebug.flags.transcriptScrollController"; + +/** Read the flag synchronously. Safe to call at module-load time. */ +export function getTranscriptScrollControllerEnabled(): boolean { + if (typeof window === "undefined") return false; + try { + return window.localStorage.getItem(STORAGE_KEY) === "true"; + } catch { + // Private-browsing modes or sandboxed contexts can throw on + // localStorage access. Treat any throw as "flag off". + return false; + } +} + +/** Persist the flag, log the new value, and reload the page so the + * dispatcher re-resolves. `value === undefined` flips the current + * value (the most common interactive case). */ +export function setTranscriptScrollControllerEnabled(value?: boolean): boolean { + if (typeof window === "undefined") return false; + const next = + value === undefined ? !getTranscriptScrollControllerEnabled() : !!value; + try { + window.localStorage.setItem(STORAGE_KEY, String(next)); + } catch { + // Persistence failed — log and bail so the user knows their + // toggle didn't stick. + console.warn( + "[vellumDebug] failed to persist transcriptScrollController flag", + ); + return getTranscriptScrollControllerEnabled(); + } + console.info( + `[vellumDebug] transcriptScrollController = ${next} — reloading…`, + ); + window.location.reload(); + return next; +} + +/** The flag value resolved exactly once at module load. The + * dispatcher reads this constant so hook-rule order stays stable + * across the page lifetime. */ +export const TRANSCRIPT_SCROLL_CONTROLLER_ENABLED = + getTranscriptScrollControllerEnabled(); diff --git a/apps/web/src/domains/chat/transcript/use-transcript-scroll.test.ts b/apps/web/src/domains/chat/transcript/use-deprecated-transcript-scroll.test.ts similarity index 99% rename from apps/web/src/domains/chat/transcript/use-transcript-scroll.test.ts rename to apps/web/src/domains/chat/transcript/use-deprecated-transcript-scroll.test.ts index 961a959f5ae..184a6a9ecc6 100644 --- a/apps/web/src/domains/chat/transcript/use-transcript-scroll.test.ts +++ b/apps/web/src/domains/chat/transcript/use-deprecated-transcript-scroll.test.ts @@ -27,7 +27,7 @@ import { PINNED_THRESHOLD_PX, SHOW_SCROLL_BUTTON_THRESHOLD_PX, type TranscriptHandle, -} from "@/domains/chat/transcript/use-transcript-scroll.js"; +} from "@/domains/chat/transcript/use-deprecated-transcript-scroll.js"; // --------------------------------------------------------------------------- // Fixtures diff --git a/apps/web/src/domains/chat/transcript/use-transcript-scroll.ts b/apps/web/src/domains/chat/transcript/use-deprecated-transcript-scroll.ts similarity index 92% rename from apps/web/src/domains/chat/transcript/use-transcript-scroll.ts rename to apps/web/src/domains/chat/transcript/use-deprecated-transcript-scroll.ts index 678624490ac..ebc8c1ead40 100644 --- a/apps/web/src/domains/chat/transcript/use-transcript-scroll.ts +++ b/apps/web/src/domains/chat/transcript/use-deprecated-transcript-scroll.ts @@ -34,6 +34,7 @@ import { import type { TranscriptItem } from "@/domains/chat/transcript/types.js"; import type { TranscriptHandle } from "@/domains/chat/transcript/transcript.js"; +import { TRANSCRIPT_SCROLL_CONTROLLER_ENABLED } from "@/domains/chat/transcript/transcript-scroll-flag.js"; export type { TranscriptHandle }; @@ -59,7 +60,7 @@ export const LOAD_OLDER_THRESHOLD_PX = 200; // Public hook API // --------------------------------------------------------------------------- -export interface UseTranscriptScrollArgs { +export interface UseDeprecatedTranscriptScrollArgs { transcriptRef: RefObject; items: TranscriptItem[]; conversationId: string | null; @@ -68,11 +69,10 @@ export interface UseTranscriptScrollArgs { onLoadOlder: () => void; } -export interface UseTranscriptScrollReturn { +export interface UseDeprecatedTranscriptScrollReturn { isPinnedToLatest: boolean; showScrollToLatest: boolean; scrollToLatest: (opts?: { behavior?: "auto" | "smooth" }) => void; - handleScroll: (event: Event) => void; } // --------------------------------------------------------------------------- @@ -226,9 +226,30 @@ export function decideItemsChangeAction( // Hook implementation // --------------------------------------------------------------------------- -export function useTranscriptScroll( - args: UseTranscriptScrollArgs, -): UseTranscriptScrollReturn { +/** Returned when the dev flag has turned this hook off. The transcript + * then runs with no JavaScript scroll coordination at all — the + * defaults below match "nothing is happening". */ +const DISABLED_RESULT: UseDeprecatedTranscriptScrollReturn = { + isPinnedToLatest: true, + showScrollToLatest: false, + scrollToLatest: () => {}, +}; + +export function useDeprecatedTranscriptScroll( + args: UseDeprecatedTranscriptScrollArgs, +): UseDeprecatedTranscriptScrollReturn { + // `TRANSCRIPT_SCROLL_CONTROLLER_ENABLED` is a module-load constant + // resolved once from localStorage at page load. It does NOT change + // across renders within a page lifetime (toggling the flag reloads + // the page). That means this early return is taken consistently for + // every render of every instance of this hook on a given page — + // either the no-op path runs forever or the full hook runs forever + // — which keeps React's hook-order rules satisfied even though no + // hooks are called on the no-op path. + if (TRANSCRIPT_SCROLL_CONTROLLER_ENABLED) { + return DISABLED_RESULT; + } + const { transcriptRef, items, @@ -645,10 +666,26 @@ export function useTranscriptScroll( [transcriptRef, engageAutoPin], ); + // ----------------------------------------------------------------------- + // Attach the scroll-event listener. The hook owns its own listener + // so the orchestrator does not have to wire one externally. + // + // Re-runs on `items` so a transcript remount (inside ResizablePanel) + // re-binds to the newly mounted scroll element. `handleScroll` is + // stable across renders so it does not contribute to re-binding. + // ----------------------------------------------------------------------- + useEffect(() => { + const el = transcriptRef.current?.getScrollElement(); + if (!el) return; + el.addEventListener("scroll", handleScroll, { passive: true }); + return () => { + el.removeEventListener("scroll", handleScroll); + }; + }, [handleScroll, transcriptRef, items, conversationId]); + return { isPinnedToLatest, showScrollToLatest, scrollToLatest, - handleScroll, }; } diff --git a/apps/web/src/domains/chat/utils/debug-api.test.ts b/apps/web/src/domains/chat/utils/debug-api.test.ts index 48c07286b20..8cf84d6f159 100644 --- a/apps/web/src/domains/chat/utils/debug-api.test.ts +++ b/apps/web/src/domains/chat/utils/debug-api.test.ts @@ -6,7 +6,7 @@ import { describe, expect, test } from "bun:test"; import type { MutableRefObject } from "react"; import type { ChatEventStream } from "@/domains/chat/api/stream.js"; -import type { TranscriptHandle } from "@/domains/chat/transcript/use-transcript-scroll.js"; +import type { TranscriptHandle } from "@/domains/chat/transcript/use-deprecated-transcript-scroll.js"; import type { TranscriptItem } from "@/domains/chat/transcript/types.js"; import type { DisplayMessage } from "@/domains/chat/utils/reconcile.js"; import type { RuntimeMessage } from "@/domains/chat/api/messages.js"; @@ -697,25 +697,34 @@ type DebugWindow = Window & { _vellumDebug?: { chat?: unknown; events?: { getClients: unknown; getEvents: unknown }; + flags?: { toggleTranscriptScrollController?: (v?: boolean) => boolean }; other?: unknown; }; }; +const makeFlagsApi = () => ({ + toggleTranscriptScrollController: (_value?: boolean): boolean => false, +}); + describe("installVellumDebugApi", () => { - test("attaches both .events and .chat in one call", () => { + test("attaches .events, .chat, and .flags in one call", () => { const api = createChatDebugApi(makeRefs()); - const uninstall = installVellumDebugApi(api); + const flags = makeFlagsApi(); + const uninstall = installVellumDebugApi(api, flags); const root = (globalThis as unknown as DebugWindow)._vellumDebug; expect(root?.chat).toBe(api); expect(root?.events).toBeDefined(); expect(typeof root?.events?.getClients).toBe("function"); expect(typeof root?.events?.getEvents).toBe("function"); + expect(typeof root?.flags?.toggleTranscriptScrollController).toBe( + "function", + ); uninstall(); }); - test("removes both .events and .chat on uninstall", () => { + test("removes .events, .chat, and .flags on uninstall", () => { const api = createChatDebugApi(makeRefs()); - const uninstall = installVellumDebugApi(api); + const uninstall = installVellumDebugApi(api, makeFlagsApi()); uninstall(); const root = (globalThis as unknown as DebugWindow)._vellumDebug; // Root should be gone entirely since nothing else was attached. @@ -727,10 +736,11 @@ describe("installVellumDebugApi", () => { win.window._vellumDebug = { other: "keep" }; const api = createChatDebugApi(makeRefs()); - const uninstall = installVellumDebugApi(api); + const uninstall = installVellumDebugApi(api, makeFlagsApi()); uninstall(); expect(win.window._vellumDebug?.chat).toBeUndefined(); expect(win.window._vellumDebug?.events).toBeUndefined(); + expect(win.window._vellumDebug?.flags).toBeUndefined(); expect(win.window._vellumDebug?.other).toBe("keep"); // Cleanup so we don't leak state into other tests. @@ -739,10 +749,10 @@ describe("installVellumDebugApi", () => { test("identity-checks chat on teardown so a newer mount isn't clobbered", () => { const first = createChatDebugApi(makeRefs()); - const uninstallFirst = installVellumDebugApi(first); + const uninstallFirst = installVellumDebugApi(first, makeFlagsApi()); const second = createChatDebugApi(makeRefs()); - installVellumDebugApi(second); + installVellumDebugApi(second, makeFlagsApi()); // First mount's teardown runs after second mount installed — // simulates strict-mode double-mount or hot-reload races. @@ -751,6 +761,7 @@ describe("installVellumDebugApi", () => { const root = (globalThis as unknown as DebugWindow)._vellumDebug; expect(root?.chat).toBe(second); expect(root?.events).toBeDefined(); + expect(root?.flags).toBeDefined(); }); }); diff --git a/apps/web/src/domains/chat/utils/debug-api.ts b/apps/web/src/domains/chat/utils/debug-api.ts index 8fe45f95d95..1b099527975 100644 --- a/apps/web/src/domains/chat/utils/debug-api.ts +++ b/apps/web/src/domains/chat/utils/debug-api.ts @@ -43,10 +43,11 @@ import type { import { recordChatDiagnostic } from "@/domains/chat/utils/diagnostics.js"; import type { DisplayMessage } from "@/domains/chat/utils/reconcile.js"; import type { ReconcileActiveConversationResult } from "@/domains/chat/hooks/use-message-reconciliation.js"; +import { setTranscriptScrollControllerEnabled } from "@/domains/chat/transcript/transcript-scroll-flag.js"; import { classifyScrollPosition, type TranscriptHandle, -} from "@/domains/chat/transcript/use-transcript-scroll.js"; +} from "@/domains/chat/transcript/use-deprecated-transcript-scroll.js"; import type { TranscriptItem } from "@/domains/chat/transcript/types.js"; import { type TerminalReason, @@ -313,6 +314,7 @@ export interface ChatDebugApi { const DEFAULT_CLIENT_MESSAGES_LIMIT = 20; const ROOT_NS = "_vellumDebug"; const CHAT_NS = "chat"; +const FLAGS_NS = "flags"; /** * Refs the API reads to surface client state and trigger actions. All are @@ -690,10 +692,25 @@ export function createChatDebugApi(refs: ChatDebugRefs): ChatDebugApi { const EVENTS_NS = "events"; const API_NS = "api"; +/** + * Dev-only toggle surface. Each function is a single-purpose imperative + * flip — call from the console to flip a localStorage-persisted flag. + * Toggles that change React hook ordering (e.g. swapping which scroll + * coordinator runs) reload the page so the new value takes effect + * cleanly. See `transcript-scroll-flag.ts` for the storage layer. + */ +export interface VellumDebugFlagsApi { + /** Flip the parallel `useTranscriptScrollController` path on or off. + * Persists to localStorage and reloads the page. Pass `true`/`false` + * to force a specific value; omit to flip the current value. */ + toggleTranscriptScrollController(value?: boolean): boolean; +} + interface VellumDebugRoot extends Record { [EVENTS_NS]?: ChatDebugEventsApi; [CHAT_NS]?: ChatDebugApi; [API_NS]?: typeof assistantApi; + [FLAGS_NS]?: VellumDebugFlagsApi; } declare global { @@ -713,6 +730,9 @@ declare global { * - `api` — the full `@vellumai/assistant-api` namespace, so a developer * can pull canonical SSE schemas (`RelationshipStateUpdatedSchema`, …) * out of the shipped bundle from the console. + * - `flags` — dev-toggleable feature flags (currently: + * `toggleTranscriptScrollController`). Stable singleton; pure + * module exports backed by localStorage. * * Consolidating these into one installer guarantees they're set at the * same time and torn down together, so DevTools never sees one namespace @@ -723,13 +743,17 @@ declare global { * root object if it's empty afterwards. Safe to call on the server — * no-op when `window` is undefined. */ -export function installVellumDebugApi(chatApi: ChatDebugApi): () => void { +export function installVellumDebugApi( + chatApi: ChatDebugApi, + flagsApi: VellumDebugFlagsApi, +): () => void { if (typeof window === "undefined") return () => {}; const win = window as Omit & { [ROOT_NS]?: VellumDebugRoot }; const existing: VellumDebugRoot = (win[ROOT_NS] ?? {}) as VellumDebugRoot; existing[EVENTS_NS] = eventsDebugApi; existing[CHAT_NS] = chatApi; existing[API_NS] = assistantApi; + existing[FLAGS_NS] = flagsApi; win[ROOT_NS] = existing; return () => { const current = win[ROOT_NS]; @@ -737,12 +761,14 @@ export function installVellumDebugApi(chatApi: ChatDebugApi): () => void { // Gate every deletion on the chat-API identity check. If a newer // mount has already replaced our chatApi (strict-mode double-mount, // hot reload, etc.), our teardown is stale — leave the world alone. - // `events` and `api` lifecycles are paired with `chat` because they - // are stable singletons; identity-checking them would always pass. + // `events`, `api`, and `flags` lifecycles are paired with `chat` + // because they are stable singletons (pure module exports); + // identity-checking them would always pass. if (current[CHAT_NS] === chatApi) { delete current[CHAT_NS]; delete current[EVENTS_NS]; delete current[API_NS]; + delete current[FLAGS_NS]; } // Only remove the root if we left it empty — other debug domains // may have attached siblings under the same namespace. @@ -792,7 +818,10 @@ export function useChatDebugApi(refs: ChatDebugRefs): void { historyFetcher: refs.historyFetcher, }; const api = createChatDebugApi(stableRefs); - const uninstall = installVellumDebugApi(api); + const flagsApi: VellumDebugFlagsApi = { + toggleTranscriptScrollController: setTranscriptScrollControllerEnabled, + }; + const uninstall = installVellumDebugApi(api, flagsApi); return uninstall; }, []); }