Files
PaperClipAI/ui/src/components/task-chat/TaskMessageScroller.test.tsx
T
DottaandPaperclip fa50cab568 perf(ui): speed task loading without shifting the conversation (#16849)
## Thinking Path

> - Paperclip's board should open a task quickly while keeping its
conversation and composer stable.
> - The browser currently downloads unrelated screens and unopened
dialog implementations before task content appears.
> - Splitting routes alone creates many shared requests and did not
improve measured cold loading.
> - This change groups the startup and task dependency trees, preloads
the task chunk, and defers optional routes and dialogs.
> - Existing coordinated history reveal stays intact so late data does
not insert content above the reader.
> - Hidden routes must not overwrite saved scroll positions with
zero-height geometry.
> - Repeated browser measurements and delayed-response tests establish
the result independently of bundle-file size.

## Linked Issues or Issue Description

Refs #14758. Related prior work: #10463 and #14727.

**What existing behavior does this improve?**

Cold task-page opening downloads and evaluates code for other screens
and globally mounted, unopened dialogs. A direct task link should reach
its saved conversation sooner without moving already-visible messages or
the composer.

**Subsystem affected**

Board UI startup, route boundaries, dialog loading, task scroll
restoration, and the issue-detail performance harness.

**Proposed behavior**

Load optional screens and dialogs when needed, keep startup requests
bounded, and preload the task route. Preserve the current history reveal
policy. Keep the shell mounted during route loading and preserve the
reader's position when a route is temporarily hidden.

**Additional context**

The older bundle-splitting attempt in #10463 is still open from August.
This implementation targets current master and deliberately preserves
today's editor, composer, and transcript reveal behavior. It does not
implement the alternative transcript-row design discussed in #14758.
ROADMAP.md does not describe overlapping startup-bundle work.

## What Changed

- Lazy-load board routes, standalone screens, the legacy layout,
onboarding, and unopened create dialogs.
- Keep activated dialogs mounted across close/reopen to retain their
existing draft lifecycle.
- Group startup and issue dependencies and preload the emitted issue
chunk from the production HTML.
- Contain route loading within the shell and retain existing chunk-error
recovery.
- Contain dialog import failures locally, support cancellation, and keep
setup dismissed after a late download. Warm the task module from task
lists and keep the small chat redirect eager.
- Wait for task-creation dialog fields in the browser fixture before
filling an explicit title.
- Ignore zero-height scroll geometry while a route is hidden.
- Reuse the same persisted performance fixture between builds, record
actual network spans, and distinguish concealed loading content from the
ready thread in browser assertions.

## Verification

- Production UI build and UI TypeScript check passed.
- Recovery follow-up: 74 focused UI tests passed; the cancellation
change passed 23 focused tests plus all 131 onboarding tests (the latter
with a 15-second per-test timeout on the contended local host).
- Setup cancellation was reproduced against the prior production build
and verified fixed after releasing the held real wizard download.
- 91 focused UI tests passed; the final route registrations were checked
again after making standalone screens lazy (33 routing tests passed).
- All eight real-browser layout scenarios passed: delayed comments, late
media, properties changes, growing drafts, older history, Back
navigation, deep links, desktop/mobile same-task links, request
failure/retry, and stalled native/log reads.
- Browser walkthrough: read a populated task, submit one reply, refresh,
navigate to Projects, and return. The reply persisted once and the
normal composer remained usable.
- Token design gates passed. The separate repository-wide publish-token
scan fails on pre-existing references to the local maintainer username
in unchanged files; this change adds none.
- Production-build measurements use the same persisted task, local
API/database, fresh browser contexts for cold opens, and five samples
per scenario/profile. These measurements do not establish deployed
staging latency.
- Final head `076466816a`, consecutive before/after production builds:
five samples per scenario/profile, same saved task and server. Cold Fast
4G/4x CPU content paint: **5,775 → 4,248 ms (26%)**; issue API server
timing was 23 ms in both sets. Warm Fast 4G: 989 → 971 ms. JavaScript
transferred before content: **2,596 → 1,722 KiB (34%)**.
- Unthrottled cold content paint: 1,306 → 676 ms; warm: 269 → 224 ms.
These local timings vary with host load. Earlier measurements showed
about 20% cold improvement, and a contended intermediate run had
substantially higher API latency; those runs are retained in the local
evidence. The final matched pair meets the proposed 25% target, but it
does not establish a guaranteed 25% improvement across environments.
- All four browser scenarios that failed on the first CI run passed
locally after the readiness fixes; the subsequent CI run passed all
eight browser shards.
- Fault-injection browser checks: delayed route loading preserves shell
geometry; a failed route recovers through Back; cancelled or failed
dialog loading preserves an unsent reply; Back dismisses pending setup
and contains its late failure.

- Final head `076466816a`: all CI gates green (55 successful checks, no
pending or failed checks); Greptile 5/5 with no actionable findings and
all review threads resolved.

## Risks

- Dynamic chunks add a loading state on first entry to optional screens
and dialogs. Route failures use the existing error boundary. Optional
dialog failures are dismissible without unmounting the board; retrying a
rejected import requires a reload.
- The task preload prioritizes the primary board journey, including when
another screen is opened first.
- Chunk grouping should be revisited when the dependency graph changes;
bundle size alone does not establish a speedup.
- No API, authorization, migration, editor semantics, or
transcript-reveal policy changes.

## Model Used

OpenAI Codex, GPT-6, with reasoning, repository tools, code execution,
and browser verification. The runtime does not expose a more precise
model identifier or context-window size.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-19 10:16:23 -05:00

431 lines
14 KiB
TypeScript

// @vitest-environment jsdom
import { flushSync } from "react-dom";
import { createRoot, type Root } from "react-dom/client";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { TaskMessageScroller } from "./TaskMessageScroller";
import { TaskChatScrollNavigation } from "./scroll-navigation";
const PILL_SELECTOR = 'button[aria-label="Scroll to latest"]';
/**
* jsdom has no layout, so scroll geometry is faked per element: scrollHeight /
* clientHeight are pinned and scrollTop is backed by a plain variable.
*/
function fakeGeometry(el: HTMLElement, { scrollHeight = 1000, clientHeight = 400 } = {}) {
let scrollTop = 0;
Object.defineProperty(el, "scrollHeight", { value: scrollHeight, configurable: true });
Object.defineProperty(el, "clientHeight", { value: clientHeight, configurable: true });
Object.defineProperty(el, "scrollTop", {
get: () => scrollTop,
set: (v: number) => {
scrollTop = v;
},
configurable: true,
});
}
function fakeResizableGeometry(
el: HTMLElement,
{ scrollHeight = 1000, clientHeight = 400 } = {},
) {
let currentClientHeight = clientHeight;
let scrollTop = 0;
Object.defineProperty(el, "scrollHeight", {
value: scrollHeight,
configurable: true,
});
Object.defineProperty(el, "clientHeight", {
get: () => currentClientHeight,
configurable: true,
});
Object.defineProperty(el, "scrollTop", {
get: () => scrollTop,
set: (value: number) => {
scrollTop = value;
},
configurable: true,
});
return {
setClientHeight(value: number) {
currentClientHeight = value;
},
};
}
/**
* Scroll/wheel are continuous-priority events, so React flushes the resulting
* state updates asynchronously — wait a macrotask after dispatching.
*/
function flushEvents() {
return new Promise<void>((resolve) => {
setTimeout(resolve);
});
}
describe("TaskMessageScroller", () => {
let container: HTMLDivElement;
let root: Root;
function render(contentKey: unknown = 0) {
flushSync(() => {
root.render(
<TaskMessageScroller contentKey={contentKey}>
<div>messages</div>
</TaskMessageScroller>,
);
});
}
function scroller(): HTMLDivElement {
return container.querySelector<HTMLDivElement>('[data-testid="task-chat-scroller"]')!;
}
function pill(): HTMLButtonElement | null {
return container.querySelector<HTMLButtonElement>(PILL_SELECTOR);
}
/**
* Wait for the pill to reach a state, rather than for a fixed number of turns.
* `flushEvents` yields exactly one macrotask, which is enough on an idle
* machine and not when the suite runs many workers in parallel — React
* flushes continuous-priority updates asynchronously, so the DOM read can
* land before the state does. Sites that dereference `pill()!` turn that into
* a hard failure rather than a retry.
*/
async function waitForPill(present: boolean): Promise<void> {
await vi.waitFor(() =>
present ? expect(pill()).not.toBeNull() : expect(pill()).toBeNull(),
);
}
/** Set scrollTop and fire a scroll event, like a user or the browser would. */
async function scrollTo(el: HTMLElement, top: number) {
el.scrollTop = top;
el.dispatchEvent(new Event("scroll", { bubbles: true }));
await flushEvents();
}
async function dispatch(el: HTMLElement, event: Event) {
el.dispatchEvent(event);
await flushEvents();
}
beforeEach(() => {
container = document.createElement("div");
document.body.appendChild(container);
root = createRoot(container);
});
afterEach(() => {
flushSync(() => root.unmount());
container.remove();
document.documentElement.style.removeProperty("--motion-scrollbar-idle-delay");
vi.useRealTimers();
vi.unstubAllGlobals();
});
it("applies same-task hash changes and restores each history entry without remounting", async () => {
let initialized = false;
function navigate(key: string, hash: string, restore = false) {
flushSync(() => root.render(
<TaskChatScrollNavigation.Provider value={{ key, hash, restore }}>
<TaskMessageScroller contentKey="unchanged">
<div ref={(node) => {
if (node && !initialized) {
fakeGeometry(node.parentElement!);
initialized = true;
}
}}>
{[100, 500].map((top, index) => <div key={index} id={`nav-comment-${index}`} data-thread-anchor={`nav-comment-${index}`} ref={(node) => {
if (node) node.getBoundingClientRect = () => ({ top: top - scroller().scrollTop, bottom: top + 100 - scroller().scrollTop, height: 100 } as DOMRect);
}}>Comment {index}</div>)}
</div>
</TaskMessageScroller>
</TaskChatScrollNavigation.Provider>,
));
}
navigate("desktop-entry-one", "#nav-comment-0");
const viewport = scroller();
expect(viewport.scrollTop).toBe(100);
await scrollTo(viewport, 150);
navigate("desktop-entry-two", "#nav-comment-1");
expect(scroller()).toBe(viewport);
expect(viewport.scrollTop).toBe(500);
navigate("desktop-entry-one", "#nav-comment-0", true);
expect(viewport.scrollTop).toBe(150);
navigate("desktop-entry-one", "#nav-comment-1", true);
expect(viewport.scrollTop).toBe(500);
});
it("does not replace a saved reading position with a hidden route's geometry", async () => {
function mount(restore: boolean) {
flushSync(() => root.render(
<TaskChatScrollNavigation.Provider value={{ key: "hidden-route-entry", hash: "", restore }}>
<TaskMessageScroller contentKey="saved-thread">
<div ref={(node) => { if (node) fakeGeometry(node.parentElement!); }}>Saved messages</div>
</TaskMessageScroller>
</TaskChatScrollNavigation.Provider>,
));
}
mount(false);
const viewport = scroller();
await scrollTo(viewport, 150);
Object.defineProperty(viewport, "clientHeight", { value: 0, configurable: true });
await scrollTo(viewport, 0);
flushSync(() => root.render(null));
mount(true);
expect(scroller().scrollTop).toBe(150);
});
it("renders children inside the scroll container, pill hidden, scrolled to bottom on mount", () => {
render();
const el = scroller();
expect(el.textContent).toContain("messages");
expect(pill()).toBeNull();
// Mount effect pins to the bottom.
expect(el.scrollTop).toBe(el.scrollHeight);
});
it("extends only the streamlined scroll box through the page gutter", () => {
render();
const el = scroller();
const frame = el.parentElement;
expect(frame?.className).toBe("relative min-h-0 flex-1");
expect(el.classList).toContain("-right-4");
expect(el.classList).toContain("pr-4");
expect(el.classList).toContain("md:-right-6");
expect(el.classList).toContain("md:pr-6");
expect(el.classList).not.toContain("right-0");
});
it("shows the scrollbar only while scroll activity is recent", () => {
vi.useFakeTimers();
document.documentElement.style.setProperty("--motion-scrollbar-idle-delay", "600ms");
render();
const el = scroller();
fakeGeometry(el);
expect(el.className).toContain("scrollbar-while-scrolling");
expect(el.getAttribute("data-scroll-active")).toBeNull();
el.scrollTop = 100;
el.dispatchEvent(new Event("scroll", { bubbles: true }));
expect(el.getAttribute("data-scroll-active")).toBe("true");
vi.advanceTimersByTime(599);
expect(el.getAttribute("data-scroll-active")).toBe("true");
vi.advanceTimersByTime(1);
expect(el.getAttribute("data-scroll-active")).toBeNull();
});
it("contains horizontal overflow so no scrollbar appears above the composer", () => {
render();
expect(scroller().classList).toContain("overflow-x-hidden");
expect(scroller().classList).toContain("overflow-y-auto");
});
it("auto-follows content instantly while pinned", async () => {
render(1);
const el = scroller();
fakeGeometry(el);
await scrollTo(el, 600); // bottom: 1000 - 600 - 400 = 0 → pinned
await waitForPill(false); // pinned state settled before content grows
render(2);
expect(el.scrollTop).toBe(1000);
expect(pill()).toBeNull();
});
it("holds position and shows the icon-only pill when scrolled up", async () => {
render(1);
const el = scroller();
fakeGeometry(el);
await scrollTo(el, 100); // 500px from bottom → unpinned
await waitForPill(true);
const btn = pill();
expect(btn).not.toBeNull();
expect(btn!.className).toContain("tc-scroll-pill-in");
expect(btn!.className).toContain("size-8");
expect(btn!.className).toContain("bottom-7");
expect(btn!.className).not.toContain("bottom-3");
expect(btn!.className).not.toContain("-translate-x-1/2");
// Icon-only: no visible text.
expect(btn!.textContent).toBe("");
// New content must not yank the held position.
render(2);
expect(el.scrollTop).toBe(100);
});
it("follows a shrinking viewport while pinned so a growing composer cannot cover the latest text", async () => {
let triggerResize = () => {};
vi.stubGlobal(
"ResizeObserver",
class {
constructor(callback: ResizeObserverCallback) {
triggerResize = () =>
callback([], this as unknown as ResizeObserver);
}
observe() {}
disconnect() {}
},
);
render();
const el = scroller();
const geometry = fakeResizableGeometry(el);
triggerResize();
await scrollTo(el, 600);
geometry.setClientHeight(200);
triggerResize();
expect(el.scrollTop).toBe(el.scrollHeight);
expect(pill()).toBeNull();
});
it("holds a reader's position when the composer grows while they are scrolled up", async () => {
let triggerResize = () => {};
vi.stubGlobal(
"ResizeObserver",
class {
constructor(callback: ResizeObserverCallback) {
triggerResize = () =>
callback([], this as unknown as ResizeObserver);
}
observe() {}
disconnect() {}
},
);
render();
const el = scroller();
const geometry = fakeResizableGeometry(el);
triggerResize();
await scrollTo(el, 100);
await waitForPill(true);
geometry.setClientHeight(200);
triggerResize();
expect(el.scrollTop).toBe(100);
expect(pill()).not.toBeNull();
});
it("small drifts within the pin threshold do not show the pill", async () => {
render();
const el = scroller();
fakeGeometry(el);
await scrollTo(el, 560); // 40px from bottom, threshold is 48
expect(pill()).toBeNull();
});
it("clicking the pill smooth-scrolls, ignores intermediate scroll events, re-pins on arrival", async () => {
vi.stubGlobal("matchMedia", vi.fn().mockReturnValue({ matches: false }));
render(1);
const el = scroller();
fakeGeometry(el);
const scrollToSpy = vi.fn();
el.scrollTo = scrollToSpy as unknown as typeof el.scrollTo;
await scrollTo(el, 100);
await waitForPill(true);
const btn = pill()!;
btn.click();
await flushEvents();
expect(scrollToSpy).toHaveBeenCalledWith({ top: 1000, behavior: "smooth" });
// Intermediate glide positions fire scroll events; they must not unpin
// (i.e. the easing flag swallows them and the pill state stays put).
await scrollTo(el, 250);
await scrollTo(el, 400);
expect(pill()).not.toBeNull();
// Arrival within the threshold re-pins and hides the pill (immediately
// here: no matchMedia in jsdom → reduced-motion/unmount-now path).
vi.stubGlobal("matchMedia", vi.fn().mockReturnValue({ matches: true }));
await scrollTo(el, 600);
expect(pill()).toBeNull();
// Re-pinned: content growth follows instantly again.
render(2);
expect(el.scrollTop).toBe(1000);
});
it("finishes following at the new bottom when content changes during the latest glide", async () => {
vi.stubGlobal("matchMedia", vi.fn().mockReturnValue({ matches: false }));
render(1);
const el = scroller();
fakeGeometry(el);
el.scrollTo = vi.fn() as unknown as typeof el.scrollTo;
await scrollTo(el, 100);
await waitForPill(true);
pill()!.click();
await flushEvents();
await scrollTo(el, 250);
render(2);
expect(el.scrollTop).toBe(1000);
});
it("a wheel gesture during the glide cancels easing and stays unpinned", async () => {
vi.stubGlobal("matchMedia", vi.fn().mockReturnValue({ matches: false }));
render(1);
const el = scroller();
fakeGeometry(el);
el.scrollTo = vi.fn() as unknown as typeof el.scrollTo;
await scrollTo(el, 100);
await waitForPill(true);
pill()!.click();
await flushEvents();
await dispatch(el, new Event("wheel", { bubbles: true }));
// Easing cancelled → user is unpinned, pill visible, content holds.
expect(pill()).not.toBeNull();
render(2);
expect(el.scrollTop).toBe(100);
// Scroll events far from the bottom now behave normally (still unpinned).
await scrollTo(el, 200);
expect(pill()).not.toBeNull();
});
it("plays the exit animation before unmounting when motion is enabled", async () => {
vi.stubGlobal(
"matchMedia",
vi.fn().mockReturnValue({ matches: false }), // motion allowed
);
render();
const el = scroller();
fakeGeometry(el);
await scrollTo(el, 100);
await vi.waitFor(() => expect(pill()?.className).toContain("tc-scroll-pill-in"));
// Scrolling back to the bottom starts the exit animation but keeps the
// pill mounted until animationend.
await scrollTo(el, 600);
await vi.waitFor(() => expect(pill()?.className).toContain("tc-scroll-pill-out"));
const exiting = pill();
expect(exiting).not.toBeNull();
// jsdom has no window.AnimationEvent, so React's vendor-prefix detection
// maps onAnimationEnd to "webkitAnimationEnd" here (real browsers get the
// unprefixed event).
await dispatch(exiting!, new Event("webkitAnimationEnd", { bubbles: true }));
await waitForPill(false);
});
it("unmounts immediately on hide under prefers-reduced-motion", async () => {
vi.stubGlobal(
"matchMedia",
vi.fn().mockReturnValue({ matches: true }), // reduce
);
render();
const el = scroller();
fakeGeometry(el);
await scrollTo(el, 100);
await waitForPill(true);
await scrollTo(el, 600);
await waitForPill(false); // no animationend needed
});
});