Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions webviews/src/agent-session/acpmux/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ import { startNewTabTransition } from "./newtab/startTransition";
import { isAgentHome, projectLabel, recentLocalFolders, sessionTitle, type AcpmuxSessionEntry } from "./sessionList";
import { ThreadMinimap } from "./threadMinimap/ThreadMinimap";
import { railOwnsWheel } from "./threadMinimap/railWheel";
import { animateScroll, stopScroll } from "./threadMinimap/scrollMotion";
import { animateScroll, MOTION_SCROLL_EVENT, stopScroll } from "./threadMinimap/scrollMotion";
import { JUMP_CURVE, jumpDuration, jumpGlideStart } from "./threadMinimap/model";
import { cssTimeMs, foldMotion, foldMs } from "./conversation/fold";
import { ChatMinimap } from "./chatMinimap/ChatMinimap";
Expand Down Expand Up @@ -3029,7 +3029,7 @@ export function VirtualTranscript({
});
});
});
const queueScroll = (top: number, atLatest: boolean) => {
const queueScroll = (top: number, atLatest: boolean, immediate = false) => {
// Keep the intent ref synchronous so a second scroll event before the next frame sees the
// reader's latest state; React work itself is coalesced by scrollPacer.
followingRef.current = atLatest;
Expand All @@ -3040,7 +3040,8 @@ export function VirtualTranscript({
// CI runner, mounted rows a step behind the view).
scrollRef.current = { top, delta: top - scrollRef.current.top || scrollRef.current.delta };
scheduleScrollLeadReset();
scrollPacer.current!.enqueue({ top, atLatest });
if (immediate) scrollPacer.current!.flush({ top, atLatest });
else scrollPacer.current!.enqueue({ top, atLatest });
};
useEffect(() => {
transcriptMounted.current = true;
Expand Down Expand Up @@ -5063,14 +5064,13 @@ export function VirtualTranscript({
}, [sends]);
/// The older-history check (below), which every scroll event runs too.
const checkOlderNow = useRef<() => void>(() => {});
const onScroll = (event: React.UIEvent<HTMLDivElement>) => {
const handleScroll = (node: HTMLDivElement, immediate = false) => {
pacing.scrolled();
// Before the chat opens its offset moves only as pages land unseen: none of it is the reader's.
if (!didOpenAtLatest.current) return;
// A hidden scroller has no height, so every offset would read as scrolled up (cx-t0zqt): the view
// keeps following until the reader scrolls a visible one.
if (event.currentTarget.clientHeight === 0) return;
const node = event.currentTarget;
if (node.clientHeight === 0) return;
// The room above the loaded rows holds history still to load (conversation/olderReserve.ts):
// the reader stops at the loaded rows' top, as at the top of the chat, until a page lands into
// the room. A view inside it showed nothing, and the pages then landed under it unseen.
Expand All @@ -5089,7 +5089,7 @@ export function VirtualTranscript({
scrolledTo.current = { top: next, atLatest: true };
seenAnchor.current = null;
followTrace.set(next);
queueScroll(next, true);
queueScroll(next, true, immediate);
// The kept place is the latest message again (a reopened chat opens there, not where the
// reader was before the jump).
if (onAnchor) {
Expand Down Expand Up @@ -5120,7 +5120,7 @@ export function VirtualTranscript({
const resting = node.scrollTop;
scrolledTo.current = { top: resting, atLatest: true };
followTrace.set(resting);
queueScroll(resting, true);
queueScroll(resting, true, immediate);
return;
}
const was = scrolledTo.current;
Expand All @@ -5146,12 +5146,24 @@ export function VirtualTranscript({
});
followTrace.update(node, layout.totalHeight, scrolledTo.current.atLatest);
seenAnchor.current = restingAnchor(layoutScrollTop(node), scrolledTo.current.atLatest, layout, rows);
queueScroll(next, scrolledTo.current.atLatest);
queueScroll(next, scrolledTo.current.atLatest, immediate);
if (onAnchor) {
clearTimeout(anchorTimer.current);
anchorTimer.current = setTimeout(reportAnchor, SCROLL_ANCHOR_SETTLE_MS);
}
};
const motionScroll = useRef(handleScroll);
motionScroll.current = handleScroll;
useLayoutEffect(() => {
const node = ref.current;
if (!node) return;
// The shared motion clock emits only from RAF callbacks, after writing scrollTop. Mount
// the visible rows before that frame paints; native scroll arrives a frame too late.
const moved = () => motionScroll.current(node, true);
node.addEventListener(MOTION_SCROLL_EVENT, moved);
return () => node.removeEventListener(MOTION_SCROLL_EVENT, moved);
}, []);
const onScroll = (event: React.UIEvent<HTMLDivElement>) => handleScroll(event.currentTarget);
// Jump to latest (the button, ChatGPT and Codex style): a smooth scroll to the latest message, then a
// landing on it, so a reply that grew meanwhile is followed again.
const jumpToLatest = (focused: boolean) => {
Expand Down
2 changes: 2 additions & 0 deletions webviews/src/agent-session/acpmux/debug.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { announceScroll } from "./threadMinimap/scrollMotion";
import { userScrollIntent } from "./conversation/userScroll";
import * as automation from "./automation";
import type { AcpmuxRow } from "./model";
Expand Down Expand Up @@ -231,6 +232,7 @@ export function createAcpmuxDebug(
timestamps.push(now);
const progress = Math.min(1, (now - start) / duration);
scroller.scrollTop = from * (1 - progress);
announceScroll(scroller);
acpmuxPerf.markFrame(
now,
isBlank(acpmuxPerf.mounted, scroller.scrollTop, scroller.clientHeight, scroller.scrollHeight),
Expand Down
26 changes: 20 additions & 6 deletions webviews/src/agent-session/acpmux/scrollPacer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ type CancelFrame = (handle: number) => void;

/**
* Coalesces transcript scroll updates so one display frame commits the latest offset.
* `cancel` is called when the transcript unmounts and drops any pending frame.
* `flush` commits a motion write before its current frame paints and cancels queued work.
* `cancel` is called when the transcript unmounts or placement takes over.
*/
export function createScrollPacer(
commit: (frame: ScrollFrame) => void,
Expand All @@ -34,9 +35,19 @@ export function createScrollPacer(
) {
let pending: ScrollFrame | undefined;
let handle: number | undefined;
// A motion callback has already committed this position before paint. Its later native
// scroll event needs no second commit, unless the offset or follow intent changed.
let immediate: ScrollFrame | undefined;
const cancelPending = () => {
pending = undefined;
if (handle !== undefined) cancelFrame(handle);
handle = undefined;
};

return {
enqueue(frame: ScrollFrame) {
if (immediate?.top === frame.top && immediate.atLatest === frame.atLatest) return;
immediate = undefined;
pending = frame;
if (handle !== undefined) return;
handle = requestFrame(() => {
Expand All @@ -46,12 +57,15 @@ export function createScrollPacer(
if (next) commit(next);
});
},
flush(frame: ScrollFrame) {
cancelPending();
if (immediate?.top === frame.top && immediate.atLatest === frame.atLatest) return;
immediate = frame;
commit(frame);
},
cancel() {
pending = undefined;
if (handle !== undefined) {
cancelFrame(handle);
handle = undefined;
}
cancelPending();
immediate = undefined;
},
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ const running = new WeakMap<HTMLElement, () => void>();
/// paints, so what must move with the scroll on its own frame (the minimap's band) listens for this too.
export const MOTION_SCROLL_EVENT = "acpmux-motion-scroll";

/// Tells `node`'s listeners that the motion just moved it (`MOTION_SCROLL_EVENT`).
function announceScroll(node: HTMLElement): void {
/// Tells `node`'s listeners that a RAF-driven motion just moved it (`MOTION_SCROLL_EVENT`).
/// Never call from React layout work: transcript listeners synchronously commit visible rows.
export function announceScroll(node: HTMLElement): void {
// A stand-in scroller (a unit test's) may have no document or events of its own.
if (typeof node.dispatchEvent !== "function") return;
const Event_ = node.ownerDocument?.defaultView?.Event ?? globalThis.Event;
Expand Down
34 changes: 34 additions & 0 deletions webviews/test/scroll-pacer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,37 @@ test("scroll updates commit once per frame and keep the latest offset", () => {
expect(committed).toEqual([{ top: 80, atLatest: true }]);
expect(callbacks).toHaveLength(0);
});

test("motion commits before paint, replaces a queued frame, and deduplicates its native event", () => {
const callbacks = new Map<number, FrameRequestCallback>();
let handle = 0;
const committed: ScrollFrame[] = [];
const pacer = createScrollPacer(
(frame) => committed.push(frame),
(callback) => {
callbacks.set(++handle, callback);
return handle;
},
(id) => callbacks.delete(id),
);
pacer.enqueue({ top: 80, atLatest: true });
pacer.flush({ top: 400, atLatest: false });
expect(committed).toEqual([{ top: 400, atLatest: false }]);
expect(callbacks).toHaveLength(0);
pacer.enqueue({ top: 400, atLatest: false });
pacer.flush({ top: 400, atLatest: false });
expect(committed).toHaveLength(1);
expect(callbacks).toHaveLength(0);

// An intent change at the same offset must survive duplicate suppression.
pacer.enqueue({ top: 400, atLatest: true });
expect(callbacks).toHaveLength(1);
callbacks.get(handle)!(16);
callbacks.clear();
expect(committed.at(-1)).toEqual({ top: 400, atLatest: true });
pacer.flush({ top: 500, atLatest: false });
expect(committed.at(-1)).toEqual({ top: 500, atLatest: false });
pacer.cancel();
pacer.enqueue({ top: 500, atLatest: false });
expect(callbacks).toHaveLength(1);
});
79 changes: 79 additions & 0 deletions webviews/test/transcript-fling.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { afterAll, beforeAll, describe, expect, setDefaultTimeout, test } from "bun:test";
import path from "node:path";
import { webkit, type Browser } from "playwright";
import { startScrollCoverage } from "../bench/scrollCoverage";
import { installPaintedFrames } from "./support/paintedFrames";
import { requireBrowserLane } from "./support/requireBrowserLane";
import { warmViteServer } from "./support/warmDevServer";
Expand Down Expand Up @@ -133,4 +134,82 @@ await requireBrowserLane("transcript-fling.test.ts", async () => {
expect(settledMounted).toBeLessThan(movingMounted);
await page.close();
});
for (const input of ["motion", "wheel"] as const) {
test(`${input}: first movement from rest keeps every painted viewport covered`, async () => {
const { port } = vite!.httpServer!.address() as { port: number };
const page = await browser!.newPage({ viewport: { width: 900, height: 700 } });
const errors: string[] = [];
page.on("pageerror", (error) => errors.push(error.message));
page.on("console", (message) => {
if (message.type() === "error") errors.push(message.text());
});
try {
await installPaintedFrames(page);
await page.goto(`http://127.0.0.1:${port}/test/browser/transcript-fling.html`);
await page.waitForFunction(() => "__debug" in window);
await page.evaluate(`window.__debug.seedRows(3000)`);
await page.locator(".acpmux-scroll").hover();
// Real reader input releases follow before positioning the test in history.
await page.mouse.wheel(0, -100);
await page.evaluate(() => window.cmuxPaintFrame());
const start = await page.evaluate(async () => {
const node = document.querySelector<HTMLElement>(".acpmux-scroll")!;
node.scrollTop = (node.scrollHeight - node.clientHeight) / 2;
const started = performance.now();
return await new Promise<{ top: number; viewport: number }>((resolve, reject) => {
let previous = "";
let stable = 0;
window.cmuxAfterPaint(() => {
const extent = node.scrollHeight - node.clientHeight;
const snapshot = `${node.scrollTop}/${extent}/${node.querySelectorAll(".acpmux-row").length}`;
stable = snapshot === previous ? stable + 1 : 0;
previous = snapshot;
// The directional lead expires after 120 ms; begin with no warmed fling range.
if (
performance.now() - started >= 180 &&
stable >= 3 &&
node.scrollTop > extent * 0.4 &&
node.scrollTop < extent * 0.6
) {
resolve({ top: node.scrollTop, viewport: node.clientHeight });
return false;
}
if (performance.now() - started > 5_000) {
reject(new Error("first-motion fixture did not settle in history"));
return false;
}
});
});
});
await page.evaluate(startScrollCoverage, { timeoutMs: 10_000 });
if (input === "motion") {
// Exercise the product clock: a short large glide exceeds the idle warm range on
// its first step. No synthetic scroll event and no first-frame exclusion.
await page.evaluate(`(async () => {
const { animateScrollBy } = await import("/src/agent-session/acpmux/threadMinimap/scrollMotion.ts");
const node = document.querySelector(".acpmux-scroll");
await new Promise(resolve => animateScrollBy(node, -24 * node.clientHeight, 160, [0, 0, 1, 1], false, resolve));
await window.cmuxPaintFrame();
})()`);
} else {
// Playwright sends real pixel wheel input through the browser's default scrolling.
for (const viewports of [6, 6, 5, 4, 3, 2, 1, 0.5]) {
await page.mouse.wheel(0, -viewports * start.viewport);
await page.evaluate(() => window.cmuxPaintFrame());
}
}
await page.evaluate(() => window.cmuxPaintFrame());
const result = await page.evaluate(() => window.__stopScrollCoverage!());
console.log(`transcript-first-${input}: ${JSON.stringify({ start, ...result })}`);
expect(result.timed_out).toBe(false);
expect(result.frames).toBeGreaterThan(0);
expect(result.distance_px).toBeGreaterThan(start.viewport * 6);
expect(result.blank_frames).toBe(0);
expect(result.max_uncovered_px).toBeLessThanOrEqual(0.5);
expect(errors).toEqual([]);
} finally {
await page.close();
}
});
}
});
Loading