From e681ea97d8c400d04106eb703853ab5decec55a1 Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Mon, 24 Aug 2026 14:02:13 +0200 Subject: [PATCH 1/5] Re-read timeline windowing geometry per trigger, not per commit TimelineWindowedItems re-read its scroll geometry (two getBoundingClientRect calls plus scrollTop) in a dependency-less layout effect after every React commit, which forced a layout per streaming delta. Replace it with a geometry-revision scheme: expand/collapse paths (ExpandableTimelineRow's isExpanded transitions) bump a revision through TimelineWindowingGeometryInvalidateContext, and the windowed list re-reads only when the revision, its item count, or scroll-root usability changes. A trigger-string ref leaves the mount read to the ResizeObserver effect so each trigger costs one geometry read; the 0.5px setScrollMargin guard is unchanged. New tests prove commits without a geometry trigger no longer force a layout read, that a revision bump re-reads and repositions the window when the owning row's expansion moves a nested list, and that a real row toggle inside ThreadTimelineRows still refreshes geometry. Co-Authored-By: Claude Fable 5 --- .../thread/timeline/ExpandableTimelineRow.tsx | 16 +++ .../thread/timeline/ThreadTimelineRows.tsx | 66 ++++++---- .../ThreadTimelineRows.windowing.test.tsx | 71 ++++++++++- .../timeline/TimelineWindowedItems.test.tsx | 119 ++++++++++++++---- .../thread/timeline/TimelineWindowedItems.tsx | 25 +++- .../timeline/TimelineWindowedItemsLoader.tsx | 16 +++ 6 files changed, 260 insertions(+), 53 deletions(-) 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..a145339383 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, @@ -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..d8cfb6adff 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,73 @@ describe("ThreadTimelineRows windowing experiment", () => { }), ); }); + + 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/TimelineWindowedItems.test.tsx b/apps/app/src/components/thread/timeline/TimelineWindowedItems.test.tsx index 25a96d572e..7503d72f83 100644 --- a/apps/app/src/components/thread/timeline/TimelineWindowedItems.test.tsx +++ b/apps/app/src/components/thread/timeline/TimelineWindowedItems.test.tsx @@ -13,12 +13,17 @@ 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 { @@ -55,42 +60,64 @@ function renderWindowedItems(options?: { configurable: true, value: options?.clientHeight ?? 96, }); - return { - ...render( + // 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) => ( + 32} + estimateItemHeight={estimateItemHeight} gap={0} - getScrollElement={() => scrollElement} + getScrollElement={getScrollElement} itemKeys={ITEM_KEYS} measurements={measurements} - renderItem={(index: number, state: TimelineWindowedItemRenderState) => ( -
- {state.isRealized ? ( - - ) : null} -
- )} - />, - { container: scrollElement }, - ), + renderItem={renderItem} + /> + + ); + let geometryRevision = 0; + const view = render(buildElement(geometryRevision), { + container: scrollElement, + }); + return { + ...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(); + spacerOffsetTop = 0; scrollElement = document.createElement("div"); document.body.append(scrollElement); Object.defineProperty(scrollElement, "clientWidth", { @@ -110,7 +137,7 @@ beforeEach(() => { 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, ); } @@ -255,4 +282,44 @@ describe("TimelineWindowedItems", () => { expect(screen.getAllByTestId(/^content-/)).toHaveLength(100), ); }); + + 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..cafddda2fa 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,7 +14,10 @@ import { type Range, type Virtualizer, } from "@tanstack/react-virtual"; -import type { TimelineWindowedItemsProps } from "./TimelineWindowedItemsLoader.js"; +import { + TimelineWindowingGeometryRevisionContext, + type TimelineWindowedItemsProps, +} from "./TimelineWindowedItemsLoader.js"; export type { TimelineWindowedItemRenderState } from "./TimelineWindowedItemsLoader.js"; @@ -95,10 +99,12 @@ export function TimelineWindowedItems({ }: TimelineWindowedItemsProps) { const configured = enabled && itemKeys.length >= minItemCount && getScrollElement !== null; + 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, @@ -264,8 +270,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..5776f7073b 100644 --- a/apps/app/src/components/thread/timeline/TimelineWindowedItemsLoader.tsx +++ b/apps/app/src/components/thread/timeline/TimelineWindowedItemsLoader.tsx @@ -24,6 +24,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; From c850dd692616c600a1ca41cfc209f9d0afed49cf Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Mon, 24 Aug 2026 17:28:53 +0200 Subject: [PATCH 2/5] Preload the windowing chunk at boot and bound the loader fallback First open of a long thread on a cold connection paid a serialized boot parse -> route chunk -> windowed chunk download while the Suspense fallback mounted and measured every loaded row, then remounted the whole list once TanStack landed. Warm the TimelineWindowedItems chunk from App.tsx next to the SplitWorkspaceRoute preload (still a dynamic import, so it stays out of the boot closure), and bound the captureMeasurements fallback to the trailing 60 rows - the bottom-anchor region a thread opens into. Verified with the timeline suites and check-bundle-budget: boot payload 443.2 / 467.8 KiB brotli, and the TimelineWindowedItems chunk appears in neither bootChunks nor the SplitWorkspaceRoute closure. Co-Authored-By: Claude Fable 5 --- apps/app/src/App.tsx | 7 +++++++ .../timeline/TimelineWindowedItems.test.tsx | 8 ++++++-- .../timeline/TimelineWindowedItemsLoader.tsx | 19 +++++++++++++++---- 3 files changed, 28 insertions(+), 6 deletions(-) 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/TimelineWindowedItems.test.tsx b/apps/app/src/components/thread/timeline/TimelineWindowedItems.test.tsx index 7503d72f83..06e8643b74 100644 --- a/apps/app/src/components/thread/timeline/TimelineWindowedItems.test.tsx +++ b/apps/app/src/components/thread/timeline/TimelineWindowedItems.test.tsx @@ -162,7 +162,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( @@ -184,8 +184,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", () => { diff --git a/apps/app/src/components/thread/timeline/TimelineWindowedItemsLoader.tsx b/apps/app/src/components/thread/timeline/TimelineWindowedItemsLoader.tsx index 5776f7073b..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 { @@ -74,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 @@ -94,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. */ From 8b0e8006293d2a4d17e01098dd0d07a6c47939bf Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Mon, 24 Aug 2026 17:31:38 +0200 Subject: [PATCH 3/5] Phone-tune timeline windowing thresholds The 40-row compact minimum and 8-row overscan were desktop numbers: 39 rows on a phone never windowed at all, and 16 retained rich rows of overscan is most of a phone's DOM budget. Window top-level lists from 16 rows on compact viewports (desktop stays 60, nested stays 20) and halve overscan to 4 there, using the same useIsCompactViewport signal that already drives the row minimum. Verified with new tests: a 20-row compact top-level timeline now windows, and the compact render retains exactly one overscan side's worth fewer wrappers than the desktop render of the same geometry. Co-Authored-By: Claude Fable 5 --- .../thread/timeline/ThreadTimelineRows.tsx | 2 +- .../ThreadTimelineRows.windowing.test.tsx | 49 ++++++++++++++++ .../timeline/TimelineWindowedItems.test.tsx | 58 ++++++++++++++----- .../thread/timeline/TimelineWindowedItems.tsx | 12 +++- 4 files changed, 104 insertions(+), 17 deletions(-) diff --git a/apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx b/apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx index a145339383..029af31304 100644 --- a/apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx +++ b/apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx @@ -2171,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]; 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 d8cfb6adff..7e0b30e4e7 100644 --- a/apps/app/src/components/thread/timeline/ThreadTimelineRows.windowing.test.tsx +++ b/apps/app/src/components/thread/timeline/ThreadTimelineRows.windowing.test.tsx @@ -243,6 +243,55 @@ 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", ""); diff --git a/apps/app/src/components/thread/timeline/TimelineWindowedItems.test.tsx b/apps/app/src/components/thread/timeline/TimelineWindowedItems.test.tsx index 06e8643b74..0bf29d1bbf 100644 --- a/apps/app/src/components/thread/timeline/TimelineWindowedItems.test.tsx +++ b/apps/app/src/components/thread/timeline/TimelineWindowedItems.test.tsx @@ -9,6 +9,7 @@ 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, @@ -49,6 +50,7 @@ function renderWindowedItems(options?: { alwaysMountedKeys?: ReadonlySet; clientHeight?: number; enabled?: boolean; + isCompactViewport?: boolean; measurements?: Map; }) { const measurements = options?.measurements ?? new Map(); @@ -84,18 +86,24 @@ function renderWindowedItems(options?: { ); const buildElement = (geometryRevision: number) => ( - - - + + + + + ); let geometryRevision = 0; const view = render(buildElement(geometryRevision), { @@ -115,9 +123,7 @@ function renderWindowedItems(options?: { }; } -beforeEach(() => { - itemHeights = new Map(); - spacerOffsetTop = 0; +function createScrollElement() { scrollElement = document.createElement("div"); document.body.append(scrollElement); Object.defineProperty(scrollElement, "clientWidth", { @@ -132,6 +138,12 @@ 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); @@ -287,6 +299,22 @@ describe("TimelineWindowedItems", () => { ); }); + 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()); diff --git a/apps/app/src/components/thread/timeline/TimelineWindowedItems.tsx b/apps/app/src/components/thread/timeline/TimelineWindowedItems.tsx index cafddda2fa..288b05c607 100644 --- a/apps/app/src/components/thread/timeline/TimelineWindowedItems.tsx +++ b/apps/app/src/components/thread/timeline/TimelineWindowedItems.tsx @@ -14,6 +14,7 @@ import { type Range, type Virtualizer, } from "@tanstack/react-virtual"; +import { useIsCompactViewport } from "@bb/shared-ui/hooks/use-compact-viewport"; import { TimelineWindowingGeometryRevisionContext, type TimelineWindowedItemsProps, @@ -23,6 +24,12 @@ export type { TimelineWindowedItemRenderState } from "./TimelineWindowedItemsLoa /** 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. */ @@ -99,6 +106,7 @@ 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); @@ -213,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, From 0631d8ead55df862d02ae6fd4b52b292b25c4f2c Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Mon, 24 Aug 2026 17:35:24 +0200 Subject: [PATCH 4/5] Default timeline windowing on for compact viewports Phones are where the unwindowed timeline hangs (1,519-5,249 DOM nodes on a 402x874 viewport in mobile hang traces), yet windowing shipped behind a default-off experiment. Resolve the enablement at ThreadTimelineSurface as `experiment ?? isCompactViewport`: compact viewports window whenever the experiment is unset, an explicitly false experiment still disables windowing everywhere (the kill switch), and desktop keeps the served value. `defaultExperiments.timelineWindowing` stays false, so server value semantics are unchanged. New surface tests assert compact default-on (fails before this change), the explicit-false opt-out, the unchanged desktop default, and the explicit-true opt-in. docs/configuration.md and the bb-cli skill surfaces now describe the compact default and kill-switch semantics. Co-Authored-By: Claude Fable 5 --- .../thread/timeline/ThreadTimelineSurface.tsx | 8 +- ...TimelineSurface.windowing-default.test.tsx | 118 ++++++++++++++++++ .../skills/builtin-skills/bb-cli/SKILL.md | 5 +- .../bb-cli/references/app-settings.md | 6 +- docs/configuration.md | 9 +- 5 files changed, 138 insertions(+), 8 deletions(-) create mode 100644 apps/app/src/components/thread/timeline/ThreadTimelineSurface.windowing-default.test.tsx 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..7c31d70bc2 --- /dev/null +++ b/apps/app/src/components/thread/timeline/ThreadTimelineSurface.windowing-default.test.tsx @@ -0,0 +1,118 @@ +// @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 } 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 Experiments | 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("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/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/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 From f1568d997dd6626ffae5bb7d1543122b91515cbc Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Tue, 25 Aug 2026 08:32:08 +0200 Subject: [PATCH 5/5] Serve only stored experiment choices from /system/config The config payload materialized every experiment from defaultExperiments, so a client could not distinguish "user chose false" from "user never chose" - and the compact-viewport windowing default rests on exactly that distinction: a fresh install's served false would defeat default-on before the user ever touched Settings. Omission now carries the meaning (AGENTS.md's optional-field rule: real semantics, not a hidden default). The /system/config experiments payload becomes a partial record (storedExperimentsSchema / getStoredExperiments) serving persisted rows only; setExperiments still writes every key on a save, so after the first explicit choice the stored values win everywhere. Server-internal policy (runtime policy, edit-message gating) keeps reading concrete booleans through getExperiments. Consumers that need a concrete boolean overlay defaultExperiments at their boundary: the Settings views (web and mobile), the editMessages readers (its default is true, so `?? false` would have regressed fresh installs), and the CLI's experiment update; the connect plugin's mobileApp gate fails closed. The offline fallback config omits timelineWindowing so an unreachable server still yields the compact windowing default. The experiments PUT contract is unchanged, and nothing crosses the server/host-daemon wire, so HOST_DAEMON_PROTOCOL_VERSION is untouched. New tests: /system/config omits never-saved experiments while getExperiments stays concrete (fails before: the payload carried all five defaults); getStoredExperiments returns only persisted rows; a compact surface windows when the payload omits the key. Co-Authored-By: Claude Fable 5 --- ...readTimelineSurface.windowing-default.test.tsx | 13 +++++++++++-- apps/app/src/lib/system-config-atoms.ts | 4 +++- apps/app/src/views/SettingsView.tsx | 7 ++++++- .../src/views/thread-detail/ThreadDetailView.tsx | 5 ++++- apps/cli/src/commands/settings.ts | 7 ++++++- .../thread-runtime/thread-runtime-mutations.ts | 7 +++++-- .../settings/ExperimentsSettingsScreen.tsx | 7 ++++++- apps/server/src/routes/system.ts | 7 ++++++- apps/server/test/system/experiments.test.ts | 9 +++++++-- packages/db/src/data/experiments.ts | 13 ++++++++++++- packages/db/src/data/index.ts | 6 +++++- packages/db/test/experiments.test.ts | 4 ++++ packages/domain/src/experiments.ts | 15 +++++++++++++++ packages/server-contract/src/api/system.ts | 11 ++++++++--- plugins/connect/src/server.ts | 7 +++++-- 15 files changed, 103 insertions(+), 19 deletions(-) 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 index 7c31d70bc2..6c02af9cf8 100644 --- a/apps/app/src/components/thread/timeline/ThreadTimelineSurface.windowing-default.test.tsx +++ b/apps/app/src/components/thread/timeline/ThreadTimelineSurface.windowing-default.test.tsx @@ -3,13 +3,13 @@ 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 } from "@bb/domain"; +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 Experiments | undefined, + experiments: undefined as StoredExperiments | undefined, })); vi.mock("@/hooks/queries/system-queries", () => ({ @@ -95,6 +95,15 @@ describe("ThreadTimelineSurface windowing default", () => { 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 }); 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/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/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(