diff --git a/design-system/AGENTS.md b/design-system/AGENTS.md index 2cc72f685c..1e21892aec 100644 --- a/design-system/AGENTS.md +++ b/design-system/AGENTS.md @@ -17,6 +17,12 @@ This file applies to `design-system/**`. Repository-wide rules in the root `AGEN - Design Lab may alias `@openbitfun/ui` to source only during Vite development for HMR. Its production build must consume package exports. - `@openbitfun/ui/registry` is the source of truth for published components. Design Lab derives navigation, counts, token scopes, and detail routes from that registry; Lab-only previews or copy must never add, retain, or remove a package component. +## CSS invalidation + +- Prefer component-owned classes for sibling rules in dynamic views; tag-only or universal sibling selectors can invalidate unrelated elements even under a scoped ancestor or CSS Module. +- Use layout `gap` only where spacing semantics remain equivalent. Preserve adjacency, specificity, and state overrides when narrowing selectors; `:where()` can add class constraints without raising specificity. +- Use Selector Stats for cause analysis and ordinary traces for performance comparisons. Invalidation records alone do not prove a scrolling speedup. See `src/web-ui/AGENTS.md` for generated-content guidance; do not blanket-ban sibling selectors. + ## Publication boundary - Public manifests expose only `dist/`, README, and package metadata. diff --git a/design-system/packages/ui/README.md b/design-system/packages/ui/README.md index a29c4c7e97..3aa189fa36 100644 --- a/design-system/packages/ui/README.md +++ b/design-system/packages/ui/README.md @@ -189,6 +189,18 @@ overrides that text; `title=""` opts out when a surrounding native title owns th content. An explicit enclosing `Tooltip` suppresses automatic nested tooltips. Do not use marquee as the sole way to access information on touch surfaces. +Overflow measurement is deferred to a shared animation-frame queue per window. +Mount, content, resize, and font notifications coalesce; all queued labels read +geometry before publishing state. Unmounted labels cancel their pending work. +Overflow indicators and automatic tooltips become available after that frame, +while the complete accessible text is present immediately. This avoids forcing +layout separately inside each label's React mount effect. +Requests made during a batch survive for the next frame; cancellation also +discards unpublished results. A failing label does not abort other labels, +and its error is reported asynchronously. This scheduling contract does not +claim an overall scrolling speedup; first-frame visual behavior requires +browser validation. + Multi-line descriptions should normally wrap. Editable fields, source code, structured paths that need to preserve their suffix, and native controls keep their appropriate text treatment instead of receiving a blanket fade rule. diff --git a/design-system/packages/ui/src/components/NumberInput/NumberInput.module.css b/design-system/packages/ui/src/components/NumberInput/NumberInput.module.css index a75e39ce0c..35578ce10d 100644 --- a/design-system/packages/ui/src/components/NumberInput/NumberInput.module.css +++ b/design-system/packages/ui/src/components/NumberInput/NumberInput.module.css @@ -17,7 +17,11 @@ .root[data-disabled="true"] .unit { color: var(--openbitfun-color-content-disabled); } .root[data-variant="default"] .buttons { grid-template-columns: 1fr; grid-template-rows: repeat(2, minmax(0, 1fr)); } .root[data-variant="default"] .buttons button { min-inline-size: 20px; font-size: var(--openbitfun-type-meta-font-size); line-height: var(--openbitfun-type-meta-line-height); } - .root[data-variant="default"] .buttons button + button { border-block-start: var(--openbitfun-border-width-default) solid var(--openbitfun-color-field-border); } + /* Chromium's former button + button invalidation set also selected unrelated + transcript buttons. Scope sibling invalidation to this component's step + buttons. A desktop trace recorded 289 tag invalidations across 77 nodes + with zero final selector matches; an overall scroll speedup is unconfirmed. */ + .root[data-variant="default"] .buttons .stepButton + .stepButton { border-block-start: var(--openbitfun-border-width-default) solid var(--openbitfun-color-field-border); } @media (prefers-reduced-motion: reduce) { .control { transition: none; } } @media (forced-colors: active) { .control { border-color: ButtonText; } .root .control:focus-within { border-color: Highlight; } } } diff --git a/design-system/packages/ui/src/components/NumberInput/NumberInput.tsx b/design-system/packages/ui/src/components/NumberInput/NumberInput.tsx index cadcf33f9d..1d7c2f3afa 100644 --- a/design-system/packages/ui/src/components/NumberInput/NumberInput.tsx +++ b/design-system/packages/ui/src/components/NumberInput/NumberInput.tsx @@ -147,8 +147,8 @@ export const NumberInput = forwardRef(functi {unit && {unit}} {showButtons && variant !== "compact" && ( - - + + )} diff --git a/design-system/packages/ui/src/primitives/OverflowText/OverflowText.tsx b/design-system/packages/ui/src/primitives/OverflowText/OverflowText.tsx index 090940a449..6587a20c2a 100644 --- a/design-system/packages/ui/src/primitives/OverflowText/OverflowText.tsx +++ b/design-system/packages/ui/src/primitives/OverflowText/OverflowText.tsx @@ -15,6 +15,7 @@ import { classNames } from "../../internal/classNames"; import { TooltipTriggerContext } from "../../internal/tooltipTriggerContext"; import { Tooltip } from "../../components/Tooltip"; import styles from "./OverflowText.module.css"; +import { cancelOverflowMeasurement, scheduleOverflowMeasurement } from "./overflowMeasurementQueue"; const useIsomorphicLayoutEffect = typeof window === "undefined" ? useEffect @@ -86,7 +87,7 @@ export const OverflowText = forwardRef( assignRef(forwardedRef, element); }, [forwardedRef]); - const updateOverflow = useCallback(() => { + const readOverflow = useCallback(() => { const element = elementRef.current; const content = contentRef.current ?? element; if (!element || !content) return; @@ -104,10 +105,17 @@ export const OverflowText = forwardRef( if (current.distance === distance && current.isOverflowing === isOverflowing) return; const next = { distance, isOverflowing }; - measurementRef.current = next; - setMeasurement(next); + return () => { + measurementRef.current = next; + setMeasurement(next); + }; }, [lines]); + const updateOverflow = useCallback(() => { + const view = elementRef.current?.ownerDocument.defaultView; + if (view) scheduleOverflowMeasurement(view, readOverflow); + }, [readOverflow]); + const prepareTooltip = useCallback(() => { const element = elementRef.current; const trigger = triggerRef.current; @@ -130,7 +138,9 @@ export const OverflowText = forwardRef( useIsomorphicLayoutEffect(() => { updateOverflow(); - }, [behavior, children, lines, overflowStyle, updateOverflow]); + const view = elementRef.current?.ownerDocument.defaultView; + return () => { if (view) cancelOverflowMeasurement(view, readOverflow); }; + }, [behavior, children, lines, overflowStyle, readOverflow, updateOverflow]); useEffect(() => { if (measurementRef.current.isOverflowing) prepareTooltip(); diff --git a/design-system/packages/ui/src/primitives/OverflowText/overflowMeasurementQueue.ts b/design-system/packages/ui/src/primitives/OverflowText/overflowMeasurementQueue.ts new file mode 100644 index 0000000000..5228be955d --- /dev/null +++ b/design-system/packages/ui/src/primitives/OverflowText/overflowMeasurementQueue.ts @@ -0,0 +1,69 @@ +type ReadMeasurement = () => (() => void) | undefined; + +interface MeasurementJob { + read: ReadMeasurement; + cancelled: boolean; +} +interface MeasurementQueue { + pending: Map; + active: Map; + frame: number | null; +} + +const queues = new WeakMap(); + +/** Batch geometry reads before React state publication. Overflow UI may update + * on the next frame; this is a scheduling contract, not a scroll speed guarantee. + */ +export function scheduleOverflowMeasurement(view: Window, read: ReadMeasurement): void { + let queue = queues.get(view); + if (!queue) { + queue = { pending: new Map(), active: new Map(), frame: null }; + queues.set(view, queue); + } + // A new request supersedes any result already read in the current batch. + const active = queue.active.get(read); + if (active) active.cancelled = true; + queue.pending.set(read, { read, cancelled: false }); + if (queue.frame !== null) return; + const current = queue; + current.frame = view.requestAnimationFrame(() => { + current.frame = null; + // Detach this batch so reentrant requests survive for the next frame. + const batch = current.pending; + current.pending = new Map(); + current.active = batch; + const publications: Array<{ job: MeasurementJob; publish: () => void }> = []; + const report = (error: unknown) => { + // Preserve uncaught-error reporting without aborting other labels. + view.setTimeout(() => { throw error; }, 0); + }; + try { + for (const job of batch.values()) { + if (job.cancelled) continue; + try { + const publish = job.read(); + if (publish) publications.push({ job, publish }); + } catch (error) { report(error); } + } + for (const { job, publish } of publications) { + if (job.cancelled) continue; + try { publish(); } catch (error) { report(error); } + } + } finally { + current.active = new Map(); + } + }); +} + +export function cancelOverflowMeasurement(view: Window, read: ReadMeasurement): void { + const queue = queues.get(view); + if (!queue) return; + const active = queue.active.get(read); + if (active) active.cancelled = true; + queue.pending.delete(read); + if (queue.pending.size === 0 && queue.frame !== null) { + view.cancelAnimationFrame(queue.frame); + queue.frame = null; + } +} diff --git a/design-system/packages/ui/tests/overflow-text.test.mjs b/design-system/packages/ui/tests/overflow-text.test.mjs index 4a57456491..53d2489bcf 100644 --- a/design-system/packages/ui/tests/overflow-text.test.mjs +++ b/design-system/packages/ui/tests/overflow-text.test.mjs @@ -64,8 +64,9 @@ test("OverflowText measures real clipping for fade and marquee treatments", asyn assert.match(source, /measurementRef\.current/); assert.match( source, - /useIsomorphicLayoutEffect\(\(\) => \{\s*updateOverflow\(\);\s*\}, \[behavior, children, lines, overflowStyle, updateOverflow\]\);/s, + /scheduleOverflowMeasurement\(view, readOverflow\)/, ); + assert.match(source, /cancelOverflowMeasurement\(view, readOverflow\)/); assert.match(source, /new ResizeObserver\(updateOverflow\)/); assert.match(source, /resizeObserver\?\.observe\(contentRef\.current\)/); assert.match(source, /--_overflow-text-marquee-distance/); diff --git a/src/web-ui/AGENTS-CN.md b/src/web-ui/AGENTS-CN.md index b00d94d96e..d9b3be5a63 100644 --- a/src/web-ui/AGENTS-CN.md +++ b/src/web-ui/AGENTS-CN.md @@ -45,6 +45,13 @@ Remote Connect 使用全局 GitHub 账户和官方版本化 Relay。账户控件 `i18nService.t(...)` 必须有 bootstrap namespace 覆盖。 - 遵循 `src/web-ui/LOGGING.md`：仅英文、无 emoji、结构化日志 +## 动态界面的 CSS 失效范围 + +- 对频繁增删 DOM 的组件自有兄弟关系样式，优先使用专属类名，避免仅按标签或通配符匹配。CSS Modules 和祖先作用域不能隔离浏览器的样式失效工作。 +- 现有布局与间距语义允许时使用 flex/grid 的 `gap`；不要机械地将同类相邻关系替换为 `:not(:first-child)`，也不要仅为此切换布局模式。 +- 生成内容可以保留语义标签选择器。对实测热点添加渲染器专属类名；用 `:where(.owned-class)` 限定复合选择器时可保持原优先级。保留内嵌 HTML、嵌套列表、混合表格单元格及数学内容的行为。 +- 改写后核对匹配集合与层叠效果。短时 Selector Stats 录制用于定位失效原因，普通录制用于验证收益；失效次数不等于可感知提升。不全面禁止兄弟选择器。 + ## 命令 这里只维护开发/构建入口；验证命令统一放在下方“验证”章节。 diff --git a/src/web-ui/AGENTS.md b/src/web-ui/AGENTS.md index 2b125b2805..1edaad58d8 100644 --- a/src/web-ui/AGENTS.md +++ b/src/web-ui/AGENTS.md @@ -56,6 +56,13 @@ workspace connections remain independent of Relay sign-in. namespace coverage. - Follow `src/web-ui/LOGGING.md`: English only, no emojis, structured logs +## CSS invalidation in dynamic views + +- For component-owned sibling styles in frequently changing DOM, prefer dedicated classes over tag-only or universal sibling selectors. CSS Modules and an ancestor scope do not isolate browser invalidation work. +- Use flex/grid `gap` when the existing layout and spacing semantics permit it. Do not mechanically replace same-tag adjacency with `:not(:first-child)` or change layout mode. +- Generated content may retain semantic tag selectors. For measured hotspots, add renderer-owned classes; `:where(.owned-class)` can narrow a compound selector without increasing specificity. Preserve raw HTML, nested lists, mixed table cells, and math rendering behavior. +- Check matching and cascade equivalence when rewriting selectors. Use short Selector Stats captures to identify invalidation causes and ordinary traces to measure speedups; invalidation counts alone do not establish user-visible gains. Do not impose a blanket ban on sibling selectors. + ## Commands Keep development/build entry points here. Verification commands are maintained diff --git a/src/web-ui/eslint.fence.regression.test.ts b/src/web-ui/eslint.fence.regression.test.ts index 3c4aa40e0b..5b5c540a78 100644 --- a/src/web-ui/eslint.fence.regression.test.ts +++ b/src/web-ui/eslint.fence.regression.test.ts @@ -16,7 +16,7 @@ * `adapters/**`. Run via `pnpm vitest run`. */ import { describe, expect, it } from 'vitest'; -import { spawnSync } from 'node:child_process'; +import { ESLint } from 'eslint'; import { resolve } from 'node:path'; const webUiRoot = resolve(__dirname); @@ -69,43 +69,27 @@ const cases: ProbeCase[] = [ }, ]; -/** - * Resolve the eslint CLI entry as an absolute path and run it with `node`, - * without a shell. Going through `pnpm`/`pnpm.cmd` needed `shell: true` on - * Windows (a `.cmd` shim cannot be spawned with `shell: false`), which triggers - * Node's DEP0190 security deprecation. Running the eslint JS entry directly - * via `node` keeps `shell: false` on every platform and avoids the warning. - */ -function eslintBinPath(): string { - return resolve(webUiRoot, 'node_modules/eslint/bin/eslint.js'); -} +// Exercise the real file-selected config with one ESLint instance. Spawning a +// fresh CLI for each probe made cold Node/config loading exceed the test budget +// under the full suite's worker load. Load config during module collection, +// like the other imports, and keep assertions about the actual rule outcomes. +const eslint = new ESLint({ cwd: webUiRoot }); +await eslint.calculateConfigForFile(cases[0].filename); -function lintProbe(probe: ProbeCase): { hasError: boolean; output: string } { - // --stdin + --stdin-filename make the rule's path selectors see the probe as - // if it lived at that path, so the fence applies per the probe's location. - const args = [ - eslintBinPath(), - '--stdin', - '--stdin-filename', - probe.filename, - ]; - const result = spawnSync(process.execPath, args, { - cwd: webUiRoot, - input: probe.source, - encoding: 'utf8', - shell: false, - }); - const combined = `${result.stdout ?? ''}${result.stderr ?? ''}`; - // ESLint exits non-zero and reports the restricted-imports/syntax error when - // the fence fires; a clean probe exits 0 with no error lines. - const hasError = /no-restricted-(imports|syntax)/.test(combined); - return { hasError, output: combined }; +async function lintProbe(probe: ProbeCase): Promise<{ hasError: boolean; errorCount: number; output: string }> { + const results = await eslint.lintText(probe.source, { filePath: probe.filename }); + const messages = results.flatMap(result => result.messages); + return { + hasError: messages.some(message => /^no-restricted-(imports|syntax)$/.test(message.ruleId ?? '')), + errorCount: results.reduce((count, result) => count + result.errorCount, 0), + output: messages.map(message => `${message.ruleId ?? 'config'}: ${message.message}`).join('\n'), + }; } describe('adapter fence regression', () => { for (const probe of cases) { - it(probe.name, () => { - const { hasError, output } = lintProbe(probe); + it(probe.name, async () => { + const { hasError, errorCount, output } = await lintProbe(probe); if (probe.expectError) { expect( hasError, @@ -116,6 +100,7 @@ describe('adapter fence regression', () => { hasError, `expected the adapter exception to allow invoke at ${probe.filename}, but the fence fired:\n${output}`, ).toBe(false); + expect(errorCount, output).toBe(0); } }); } diff --git a/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.trust.test.tsx b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.trust.test.tsx index 9f82ad72a4..a06c83e6ef 100644 --- a/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.trust.test.tsx +++ b/src/web-ui/src/app/components/panels/review-platform/ReviewPlatformPanel.trust.test.tsx @@ -52,7 +52,7 @@ const error = new TauriCommandError('Command failed', { beforeEach(async () => { const { JSDOM } = await import('jsdom'); - dom = new JSDOM('', { url: 'http://localhost' }); + dom = new JSDOM('', { url: 'http://localhost', pretendToBeVisual: true }); vi.stubGlobal('window', dom.window); vi.stubGlobal('document', dom.window.document); vi.stubGlobal('localStorage', dom.window.localStorage); diff --git a/src/web-ui/src/features/dispatch/DispatchInstallDialog.scss b/src/web-ui/src/features/dispatch/DispatchInstallDialog.scss index d594c280e7..b6693c3061 100644 --- a/src/web-ui/src/features/dispatch/DispatchInstallDialog.scss +++ b/src/web-ui/src/features/dispatch/DispatchInstallDialog.scss @@ -100,16 +100,20 @@ display: grid; gap: 0; + // The former div + div rule invalidated unrelated transcript divs in Chromium. + // Match owned check-row classes instead. Equal-DOM trace sample after the + // metadata fix: 93 added / 1161 retained elements, style 46 -> 23ms and + // elementCount 3111 -> 1432 (single desktop run, not a performance guarantee). + > .dispatch-install-dialog__check-row + .dispatch-install-dialog__check-row { + border-top: 1px solid var(--openbitfun-color-border-subtle); + } + > div { display: grid; grid-template-columns: minmax(104px, auto) minmax(0, 1fr); gap: var(--openbitfun-space-2); padding: var(--openbitfun-space-2) 0; - + div { - border-top: 1px solid var(--openbitfun-color-border-subtle); - } - > span { color: var(--openbitfun-color-content-muted); font-size: var(--openbitfun-type-label-sm-font-size); diff --git a/src/web-ui/src/features/dispatch/DispatchInstallDialog.tsx b/src/web-ui/src/features/dispatch/DispatchInstallDialog.tsx index c7cb0365a2..dd2e2bc3c4 100644 --- a/src/web-ui/src/features/dispatch/DispatchInstallDialog.tsx +++ b/src/web-ui/src/features/dispatch/DispatchInstallDialog.tsx @@ -479,6 +479,7 @@ export const DispatchInstallDialog: React.FC = ({ {probe ? (
38:46` window change clearing `reached` while retaining +`armed=false`. The prepend snapshot matched and compensation succeeded, but the +first off-head observation was now false -> false. Re-arming required true -> +false, so a second visit to the head was refused indefinitely. A prefetch that +never reached the physical boundary had the same structural weakness. + +`flowChatHistoryPager.ts` now owns a state machine per direction: + +- `ready`: an initial ask is available once opening/follow ownership permits it. +- `requesting`: one ticket owns the asynchronous request; further reader demand + is coalesced into a boolean, never a queue of wheel events. +- `awaiting-layout`: an applied result has arrived, but its presentation commit + has not yet been acknowledged. +- `waiting-for-reader`: result and layout have both arrived; fresh demand can + dispatch another page while the boundary is within the lead. +- `exhausted`: no more content at the window/boundary that answered. + +The list reserves a ticket before invoking the handler. Its existing +`prepareViewportForPresentationCommit` callback checks that the ticket is still +current. A layout effect **after** measurement, prepend compensation, and +prepared navigation acknowledges the presentation. Layout can precede or follow +the promise result. A completion acknowledgement also renders when the returned +page was already projected. This acknowledges a commit, not convergence of all +future row measurements; later measurement callbacks cannot create reader demand. + +Directional wheel/touch/key intent can ask even at scrollTop=0, where no native +scroll event occurs. Native travel covers scrollbar dragging and inertia. The +viewport register subtracts its actual synchronous writes/shifts (including +clamping), so a delayed scroll event caused by compensation adds no demand. +Owned smooth navigation/follow scrolls are excluded. Content commits rebaseline +travel because replacing content can clamp the browser's scroll position. + +Demand accumulated during a fetch survives until the new layout is evaluated. +If the new boundary lies outside the lead, or the reader reverses direction, it +is discarded. Repeated rendering, measurement, and correction never manufacture +another page. A new gesture or travel toward the boundary can ask again without +requiring the reader to first hit, leave, and revisit the physical edge. + +Navigation invalidates request tickets. An old result cannot clear a newer +request or attach exhaustion to a different window. Non-applied outcomes leave +no pending ticket; errors/cancellation require a new reader action rather than +a render-driven retry loop. Exhaustion is forgotten when its window/boundary +changes. Session instances isolate the controller across session switches. + +Opening and follow-output guards remain synchronous (`isFollowingOutputNow`), +so the gesture that releases follow is evaluated against the new owner. Geometry +with no visible range remains a diagnostic refusal. Request/refusal diagnostics +report the phase and request identity without transcript content. + +Automated tests cover event ordering, consecutive real prepends, queued intent, +passive layout repetition, stale completions, failed requests and registered +scroll travel. The user confirmed that the original second-page failure was +resolved in the September 22 reproduction. The broader input-device matrix and +remote transports still require separate manual validation; unit geometry is +not performance evidence. ## Keeping the Viewport on the Reader's Content @@ -664,19 +653,17 @@ diagnostics turned on first. It warns once per session, so scrolling against a dead boundary cannot flood the log. Turn the flag on only when the trail's 30-event cap is not enough. -Two detectors raise it: - -- `exhausted` returned for `beyond-known-total`. That result **latches the - direction off for the rest of the session** and only `applied` clears it, so - reaching it on an unknown or contradictory total is how history goes - permanently missing rather than merely late. -- A `before` request blocked by that latch while the session is still - `isPartial`. This fires at the moment the user scrolls up and nothing happens. +The container raises this warning for an inconsistent `beyond-known-total` +answer or a declined viewport preparation. The former session-wide +`latched-exhausted-while-partial` detector in the list has been replaced by the +pager's window-scoped exhaustion and request-phase diagnostics. A partial live +tail alone does not prove that a particular rendered history boundary has more +content. When the report is "scrolling up shows no history, but the Turn Rail can still load those Turns", search the log for `declined to page older Turns`. Turn Rail navigation goes through `loadSessionTurnWindow` directly and bypasses the -boundary latch entirely, which is why it keeps working. The accompanying +viewport paging controller, which is why it keeps working. The accompanying `FlowChat history paging trail` warning carries the preceding events, including `anchor_capture_failed` — `captureHistoryPrependAnchor` returning `false` cancels a window that was already fetched. diff --git a/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_VERIFICATION.md b/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_VERIFICATION.md index dda1220a22..550b6c2d4a 100644 --- a/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_VERIFICATION.md +++ b/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_VERIFICATION.md @@ -33,11 +33,13 @@ each missing what the other had. | `flowChatCollapseMotion.test.ts` | collapse does not move earlier content | | `useFlowChatFollowOutput.test.tsx` | one-shot new-Turn reveal, frame loop, blank crossing, resize realign, opening readback publication and ownership/lifecycle gates | | `../../tool-cards/useToolCardHeightContract.test.tsx` | tool cards reflow rather than compensate | -| `flowChatHistoryBoundary.test.ts` | the screenful lead, and the latch's own predicate | +| `flowChatHistoryBoundary.test.ts` | the screenful lead and physical boundary geometry | +| `flowChatHistoryPager.test.ts` | request/layout ordering, coalesced demand, prefetch without physical arrival, stale tickets, exhaustion and retry eligibility | +| `useFlowChatViewportOwner.test.tsx` | synchronous write/shift accounting preserves reader travel and actual clamping | | `flowChatLiveTailWindow.test.ts` | "does the transcript still reach the newest Turn" | | `flowChatViewportAnchor.test.ts` | anchor geometry and the DOM contract | | `useFlowChatViewportAnchor.test.tsx` | capture, restore, carry, the settle window | -| `VirtualMessageList.session-boundary.test.tsx` | prepend compensation, the ask, navigation-target current Turn with gesture/follow/session handoff, and search placement only outside the readable viewport | +| `VirtualMessageList.session-boundary.test.tsx` | prepend compensation, consecutive paging with/without queued input, passive scroll suppression, navigation-target current Turn with gesture/follow/session handoff, and search placement only outside the readable viewport | | `FlowChatOpeningBoundary.test.tsx` | opening-only activation/scroll isolation, bidirectional focus skipping, programmatic focus return, and reveal cleanup; DOM contracts only | | `ModernFlowChatContainer.history-state.test.tsx` | history presentation and the submission event | | `flowChatViewportOwnership.test.ts` | the priority order, preemption, expiry | @@ -336,3 +338,20 @@ Keyboard focus has its own outline and does not navigate until activation. neither retains the old navigation selection. 11. With reduced motion enabled, the hover fan changes without animation. Touch navigation must not leave a hover fan behind. + +### History paging demand + +1. Page upward at least three times from a long session's live tail, and page + downward again through a history window. Both directions must keep working. +2. Trigger prefetch before reaching the physical head; stop while it loads. + Correction/measurement alone must not cascade through subsequent pages. +3. Keep scrolling during a slow page; queued demand should continue once the + page commits if the new boundary is still near. Reverse or move away during + the fetch and verify the obsolete demand does not load another page. +4. On a transcript shorter than one viewport, wheel upward at the hard top. + Test keyboard, touch/inertia and scrollbar dragging as well. +5. Navigate elsewhere or switch sessions during a slow fetch. Its eventual + result must not block the new boundary or move the new presentation. +6. Repeat on remote workspace and Peer Device surfaces with transport latency; + local unit fixtures do not establish those behaviors. Remote-control/mobile + and detached-dispatch surfaces do not use this list controller directly. diff --git a/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_VIRTUALIZATION.md b/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_VIRTUALIZATION.md index 24c399b3e5..678428d518 100644 --- a/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_VIRTUALIZATION.md +++ b/src/web-ui/src/flow_chat/components/modern/FLOWCHAT_VIRTUALIZATION.md @@ -20,6 +20,31 @@ cancelled rounds precede it. That boundary prevents cross-round grouping from hiding the label; ordinary within-round tool folding remains available. Round ids and virtual row keys stay unchanged, with no viewport writes or mount animation. +## Measurement compensation and cached offsets + +When the viewport owner accepts a shift for a measured row wholly above the +reader, the virtualizer publishes the actual scroll offset after updating its +size cache, before selecting the next rendered window. This applies during +ordinary reading as well as opening reconciliation. Waiting for the native +scroll event leaves the old offset paired with new row positions and can remove +newly measured rows, then mount them again on that event. A refused shift does +not trigger this readback. All viewport writes remain with the existing owner. +`useFlowChatVirtualizer.initial-window.test.tsx` covers delayed scroll and +scroll-end delivery using the real virtualizer and supplied geometry; it does +not establish browser performance. + +## Collapsed thinking content lifetime + +Thinking cards mount their Markdown body only while expanded or finishing a +collapse transition. Initially collapsed rows therefore do not parse or build +hidden Markdown when virtualization remounts them. Closing content is released +when the actual grid transition finishes or is cancelled; without a transition +(including reduced motion), it is released immediately. Reopening invalidates +the pending release. The typewriter and reveal gate retain their existing +lifetime. `ModelThinkingDisplay.test.tsx` covers this lifecycle with supplied +animation promises; browser animation fidelity and scroll performance still +require runtime verification. + ## Embedded session lifetime `BtwSessionPanel` keeps a lightweight tab-owned wrapper while its content is diff --git a/src/web-ui/src/flow_chat/components/modern/FlowChatPrependSnapshot.test.tsx b/src/web-ui/src/flow_chat/components/modern/FlowChatPrependSnapshot.test.tsx new file mode 100644 index 0000000000..46983483f4 --- /dev/null +++ b/src/web-ui/src/flow_chat/components/modern/FlowChatPrependSnapshot.test.tsx @@ -0,0 +1,42 @@ +// @vitest-environment jsdom +import React, { act, useLayoutEffect } from 'react'; +import { createRoot } from 'react-dom/client'; +import { expect, it, vi } from 'vitest'; +import { FlowChatPrependSnapshot } from './FlowChatPrependSnapshot'; + +it('captures current pre-mutation geometry only for a prepend, before parent layout effects', () => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true; + const host = document.createElement('div'); + document.body.append(host); + const root = createRoot(host); + const scrollerRef = { current: null as HTMLDivElement | null }; + const snapshotRef = { current: null as { firstKey: string; scrollHeight: number } | null }; + const reads = vi.fn(() => Number(scrollerRef.current?.dataset.height)); + const observed: unknown[] = []; + function List({ keys, height }: { keys: string[]; height: number }) { + useLayoutEffect(() => { observed.push(snapshotRef.current); }); + return +
+ ; + } + try { + act(() => root.render()); + Object.defineProperty(scrollerRef.current, 'scrollHeight', { get: reads }); + act(() => root.render()); + act(() => root.render()); + expect(reads).not.toHaveBeenCalled(); + // A layout change between React commits must be reflected in the baseline. + scrollerRef.current!.dataset.height = '225'; + act(() => root.render()); + expect(reads).toHaveBeenCalledOnce(); + expect(observed.at(-1)).toEqual({ firstKey: 'b', scrollHeight: 225 }); + expect(scrollerRef.current!.dataset.height).toBe('400'); + act(() => root.render()); + expect(observed.at(-1)).toBeNull(); + act(() => root.render()); + expect(reads).toHaveBeenCalledOnce(); + } finally { + act(() => root.unmount()); + host.remove(); + } +}); diff --git a/src/web-ui/src/flow_chat/components/modern/FlowChatPrependSnapshot.tsx b/src/web-ui/src/flow_chat/components/modern/FlowChatPrependSnapshot.tsx new file mode 100644 index 0000000000..5ee67b7ce2 --- /dev/null +++ b/src/web-ui/src/flow_chat/components/modern/FlowChatPrependSnapshot.tsx @@ -0,0 +1,32 @@ +import React from 'react'; + +interface Props { + itemKeys: readonly string[]; + scrollerRef: React.RefObject; + snapshotRef: React.MutableRefObject<{ firstKey: string; scrollHeight: number } | null>; + children: React.ReactNode; +} + +/** Read the old DOM only when history is actually inserted before its head. + * A height saved after the previous commit can become stale as images or fonts + * settle between commits. Capture immediately before mutation instead; the + * viewport owner still decides and bounds compensation after mutation. + */ +export class FlowChatPrependSnapshot extends React.Component { + getSnapshotBeforeUpdate(previous: Props): null { + this.props.snapshotRef.current = null; + const firstKey = previous.itemKeys[0]; + if (firstKey && this.props.itemKeys[0] !== firstKey + && this.props.itemKeys.indexOf(firstKey) > 0) { + const scroller = this.props.scrollerRef.current; + if (scroller) this.props.snapshotRef.current = { firstKey, scrollHeight: scroller.scrollHeight }; + } + return null; + } + + // React requires this lifecycle with getSnapshotBeforeUpdate. The parent + // consumes the ref in its layout effect, after DOM mutation and before paint. + componentDidUpdate(): void {} + + render() { return this.props.children; } +} diff --git a/src/web-ui/src/flow_chat/components/modern/ModernFlowChatContainer.tsx b/src/web-ui/src/flow_chat/components/modern/ModernFlowChatContainer.tsx index eeadd9e868..23840ff4d9 100644 --- a/src/web-ui/src/flow_chat/components/modern/ModernFlowChatContainer.tsx +++ b/src/web-ui/src/flow_chat/components/modern/ModernFlowChatContainer.tsx @@ -2287,7 +2287,11 @@ export const ModernFlowChatContainer: React.FC = ( return 'not-ready'; } })().finally(() => { - historyBoundaryRequestsRef.current[direction] = null; + // A session switch can install another request before this one settles. + // Only the request that owns the slot may release it. + if (historyBoundaryRequestsRef.current[direction] === request) { + historyBoundaryRequestsRef.current[direction] = null; + } }); historyBoundaryRequestsRef.current[direction] = request; return request; diff --git a/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.session-boundary.test.tsx b/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.session-boundary.test.tsx index 8df92d20f7..22c33335d7 100644 --- a/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.session-boundary.test.tsx +++ b/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.session-boundary.test.tsx @@ -1002,17 +1002,19 @@ describe('VirtualMessageList natural scroll contract', () => { options: { scrollHeightPx: number; growthPx: number; scrollTopPx?: number }, run: (scroller: HTMLElement) => void, ) { - let scrollHeightPx = options.scrollHeightPx; const restoreLayout = fakeLayout({ clientHeight: 600, - scrollHeight: () => scrollHeightPx, + // DOM geometry grows at mutation, not when new props are prepared. + // The prepend snapshot must still see the old range before that point. + scrollHeight: () => options.scrollHeightPx + ( + container.querySelector('[data-turn-id="turn-old-0"]') ? options.growthPx : 0 + ), turnTopFromScrollerTop: 500, }); try { act(() => root.render()); const scroller = container.querySelector('[data-flowchat-scroller]')!; scroller.scrollTop = options.scrollTopPx ?? 500; - scrollHeightPx += options.growthPx; run(scroller); } finally { restoreLayout(); @@ -1029,6 +1031,55 @@ describe('VirtualMessageList natural scroll contract', () => { act(() => root.render()); } + it('consumes each consecutive prepend once', () => { + const restoreLayout = fakeLayout({ + clientHeight: 600, + scrollHeight: () => 3000 + container.querySelectorAll('[data-turn-id^="batch-"]').length * 40, + turnTopFromScrollerTop: 500, + }); + try { + act(() => root.render()); + const scroller = container.querySelector('[data-flowchat-scroller]')!; + scroller.scrollTop = 500; + for (const batch of ['batch-a', 'batch-b']) { + mocks.items = [userMessage(batch, `${batch}-message`, 'Older'), ...mocks.items]; + act(() => root.render()); + } + expect(scroller.scrollTop).toBe(580); + act(() => root.render()); + expect(scroller.scrollTop).toBe(580); + } finally { restoreLayout(); } + }); + + it('does not replay a prepend received while the viewport is suspended', () => { + const layout = { + clientWidth: 1000, clientHeight: 600, + scrollHeight: () => 3000 + (container.querySelector('[data-turn-id="turn-old-0"]') ? 80 : 0), + turnTopFromScrollerTop: 500, + }; + const restoreLayout = fakeLayout(layout); + try { + act(() => root.render()); + const scroller = container.querySelector('[data-flowchat-scroller]')!; + scroller.scrollTop = 500; + const observer = resizeObservers.find(candidate => candidate.targets.has(scroller))!; + layout.clientHeight = 0; + act(() => observer.notify()); + mocks.items = [userMessage('turn-old-0', 'message-old-0', 'Older'), ...mocks.items]; + act(() => root.render()); + expect(scroller.scrollTop).toBe(500); + animationFrames.clear(); + layout.clientHeight = 600; + act(() => observer.notify()); + const resume = [...animationFrames.values()][0]; + expect(resume).toBeDefined(); + act(() => resume(16)); + const afterResume = scroller.scrollTop; + act(() => root.render()); + expect(scroller.scrollTop).toBe(afterResume); + } finally { restoreLayout(); } + }); + it('moves the viewport by the height that was prepended', () => { // Three 40px items arrived above, so the reader's content is 120px lower // and the viewport follows it. Anything less leaves them looking at @@ -1048,7 +1099,7 @@ describe('VirtualMessageList natural scroll contract', () => { * the boundary never re-armed because the reader was still at the head. */ withGrowingRange({ scrollHeightPx: 3000, growthPx: 80 }, scroller => { - act(() => { scroller.dispatchEvent(new Event('wheel')); }); + act(() => { scroller.dispatchEvent(new WheelEvent('wheel', { deltaY: -100 })); }); prependOlderTurns(2); expect(scroller.scrollTop).toBe(580); }); @@ -1188,8 +1239,9 @@ describe('VirtualMessageList natural scroll contract', () => { await act(async () => { root.render( { + onHistoryWindowBoundaryIntent={(direction, options) => { asked.push(direction); + options?.prepareViewportForPresentationCommit?.(); return 'applied'; }} />, @@ -1210,31 +1262,95 @@ describe('VirtualMessageList natural scroll contract', () => { } } - it('asks again once the reader is off the boundary, still within the lead', async () => { - /* - * The latch re-arms on the reader being *off* the boundary, and the ask - * goes out a screenful before they reach it — so those cannot be the same - * predicate. When they were, one page landed and everything after it was - * refused as `not-rearmed`: measured, six minutes of refusals while the - * reader scrolled into a wall two Turns from the top of what was loaded. - */ + it('asks again when the reader returns toward the boundary within the lead', async () => { + // Prefetch must not require reaching the physical head first. Moving + // away alone must also not ask for another older page. await withPagedTranscript(async (scroller, asked) => { // Rows 3..7 are on screen, so nothing is reached; the head is 120px up, // which is inside the one-screen lead. + await scrollTo(scroller, 300); + expect(asked).toEqual([]); await scrollTo(scroller, 120); expect(asked).toEqual(['before']); }); }); it('does not ask again while the reader is still on the head', async () => { - // The latch's own job, unchanged: after a prepend the visible range reads - // as the head for a commit, and that must not dispatch a second page. + // Moving away from the head is not demand for another older page. await withPagedTranscript(async (scroller, asked) => { await scrollTo(scroller, ROW_PX); expect(asked).toEqual([]); }); }); + it.each([false, true])('continues after a real tail prepend (queued intent: %s)', async queued => { + mocks.items = Array.from({ length: 6 }, (_, index) => ( + userMessage(`turn-${40 + index}`, `message-${40 + index}`, 'Body') + )); + const restoreLayout = fakeLayout({ + clientHeight: VIEWPORT_PX, + scrollHeight: () => container.querySelectorAll('.virtual-item-wrapper[data-turn-id]').length * ROW_PX, + turnTopFromScrollerTop: 0, + }); + let resolvePage!: (result: 'applied') => void; + let prepareCommit: (() => boolean | void | Promise) | undefined; + const ask = vi.fn((direction: string, options?: { + prepareViewportForPresentationCommit?: () => boolean | void | Promise; + }) => { + if (direction === 'after') return 'exhausted' as const; + prepareCommit = options?.prepareViewportForPresentationCommit; + return new Promise<'applied'>(resolve => { resolvePage = resolve; }); + }); + const beforeCount = () => ask.mock.calls.filter(([direction]) => direction === 'before').length; + const render = (history = false) => root.render( + , + ); + try { + await act(async () => { render(); }); + await settleOpenReveal(); + expect(beforeCount()).toBe(1); + const scroller = container.querySelector('[data-flowchat-scroller]')!; + if (queued) { + await act(async () => { + for (let i = 0; i < 5; i++) scroller.dispatchEvent(new WheelEvent('wheel', { deltaY: -100 })); + }); + expect(beforeCount()).toBe(1); + } + await act(async () => { + expect(await prepareCommit?.()).toBe(true); + mocks.items = [ + ...Array.from({ length: 4 }, (_, index) => userMessage(`turn-${36 + index}`, `message-${36 + index}`, 'Body')), + ...mocks.items, + ]; + render(true); + resolvePage('applied'); + }); + expect(scroller.scrollTop).toBeGreaterThan(0); + expect(beforeCount()).toBe(queued ? 2 : 1); + // Delayed native events from compensation and another render are not + // reader demand, even though the new head is still within the lead. + await act(async () => { + scroller.dispatchEvent(new Event('scroll')); + render(true); + }); + expect(beforeCount()).toBe(queued ? 2 : 1); + if (!queued) { + await act(async () => { + scroller.dispatchEvent(new WheelEvent('wheel', { deltaY: -100 })); + }); + expect(beforeCount()).toBe(2); + } + } finally { + restoreLayout(); + } + }); + it('asks on a gesture that moves nothing, because at the top none of them do', async () => { /* * The deadlock this closes, measured on a tail window of three Turns that @@ -1281,7 +1397,7 @@ describe('VirtualMessageList natural scroll contract', () => { // A wheel and nothing else: no scroll event, because there is nowhere // for the offset to go. await act(async () => { - scroller.dispatchEvent(new Event('wheel')); + scroller.dispatchEvent(new WheelEvent('wheel', { deltaY: -100 })); await Promise.resolve(); await Promise.resolve(); }); @@ -1334,7 +1450,7 @@ describe('VirtualMessageList natural scroll contract', () => { const scroller = container.querySelector('[data-flowchat-scroller]')!; await act(async () => { - scroller.dispatchEvent(new Event('wheel')); + scroller.dispatchEvent(new WheelEvent('wheel', { deltaY: -100 })); await Promise.resolve(); await Promise.resolve(); }); @@ -1389,7 +1505,7 @@ describe('VirtualMessageList natural scroll contract', () => { // Latched: asking again from the same window changes nothing. const scroller = container.querySelector('[data-flowchat-scroller]')!; await act(async () => { - scroller.dispatchEvent(new Event('wheel')); + scroller.dispatchEvent(new WheelEvent('wheel', { deltaY: -100 })); await Promise.resolve(); await Promise.resolve(); }); @@ -1403,7 +1519,7 @@ describe('VirtualMessageList natural scroll contract', () => { await Promise.resolve(); }); await act(async () => { - scroller.dispatchEvent(new Event('wheel')); + scroller.dispatchEvent(new WheelEvent('wheel', { deltaY: -100 })); await Promise.resolve(); await Promise.resolve(); }); diff --git a/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx b/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx index d440c49c24..e4c6a7a037 100644 --- a/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx +++ b/src/web-ui/src/flow_chat/components/modern/VirtualMessageList.tsx @@ -64,6 +64,7 @@ import { isUsableFlowChatViewportRect, useFlowChatVirtualizer, } from './useFlowChatVirtualizer'; +import { FlowChatHistoryPager, type HistoryPageResult } from './flowChatHistoryPager'; import { useFlowChatViewportOwner } from './useFlowChatViewportOwner'; import { ONE_SHOT_NAVIGATION_HOLD_MS, @@ -72,10 +73,10 @@ import { import { USER_DRIVEN_SCROLL_WINDOW_MS } from './flowChatViewportAnchor'; import { historyBoundariesForVisibleRange, - historyBoundariesReached, type HistoryBoundaryProximity, } from './flowChatHistoryBoundary'; import { VirtualItemRenderer } from './VirtualItemRenderer'; +import { FlowChatPrependSnapshot } from './FlowChatPrependSnapshot'; import { FlowChatOpeningBoundary } from './FlowChatOpeningBoundary'; import { useFlowChatVolatileContext } from './FlowChatContext'; import { @@ -86,7 +87,6 @@ import { resolveVisibleFlowChatTurnIds } from './flowChatVisibleTurns'; import type { FlowChatViewportSnapshot } from './flowChatViewportSnapshot'; import { getVirtualItemStableKey } from './virtualItemIdentity'; import { isAmbientToolRunContinuationAfter } from './flowChatRhythm'; -import { warnHistoryPagingRefusedWithPendingTurns } from '../../services/historySessionDiagnostics'; import { VIEWPORT_PLACEMENT_SETTLE_MS, roundViewportPx, @@ -145,11 +145,7 @@ export interface TurnNavigationOptions { behavior?: ScrollBehavior; } -export type HistoryWindowBoundaryIntentResult = - | 'applied' - | 'exhausted' - | 'not-ready' - | 'cancelled'; +export type HistoryWindowBoundaryIntentResult = HistoryPageResult; type HistoryWindowBoundaryIntentResponse = | HistoryWindowBoundaryIntentResult @@ -462,62 +458,13 @@ const VirtualMessageListSession = forwardRef(null); // Selection identity only; this must never hold or reposition the viewport. const navigatedTurnIdRef = useRef(null); - const boundaryRequestRef = useRef | null>>({ - before: null, - after: null, - }); - const exhaustedBoundaryRef = useRef>({ - before: false, - after: false, - }); - /** Re-arming is a transition out of a boundary, not a repeated level read. */ - const boundaryReachedRef = useRef>({ - before: false, - after: false, - }); - /** - * `exhausted` describes the window that asked, not the session. - * - * "There is nothing before this" is true of a *start ordinal*. Navigate to - * the first Turn and the store answers `reached-start` for `targetOrdinal: - * -1`, correctly — and the latch then outlived the window by the rest of the - * session. Measured: 3 Turns of 43 loaded, `before` latched off from a visit - * to Turn 1, and after jumping back to the tail the reader could not page at - * all. The alarm fired (`latched-exhausted-while-partial`) and nothing acted - * on it. - * - * So the latch is cleared whenever the window moves. Only `applied` used to - * clear it, which is the one case where the window moves *because* of the - * page — every other way it moves left a stale answer behind. - */ + const [historyPager] = useState(() => new FlowChatHistoryPager()); + const [historyPageRevision, acknowledgeHistoryPage] = useState(0); + const readerScrollPositionRef = useRef(null); + const pagingLayoutKeyRef = useRef(null); const windowBoundsKey = historyWindow ? `${historyWindow.startOrdinal}:${historyWindow.endOrdinalExclusive}` : presentationMode; - const previousWindowBoundsKeyRef = useRef(windowBoundsKey); - if (previousWindowBoundsKeyRef.current !== windowBoundsKey) { - previousWindowBoundsKeyRef.current = windowBoundsKey; - exhaustedBoundaryRef.current = { before: false, after: false }; - boundaryReachedRef.current = { before: false, after: false }; - } - /** - * Whether a boundary may be asked about again. - * - * Prepend compensation puts the viewport back on the reader's content, but - * the virtualizer places its rows from a scroll offset it only refreshes on - * the next frame, so for one commit the visible range is still read against - * the head. Asking from that commit pages again and produces another one just - * like it: measured, a single junction paged a transcript back to its first - * Turn while the reader held still. - * - * A direction is armed by the visible range leaving it. react-virtuoso had - * this for free — the range it reported was absolute, so a prepend moved the - * local start index by the number of items added and the rule stopped - * applying by itself. - */ - const boundaryArmedRef = useRef>({ - before: true, - after: true, - }); /** Assigned below, once the boundary evaluation it stands for exists. */ const evaluateHistoryBoundariesRef = useRef<() => void>(() => {}); const searchNavigationRequestIdRef = useRef(0); @@ -525,7 +472,8 @@ const VirtualMessageListSession = forwardRef () => { searchNavigationRequestIdRef.current += 1; - }, [activeSessionId]); + historyPager.reset(); + }, [activeSessionId, historyPager]); const reconcileOpeningMeasurementRef = useRef<() => boolean>(() => false); const virtualizer = useFlowChatVirtualizer({ @@ -780,11 +728,8 @@ const VirtualMessageListSession = forwardRef(null); - /** - * The scroll range as of the last render, so a prepend can be told what the - * transcript actually grew by rather than only what was reserved for it. - */ - const previousScrollHeightRef = useRef(0); + const prependSnapshotRef = useRef<{ firstKey: string; scrollHeight: number } | null>(null); + const prependItemKeys = useMemo(() => virtualItems.map(getVirtualItemStableKey), [virtualItems]); useLayoutEffect(() => { const previousFirstKey = firstItemKeyRef.current; const nextFirstKey = virtualItems[0] ? getVirtualItemStableKey(virtualItems[0]) : null; @@ -792,10 +737,12 @@ const VirtualMessageListSession = forwardRef { navigatedTurnIdRef.current = turnId; + if (turnId !== null) historyPager.reset(); // Different tail Turns can land at the same offset, emitting no scroll. scheduleVisibleTurnInfoUpdate(); - }, [scheduleVisibleTurnInfoUpdate]); + }, [historyPager, scheduleVisibleTurnInfoUpdate]); - const notifyUserScrollIntent = useCallback(() => { + const notifyUserScrollIntent = useCallback((direction?: SessionHistoryWindowDirection) => { + if (direction) historyPager.readerIntent(direction); /* * The reader outranks everything, and the claim is what makes that true of * writers that are already in flight rather than only of ones yet to @@ -1057,6 +1006,7 @@ const VirtualMessageListSession = forwardRef { if (isViewportSuspendedRef.current) return; + const position = viewportOwner.readReaderScrollPosition(); + const previous = readerScrollPositionRef.current; + readerScrollPositionRef.current = position; + const owner = viewportOwner.currentOwner(); + const delta = previous === null ? 0 : position - previous; + const direction = delta < -0.5 ? 'before' : delta > 0.5 ? 'after' : undefined; + // Synchronous corrections are removed by the register. Smooth owned + // navigation/follow scrolls are excluded here; unowned momentum counts. + if (direction && (owner === null || owner === 'user-gesture')) { + historyPager.readerIntent(direction); + } /* * A scroll under a scrollbar press is the one case where a plain scroll * event does carry intent — the press is what qualifies it. Left @@ -1575,7 +1536,7 @@ const VirtualMessageListSession = forwardRef notifyUserScrollIntent(); - const handleTouchMove = () => notifyUserScrollIntent(); + const handleWheel = (event: WheelEvent) => { + notifyUserScrollIntent(event.deltaY < 0 ? 'before' : event.deltaY > 0 ? 'after' : undefined); + }; + let touchY: number | null = null; + const handleTouchStart = (event: TouchEvent) => { touchY = event.touches?.[0]?.clientY ?? null; }; + const handleTouchMove = (event: TouchEvent) => { + const y = event.touches?.[0]?.clientY ?? null; + const delta = y !== null && touchY !== null ? touchY - y : 0; + touchY = y; + notifyUserScrollIntent(delta < 0 ? 'before' : delta > 0 ? 'after' : undefined); + }; const handleKeyDown = (event: KeyboardEvent) => { + if (event.target instanceof Element && event.target.closest('input, textarea, select, [contenteditable="true"]')) return; if (['ArrowUp', 'ArrowDown', 'PageUp', 'PageDown', 'Home', 'End', ' '].includes(event.key)) { - notifyUserScrollIntent(); + const before = ['ArrowUp', 'PageUp', 'Home'].includes(event.key) || (event.key === ' ' && event.shiftKey); + notifyUserScrollIntent(before ? 'before' : 'after'); } }; /* @@ -1614,6 +1586,7 @@ const VirtualMessageListSession = forwardRef { scrollerElement.removeEventListener('scroll', handleNativeScroll); scrollerElement.removeEventListener('wheel', handleWheel); + scrollerElement.removeEventListener('touchstart', handleTouchStart); scrollerElement.removeEventListener('touchmove', handleTouchMove); scrollerElement.removeEventListener('keydown', handleKeyDown); scrollerElement.removeEventListener('pointerdown', handlePointerDown); @@ -1630,6 +1604,7 @@ const VirtualMessageListSession = forwardRef { if (!turnId || !activeSessionId) return 'rejected'; + historyPager.reset(); exitFollowOutput('scroll-to-turn'); preparedTurnNavigationRef.current = { turnId, behavior: options?.behavior ?? 'auto', }; return 'pending'; - }, [activeSessionId, exitFollowOutput]); + }, [activeSessionId, exitFollowOutput, historyPager]); useLayoutEffect(() => { const prepared = preparedTurnNavigationRef.current; @@ -2204,156 +2180,53 @@ const VirtualMessageListSession = forwardRef { - /* - * Three refusals, all asking whether the ask describes the reader. - * - * The first is the opening reveal. Until it ends the transcript is hidden - * and still being placed — item heights are estimates and the viewport is - * walking down to the content end — so the offset a boundary would be - * judged from is not a position anybody chose, and on a session whose - * loaded tail is shorter than one viewport the head is trivially "reached" - * at offset 0. Worse, what the page then does is prepend history *above* - * that viewport, and the compensation for it is deliberately left to - * whoever holds a target; landing it in the middle of the opening - * placement puts those two in a race that the reader loses by eight Turns. - * Measured: a re-opened session paged 140ms after mount, from a viewport - * still at 0, and was revealed at the top of the window it had just pulled - * in. Deferred rather than dropped — the reveal settling asks again. - * - * While the follow rule owns the viewport, the position the ask was derived - * from is our own placement — as true of a history window being opened as - * of the live tail, so the test is ownership and not which presentation is - * on screen. Ownership ends the moment the reader scrolls, which is exactly - * when the ask starts meaning something. Measured on session open: five - * pages landed in 890ms, each one displacing the viewport and so requesting - * the next, until history ran out. - * - * The third is the arming latch; see `boundaryArmedRef`. - * - * All three are invisible when they fire: the boundary status stays idle, - * which is also what "there is no more history" looks like. The faults have - * been at either extreme — pages arriving in a chain because no refusal - * fired, and a boundary that never pages because the latch stayed shut — so - * the reason is recorded rather than inferred from what did not happen. - */ - if (isOpeningViewport()) { - traceViewportRepeating(`paging|${direction}|opening`, { - location: 'historyPaging.refused', - message: 'the transcript is still being placed, so the boundary is not the reader\'s', - data: () => ({ - direction, - reason: 'opening-reveal', - viewportId, - scrollTopPx: roundViewportPx(scrollerElementRef.current?.scrollTop ?? 0), - }), - }); - return; - } - if (isFollowingOutputNow()) { - traceViewportRepeating(`paging|${direction}|following`, { + const opening = isOpeningViewport(); + const following = isFollowingOutputNow(); + // Opening/follow placements are not reader demand. Keep the initial ask + // available until their ownership ends, as before. + if (opening || following || !onHistoryWindowBoundaryIntent) { + const reason = opening ? 'opening-reveal' : following ? 'follow-output-owns-the-viewport' : 'no-handler'; + traceViewportRepeating(`paging|${direction}|${reason}`, { location: 'historyPaging.refused', - message: 'the boundary was reached by our own placement, not by the reader', - data: () => ({ direction, reason: 'follow-output-owns-the-viewport' }), + message: 'history paging is waiting for viewport ownership or a handler', + data: () => ({ direction, viewportId, reason }), }); return; } - if (!boundaryArmedRef.current[direction]) { - traceViewportRepeating(`paging|${direction}|unarmed`, { + const ticket = historyPager.begin(direction); + if (!ticket) { + traceViewportRepeating(`paging|${direction}|${historyPager.snapshot(direction).phase}`, { location: 'historyPaging.refused', - message: 'the boundary has not been left since the last page', - data: () => ({ direction, reason: 'not-rearmed' }), + message: 'history paging is waiting for its request, layout, or reader', + data: () => ({ direction, ...historyPager.snapshot(direction) }), }); return; } - const latchedExhausted = exhaustedBoundaryRef.current[direction]; - if ( - !onHistoryWindowBoundaryIntent || - boundaryRequestRef.current[direction] || - latchedExhausted - ) { - /* - * The user has reached the head of the loaded window and we are declining - * to fetch more. That is correct once history really is exhausted, and a - * silent data loss when it is not — the boundary status stays idle either - * way, so the transcript looks like it simply has no earlier Turns. - * - * The head, and only the head. `after` latches the moment the reader - * reaches the newest Turn, which is every session that has ever been - * paged, and a partial session stays partial throughout — so raising this - * for it is a warning that fires on the ordinary case and means nothing. - * Measured: four of them in one recording, all `after`, all correct - * behaviour. `resolveHistoryBoundaryTarget` draws the same line one layer - * down, between `reached-latest` and `beyond-known-total`. - */ - const session = activeSessionRef.current; - if ( - direction === 'before' - && latchedExhausted - && session?.sessionId - && session.isPartial === true - ) { - warnHistoryPagingRefusedWithPendingTurns(session.sessionId, { - direction, - reason: 'latched-exhausted-while-partial', - isPartial: true, - latchedExhausted: true, - loadedTurnCount: session.dialogTurns.length, - totalTurnCount: session.totalTurnCount ?? 0, - }); - } - return; - } - // Disarmed for as long as this page is the reason the window sits at the - // boundary. Only the window moving off it arms the direction again. - boundaryArmedRef.current[direction] = false; - /* - * The ask itself, and the viewport it was derived from. - * - * Every refusal above is recorded, and the one that goes through was not — - * so a page that arrives while the transcript is still opening, from a - * viewport nobody is following and at an offset the reader never chose, - * looked exactly like a page the reader asked for. That page prepends - * history above them, and the compensation for it is deliberately left to - * whoever holds a target; with nothing holding one, this line is where that - * chain starts. - */ traceViewport({ location: 'historyPaging.asked', - message: 'the boundary was reached by the reader, so history was asked for', - data: () => ({ - direction, - viewportId, - isOpening: isOpeningViewport(), - presentationMode, - itemCount: virtualItems.length, - scrollTopPx: roundViewportPx(scrollerElementRef.current?.scrollTop ?? 0), - }), + message: 'reader demand dispatched a history page', + data: () => ({ direction, viewportId, requestId: ticket.id }), }); - const request = Promise.resolve(onHistoryWindowBoundaryIntent(direction)).then( - normalizeBoundaryResult, - ).then(result => { - if (result === 'exhausted') { - exhaustedBoundaryRef.current[direction] = true; - } else if (result === 'applied') { - exhaustedBoundaryRef.current[direction] = false; - } - // Nothing was prepended, so the window sitting at the boundary is still - // the reader's own position rather than a consequence of this page. - if (result !== 'applied') { - boundaryArmedRef.current[direction] = true; + // Reserve the ticket before invoking: even a synchronous callback/re-entry + // cannot dispatch it twice. Rejections and synchronous throws share cleanup. + void Promise.resolve().then(() => historyPager.isCurrent(ticket) + ? onHistoryWindowBoundaryIntent(direction, { + prepareViewportForPresentationCommit: () => historyPager.prepareCommit(ticket), + }) + : 'cancelled' as const).then(normalizeBoundaryResult).catch(() => 'not-ready' as const).then(result => { + const accepted = historyPager.finish(ticket, result); + traceViewport({ + location: 'historyPaging.completed', + message: accepted ? 'history page result accepted' : 'obsolete history page result ignored', + data: () => ({ direction, completedRequestId: ticket.id, result, ...historyPager.snapshot(direction) }), + }); + if (accepted && result === 'applied') { + // Also covers a page that was already projected: the acknowledgement + // commits after the parent's presentation updates, without a timer. + acknowledgeHistoryPage(revision => revision + 1); } - }).finally(() => { - boundaryRequestRef.current[direction] = null; }); - boundaryRequestRef.current[direction] = request; - }, [ - isFollowingOutputNow, - isOpeningViewport, - onHistoryWindowBoundaryIntent, - presentationMode, - viewportId, - virtualItems.length, - ]); + }, [historyPager, isFollowingOutputNow, isOpeningViewport, onHistoryWindowBoundaryIntent, viewportId]); /** * Whether either end of the loaded transcript is on screen, and act on it. @@ -2406,41 +2279,20 @@ const VirtualMessageListSession = forwardRef { + if (isViewportSuspendedRef.current) return; + historyPager.commitLayout(windowBoundsKey, { + before: virtualItems[0] ? getVirtualItemStableKey(virtualItems[0]) : null, + after: virtualItems.length ? getVirtualItemStableKey(virtualItems[virtualItems.length - 1]) : null, + }); + // Content replacement can clamp native scrollTop. That is not user travel. + const layoutKey = `${windowBoundsKey}:${virtualItems.length}:${virtualItems[0] ? getVirtualItemStableKey(virtualItems[0]) : ''}`; + if (pagingLayoutKeyRef.current !== layoutKey) { + pagingLayoutKeyRef.current = layoutKey; + readerScrollPositionRef.current = viewportOwner.readReaderScrollPosition(); + } + }); + evaluateHistoryBoundariesRef.current = evaluateHistoryBoundaries; useEffect(() => { @@ -2455,6 +2323,7 @@ const VirtualMessageListSession = forwardRef { + historyPager.reset(); setNavigatedTurn(null); enterFollowOutput('jump-to-latest'); updateIsAtBottom(); - }, [enterFollowOutput, setNavigatedTurn, updateIsAtBottom]); + }, [enterFollowOutput, historyPager, setNavigatedTurn, updateIsAtBottom]); const scrollToLatestEndPosition = useCallback(() => { + historyPager.reset(); onUserScrollIntent?.(); setNavigatedTurn(null); enterFollowOutput('jump-to-latest'); // Entering follow can leave the viewport exactly where it is, which // produces no scroll event to recompute the band from. updateIsAtBottom(); - }, [enterFollowOutput, onUserScrollIntent, setNavigatedTurn, updateIsAtBottom]); + }, [enterFollowOutput, historyPager, onUserScrollIntent, setNavigatedTurn, updateIsAtBottom]); useImperativeHandle(ref, () => ({ scrollToTurn, @@ -2616,6 +2487,7 @@ const VirtualMessageListSession = forwardRef + ); }); diff --git a/src/web-ui/src/flow_chat/components/modern/flowChatHistoryBoundary.ts b/src/web-ui/src/flow_chat/components/modern/flowChatHistoryBoundary.ts index 8897486544..7912b5fbac 100644 --- a/src/web-ui/src/flow_chat/components/modern/flowChatHistoryBoundary.ts +++ b/src/web-ui/src/flow_chat/components/modern/flowChatHistoryBoundary.ts @@ -7,19 +7,9 @@ * to the container, and how the position was arrived at belongs to whatever is * placing the items. * - * Two questions, deliberately two functions, because one predicate cannot serve - * both and it was tried: - * - * - **Has the reader arrived?** `historyBoundariesReached`, from the item range - * alone. This is what the container's arming latch turns on. - * - **Is it worth asking?** `historyBoundariesForVisibleRange`, which adds a - * screenful of lead so the page lands off screen. - * - * Serving the latch from the wider answer walls it shut. The latch disarms a - * direction on dispatch and re-arms it when the reader is no longer at the - * boundary, so a boundary that counts as reached from a screen away is one the - * reader can never be off — measured: one page fetched, then `not-rearmed` - * refusals for eleven seconds while the reader kept scrolling into a wall. + * Physical arrival and the one-screen prefetch lead are separate geometry + * questions. Neither creates demand or re-arms requests: FlowChatHistoryPager + * owns permission from reader intent, request results and committed layout. */ import type { SessionHistoryWindowDirection } from '../../store/FlowChatStore'; @@ -94,9 +84,7 @@ export interface HistoryBoundaryProximity { /** * Directions the visible range is sitting on — the reader has arrived. * - * The arming latch's question, and only ever this one. It has to be answerable - * as false while the reader is still near the boundary, or a direction disarmed - * on dispatch has no way back. + * This excludes the prefetch lead and is useful for describing actual arrival. * * `'after'` only applies to a history window. A tail presentation is already * anchored to the newest Turn, so there is nothing past its end to fetch. diff --git a/src/web-ui/src/flow_chat/components/modern/flowChatHistoryPager.test.ts b/src/web-ui/src/flow_chat/components/modern/flowChatHistoryPager.test.ts new file mode 100644 index 0000000000..4725b2751e --- /dev/null +++ b/src/web-ui/src/flow_chat/components/modern/flowChatHistoryPager.test.ts @@ -0,0 +1,154 @@ +import { describe, expect, it } from 'vitest'; +import { FlowChatHistoryPager } from './flowChatHistoryPager'; + +function setup() { + const pager = new FlowChatHistoryPager(); + pager.commitLayout('tail', { before: 'turn-40', after: 'turn-46' }); + return pager; +} + +describe('FlowChatHistoryPager', () => { + it('acknowledges an already-projected legacy result without a prepare callback', () => { + const pager = setup(); + const ticket = pager.begin('before')!; + pager.readerIntent('before'); + pager.finish(ticket, 'applied'); + expect(pager.begin('before')).toBeNull(); + pager.commitLayout('tail', { before: 'turn-40', after: 'turn-46' }); + expect(pager.begin('before')).not.toBeNull(); + }); + + it('uses the same request/layout/demand policy for newer history', () => { + const pager = setup(); + const ticket = pager.begin('after')!; + pager.prepareCommit(ticket); + pager.readerIntent('after'); + pager.finish(ticket, 'applied'); + pager.commitLayout('40:54', { before: 'turn-40', after: 'turn-54' }); + expect(pager.begin('after')).not.toBeNull(); + }); + + it.each(['layout-first', 'result-first'] as const)('waits for both result and layout (%s)', order => { + const pager = setup(); + const ticket = pager.begin('before')!; + pager.prepareCommit(ticket); + pager.readerIntent('before'); + const commit = () => pager.commitLayout('38:46', { before: 'turn-38', after: 'turn-46' }); + if (order === 'layout-first') commit(); + else pager.finish(ticket, 'applied'); + expect(pager.begin('before')).toBeNull(); + if (order === 'layout-first') pager.finish(ticket, 'applied'); + else commit(); + expect(pager.begin('before')).not.toBeNull(); + }); + + it('pages twice across tail -> history without ever reaching the physical boundary', () => { + const pager = setup(); + const first = pager.begin('before')!; + pager.prepareCommit(first); + pager.finish(first, 'applied'); + pager.commitLayout('38:46', { before: 'turn-38', after: 'turn-46' }); + pager.observeProximity(new Set(['before'])); + expect(pager.begin('before')).toBeNull(); + pager.readerIntent('before'); + const second = pager.begin('before')!; + expect(second.id).not.toBe(first.id); + pager.prepareCommit(second); + pager.commitLayout('30:46', { before: 'turn-30', after: 'turn-46' }); + pager.finish(second, 'applied'); + expect(pager.begin('before')).toBeNull(); + }); + + it('coalesces ongoing reader demand and never converts repeated layout into demand', () => { + const pager = setup(); + const first = pager.begin('before')!; + for (let i = 0; i < 10; i++) pager.readerIntent('before'); + pager.prepareCommit(first); + pager.commitLayout('38:46', { before: 'turn-38', after: 'turn-46' }); + pager.finish(first, 'applied'); + const second = pager.begin('before')!; + pager.prepareCommit(second); + pager.finish(second, 'applied'); + for (let i = 0; i < 10; i++) { + pager.commitLayout('30:46', { before: 'turn-30', after: 'turn-46' }); + pager.observeProximity(new Set(['before'])); + expect(pager.begin('before')).toBeNull(); + } + }); + + it('accepts fresh top-edge intent when the short page cannot scroll', () => { + const pager = setup(); + const ticket = pager.begin('before')!; + pager.prepareCommit(ticket); + pager.finish(ticket, 'applied'); + pager.commitLayout('38:46', { before: 'turn-38', after: 'turn-46' }); + pager.readerIntent('before'); + expect(pager.begin('before')).not.toBeNull(); + }); + + it('drops queued demand when the reader reverses or the new boundary is far away', () => { + const pager = setup(); + const ticket = pager.begin('before')!; + pager.readerIntent('before'); + pager.readerIntent('after'); + expect(pager.snapshot('before').demand).toBe(false); + pager.readerIntent('before'); + pager.prepareCommit(ticket); + pager.finish(ticket, 'applied'); + pager.commitLayout('30:46', { before: 'turn-30', after: 'turn-46' }); + pager.observeProximity(new Set()); + expect(pager.begin('before')).toBeNull(); + pager.readerIntent('before'); + pager.observeProximity(new Set(['before'])); + expect(pager.begin('before')).not.toBeNull(); + }); + + it.each(['applied', 'exhausted', 'not-ready', 'cancelled'] as const)('ignores stale %s after navigation', result => { + const pager = setup(); + const old = pager.begin('before')!; + pager.reset(); + pager.commitLayout('10:18', { before: 'turn-10', after: 'turn-18' }); + const current = pager.begin('before')!; + expect(pager.prepareCommit(old)).toBe(false); + expect(pager.finish(old, result)).toBe(false); + expect(pager.snapshot('before').requestId).toBe(current.id); + }); + + it('invalidates an unprepared fetch when a different presentation commits', () => { + const pager = setup(); + const ticket = pager.begin('before')!; + pager.commitLayout('10:18', { before: 'turn-10', after: 'turn-18' }); + expect(pager.prepareCommit(ticket)).toBe(false); + expect(pager.finish(ticket, 'exhausted')).toBe(false); + expect(pager.begin('before')).not.toBeNull(); + }); + + it('does not cancel an older-page fetch when live output extends the other end', () => { + const pager = setup(); + const ticket = pager.begin('before')!; + pager.commitLayout('40:47', { before: 'turn-40', after: 'turn-47' }); + expect(pager.begin('before')).toBeNull(); + expect(pager.prepareCommit(ticket)).toBe(true); + pager.commitLayout('32:47', { before: 'turn-32', after: 'turn-47' }); + expect(pager.finish(ticket, 'applied')).toBe(true); + }); + + it('keeps exhaustion scoped to the boundary that answered', () => { + const pager = setup(); + pager.finish(pager.begin('before')!, 'exhausted'); + pager.readerIntent('before'); + expect(pager.begin('before')).toBeNull(); + pager.commitLayout('10:18', { before: 'turn-10', after: 'turn-18' }); + expect(pager.begin('before')).not.toBeNull(); + }); + + it.each(['not-ready', 'cancelled'] as const)('does not retry %s on passive evaluations', result => { + const pager = setup(); + const ticket = pager.begin('before')!; + pager.readerIntent('before'); + pager.finish(ticket, result); + expect(pager.begin('before')).toBeNull(); + pager.readerIntent('before'); + expect(pager.begin('before')).not.toBeNull(); + }); +}); diff --git a/src/web-ui/src/flow_chat/components/modern/flowChatHistoryPager.ts b/src/web-ui/src/flow_chat/components/modern/flowChatHistoryPager.ts new file mode 100644 index 0000000000..8ad3bd31da --- /dev/null +++ b/src/web-ui/src/flow_chat/components/modern/flowChatHistoryPager.ts @@ -0,0 +1,132 @@ +import type { SessionHistoryWindowDirection } from '../../store/FlowChatStore'; + +type Direction = SessionHistoryWindowDirection; +export type HistoryPageResult = 'applied' | 'exhausted' | 'not-ready' | 'cancelled'; +type Phase = 'ready' | 'requesting' | 'awaiting-layout' | 'waiting-for-reader' | 'exhausted'; + +export interface HistoryPageTicket { + readonly id: number; + readonly direction: Direction; + readonly window: string; + readonly boundary: string | null; +} + +interface DirectionState { + phase: Phase; + demand: boolean; + ticket: HistoryPageTicket | null; + prepared: boolean; + layoutObserved: boolean; +} + +const directions = ['before', 'after'] as const; +const initialState = (): DirectionState => ({ + phase: 'ready', demand: false, ticket: null, prepared: false, layoutObserved: false, +}); + +/** + * Request completion and React's layout commit can arrive in either order. + * Neither grants another page: only fresh reader demand does. In particular, + * prepend compensation and repeated geometry observations cannot page a whole + * transcript while the reader is still. A demand is coalesced, not counted. + * + * The former armed/reached pair lost its true -> false edge on tail -> history + * transitions (confirmed by paging probes). It also required a prefetch to have + * reached the physical boundary. This controller needs neither assumption. + */ +export class FlowChatHistoryPager { + private states: Record = { before: initialState(), after: initialState() }; + private window = ''; + private boundaries: Record = { before: null, after: null }; + private nextId = 0; + + reset(): void { + this.states = { before: initialState(), after: initialState() }; + } + + readerIntent(direction: Direction): void { + this.states[direction].demand = true; + // Reversing direction supersedes a queued request from an earlier gesture. + this.states[direction === 'before' ? 'after' : 'before'].demand = false; + } + + observeProximity(asking: ReadonlySet): void { + for (const direction of directions) { + if (!asking.has(direction)) this.states[direction].demand = false; + } + } + + /** Called after the list's measurement/prepend compensation layout effects. */ + commitLayout(window: string, boundaries: Record): void { + for (const direction of directions) { + const state = this.states[direction]; + const boundaryMoved = this.boundaries[direction] !== boundaries[direction]; + const moved = this.window !== window || boundaryMoved; + if (moved && state.phase === 'exhausted') this.states[direction] = initialState(); + if (state.ticket && boundaryMoved && !state.prepared) { + // A different presentation won while the request was still fetching. + // Movement at the opposite end (e.g. live output) cannot cancel this + // direction's fetch while its rendered boundary is unchanged. + this.states[direction] = initialState(); + } else if (state.ticket && state.prepared && ( + this.boundaries[direction] !== boundaries[direction] || state.phase === 'awaiting-layout' + )) { + state.layoutObserved = true; + if (state.phase === 'awaiting-layout') { + state.phase = 'waiting-for-reader'; + state.ticket = null; + } + } + } + this.window = window; + this.boundaries = boundaries; + } + + snapshot(direction: Direction): { phase: Phase; demand: boolean; requestId: number | null } { + const state = this.states[direction]; + return { phase: state.phase, demand: state.demand, requestId: state.ticket?.id ?? null }; + } + + begin(direction: Direction): HistoryPageTicket | null { + const state = this.states[direction]; + if (state.phase !== 'ready' && !(state.phase === 'waiting-for-reader' && state.demand)) return null; + const ticket: HistoryPageTicket = { + id: ++this.nextId, direction, window: this.window, boundary: this.boundaries[direction], + }; + this.states[direction] = { + phase: 'requesting', demand: false, ticket, prepared: false, layoutObserved: false, + }; + return ticket; + } + + isCurrent(ticket: HistoryPageTicket): boolean { + return this.states[ticket.direction].ticket === ticket; + } + + prepareCommit(ticket: HistoryPageTicket): boolean { + if (!this.isCurrent(ticket)) return false; + this.states[ticket.direction].prepared = true; + return true; + } + + finish(ticket: HistoryPageTicket, result: HistoryPageResult): boolean { + if (!this.isCurrent(ticket)) return false; + const state = this.states[ticket.direction]; + if (result === 'applied') { + // Legacy handlers can return an already-projected page without needing + // the optional pre-commit hook. The following acknowledgement commit is + // still required before another request is eligible. + state.prepared = true; + state.phase = state.layoutObserved ? 'waiting-for-reader' : 'awaiting-layout'; + if (state.layoutObserved) state.ticket = null; + } else { + const sameBoundary = ticket.window === this.window && ticket.boundary === this.boundaries[ticket.direction]; + state.phase = result === 'exhausted' && sameBoundary ? 'exhausted' : 'waiting-for-reader'; + state.ticket = null; + // Errors/cancellation need a new reader action; rendering an error must + // not turn a failed load into a self-sustaining retry loop. + state.demand = false; + } + return true; + } +} diff --git a/src/web-ui/src/flow_chat/components/modern/useFlowChatViewportOwner.test.tsx b/src/web-ui/src/flow_chat/components/modern/useFlowChatViewportOwner.test.tsx new file mode 100644 index 0000000000..3b6b919844 --- /dev/null +++ b/src/web-ui/src/flow_chat/components/modern/useFlowChatViewportOwner.test.tsx @@ -0,0 +1,63 @@ +// @vitest-environment jsdom +import React, { act, useRef } from 'react'; +import { createRoot } from 'react-dom/client'; +import { describe, expect, it } from 'vitest'; +import { useFlowChatViewportOwner, type FlowChatViewportOwnerApi } from './useFlowChatViewportOwner'; + +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + +describe('viewport reader travel', () => { + it('removes registered writes and shifts but keeps intervening reader travel', async () => { + const host = document.createElement('div'); + const scroller = document.createElement('div'); + const root = createRoot(host); + let owner!: FlowChatViewportOwnerApi; + function Harness() { + owner = useFlowChatViewportOwner(useRef(scroller)); + return null; + } + try { + await act(async () => root.render()); + expect(owner.readReaderScrollPosition()).toBe(0); + owner.write({ owner: 'layout-correction', topPx: 100 }); + expect(owner.readReaderScrollPosition()).toBe(0); + owner.claim('user-gesture'); + scroller.scrollTop = 60; + expect(owner.readReaderScrollPosition()).toBe(-40); + owner.shift(200); + expect(owner.readReaderScrollPosition()).toBe(-40); + scroller.scrollTop -= 30; + expect(owner.readReaderScrollPosition()).toBe(-70); + } finally { + await act(async () => root.unmount()); + } + }); + + it('accounts for actual clamped travel and does not count refused writes', async () => { + const host = document.createElement('div'); + const scroller = document.createElement('div'); + let top = 0; + Object.defineProperty(scroller, 'scrollTop', { + get: () => top, set: (next: number) => { top = Math.max(0, Math.min(100, next)); }, + }); + const root = createRoot(host); + let owner!: FlowChatViewportOwnerApi; + function Harness() { + owner = useFlowChatViewportOwner(useRef(scroller)); + return null; + } + try { + await act(async () => root.render()); + owner.shift(300); + expect(top).toBe(100); + expect(owner.readReaderScrollPosition()).toBe(0); + owner.claim('user-gesture', { holdForMs: 1000 }); + expect(owner.write({ owner: 'follow-output', topPx: 0 })).toBe(false); + expect(owner.readReaderScrollPosition()).toBe(0); + scroller.scrollTop = 80; + expect(owner.readReaderScrollPosition()).toBe(-20); + } finally { + await act(async () => root.unmount()); + } + }); +}); diff --git a/src/web-ui/src/flow_chat/components/modern/useFlowChatViewportOwner.ts b/src/web-ui/src/flow_chat/components/modern/useFlowChatViewportOwner.ts index 9b4d01a6ec..ea175585e4 100644 --- a/src/web-ui/src/flow_chat/components/modern/useFlowChatViewportOwner.ts +++ b/src/web-ui/src/flow_chat/components/modern/useFlowChatViewportOwner.ts @@ -71,12 +71,15 @@ export interface FlowChatViewportOwnerApi { * transcript, and this is what keeps that position meaning the same thing. */ shift: (byPx: number) => boolean; + /** Offset with this register's synchronous writes removed, for reader travel. */ + readReaderScrollPosition: () => number; } export function useFlowChatViewportOwner( scrollerRef: RefObject, ): FlowChatViewportOwnerApi { const claimRef = useRef(null); + const writtenTravelRef = useRef(0); /** * Take the viewport and report who was holding it, which the register drops @@ -171,7 +174,9 @@ export function useFlowChatViewportOwner( // Assigned rather than `scrollTo({behavior:'auto'})`: an assignment also // cancels any animation still running, which is what a writer taking the // viewport from an animated one means to do. + const before = scroller.scrollTop; scroller.scrollTop = request.topPx; + writtenTravelRef.current += scroller.scrollTop - before; } return true; }, [scrollerRef, takeViewport]); @@ -197,10 +202,16 @@ export function useFlowChatViewportOwner( }); } if (!allowed) return false; + const before = scroller.scrollTop; scroller.scrollTop += byPx; + writtenTravelRef.current += scroller.scrollTop - before; return true; }, [canShift, currentOwner, scrollerRef]); + const readReaderScrollPosition = useCallback(() => ( + (scrollerRef.current?.scrollTop ?? 0) - writtenTravelRef.current + ), [scrollerRef]); + return useMemo(() => ({ claim, release, @@ -208,5 +219,6 @@ export function useFlowChatViewportOwner( currentOwner, write, shift, - }), [canShift, claim, currentOwner, release, shift, write]); + readReaderScrollPosition, + }), [canShift, claim, currentOwner, release, shift, write, readReaderScrollPosition]); } diff --git a/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.initial-window.test.tsx b/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.initial-window.test.tsx index 6f68774c3c..874f8ef51b 100644 --- a/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.initial-window.test.tsx +++ b/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.initial-window.test.tsx @@ -11,6 +11,7 @@ let latestApi: FlowChatVirtualizer; let reconcileEnabled = false; let shortOverscan = false; let viewportSuspended = false; +let shiftEnabled = false; function Harness({ count, tail }: { count: number; tail: boolean }) { const scrollerRef = useRef(null); const headerRef = useRef(null); @@ -21,6 +22,12 @@ function Harness({ count, tail }: { count: number; tail: boolean }) { estimateItemHeightPx: () => 100, startAtTailOnMount: tail, isViewportSuspended: () => viewportSuspended, + shiftViewport: delta => { + const scroller = scrollerRef.current; + if (!shiftEnabled || !scroller) return false; + scroller.scrollTop += delta; + return true; + }, reconcileOpeningMeasurement: () => { const scroller = scrollerRef.current; if (!reconcileEnabled || !scroller) return false; @@ -56,6 +63,7 @@ describe('initial virtual window with the real virtualizer', () => { reconcileEnabled = false; shortOverscan = false; viewportSuspended = false; + shiftEnabled = false; vi.useFakeTimers(); vi.stubGlobal('requestAnimationFrame', vi.fn().mockReturnValue(1)); vi.stubGlobal('cancelAnimationFrame', vi.fn()); @@ -170,6 +178,28 @@ describe('initial virtual window with the real virtualizer', () => { expect(windows.at(-1)![0]).toBe(0); }); + it('keeps the measured reading window stable before the compensation scroll event arrives', () => { + shortOverscan = true; + render(34, true); + shiftEnabled = true; + const scroller = host.querySelector('[data-scroller]')!; + act(() => { + scroller.scrollTop = 2500; + scroller.dispatchEvent(new Event('scroll')); + }); + act(() => latestApi.measureRenderedItems()); + expect(scroller.scrollTop).toBeLessThan(2500); + const committedWindow = [...windows.at(-1)!]; + const mountedRows = [...host.querySelectorAll('[data-virtual-index]')]; + // Scroll-end still holds the offset from before the owner applied the shift. + act(() => vi.advanceTimersByTime(200)); + expect(windows.at(-1)).toEqual(committedWindow); + expect([...host.querySelectorAll('[data-virtual-index]')]).toEqual(mountedRows); + act(() => scroller.dispatchEvent(new Event('scroll'))); + expect(windows.at(-1)).toEqual(committedWindow); + expect([...host.querySelectorAll('[data-virtual-index]')]).toEqual(mountedRows); + }); + it.each([false, true])('reconciles measured overscan before delayed events (enabled=%s)', enabled => { shortOverscan = true; render(34, true); diff --git a/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.ts b/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.ts index 02cbc5f5fe..47b21ae4b9 100644 --- a/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.ts +++ b/src/web-ui/src/flow_chat/components/modern/useFlowChatVirtualizer.ts @@ -348,6 +348,7 @@ export function useFlowChatVirtualizer({ const reconcileOpeningMeasurementRef = useRef(reconcileOpeningMeasurement); reconcileOpeningMeasurementRef.current = reconcileOpeningMeasurement; const pendingMeasurementRef = useRef(false); + const pendingMeasurementShiftRef = useRef(false); const publishMeasuredOffsetRef = useRef<((actualOffsetPx?: number) => void) | null>(null); const syncViewportOffset = useCallback((actualOffsetPx: number) => { if (Number.isFinite(actualOffsetPx)) publishMeasuredOffsetRef.current?.(actualOffsetPx); @@ -467,13 +468,19 @@ export function useFlowChatVirtualizer({ onChange: (_instance, sync) => { if (sync || !pendingMeasurementRef.current) return; pendingMeasurementRef.current = false; + const shifted = pendingMeasurementShiftRef.current; + pendingMeasurementShiftRef.current = false; // A trace showed rows 22..26 shrinking by 729px while range selection // still used 8669px. They unmounted, then remounted on the delayed 7940px // scroll event (113.8ms style work on removal). Reconcile through follow, // then publish the real offset before React selects the next window. // Retest: five row cleanups became zero; post-reveal sampling advanced // from 786.8ms to 596.4ms (single desktop trace, not paint timing). - if (reconcileOpeningMeasurementRef.current?.()) publishMeasuredOffsetRef.current?.(); + const reconciled = reconcileOpeningMeasurementRef.current?.(); + // Ordinary reading also shifts the real viewport when measured rows above + // it shrink. Publish that readback after the size cache updates, before + // selecting a window from the old offset and unmounting those same rows. + if (reconciled || shifted) publishMeasuredOffsetRef.current?.(); }, estimateSize, getItemKey: resolveItemKey, @@ -515,6 +522,7 @@ export function useFlowChatVirtualizer({ // move the reader's existing content and needs a viewport shift. const fullyAboveViewport = isItemFullyAboveViewport(item.end, beforeScrollTopPx); const applied = fullyAboveViewport ? shiftViewport(delta) : false; + if (applied) pendingMeasurementShiftRef.current = true; const virtualItem = itemsRef.current[item.index]; if (isViewportDiagnosticsEnabled()) { const diagnosticItem = virtualItem as { diff --git a/src/web-ui/src/flow_chat/components/usage/SessionUsageReportCard.scss b/src/web-ui/src/flow_chat/components/usage/SessionUsageReportCard.scss index 97ee8b1478..94c3c353b4 100644 --- a/src/web-ui/src/flow_chat/components/usage/SessionUsageReportCard.scss +++ b/src/web-ui/src/flow_chat/components/usage/SessionUsageReportCard.scss @@ -469,7 +469,11 @@ font-size: var(--openbitfun-type-support-font-size); line-height: var(--openbitfun-type-support-line-height); - span + span { + // Chromium associated the former span + span rule with unrelated transcript + // spans during virtual row changes. Use owned metadata classes to narrow the + // invalidation set. Equal-DOM trace sample: 53 added / 771 retained elements, + // style time 59 -> 31ms and elementCount 3867 -> 2027 (single desktop run). + > .session-usage-report-card__compact-meta-item + .session-usage-report-card__compact-meta-item { padding-inline-start: var(--openbitfun-space-2); border-inline-start: var(--openbitfun-border-width-default) solid var(--openbitfun-color-border-subtle); } diff --git a/src/web-ui/src/flow_chat/components/usage/SessionUsageReportCard.tsx b/src/web-ui/src/flow_chat/components/usage/SessionUsageReportCard.tsx index 766063532f..cd226347d7 100644 --- a/src/web-ui/src/flow_chat/components/usage/SessionUsageReportCard.tsx +++ b/src/web-ui/src/flow_chat/components/usage/SessionUsageReportCard.tsx @@ -246,8 +246,8 @@ export const SessionUsageReportCard: React.FC = ({
- {formatUsageTimestamp(generatedAt ?? report.generatedAt, t)} - {t('usage.card.turns', { count: report.scope.turnCount })} + {formatUsageTimestamp(generatedAt ?? report.generatedAt, t)} + {t('usage.card.turns', { count: report.scope.turnCount })}
diff --git a/src/web-ui/src/flow_chat/hooks/useTypewriter.ts b/src/web-ui/src/flow_chat/hooks/useTypewriter.ts index 8a2838c2f3..e08ea25d37 100644 --- a/src/web-ui/src/flow_chat/hooks/useTypewriter.ts +++ b/src/web-ui/src/flow_chat/hooks/useTypewriter.ts @@ -113,6 +113,10 @@ export const TYPEWRITER_MIN_PAINT_INTERVAL_MS = 16; export const TYPEWRITER_FINISH_MIN_PAINT_INTERVAL_MS = 8; export interface TypewriterOptions { + /** Skip playback and drain immediately, e.g. when the owning body is hidden. + * Unlike animate=false (stream finished), this does not animate the backlog. + */ + revealImmediately?: boolean; /** * Replay the whole current text on mount. Defaults to false: mounting starts * from the current text and only reveals later appended content. @@ -209,8 +213,9 @@ export function useTypewriter( options: TypewriterOptions = {} ): TypewriterResult { const replayOnMount = options.replayOnMount ?? false; + const revealImmediately = options.revealImmediately ?? false; const [prefersReducedMotion, setPrefersReducedMotion] = useState(getPrefersReducedMotion); - const shouldReplayInitialText = animate && replayOnMount && !prefersReducedMotion; + const shouldReplayInitialText = animate && replayOnMount && !prefersReducedMotion && !revealImmediately; const [displayText, setDisplayText] = useState(shouldReplayInitialText ? '' : targetText); const revealedRef = useRef(shouldReplayInitialText ? 0 : targetText.length); const targetRef = useRef(targetText); @@ -220,7 +225,7 @@ export function useTypewriter( const lastPaintMsRef = useRef(0); const fractionalCarryRef = useRef(0); - const isRevealing = !prefersReducedMotion + const isRevealing = !revealImmediately && !prefersReducedMotion && (animate || displayText.length < targetText.length); useEffect(() => { @@ -229,7 +234,7 @@ export function useTypewriter( const pageIsHidden = typeof document !== 'undefined' && document.visibilityState === 'hidden'; - if (prefersReducedMotion || pageIsHidden) { + if (revealImmediately || prefersReducedMotion || pageIsHidden) { if (rafRef.current !== null) { cancelAnimationFrame(rafRef.current); rafRef.current = null; @@ -342,7 +347,7 @@ export function useTypewriter( if (rafRef.current === null && targetText.length > revealedRef.current) { rafRef.current = requestAnimationFrame(tick); } - }, [targetText, animate, prefersReducedMotion]); + }, [targetText, animate, prefersReducedMotion, revealImmediately]); useEffect(() => { if (typeof window === 'undefined' || typeof window.matchMedia !== 'function') return; @@ -380,5 +385,5 @@ export function useTypewriter( }; }, []); - return { displayText, isRevealing }; + return { displayText: revealImmediately ? targetText : displayText, isRevealing }; } diff --git a/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.test.tsx b/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.test.tsx index a8ed7c303c..e0ffd1bed1 100644 --- a/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.test.tsx +++ b/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.test.tsx @@ -5,6 +5,11 @@ import { createRoot, type Root } from 'react-dom/client'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { FlowThinkingItem } from '../types/flow-chat'; import { ModelThinkingDisplay } from './ModelThinkingDisplay'; +import { latestReasoningSummaryPreview } from '../utils/reasoningSummaryPresentation'; + +vi.mock('../utils/reasoningSummaryPresentation', { spy: true }); + +const markdownRender = vi.hoisted(() => vi.fn()); vi.mock('react-i18next', () => ({ useTranslation: () => ({ @@ -40,9 +45,10 @@ vi.mock('./useToolCardHeightContract', () => ({ })); vi.mock('@/infrastructure/markdown', () => ({ - MarkdownRenderer: ({ content }: { content: string }) => ( -
{content}
- ), + MarkdownRenderer: ({ content }: { content: string }) => { + markdownRender(content); + return
{content}
; + }, })); function summaryItem(content: string): FlowThinkingItem { @@ -72,6 +78,8 @@ describe('ModelThinkingDisplay reasoning summary', () => { container = document.createElement('div'); document.body.appendChild(container); root = createRoot(container); + markdownRender.mockClear(); + vi.mocked(latestReasoningSummaryPreview).mockClear(); }); afterEach(() => { @@ -80,6 +88,27 @@ describe('ModelThinkingDisplay reasoning summary', () => { vi.unstubAllGlobals(); }); + it.each(['reasoning', undefined] as const)( + 'skips summary processing for streaming reasoning (kind=%s)', async reasoningKind => { + const item = { ...summaryItem('**Reasoning**\n\n'.repeat(7000)), reasoningKind }; + await act(async () => root.render()); + const content = `${item.content}More`; + await act(async () => root.render()); + expect(latestReasoningSummaryPreview).not.toHaveBeenCalled(); + expect(container.querySelector('[data-testid="thinking-markdown"]')?.textContent).toBe(content); + }, + ); + + it('computes the preview when the kind changes to summary with unchanged content', async () => { + const item = summaryItem('**Latest summary**'); + await act(async () => root.render()); + expect(latestReasoningSummaryPreview).not.toHaveBeenCalled(); + await act(async () => root.render()); + expect(latestReasoningSummaryPreview).toHaveBeenCalledWith(item.content); + expect(container.querySelector('[data-openbitfun-part="label"]')?.textContent).toBe('Latest summary'); + }); + it('defaults to a collapsed single-line preview of the latest summary part', async () => { await act(async () => { root.render( { expect(panel?.getAttribute('data-expanded')).toBe('false'); expect(label?.textContent).toBe('Preparing the repair'); expect(label?.textContent).not.toContain('characters'); + expect(markdownRender).not.toHaveBeenCalled(); }); + it('does not render a large collapsed reasoning body, including content updates', async () => { + const item = { ...summaryItem('**Reasoning**\n\n'.repeat(6000)), + reasoningKind: 'reasoning' as const, isStreaming: false, status: 'completed' as const }; + await act(async () => root.render()); + await act(async () => root.render()); + expect(markdownRender).not.toHaveBeenCalled(); + expect(container.querySelector('[data-testid="chat-thinking-content"]')).toBeNull(); + }); + + it('mounts and releases content when forced expansion changes without an animation', async () => { + const item = summaryItem('**Full summary**'); + await act(async () => root.render()); + expect(container.querySelector('[data-testid="thinking-markdown"]')?.textContent).toBe(item.content); + await act(async () => root.render()); + expect(container.querySelector('[data-testid="thinking-markdown"]')).toBeNull(); + }); + + it.each(['finish', 'cancel', 'reopen'] as const)( + 'retains closing content until the actual transition settles: %s', async outcome => { + await act(async () => root.render()); + const toggle = container.querySelector('[data-testid="chat-thinking-toggle"]') as HTMLElement; + await act(async () => toggle.click()); + const body = container.querySelector('[data-testid="thinking-markdown"]'); + let finish!: () => void; + let cancel!: () => void; + const finished = new Promise((resolve, reject) => { + finish = resolve; + cancel = () => reject(new Error('Transition cancelled')); + }); + const expandContainer = container.querySelector('[data-openbitfun-part="expandContainer"]') as HTMLElement; + Object.defineProperty(expandContainer, 'getAnimations', { + value: () => [{ transitionProperty: 'grid-template-rows', finished }], + }); + await act(async () => toggle.click()); + expect(container.querySelector('[data-testid="thinking-markdown"]')).toBe(body); + if (outcome === 'reopen') await act(async () => toggle.click()); + await act(async () => { if (outcome === 'finish') finish(); else cancel(); }); + expect(container.querySelector('[data-testid="thinking-markdown"]')).toBe(outcome === 'reopen' ? body : null); + }, + ); + it('uses design-system thinking and disclosure icons in the header', async () => { await act(async () => { root.render(); diff --git a/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.tsx b/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.tsx index 47b730c428..33eeb2a51b 100644 --- a/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.tsx +++ b/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.tsx @@ -59,11 +59,6 @@ export const ModelThinkingDisplay: React.FC = ({ } | null>(null); const isActive = isStreaming || status === 'streaming'; - const { displayText: displayContent, isRevealing } = useTypewriter( - isSummary ? '' : content, - isActive && !isSummary, - ); - useReportTypewriterReveal(thinkingItem.id, isRevealing); const shouldDefaultExpanded = forceExpanded || (!isSummary && ( displayContext === 'subagent-projection' ? isActive || isLastItem @@ -71,6 +66,40 @@ export const ModelThinkingDisplay: React.FC = ({ )); const [isExpanded, setIsExpanded] = useState(shouldDefaultExpanded); + const [retainClosingContent, setRetainClosingContent] = useState(shouldDefaultExpanded); + const expandContainerRef = useRef(null); + const shouldMountContent = isExpanded || retainClosingContent; + const { displayText: displayContent, isRevealing } = useTypewriter( + isSummary ? '' : content, + isActive && !isSummary, + // Keep playback through the closing transition, then release the reveal + // gate and track current content without animating an invisible backlog. + { revealImmediately: !shouldMountContent }, + ); + useReportTypewriterReveal(thinkingItem.id, isRevealing); + + // Keep the existing collapse transition, but never build a hidden Markdown + // tree on an initially collapsed virtual-row mount. Observe actual CSS + // transitions so reduced motion and cancelled transitions also release it. + useLayoutEffect(() => { + if (isExpanded) { + setRetainClosingContent(true); + return; + } + if (!retainClosingContent) return; + const transitions = expandContainerRef.current?.getAnimations?.().filter(animation => ( + 'transitionProperty' in animation && animation.transitionProperty === 'grid-template-rows' + )) ?? []; + if (transitions.length === 0) { + setRetainClosingContent(false); + return; + } + let cancelled = false; + void Promise.allSettled(transitions.map(animation => animation.finished)).then(() => { + if (!cancelled) setRetainClosingContent(false); + }); + return () => { cancelled = true; }; + }, [isExpanded, retainClosingContent]); const userToggledRef = useRef(false); const { cardRootRef, applyExpandedState } = useToolCardHeightContract({ toolId: thinkingItem.id, @@ -299,8 +328,10 @@ export const ModelThinkingDisplay: React.FC = ({ }, [content, t]); const summaryPreview = useMemo( - () => latestReasoningSummaryPreview(content), - [content], + // Ordinary reasoning never displays this preview. Avoid splitting and + // stripping its potentially large body on every streaming update. + () => isSummary ? latestReasoningSummaryPreview(content) : '', + [content, isSummary], ); const handleToggleClick = () => { @@ -400,6 +431,7 @@ export const ModelThinkingDisplay: React.FC = ({
= ({ data-openbitfun-component="model-thinking-display" data-openbitfun-part="expandContainer" > -
+ {shouldMountContent &&
= ({ className="thinking-markdown" />
-
+
}
); diff --git a/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.typewriter.test.tsx b/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.typewriter.test.tsx new file mode 100644 index 0000000000..a3385f8164 --- /dev/null +++ b/src/web-ui/src/flow_chat/tool-cards/ModelThinkingDisplay.typewriter.test.tsx @@ -0,0 +1,92 @@ +// @vitest-environment jsdom +import React, { act } from 'react'; +import { createRoot } from 'react-dom/client'; +import { afterEach, expect, it, vi } from 'vitest'; +import { ModelThinkingDisplay } from './ModelThinkingDisplay'; +import type { FlowThinkingItem } from '../types/flow-chat'; +import { TypewriterRevealGateContext, useCreateTypewriterRevealGate } from '../hooks/typewriterRevealGateContext'; + +vi.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }) })); +vi.mock('@openbitfun/ui', () => ({ + OverflowText: ({ children }: { children: React.ReactNode }) => {children}, + Icon: () => null, +})); +vi.mock('@/infrastructure/markdown', () => ({ + MarkdownRenderer: ({ content }: { content: string }) =>
{content}
, +})); +vi.mock('./useToolCardHeightContract', () => ({ + useToolCardHeightContract: () => ({ + cardRootRef: { current: null }, + applyExpandedState: (_old: boolean, next: boolean, set: (next: boolean) => void) => set(next), + }), +})); + +let cleanup: (() => void) | undefined; +afterEach(() => { cleanup?.(); vi.unstubAllGlobals(); }); + +function setup() { + globalThis.IS_REACT_ACT_ENVIRONMENT = true; + const frames = new Map(); + let nextId = 0; + vi.stubGlobal('requestAnimationFrame', (cb: FrameRequestCallback) => { frames.set(++nextId, cb); return nextId; }); + vi.stubGlobal('cancelAnimationFrame', (id: number) => frames.delete(id)); + vi.stubGlobal('ResizeObserver', class { observe() {} disconnect() {} }); + const host = document.createElement('div'); + document.body.append(host); + const root = createRoot(host); + cleanup = () => { act(() => root.unmount()); host.remove(); }; + function Card({ content, streaming = true }: { content: string; streaming?: boolean }) { + const gate = useCreateTypewriterRevealGate(); + const item: FlowThinkingItem = { + id: 'thinking', type: 'thinking', reasoningKind: 'reasoning', content, + isStreaming: streaming, status: streaming ? 'streaming' : 'completed', + timestamp: 1, isCollapsed: false, + }; + return + {String(gate.isAnyRevealing)} + + ; + } + const render = (content: string, streaming = true) => act(() => root.render()); + const toggle = () => act(() => (host.querySelector('[data-testid="chat-thinking-toggle"]') as HTMLElement).click()); + return { host, frames, render, toggle, gate: () => host.querySelector('output')?.textContent }; +} + +it('keeps playback while closing, drains when hidden, and resumes only new text after reopening', async () => { + const h = setup(); + h.render('Start'); + const content = 'Start' + ' more'.repeat(2000); + h.render(content); + expect(h.host.querySelector('[data-testid="body"]')?.textContent).toBe('Start'); + let finish!: () => void; + const finished = new Promise(resolve => { finish = resolve; }); + const container = h.host.querySelector('[data-openbitfun-part="expandContainer"]')!; + Object.defineProperty(container, 'getAnimations', { value: () => [{ transitionProperty: 'grid-template-rows', finished }] }); + h.toggle(); + expect(h.host.querySelector('[data-testid="body"]')).not.toBeNull(); + expect(h.gate()).toBe('true'); + await act(async () => { finish(); }); + expect(h.host.querySelector('[data-testid="body"]')).toBeNull(); + expect(h.gate()).toBe('false'); + expect(h.frames.size).toBe(0); + const latest = content + ' hidden update'; + h.render(latest); + expect(h.gate()).toBe('false'); + expect(h.frames.size).toBe(0); + h.toggle(); + expect(h.host.querySelector('[data-testid="body"]')?.textContent).toBe(latest); + h.render(latest + ' next'); + expect(h.host.querySelector('[data-testid="body"]')?.textContent).toBe(latest); + expect(h.gate()).toBe('true'); +}); + +it('releases a finishing backlog immediately when collapsed without a transition', () => { + const h = setup(); + h.render('Start'); + h.render('Start' + ' more'.repeat(2000), false); + expect(h.gate()).toBe('true'); + h.toggle(); + expect(h.gate()).toBe('false'); + expect(h.frames.size).toBe(0); + expect(h.host.querySelector('[data-testid="body"]')).toBeNull(); +}); diff --git a/src/web-ui/src/flow_chat/tool-cards/SessionControlToolCard.test.tsx b/src/web-ui/src/flow_chat/tool-cards/SessionControlToolCard.test.tsx index 7caba5c178..4e43bf0585 100644 --- a/src/web-ui/src/flow_chat/tool-cards/SessionControlToolCard.test.tsx +++ b/src/web-ui/src/flow_chat/tool-cards/SessionControlToolCard.test.tsx @@ -58,7 +58,9 @@ describe('SessionControlToolCard', () => { let root: Root; beforeEach(() => { - dom = new JSDOM('
'); + dom = new JSDOM('
', { + pretendToBeVisual: true, + }); vi.stubGlobal('window', dom.window); vi.stubGlobal('document', dom.window.document); vi.stubGlobal('HTMLElement', dom.window.HTMLElement); @@ -71,6 +73,7 @@ describe('SessionControlToolCard', () => { act(() => { root.unmount(); }); + dom.window.close(); vi.unstubAllGlobals(); }); diff --git a/src/web-ui/src/flow_chat/tool-cards/ToolTimeoutIndicator.test.tsx b/src/web-ui/src/flow_chat/tool-cards/ToolTimeoutIndicator.test.tsx index e993a451a2..183aa35de8 100644 --- a/src/web-ui/src/flow_chat/tool-cards/ToolTimeoutIndicator.test.tsx +++ b/src/web-ui/src/flow_chat/tool-cards/ToolTimeoutIndicator.test.tsx @@ -3,6 +3,7 @@ import { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { renderToStaticMarkup } from 'react-dom/server'; import { readFileSync } from 'node:fs'; +import { JSDOM } from 'jsdom'; import { I18nextProvider, initReactI18next } from 'react-i18next'; import { createInstance, type i18n as I18nInstance } from 'i18next'; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -25,11 +26,6 @@ vi.mock('@/infrastructure/api/service-api/AgentAPI', () => ({ })); let i18n: I18nInstance; -let JSDOMCtor: (new ( - html?: string, - options?: { pretendToBeVisual?: boolean; url?: string } -) => { window: Window & typeof globalThis }) | null = null; - beforeAll(async () => { i18n = createInstance(); await i18n.use(initReactI18next).init({ @@ -55,8 +51,6 @@ beforeAll(async () => { interpolation: { escapeValue: false }, }); - const jsdom = await import('jsdom'); - JSDOMCtor = jsdom.JSDOM as typeof JSDOMCtor; }); function withI18n(element: React.ReactElement): React.ReactElement { @@ -77,7 +71,7 @@ describe('ToolTimeoutIndicator', () => { let root: Root | null = null; beforeEach(() => { - dom = new JSDOMCtor!('', { + dom = new JSDOM('', { pretendToBeVisual: true, url: 'http://localhost', }) as unknown as { window: Window & typeof globalThis }; diff --git a/src/web-ui/src/infrastructure/markdown/Markdown.scss b/src/web-ui/src/infrastructure/markdown/Markdown.scss index d2829a838c..0ba0801ef7 100644 --- a/src/web-ui/src/infrastructure/markdown/Markdown.scss +++ b/src/web-ui/src/infrastructure/markdown/Markdown.scss @@ -66,7 +66,20 @@ margin-bottom: 0 !important; } -.markdown-renderer > * + * { +// Universal sibling selectors here and in the nested-list gap rule below caused +// Chromium to invalidate the virtual-message-list__items subtree on row insertion +// and removal (confirmed by StyleInvalidatorInvalidationTracking). Select the +// same non-first element children without `* + *`; :where keeps specificity +// unchanged so paragraph/heading overrides retain their existing precedence. +// Desktop trace comparison (2026-09-22, selector stats OFF), at equal DOM counts: +// - Add row 27 (37 added / 451 retained elements): style 53 -> 35ms, +// trace elementCount 3050 -> 2348. +// - Add row 20 (53 added / 771 retained): 77 -> 59ms, 5061 -> 3867. +// - Remove rows 29/30 (77 removed / 950 retained): 81 -> 46ms, 5789 -> 4301. +// These are single-run samples, not a universal speedup; elementCount is the +// trace's style calculation count, not a count of unique DOM nodes. Other costly +// invalidations remained, so this change does not claim to eliminate scroll jank. +.markdown-renderer > :where(:not(:first-child)) { margin-top: var(--markdown-block-gap); } @@ -77,7 +90,12 @@ line-height: inherit; } -.markdown-renderer p + p { +// Tag-only sibling rules caused unrelated elements to enter Chromium style +// invalidation (93 paragraph, 70 list-item and 104 table-cell records in one +// desktop trace). Owned classes narrow the candidates; :where() preserves +// specificity and same-tag adjacency, including sanitized HTML and math output. +// Overall scrolling improvement has not been measured for this change. +.markdown-renderer p:where(.markdown-paragraph) + p:where(.markdown-paragraph) { margin-top: var(--markdown-paragraph-gap); } @@ -156,7 +174,7 @@ word-wrap: break-word; } -.markdown-renderer li + li { +.markdown-renderer li:where(.markdown-list-item) + li:where(.markdown-list-item) { margin-top: var(--markdown-list-item-gap); } @@ -165,7 +183,8 @@ margin-bottom: 0; } -.markdown-renderer li > * + * { +// Same universal-sibling invalidation fix as the block-gap rule above. +.markdown-renderer li > :where(:not(:first-child)) { margin-top: var(--markdown-list-nested-gap); } @@ -457,7 +476,7 @@ vertical-align: middle; } -.markdown-renderer div[align="center"] > p + p { +.markdown-renderer div[align="center"] > p:where(.markdown-paragraph) + p:where(.markdown-paragraph) { margin-top: 0.2rem; } @@ -556,8 +575,8 @@ vertical-align: top; } -.markdown-renderer .table-wrapper th + th, -.markdown-renderer .table-wrapper td + td { +.markdown-renderer .table-wrapper th:where(.markdown-header-cell) + th:where(.markdown-header-cell), +.markdown-renderer .table-wrapper td:where(.markdown-data-cell) + td:where(.markdown-data-cell) { border-left: 1px solid var(--markdown-table-col-divider); } diff --git a/src/web-ui/src/infrastructure/markdown/MarkdownRenderer.test.tsx b/src/web-ui/src/infrastructure/markdown/MarkdownRenderer.test.tsx index fd21cca639..1e7b0216d0 100644 --- a/src/web-ui/src/infrastructure/markdown/MarkdownRenderer.test.tsx +++ b/src/web-ui/src/infrastructure/markdown/MarkdownRenderer.test.tsx @@ -185,6 +185,37 @@ describe('Markdown file links', () => { vi.clearAllMocks(); }); + it('preserves same-tag sibling matches for generated and raw HTML content', async () => { + const content = `First paragraph. + +Second paragraph. + +- First item + - Nested item + - Next nested item +- Second item + +| A | B | +| - | - | +| C | D | + +

One

text

Two

Three

+
ABCDE
`; + await act(async () => root.render()); + for (const [tag, className] of [ + ['p', 'markdown-paragraph'], ['li', 'markdown-list-item'], + ['th', 'markdown-header-cell'], ['td', 'markdown-data-cell'], + ]) { + const oldMatches = [...container.querySelectorAll(`${tag} + ${tag}`)]; + expect(oldMatches.length).toBeGreaterThan(0); + expect([...container.querySelectorAll(`${tag}:where(.${className}) + ${tag}:where(.${className})`)]) + .toEqual(oldMatches); + expect([...container.querySelectorAll(tag)].every(node => node.classList.contains(className))).toBe(true); + } + expect([...container.querySelectorAll('div[align="center"] > p:where(.markdown-paragraph) + p:where(.markdown-paragraph)')] + .map(node => node.textContent)).toEqual(['Two']); + }); + it.each([false, true])('keeps fullwidth parentheses outside bare web links (escaped=%s)', async escaped => { const url = 'http://127.0.0.1:8000'; const bare = escaped ? url.replace(':', '\\:') : url; diff --git a/src/web-ui/src/infrastructure/markdown/MarkdownRenderer.tsx b/src/web-ui/src/infrastructure/markdown/MarkdownRenderer.tsx index 0f61bee463..859fe74949 100644 --- a/src/web-ui/src/infrastructure/markdown/MarkdownRenderer.tsx +++ b/src/web-ui/src/infrastructure/markdown/MarkdownRenderer.tsx @@ -1707,14 +1707,23 @@ export const MarkdownRenderer = React.memo(({ return
    {children}
; }, - li({ children, ...props }: any) { - return
  • {children}
    • ; + li({ node: _node, children, className, ...props }: any) { + return
  • {children}
    • ; + }, + + th({ node: _node, children, className, ...props }: any) { + return {children}; + }, + + td({ node: _node, children, className, ...props }: any) { + return {children}; }, - p({ children, align, style, ...props }: any) { + p({ node: _node, children, align, style, className, ...props }: any) { return (

    {children} diff --git a/src/web-ui/src/shared/notification-system/components/NotificationItem.test.tsx b/src/web-ui/src/shared/notification-system/components/NotificationItem.test.tsx index 259887f349..76d31efa55 100644 --- a/src/web-ui/src/shared/notification-system/components/NotificationItem.test.tsx +++ b/src/web-ui/src/shared/notification-system/components/NotificationItem.test.tsx @@ -22,9 +22,11 @@ describe('NotificationItem accessibility', () => { let root: Root; beforeEach(() => { - dom = new JSDOM('

    '); - globalThis.window = dom.window as unknown as Window & typeof globalThis; - globalThis.document = dom.window.document; + dom = new JSDOM('
    ', { + pretendToBeVisual: true, + }); + vi.stubGlobal('window', dom.window); + vi.stubGlobal('document', dom.window.document); container = document.getElementById('root') as HTMLDivElement; root = createRoot(container); }); @@ -32,6 +34,7 @@ describe('NotificationItem accessibility', () => { afterEach(() => { act(() => root.unmount()); dom.window.close(); + vi.unstubAllGlobals(); }); it('announces an actionable error while focus remains in the composer', () => { diff --git a/src/web-ui/src/shared/ui/OverflowText.test.tsx b/src/web-ui/src/shared/ui/OverflowText.test.tsx index 3c5d480a40..29820cc37b 100644 --- a/src/web-ui/src/shared/ui/OverflowText.test.tsx +++ b/src/web-ui/src/shared/ui/OverflowText.test.tsx @@ -15,7 +15,11 @@ describe('overflow text full-content access', () => { const resizeCallbacks = new Set<() => void>(); const longLabel = 'Run independent tasks concurrently whenever possible'; - const render = (content: React.ReactNode) => act(() => root.render(content)); + const flushMeasurement = () => act(() => vi.advanceTimersByTime(1)); + const render = (content: React.ReactNode) => { + act(() => root.render(content)); + flushMeasurement(); + }; const hover = (element: Element) => act(() => { element.dispatchEvent(new MouseEvent('mouseenter')); }); @@ -75,6 +79,37 @@ describe('overflow text full-content access', () => { expect(button.getAttribute('aria-describedby')).toBe('help'); }); + it('defers mount reads and coalesces repeated resize notifications for all labels', () => { + const reads: string[] = []; + vi.spyOn(HTMLElement.prototype, 'scrollWidth', 'get').mockImplementation(function (this: HTMLElement) { + reads.push(this.textContent ?? ''); + // No label may publish an overflow state while this batch is reading. + expect(host.querySelector('[data-overflow="true"]')).toBeNull(); + return 500; + }); + act(() => root.render(<>FirstSecond)); + act(() => { + resizeCallbacks.forEach(callback => { callback(); callback(); }); + }); + expect(reads).toEqual([]); + flushMeasurement(); + expect(reads).toEqual(['First', 'Second']); + expect(host.querySelectorAll('[data-overflow="true"]')).toHaveLength(2); + }); + + it('cancels pending measurements on unmount and measures the latest props only', () => { + const read = vi.spyOn(HTMLElement.prototype, 'scrollWidth', 'get'); + act(() => root.render({longLabel})); + act(() => root.render(null)); + flushMeasurement(); + expect(read).not.toHaveBeenCalled(); + act(() => root.render({longLabel})); + act(() => root.render(Short)); + flushMeasurement(); + expect(read).toHaveBeenCalledOnce(); + expect(host.querySelector('[data-overflow]')?.getAttribute('data-overflow')).toBe('false'); + }); + it('keeps interaction-only ellipsis idle on virtual selection and reveals full text on focus', () => { render(