diff --git a/apps/app/src/App.tsx b/apps/app/src/App.tsx index 722e1db661..c980293f65 100644 --- a/apps/app/src/App.tsx +++ b/apps/app/src/App.tsx @@ -89,6 +89,13 @@ const ProjectSettingsView = lazy(() => const splitWorkspaceRouteModule = import("./views/SplitWorkspaceRoute"); splitWorkspaceRouteModule.catch(() => {}); const SplitWorkspaceRoute = lazy(() => splitWorkspaceRouteModule); +// Same reasoning for the timeline windowing chunk: windowing defaults on for +// compact viewports, and until the chunk lands the loader's Suspense fallback +// mounts loaded rows without virtualization, so a first long-thread open on a +// cold connection would pay boot parse → route chunk → windowed chunk in +// series. Warming it here keeps it a separate chunk (nothing static imports +// it) while React.lazy resolves from the module cache. +import("./components/thread/timeline/TimelineWindowedItems").catch(() => {}); export function LegacyAutomationDetailRedirect() { const location = useLocation(); diff --git a/apps/app/src/components/thread/timeline/ExpandableTimelineRow.tsx b/apps/app/src/components/thread/timeline/ExpandableTimelineRow.tsx index 1bac8a4fb2..dbb1f3d5dd 100644 --- a/apps/app/src/components/thread/timeline/ExpandableTimelineRow.tsx +++ b/apps/app/src/components/thread/timeline/ExpandableTimelineRow.tsx @@ -1,7 +1,10 @@ import { memo, useCallback, + useContext, useEffect, + useLayoutEffect, + useRef, useState, type CSSProperties, type FocusEvent, @@ -29,6 +32,7 @@ import { type TimelineTitleActionResolver, type TimelineTitleLinkResolver, } from "./TimelineTitleView.js"; +import { TimelineWindowingGeometryInvalidateContext } from "./TimelineWindowedItemsLoader.js"; interface ExpandableTimelineRowProps { autoExpanded?: boolean; @@ -125,6 +129,18 @@ function ExpandableTimelineRowComponent({ setCollapsedPreviewActive(false); } }, [isExpanded]); + const invalidateWindowingGeometry = useContext( + TimelineWindowingGeometryInvalidateContext, + ); + const previousIsExpandedRef = useRef(isExpanded); + // Expanding or collapsing this row moves every windowed list below it (and + // any nested list inside it) within the shared scroll root without resizing + // the root; tell mounted windowed lists to re-read scroll geometry. + useLayoutEffect(() => { + if (previousIsExpandedRef.current === isExpanded) return; + previousIsExpandedRef.current = isExpanded; + invalidateWindowingGeometry(); + }, [invalidateWindowingGeometry, isExpanded]); const horizontalPaddingClass = timelineRowHorizontalPaddingClassName(horizontalPadding); const handleToggle = useCallback((): void => { diff --git a/apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx b/apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx index c0d1ae1fc4..029af31304 100644 --- a/apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx +++ b/apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx @@ -5,6 +5,7 @@ import { useContext, useEffect, useMemo, + useReducer, useRef, useState, useSyncExternalStore, @@ -133,6 +134,8 @@ import { } from "@/components/ui/markdown-message-directives.js"; import { TimelineWindowedItemsLoader, + TimelineWindowingGeometryInvalidateContext, + TimelineWindowingGeometryRevisionContext, TimelineWindowingMeasurementsContext, TimelineWindowingScrollRootContext, type TimelineWindowedItemRenderState, @@ -2168,7 +2171,7 @@ function TimelineRowsList({ itemKeys={itemKeys} measurements={measurements} minItemCount={ - spacing === "top-level" ? (isCompactViewport ? 40 : 60) : 20 + spacing === "top-level" ? (isCompactViewport ? 16 : 60) : 20 } renderItem={(index, windowedState) => { const item = items[index]; @@ -2234,6 +2237,13 @@ function ThreadTimelineRowsComponent(props: ThreadTimelineRowsProps) { function ThreadTimelineRowsForTimelineView(props: ThreadTimelineRowsProps) { const getViewRows = useTimelineViewRowsCache(); const [windowingMeasurements] = useState(() => new Map()); + // Expand/collapse commits can move a windowed list within its scroll root + // without resizing the root; the bumped revision tells every mounted + // windowed list (top-level and nested) to re-read scroll geometry. + const [windowingGeometryRevision, invalidateWindowingGeometry] = useReducer( + (revision: number) => revision + 1, + 0, + ); const rows = useMemo( () => getViewRows(props.timelineRows), [getViewRows, props.timelineRows], @@ -2504,29 +2514,39 @@ function ThreadTimelineRowsForTimelineView(props: ThreadTimelineRowsProps) { - - - + + + + + + + {hasSelectionActions ? ( diff --git a/apps/app/src/components/thread/timeline/ThreadTimelineRows.windowing.test.tsx b/apps/app/src/components/thread/timeline/ThreadTimelineRows.windowing.test.tsx index 6e63f850e8..7e0b30e4e7 100644 --- a/apps/app/src/components/thread/timeline/ThreadTimelineRows.windowing.test.tsx +++ b/apps/app/src/components/thread/timeline/ThreadTimelineRows.windowing.test.tsx @@ -1,6 +1,6 @@ // @vitest-environment jsdom -import { cleanup, render, waitFor } from "@testing-library/react"; +import { cleanup, fireEvent, render, waitFor } from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { MemoryRouter } from "react-router-dom"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -242,4 +242,122 @@ describe("ThreadTimelineRows windowing experiment", () => { }), ); }); + + it("windows a 20-row top-level timeline on a compact viewport", async () => { + const scrollElement = document.createElement("div"); + scrollElement.setAttribute("data-test-main-scroll", ""); + Object.defineProperty(scrollElement, "clientHeight", { value: 800 }); + Object.defineProperty(scrollElement, "scrollHeight", { value: 2_000 }); + const bottomAnchor: BottomAnchorContextValue = { + captureScrollAnchor: vi.fn(), + getScrollElement: () => scrollElement, + isAtBottom: false, + scrollElementIntoView: vi.fn(), + scrollElementIntoViewClampedToMaxScroll: vi.fn(), + scrollToBottom: vi.fn(), + }; + const rows = Array.from({ length: 20 }, (_, index) => + conversationRow({ + id: `short-message-${index}`, + role: index % 2 === 0 ? "user" : "assistant", + sourceSeqEnd: index + 1, + sourceSeqStart: index + 1, + text: `Short message ${index}`, + }), + ); + const queryClient = new QueryClient(); + const view = render( + + + + + + + + + , + ); + + // Phones window from 16 top-level rows; 20 rows stayed fully mounted + // under the old 40-row floor. + await waitFor(() => + expect( + view.container.querySelector("[data-timeline-virtual-spacer]"), + ).not.toBeNull(), + ); + }); + + it("re-reads windowing scroll geometry when a row expands", async () => { + const scrollElement = document.createElement("div"); + scrollElement.setAttribute("data-test-main-scroll", ""); + Object.defineProperty(scrollElement, "clientHeight", { value: 800 }); + Object.defineProperty(scrollElement, "scrollHeight", { value: 8_000 }); + const bottomAnchor: BottomAnchorContextValue = { + captureScrollAnchor: vi.fn(), + getScrollElement: () => scrollElement, + isAtBottom: false, + scrollElementIntoView: vi.fn(), + scrollElementIntoViewClampedToMaxScroll: vi.fn(), + scrollToBottom: vi.fn(), + }; + const rows = [ + ...Array.from({ length: 80 }, (_, index) => + conversationRow({ + id: `message-${index}`, + role: index % 2 === 0 ? "user" : "assistant", + sourceSeqEnd: index + 1, + sourceSeqStart: index + 1, + text: `Message ${index}`, + }), + ), + delegationRow({ + id: "expandable-delegation", + output: "Delegation output.", + sourceSeqEnd: 81, + sourceSeqStart: 81, + }), + ]; + const queryClient = new QueryClient(); + const view = render( + + + + + + + , + ); + const toggle = view.container.querySelector( + '[data-timeline-row-id="expandable-delegation"] button[aria-expanded]', + ); + expect(toggle).not.toBeNull(); + + const boundingRectSpy = vi.mocked( + HTMLElement.prototype.getBoundingClientRect, + ); + const scrollElementReads = () => + boundingRectSpy.mock.contexts.filter( + (context) => context === scrollElement, + ).length; + const settledReads = scrollElementReads(); + + // Expanding moves every windowed list below the row within the scroll + // root without resizing the root; the expansion path must bump the + // geometry revision so the windowed list re-reads its scroll margin. + fireEvent.click(toggle as HTMLButtonElement); + + await waitFor(() => + expect(scrollElementReads()).toBeGreaterThan(settledReads), + ); + }); }); diff --git a/apps/app/src/components/thread/timeline/ThreadTimelineSurface.tsx b/apps/app/src/components/thread/timeline/ThreadTimelineSurface.tsx index 4613f550ad..8253b18f97 100644 --- a/apps/app/src/components/thread/timeline/ThreadTimelineSurface.tsx +++ b/apps/app/src/components/thread/timeline/ThreadTimelineSurface.tsx @@ -11,6 +11,7 @@ import { ConversationTimeline } from "@/components/ui/conversation.js"; import { HeightTransition } from "@/components/ui/height-transition.js"; import { Icon } from "@bb/shared-ui/icon"; import { Skeleton } from "@bb/shared-ui/skeleton"; +import { useIsCompactViewport } from "@bb/shared-ui/hooks/use-compact-viewport"; import { useSystemConfig } from "@/hooks/queries/system-queries"; import { toUserAttachmentImageSrc } from "@/lib/user-attachment-images"; import { ThreadTimelineRows } from "./ThreadTimelineRows.js"; @@ -180,8 +181,13 @@ export function ThreadTimelineSurface({ workspaceRootPath, }: ThreadTimelineSurfaceProps) { const systemConfigQuery = useSystemConfig(); + const isCompactViewport = useIsCompactViewport(); + // Compact viewports default timeline windowing on: phones are where the + // unwindowed tree hangs, and the server cannot own this default because it + // depends on the viewport. The experiment stays the kill switch — a served + // false still disables windowing here; desktop keeps the served value. const timelineWindowingEnabled = - systemConfigQuery.data?.experiments.timelineWindowing ?? false; + systemConfigQuery.data?.experiments.timelineWindowing ?? isCompactViewport; const showActiveThinking = activeThinking !== null && ongoingIndicatorLabel === undefined; const activeThinkingText = activeThinking?.text.trim() ?? ""; diff --git a/apps/app/src/components/thread/timeline/ThreadTimelineSurface.windowing-default.test.tsx b/apps/app/src/components/thread/timeline/ThreadTimelineSurface.windowing-default.test.tsx new file mode 100644 index 0000000000..6c02af9cf8 --- /dev/null +++ b/apps/app/src/components/thread/timeline/ThreadTimelineSurface.windowing-default.test.tsx @@ -0,0 +1,127 @@ +// @vitest-environment jsdom + +import { cleanup, render, screen } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { Experiments, StoredExperiments } from "@bb/domain"; +import { CompactViewportOverrideProvider } from "@bb/shared-ui/hooks/use-compact-viewport"; +import { conversationRow } from "@/test/fixtures/thread-timeline-rows"; +import { ThreadTimelineSurface } from "./ThreadTimelineSurface.js"; + +const mocks = vi.hoisted(() => ({ + experiments: undefined as StoredExperiments | undefined, +})); + +vi.mock("@/hooks/queries/system-queries", () => ({ + useSystemConfig: () => ({ + data: + mocks.experiments === undefined + ? undefined + : { experiments: mocks.experiments }, + }), +})); + +vi.mock("./ThreadTimelineRows.js", () => ({ + ThreadTimelineRows: ({ + timelineWindowingEnabled, + }: { + timelineWindowingEnabled?: boolean; + }) => ( +
+ ), +})); + +vi.mock("@/components/ui/conversation.js", () => ({ + ConversationTimeline: ({ children }: { children?: ReactNode }) => ( +
{children}
+ ), +})); + +function experimentsWithTimelineWindowing(value: boolean): Experiments { + return { + changelogPreview: false, + editMessages: true, + mobileApp: false, + providerSessionReaping: false, + timelineWindowing: value, + }; +} + +function renderSurface({ isCompactViewport }: { isCompactViewport: boolean }) { + return render( + + + , + ); +} + +function windowingEnabledAttribute(): string | undefined { + return screen.getByTestId("timeline-rows-stub").dataset.windowingEnabled; +} + +class ResizeObserverStub implements ResizeObserver { + disconnect(): void {} + observe(): void {} + unobserve(): void {} +} + +beforeEach(() => { + vi.stubGlobal("ResizeObserver", ResizeObserverStub); +}); + +afterEach(() => { + cleanup(); + mocks.experiments = undefined; + vi.unstubAllGlobals(); +}); + +describe("ThreadTimelineSurface windowing default", () => { + it("defaults windowing on for compact viewports while the experiment is unset", () => { + mocks.experiments = undefined; + renderSurface({ isCompactViewport: true }); + + expect(windowingEnabledAttribute()).toBe("true"); + }); + + it("defaults windowing on for compact viewports when the config payload omits the key", () => { + // A fresh install: /system/config serves only saved choices, so a + // never-toggled timelineWindowing arrives omitted, not false. + mocks.experiments = {}; + renderSurface({ isCompactViewport: true }); + + expect(windowingEnabledAttribute()).toBe("true"); + }); + + it("keeps an explicitly false experiment as the kill switch on compact viewports", () => { + mocks.experiments = experimentsWithTimelineWindowing(false); + renderSurface({ isCompactViewport: true }); + + expect(windowingEnabledAttribute()).toBe("false"); + }); + + it("keeps desktop off while the experiment is unset", () => { + mocks.experiments = undefined; + renderSurface({ isCompactViewport: false }); + + expect(windowingEnabledAttribute()).toBe("false"); + }); + + it("honors an explicitly true experiment on desktop", () => { + mocks.experiments = experimentsWithTimelineWindowing(true); + renderSurface({ isCompactViewport: false }); + + expect(windowingEnabledAttribute()).toBe("true"); + }); +}); diff --git a/apps/app/src/components/thread/timeline/TimelineWindowedItems.test.tsx b/apps/app/src/components/thread/timeline/TimelineWindowedItems.test.tsx index 25a96d572e..0bf29d1bbf 100644 --- a/apps/app/src/components/thread/timeline/TimelineWindowedItems.test.tsx +++ b/apps/app/src/components/thread/timeline/TimelineWindowedItems.test.tsx @@ -9,16 +9,22 @@ import { waitFor, } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { CompactViewportOverrideProvider } from "@bb/shared-ui/hooks/use-compact-viewport"; import { TimelineWindowedItems, type TimelineWindowedItemRenderState, } from "./TimelineWindowedItems.js"; -import { TimelineWindowedItemsLoader } from "./TimelineWindowedItemsLoader.js"; +import { + TimelineWindowedItemsLoader, + TimelineWindowingGeometryRevisionContext, +} from "./TimelineWindowedItemsLoader.js"; const ITEM_KEYS = Array.from({ length: 100 }, (_, index) => `row-${index}`); let scrollElement: HTMLDivElement; let itemHeights = new Map(); +/** Height of content above the windowed container inside the scroll root. */ +let spacerOffsetTop = 0; function rect(top: number, height: number): DOMRect { return { @@ -44,6 +50,7 @@ function renderWindowedItems(options?: { alwaysMountedKeys?: ReadonlySet; clientHeight?: number; enabled?: boolean; + isCompactViewport?: boolean; measurements?: Map; }) { const measurements = options?.measurements ?? new Map(); @@ -55,42 +62,68 @@ function renderWindowedItems(options?: { configurable: true, value: options?.clientHeight ?? 96, }); + // Stable identities so a rerender only commits what a test changes. + const estimateItemHeight = () => 32; + const getScrollElement = () => scrollElement; + const renderItem = ( + index: number, + state: TimelineWindowedItemRenderState, + ) => ( +
+ {state.isRealized ? ( + + ) : null} +
+ ); + const buildElement = (geometryRevision: number) => ( + + + + + + ); + let geometryRevision = 0; + const view = render(buildElement(geometryRevision), { + container: scrollElement, + }); return { - ...render( - 32} - gap={0} - getScrollElement={() => scrollElement} - itemKeys={ITEM_KEYS} - measurements={measurements} - renderItem={(index: number, state: TimelineWindowedItemRenderState) => ( -
- {state.isRealized ? ( - - ) : null} -
- )} - />, - { container: scrollElement }, - ), + ...view, measurements, + /** Commit with fresh element identity but no geometry trigger. */ + rerenderWithoutGeometryTrigger: () => { + view.rerender(buildElement(geometryRevision)); + }, + rerenderWithGeometryRevision: (revision: number) => { + geometryRevision = revision; + view.rerender(buildElement(revision)); + }, }; } -beforeEach(() => { - itemHeights = new Map(); +function createScrollElement() { scrollElement = document.createElement("div"); document.body.append(scrollElement); Object.defineProperty(scrollElement, "clientWidth", { @@ -105,12 +138,18 @@ beforeEach(() => { configurable: true, value: 3_200, }); +} + +beforeEach(() => { + itemHeights = new Map(); + spacerOffsetTop = 0; + createScrollElement(); vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockImplementation( function (this: HTMLElement) { if (this === scrollElement) return rect(0, scrollElement.clientHeight); if (this.hasAttribute("data-timeline-virtual-spacer")) { return rect( - -scrollElement.scrollTop, + spacerOffsetTop - scrollElement.scrollTop, Number.parseFloat(this.style.height) || 0, ); } @@ -135,7 +174,7 @@ afterEach(() => { }); describe("TimelineWindowedItems", () => { - it("seeds exact heights while the lazy windowing implementation loads", () => { + it("seeds exact heights for a bounded trailing region while the lazy windowing implementation loads", () => { const measurements = new Map(); render( @@ -157,8 +196,12 @@ describe("TimelineWindowedItems", () => { { container: scrollElement }, ); - expect(measurements.get("row-0")).toBe(32); + // The fallback mounts and measures only the trailing bottom-anchor + // region, not all 100 loaded rows. expect(measurements.get("row-99")).toBe(32); + expect(measurements.get("row-40")).toBe(32); + expect(measurements.has("row-39")).toBe(false); + expect(measurements.size).toBe(60); }); it("keeps the control path fully mounted when the experiment is off", () => { @@ -255,4 +298,60 @@ describe("TimelineWindowedItems", () => { expect(screen.getAllByTestId(/^content-/)).toHaveLength(100), ); }); + + it("retains fewer overscan rows on compact viewports", async () => { + renderWindowedItems(); + await waitFor(() => expect(screen.getByTestId("content-0")).toBeTruthy()); + const desktopWrappers = screen.getAllByTestId(/^wrapper-/).length; + cleanup(); + createScrollElement(); + + renderWindowedItems({ isCompactViewport: true }); + await waitFor(() => expect(screen.getByTestId("content-0")).toBeTruthy()); + const compactWrappers = screen.getAllByTestId(/^wrapper-/).length; + + // Same geometry, half the overscan (4 instead of 8). At the top only the + // trailing side contributes, so the difference is one side's worth. + expect(compactWrappers).toBe(desktopWrappers - 4); + }); + + it("re-reads scroll geometry only when a geometry trigger changes", async () => { + const view = renderWindowedItems(); + await waitFor(() => expect(screen.getByTestId("content-0")).toBeTruthy()); + + const boundingRectSpy = vi.mocked( + HTMLElement.prototype.getBoundingClientRect, + ); + const scrollElementReads = () => + boundingRectSpy.mock.contexts.filter( + (context) => context === scrollElement, + ).length; + const settledReads = scrollElementReads(); + + // A commit without a geometry trigger (a streaming delta that only + // mutates row content) must not force a layout read. + view.rerenderWithoutGeometryTrigger(); + expect(scrollElementReads()).toBe(settledReads); + + // An expand/collapse path bumping the geometry revision costs exactly + // one read. + view.rerenderWithGeometryRevision(1); + expect(scrollElementReads()).toBe(settledReads + 1); + }); + + it("re-reads scroll geometry when the owning row's expansion moves a nested list", async () => { + scrollElement.scrollTop = 640; + const view = renderWindowedItems(); + + await waitFor(() => expect(screen.getByTestId("content-20")).toBeTruthy()); + expect(screen.queryByTestId("wrapper-2")).toBeNull(); + + // The owning row expanded: 320px of content appeared above the nested + // list without resizing the scroll root, and the expansion path bumped + // the geometry revision. + spacerOffsetTop = 320; + view.rerenderWithGeometryRevision(1); + + await waitFor(() => expect(screen.getByTestId("wrapper-2")).toBeTruthy()); + }); }); diff --git a/apps/app/src/components/thread/timeline/TimelineWindowedItems.tsx b/apps/app/src/components/thread/timeline/TimelineWindowedItems.tsx index e9caaf3d11..288b05c607 100644 --- a/apps/app/src/components/thread/timeline/TimelineWindowedItems.tsx +++ b/apps/app/src/components/thread/timeline/TimelineWindowedItems.tsx @@ -1,5 +1,6 @@ import { useCallback, + useContext, useLayoutEffect, useMemo, useRef, @@ -13,12 +14,22 @@ import { type Range, type Virtualizer, } from "@tanstack/react-virtual"; -import type { TimelineWindowedItemsProps } from "./TimelineWindowedItemsLoader.js"; +import { useIsCompactViewport } from "@bb/shared-ui/hooks/use-compact-viewport"; +import { + TimelineWindowingGeometryRevisionContext, + type TimelineWindowedItemsProps, +} from "./TimelineWindowedItemsLoader.js"; export type { TimelineWindowedItemRenderState } from "./TimelineWindowedItemsLoader.js"; /** Rich rows retained on each side of the visible range. */ const TIMELINE_WINDOW_OVERSCAN_ITEMS = 8; +/** + * Compact viewports keep fewer: every retained rich row is walked by the + * style/layout passes phone interactions trigger (keyboard, composer growth, + * orientation), and a phone shows fewer rows per screen to begin with. + */ +const TIMELINE_WINDOW_OVERSCAN_ITEMS_COMPACT = 4; /** TanStack's scroll-idle boundary also drives rich-content realization. */ const TIMELINE_WINDOW_IDLE_DELAY_MS = 300; /** Bound row-local interaction state retained across a long-lived session. */ @@ -95,10 +106,13 @@ export function TimelineWindowedItems({ }: TimelineWindowedItemsProps) { const configured = enabled && itemKeys.length >= minItemCount && getScrollElement !== null; + const isCompactViewport = useIsCompactViewport(); + const geometryRevision = useContext(TimelineWindowingGeometryRevisionContext); const [scrollRootUsable, setScrollRootUsable] = useState(true); const [scrollMargin, setScrollMargin] = useState(0); const [interactionPins, setInteractionPins] = useState([]); const containerElementRef = useRef(null); + const scrollGeometryTriggersRef = useRef(null); const scrollSampleRef = useRef({ at: 0, fast: false, @@ -207,7 +221,9 @@ export function TimelineWindowedItems({ isScrollingResetDelay: TIMELINE_WINDOW_IDLE_DELAY_MS, measureElement, onChange: handleVirtualizerChange, - overscan: TIMELINE_WINDOW_OVERSCAN_ITEMS, + overscan: isCompactViewport + ? TIMELINE_WINDOW_OVERSCAN_ITEMS_COMPACT + : TIMELINE_WINDOW_OVERSCAN_ITEMS, rangeExtractor, scrollMargin, useFlushSync: false, @@ -264,8 +280,21 @@ export function TimelineWindowedItems({ }, [configured, resolvedGetScrollElement, updateScrollGeometry]); // A nested list's offset can change when its owning row expands or reflows - // without resizing the scroll root itself. Re-read after those React commits. - useLayoutEffect(updateScrollGeometry); + // without resizing the scroll root itself. Expand/collapse paths bump the + // geometry revision for those commits, and row count or root usability + // changes reposition the container, so re-read after exactly those commits. + // The previous depless form re-read after every commit — a forced layout + // per streaming delta. The ref guard leaves the mount read (and the + // missing-scroll-element retry) to the effect above, keeping one geometry + // read per trigger. + useLayoutEffect(() => { + const triggers = `${geometryRevision}:${itemKeys.length}:${scrollRootUsable}`; + if (scrollGeometryTriggersRef.current === triggers) return; + const isMountRead = scrollGeometryTriggersRef.current === null; + scrollGeometryTriggersRef.current = triggers; + if (isMountRead) return; + updateScrollGeometry(); + }, [geometryRevision, itemKeys.length, scrollRootUsable, updateScrollGeometry]); const retainInteractedItem = useCallback( (event: SyntheticEvent) => { diff --git a/apps/app/src/components/thread/timeline/TimelineWindowedItemsLoader.tsx b/apps/app/src/components/thread/timeline/TimelineWindowedItemsLoader.tsx index d05a0945fa..0168408e58 100644 --- a/apps/app/src/components/thread/timeline/TimelineWindowedItemsLoader.tsx +++ b/apps/app/src/components/thread/timeline/TimelineWindowedItemsLoader.tsx @@ -8,6 +8,13 @@ import { const DEFAULT_WINDOWING_MIN_ITEM_COUNT = 20; const MAX_CONTROL_PATH_MEASUREMENTS = 2_000; +/** + * Rows the Suspense fallback mounts while the windowed chunk downloads: the + * trailing bottom-anchor region a thread opens into. Mounting (and measuring) + * every loaded row made a cold first open pay a full-tree mount before the + * windowed remount. + */ +const FALLBACK_TRAILING_ITEM_COUNT = 60; const NOOP_ITEM_REF = () => {}; export interface TimelineWindowingScrollRoot { @@ -24,6 +31,22 @@ export const TimelineWindowingMeasurementsContext = createContext | null>(null); +/** + * Bumped after a commit that can move a windowed list within its scroll root + * without resizing the root (a row above it expanding or collapsing). + * Windowed lists re-read their scroll geometry when it changes. + */ +export const TimelineWindowingGeometryRevisionContext = createContext(0); + +/** + * Stable notifier for the revision context above. Expand/collapse paths call + * it in the commit that moves content so every mounted windowed list + * re-reads geometry before paint. + */ +export const TimelineWindowingGeometryInvalidateContext = createContext< + () => void +>(() => {}); + export interface TimelineWindowedItemRenderState { isRealized: boolean; itemIndex: number | undefined; @@ -58,8 +81,12 @@ function TimelineWindowedItemsControl({ renderItem, captureMeasurements = false, }: TimelineWindowedItemsProps & { captureMeasurements?: boolean }) { - return itemKeys.map((key, index) => - renderItem(index, { + const firstRenderedIndex = captureMeasurements + ? Math.max(0, itemKeys.length - FALLBACK_TRAILING_ITEM_COUNT) + : 0; + return itemKeys.slice(firstRenderedIndex).map((key, renderedIndex) => { + const index = firstRenderedIndex + renderedIndex; + return renderItem(index, { isRealized: true, itemIndex: captureMeasurements ? index : undefined, itemRef: captureMeasurements @@ -78,8 +105,8 @@ function TimelineWindowedItemsControl({ : NOOP_ITEM_REF, itemStyle: undefined, windowingEnabled: false, - }), - ); + }); + }); } /** Keep TanStack Virtual out of the route bundle until the experiment is on. */ diff --git a/apps/app/src/lib/system-config-atoms.ts b/apps/app/src/lib/system-config-atoms.ts index 2ebe95280d..a5071f993b 100644 --- a/apps/app/src/lib/system-config-atoms.ts +++ b/apps/app/src/lib/system-config-atoms.ts @@ -21,12 +21,14 @@ const unavailableSystemConfig: SystemConfigResponse = { keybindings: [], defaultKeybindings: [], keybindingOverrides: [], + // timelineWindowing is deliberately omitted (never-chosen semantics): a + // phone that cannot reach the server still gets the compact windowing + // default instead of an explicit off. experiments: { changelogPreview: false, editMessages: false, mobileApp: false, providerSessionReaping: false, - timelineWindowing: false, }, appearance: defaultAppTheme, customThemes: [], diff --git a/apps/app/src/views/SettingsView.tsx b/apps/app/src/views/SettingsView.tsx index d1dfcf52f2..fdaac48704 100644 --- a/apps/app/src/views/SettingsView.tsx +++ b/apps/app/src/views/SettingsView.tsx @@ -919,7 +919,12 @@ export function SettingsView() { // The in-app browser only exists on desktop; hide the toggle entirely on web, // where it would have no effect. const [desktopBrowserAvailable] = useState(isDesktopBrowserAvailable); - const experiments = systemConfigQuery.data?.experiments ?? defaultExperiments; + // The config payload carries stored choices only; overlay the defaults so + // toggles display effective values and saves put a full record. + const experiments = { + ...defaultExperiments, + ...systemConfigQuery.data?.experiments, + }; const updateExperimentsMutation = useUpdateExperiments(); const generalSettings = systemConfigQuery.data?.generalSettings ?? defaultAppSettings; diff --git a/apps/app/src/views/thread-detail/ThreadDetailView.tsx b/apps/app/src/views/thread-detail/ThreadDetailView.tsx index 8870c2ba37..0e0261c6b2 100644 --- a/apps/app/src/views/thread-detail/ThreadDetailView.tsx +++ b/apps/app/src/views/thread-detail/ThreadDetailView.tsx @@ -30,6 +30,7 @@ import { requestProviderPluginFrontend } from "@/lib/plugin-frontend-lazy"; import { ThreadProviderContext } from "@/components/thread/thread-provider-context"; import { defaultAppSettings, + defaultExperiments, resolveEnvironmentMergeBaseBranch, type ThreadListEntry, type ThreadWithRuntime, @@ -1044,7 +1045,9 @@ function ThreadDetailViewInternal(props: ThreadRoutePathArgs) { // must repeat the full eligibility check on the server before changing state. const canEditSentMessages = thread !== undefined && - (systemConfigQuery.data?.experiments.editMessages ?? false) && + // The config payload omits never-saved keys; editMessages defaults on. + (systemConfigQuery.data?.experiments.editMessages ?? + defaultExperiments.editMessages) && // Declared capability, same source as the fork affordance above: an edit // is a rewind to an earlier point in the provider session. (threadProviderInfo?.capabilities.supportsSessionRewind ?? false) && diff --git a/apps/cli/src/commands/settings.ts b/apps/cli/src/commands/settings.ts index 6258b215bd..e7d9f844e7 100644 --- a/apps/cli/src/commands/settings.ts +++ b/apps/cli/src/commands/settings.ts @@ -3,11 +3,13 @@ import { appCommandIdSchema, appShortcutSchema, appSettingsSchema, + defaultExperiments, experimentKeySchema, experimentsSchema, type AppSettings, type AppShortcut, type Experiments, + type StoredExperiments, } from "@bb/domain"; import { action } from "../action.js"; import { createCliBbSdk } from "../client.js"; @@ -102,7 +104,7 @@ function updateGeneralSetting( } function updateExperiment( - experiments: Experiments, + experiments: StoredExperiments, key: string, value: string, ): Experiments { @@ -111,7 +113,10 @@ function updateExperiment( if (!experimentKey.success) { throw new Error(`Unknown experiment '${key}'.`); } + // The config payload carries stored choices only; overlay the defaults so + // the PUT persists a full record, as it always has. return experimentsSchema.parse({ + ...defaultExperiments, ...experiments, [experimentKey.data]: enabled, }); diff --git a/apps/mobile/src/data/thread-runtime/thread-runtime-mutations.ts b/apps/mobile/src/data/thread-runtime/thread-runtime-mutations.ts index 2a69a31f4d..fce35116d7 100644 --- a/apps/mobile/src/data/thread-runtime/thread-runtime-mutations.ts +++ b/apps/mobile/src/data/thread-runtime/thread-runtime-mutations.ts @@ -1,4 +1,4 @@ -import type { ThreadQueuedMessage } from "@bb/domain"; +import { defaultExperiments, type ThreadQueuedMessage } from "@bb/domain"; import { BbHttpError } from "@bb/sdk/browser"; import type { EditMessageRequest, @@ -143,7 +143,10 @@ export function useEditThreadMessage() { /** Whether the server has the `editMessages` experiment switched on. */ export function useEditMessagesExperimentEnabled(): boolean { const config = useSystemConfig(); - return config.data?.experiments.editMessages ?? false; + // The config payload omits never-saved keys; editMessages defaults on. + return ( + config.data?.experiments.editMessages ?? defaultExperiments.editMessages + ); } /** `POST /threads/:id/stop`: the thread shows `stopping` at once. */ diff --git a/apps/mobile/src/screens/settings/ExperimentsSettingsScreen.tsx b/apps/mobile/src/screens/settings/ExperimentsSettingsScreen.tsx index fb4daf300b..3337f095f3 100644 --- a/apps/mobile/src/screens/settings/ExperimentsSettingsScreen.tsx +++ b/apps/mobile/src/screens/settings/ExperimentsSettingsScreen.tsx @@ -51,7 +51,12 @@ export function ExperimentsSettingsScreen() { function ConnectedExperimentsSettingsScreen() { const configQuery = useSystemConfig(); const updateExperiments = useUpdateExperiments(); - const experiments = configQuery.data?.experiments ?? defaultExperiments; + // The config payload carries stored choices only; overlay the defaults so + // toggles display effective values and saves put a full record. + const experiments = { + ...defaultExperiments, + ...configQuery.data?.experiments, + }; const disabled = configQuery.data === undefined; return ( diff --git a/apps/server/src/routes/system.ts b/apps/server/src/routes/system.ts index 80973706e0..6971ab0fbe 100644 --- a/apps/server/src/routes/system.ts +++ b/apps/server/src/routes/system.ts @@ -2,6 +2,7 @@ import { getAppSettings, getAppKeybindingOverrides, getExperiments, + getStoredExperiments, getStoredFaviconColor, getStoredThemeId, hasActiveThreadAttention, @@ -160,7 +161,11 @@ export function registerSystemRoutes( ), defaultKeybindings: DEFAULT_APP_KEYBINDINGS, keybindingOverrides, - experiments: getExperiments(deps.db), + // Stored choices only: an omitted key means the user never chose, so + // clients can apply their own defaults (timelineWindowing defaults on + // for compact viewports, which the server cannot know). Server-internal + // policy keeps reading concrete booleans through getExperiments. + experiments: getStoredExperiments(deps.db), appearance: await resolveSelectedTheme( getStoredThemeId(deps.db), getStoredFaviconColor(deps.db), diff --git a/apps/server/src/services/skills/builtin-skills/bb-cli/SKILL.md b/apps/server/src/services/skills/builtin-skills/bb-cli/SKILL.md index fde98e13f8..b99f9056fc 100644 --- a/apps/server/src/services/skills/builtin-skills/bb-cli/SKILL.md +++ b/apps/server/src/services/skills/builtin-skills/bb-cli/SKILL.md @@ -120,8 +120,9 @@ message agents, or inspect projects, providers, and environments. failed or incomplete turns. Submitting an edit to a running thread stops and settles the current turn first. Change it with: `bb settings experiment editMessages `. -- The default-off `timelineWindowing` experiment mounts only nearby rows in - long timelines and large expanded timeline details. Change it with +- The `timelineWindowing` experiment mounts only nearby rows in long + timelines and large expanded timeline details: on by default for compact + viewports (phones), default-off on desktop. Change it with `bb settings experiment timelineWindowing `. - Thread timeline windows are capped by event count as well as by user-message count (`BB_FF_TIMELINE_WINDOW_EVENT_BUDGET`, default 1500), because a thread diff --git a/apps/server/src/services/skills/builtin-skills/bb-cli/references/app-settings.md b/apps/server/src/services/skills/builtin-skills/bb-cli/references/app-settings.md index 07194d428a..94575849ed 100644 --- a/apps/server/src/services/skills/builtin-skills/bb-cli/references/app-settings.md +++ b/apps/server/src/services/skills/builtin-skills/bb-cli/references/app-settings.md @@ -80,7 +80,9 @@ every window and client sees the same value. ## Timeline windowing -- The `timelineWindowing` experiment defaults to false. -- Enable it with `bb settings experiment timelineWindowing true`. +- Compact viewports (phones) window by default; setting the + `timelineWindowing` experiment to false is the kill switch there. +- Desktop defaults to false; enable it with + `bb settings experiment timelineWindowing true`. - It keeps stable timeline wrappers while mounting only rows near the active main or nested detail scrollport. diff --git a/apps/server/test/system/experiments.test.ts b/apps/server/test/system/experiments.test.ts index 23748a98e8..ee02e2205d 100644 --- a/apps/server/test/system/experiments.test.ts +++ b/apps/server/test/system/experiments.test.ts @@ -8,12 +8,17 @@ import { seedHostSession } from "../helpers/seed.js"; import { withTestHarness } from "../helpers/test-app.js"; describe("experiments settings", () => { - it("serves the shipped experiment defaults in /system/config", async () => { + it("omits never-saved experiments from /system/config", async () => { await withTestHarness(async (harness) => { const response = await harness.app.request("/api/v1/system/config"); expect(response.status).toBe(200); const body = systemConfigResponseSchema.parse(await readJson(response)); - expect(body.experiments).toEqual({ + // Omission is the contract: a client cannot otherwise distinguish + // "user chose false" from "user never chose", and the compact-viewport + // windowing default rests on that distinction. + expect(body.experiments).toEqual({}); + // Server-internal policy still reads concrete booleans. + expect(getExperiments(harness.db)).toEqual({ changelogPreview: false, editMessages: true, mobileApp: false, diff --git a/docs/configuration.md b/docs/configuration.md index a946129738..485741ca3e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -642,9 +642,12 @@ turns, commands, agents, workflows, and monitors keep their sessions loaded. The experiment does not gate release: BB releases idle Codex sessions with the experiment off, which is the behavior it had before this setting. -The `timelineWindowing` experiment is off by default. When enabled, long -timelines and large expanded timeline details retain stable height-preserving -wrappers while mounting only rows near their active scrollport. Toggle it with +The `timelineWindowing` experiment mounts only rows near the active +scrollport of long timelines and large expanded timeline details, behind +stable height-preserving wrappers. Compact viewports (phones) window by +default and treat the experiment as a kill switch: an explicitly `false` +value disables windowing there too. Desktop keeps the off default until the +experiment is enabled. Toggle it with `bb settings experiment timelineWindowing `. ## Thread Timeline Window diff --git a/packages/db/src/data/experiments.ts b/packages/db/src/data/experiments.ts index c8f682c0ba..2b691d05ef 100644 --- a/packages/db/src/data/experiments.ts +++ b/packages/db/src/data/experiments.ts @@ -4,12 +4,23 @@ import { experimentKeys, experimentKeySchema, type Experiments, + type StoredExperiments, } from "@bb/domain"; import type { DbConnection } from "../connection.js"; import { systemExperiments } from "../schema.js"; export function getExperiments(db: DbConnection): Experiments { - const experiments = { ...defaultExperiments }; + return { ...defaultExperiments, ...getStoredExperiments(db) }; +} + +/** + * Only the persisted choices, with never-saved keys omitted. `/system/config` + * serves this so clients can distinguish "user chose false" from "user never + * chose"; server-internal policy keeps using `getExperiments` for concrete + * booleans. + */ +export function getStoredExperiments(db: DbConnection): StoredExperiments { + const experiments: StoredExperiments = {}; const rows = db .select() .from(systemExperiments) diff --git a/packages/db/src/data/index.ts b/packages/db/src/data/index.ts index 66d44757f7..936af75efa 100644 --- a/packages/db/src/data/index.ts +++ b/packages/db/src/data/index.ts @@ -108,7 +108,11 @@ export { setAppSettings, } from "./app-settings.js"; export { getStoredThreadTabs, replaceStoredThreadTabs } from "./thread-tabs.js"; -export { getExperiments, setExperiments } from "./experiments.js"; +export { + getExperiments, + getStoredExperiments, + setExperiments, +} from "./experiments.js"; export { deleteInstalledPlugin, getInstalledPlugin, diff --git a/packages/db/test/experiments.test.ts b/packages/db/test/experiments.test.ts index b4bb1d0ad3..ef56250263 100644 --- a/packages/db/test/experiments.test.ts +++ b/packages/db/test/experiments.test.ts @@ -3,6 +3,7 @@ import { defaultExperiments } from "@bb/domain"; import { createConnection, getExperiments, + getStoredExperiments, migrate, setExperiments, } from "../src/index.js"; @@ -14,12 +15,15 @@ describe("experiments", () => { try { migrate(db); expect(getExperiments(db)).toEqual(defaultExperiments); + // Never-saved keys are omitted from the stored view, not defaulted. + expect(getStoredExperiments(db)).toEqual({}); const experiments = { ...defaultExperiments, mobileApp: true, }; setExperiments(db, experiments); + expect(getStoredExperiments(db)).toEqual(experiments); db.$client .prepare( "INSERT INTO system_experiments (key, value, updated_at) VALUES ('futureExperiment', true, 1)", diff --git a/packages/domain/src/experiments.ts b/packages/domain/src/experiments.ts index be91da5eac..ccc9143106 100644 --- a/packages/domain/src/experiments.ts +++ b/packages/domain/src/experiments.ts @@ -23,6 +23,21 @@ export type ExperimentKey = z.infer; export const experimentsSchema = z.record(experimentKeySchema, z.boolean()); export type Experiments = z.infer; +/** + * The user's persisted experiment choices only. Omission means the user never + * saved a value for the key — real semantics, not a hidden default: consumers + * that need a concrete boolean overlay `defaultExperiments`, while + * viewport-dependent defaults (`timelineWindowing` on compact viewports) + * resolve client-side where the viewport is known. `setExperiments` persists + * every key, so after the first explicit save the stored values win + * everywhere. + */ +export const storedExperimentsSchema = z.partialRecord( + experimentKeySchema, + z.boolean(), +); +export type StoredExperiments = z.infer; + /** * Values for an installation that has never saved a toggle. `setExperiments` * persists every key, so one that has keeps its stored values instead. diff --git a/packages/server-contract/src/api/system.ts b/packages/server-contract/src/api/system.ts index 8e557cc48d..65d8f84ee4 100644 --- a/packages/server-contract/src/api/system.ts +++ b/packages/server-contract/src/api/system.ts @@ -6,11 +6,11 @@ import { appKeybindingsSchema, appThemeSchema, availableModelSchema, - experimentsSchema, featureFlagsSchema, permissionModeSchema, pluginThemeMetaSchema, providerInfoSchema, + storedExperimentsSchema, } from "@bb/domain"; import { providerHealthSchema as providerHealthSchema } from "@bb/provider-bridge-protocol/provider-maintenance"; import { hostPlatformSchema } from "@bb/host-daemon-contract/local"; @@ -177,8 +177,13 @@ export const systemConfigResponseSchema = z.object({ defaultKeybindings: appDefaultKeybindingsSchema, /** Sparse per-command customizations; null shortcuts explicitly disable commands. */ keybindingOverrides: appKeybindingOverridesSchema, - /** User-opt-in experiments (Settings → Experiments), persisted server-side. */ - experiments: experimentsSchema, + /** + * User-opt-in experiments (Settings → Experiments), persisted server-side. + * Only saved choices appear; an omitted key means the user never chose, so + * clients apply `defaultExperiments` — or a viewport-dependent default + * (`timelineWindowing` defaults on for compact viewports). + */ + experiments: storedExperimentsSchema, /** Active app-wide palette (built-in id or custom theme), resolved server-side. */ appearance: appThemeSchema, /** diff --git a/plugins/connect/src/server.ts b/plugins/connect/src/server.ts index bdaf95bffb..edeb05b424 100644 --- a/plugins/connect/src/server.ts +++ b/plugins/connect/src/server.ts @@ -50,9 +50,12 @@ export default async function plugin(bb: BbPluginApi) { }); // Experiments are server-owned settings; the plugin reads them through its - // loopback SDK binding rather than through a dedicated plugin API. + // loopback SDK binding rather than through a dedicated plugin API. The + // config payload carries stored choices only, and a never-saved mobileApp + // key means the default (off), so the gate fails closed. const mobilePairing: MobilePairingGate = { - enabled: async () => (await bb.sdk.system.config()).experiments.mobileApp, + enabled: async () => + (await bb.sdk.system.config()).experiments.mobileApp ?? false, }; bb.rpc.register(