fix(desktop): keep an own send out of the frozen timeline tail

Problem:
`messaging.spec.ts:483` "long autolink wraps without widening the
timeline" flaked in Desktop Smoke E2E at :497: the timeline never
showed the sent message. The composer was empty, and a "1 new message"
pill sat at the bottom of a timeline that was already at the floor.
- seiler/BUZZ-517/desktop-upgrade-guidance:
  https://github.com/block/buzz/actions/runs/41751914119/job/126319694104
  (retry 1 passed)
The failure video shows the composer filling while the channel still
showed its skeleton, the rows rendering under the tall composer, and
the send landing about 120 ms later.

Cause:
The timeline freezes its live tail when Virtua reports that the reader
has left the bottom, and later rows wait behind the pill. A send lifts
the freeze and arms a one-shot pin for the next append
(`prepareForOwnMessage`). Nothing kept a not-at-bottom report between
the send and the sent row reaching the deferred snapshot from freezing
the tail again. The sent row then waited behind the pill, and the pin,
which waits for that append, never fired. Only a later scroll to the
floor would have released it.

Virtua 0.49.3 makes that report while the reader sits at the floor. It
applies a list length change to its store during render
(Virtualizer.tsx). React discarded the deferred render that added the
sent row, but the store kept the row's estimated height. When the
cleared composer shrank the trailing spacer, the next scroll event put
the reader 96 px from a floor the DOM didn't have: Virtua read
941 - 479 - 366 = 96, the DOM read 917 - 551 - 366 = 0. The freeze it
queued renders at default priority, ahead of the deferred render, which
then restarted against the frozen tail.

Fix:
While the send's pin is armed, a not-at-bottom report doesn't freeze
the tail. `useAnchoredScroll` exposes `isBottomPinArmed()`, which reads
the pin's own flag, so the hold ends when the pin fires or the channel
changes. `prepareForOwnMessage` also cancels a freeze queued before the
send, which would otherwise land after its release.

Bottom state still comes from Virtua. Reading it from the DOM would add
false gaps of its own, because unmeasured rows overflow their slots: a
passing run read 51 px from the DOM where Virtua read 0.

A scroll up between the send and its row no longer freezes the tail.
The pin brings the reader down to the sent row, as it already does when
the row lands first. A send that adds no row leaves the pin armed until
the next append, as before, and the hold follows it.

Validation:
- New `MessageTimeline.ownSend.jsdom-test.mjs` renders the real
  MessageTimeline over Virtua with a modelled layout and manual frames.
  It sends with a gap reported after the send, and with a freeze
  already queued at the send. The sent row must render at the floor
  with no pill. Without the pin guard the first case fails, and
  without the cancel the second does, each showing "1 new message".
- Repro, using temporary, uncommitted changes: the test with a 600 ms
  channel-head delay, filling during the skeleton, sending once rows
  render, with CPU throttled 4x over CDP. Without the fix: 3/20 failed
  with the CI signature, and instrumentation showed the 96 px report
  above. With the fix: 40/40 passed, and 6 of those runs logged a
  not-at-bottom report while the pin was armed.
- `messaging.spec.ts --grep "long autolink"`: 30/30 passed at 4
  workers, 10/10 serially.
- messaging, virtualization, scroll-history, stream, smoke,
  thread-follow, file-attachment and message-target-highlight specs,
  run twice at 4 workers: 185 passed and 1 failed each time, a
  different test each run. Repeating those two fails as often without
  the fix: `:2785` (own avatar) 22/25 vs 20/25 at 4 workers and 8/10
  vs 9/10 serially; `:3170` (thread panel width) 4/25 vs 8/25 and 1/10
  vs 3/10. `:2785` fails on the avatar image after its message has
  rendered, and `:3170` never sends, so the change can't reach it.
- `tsc --noEmit`, `biome check`, and the messages UI unit (337) and
  jsdom (9) tests pass.

A green run doesn't prove the flake is gone. Hosted CI covers broad
validation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: mtoohey <mtoohey@squareup.com>
(cherry picked from commit 4c9272a362eb6f01aacb8d0133fbe41066eaf308)
This commit is contained in:
mtoohey
2026-11-04 18:07:53 +00:00
parent 64d12e2975
commit f2c57afb51
3 changed files with 254 additions and 3 deletions

View File

@@ -0,0 +1,233 @@
/**
* The user's own send reaches the bottom of the timeline even when Virtua
* reports a gap from the floor around it.
*
* MessageTimeline freezes its live tail when Virtua reports that the reader
* has left the bottom, and a send arms a one-shot pin for the next append. If
* the freeze lands before the sent row reaches the deferred snapshot, the row
* waits behind a "1 new message" pill, and the pin never sees its append.
* Virtua can report that gap while the reader is at the floor: it counts rows
* in its store during render, so a deferred render that React discards leaves
* the sent row counted. The next scroll event, from the composer shrinking
* after the send, then reads that row's height as distance from the bottom.
*/
import assert from "node:assert/strict";
import { afterEach, test } from "node:test";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import {
RouterContextProvider,
createMemoryHistory,
createRootRoute,
createRouter,
} from "@tanstack/react-router";
import React, { act } from "react";
import { createRoot } from "react-dom/client";
import { MessageTimeline } from "./MessageTimeline.tsx";
const ROW = 60;
const VIEWPORT = 400;
// jsdom has no layout. Model what the timeline reads: the scroller's offset
// is clamped to the height Virtua gives its inner container, and the list
// host is as tall as the scroller so Virtua keeps an offscreen buffer.
const offsets = new WeakMap();
const isScroller = (element) => element?.style?.overflowY === "auto";
const maxOffset = (element) =>
Math.max(
(Number.parseFloat(element.firstElementChild?.style.height) || 0) -
VIEWPORT,
0,
);
const clampOffset = (element, value) =>
Math.min(Math.max(value, 0), maxOffset(element));
Object.defineProperties(window.HTMLElement.prototype, {
clientHeight: {
configurable: true,
get() {
return isScroller(this) || isScroller(this.firstElementChild)
? VIEWPORT
: 0;
},
},
scrollHeight: {
configurable: true,
get() {
return isScroller(this) ? maxOffset(this) + VIEWPORT : 0;
},
},
scrollTop: {
configurable: true,
get() {
const offset = clampOffset(this, offsets.get(this) ?? 0);
offsets.set(this, offset);
return offset;
},
set(value) {
offsets.set(this, clampOffset(this, value));
},
},
offsetParent: {
configurable: true,
get() {
return this.parentElement;
},
},
});
const resizeObservers = new Set();
globalThis.ResizeObserver = window.ResizeObserver = class {
constructor(callback) {
this.callback = callback;
this.targets = new Set();
resizeObservers.add(this);
}
observe(target) {
this.targets.add(target);
}
unobserve(target) {
this.targets.delete(target);
}
disconnect() {
resizeObservers.delete(this);
}
};
// Frames run only when the test flushes them, so it decides whether a queued
// freeze lands before or after the send.
let frames = new Map();
let nextFrameId = 1;
globalThis.requestAnimationFrame = window.requestAnimationFrame = (
callback,
) => {
const id = nextFrameId++;
frames.set(id, callback);
return id;
};
globalThis.cancelAnimationFrame = window.cancelAnimationFrame = (id) => {
frames.delete(id);
};
function flushFrames() {
act(() => {
const pending = frames;
frames = new Map();
for (const callback of pending.values()) callback(performance.now());
});
}
function message(index, body = `message ${index}`) {
return {
id: index.toString(16).padStart(64, "0"),
createdAt: 1_700_000_000 + index,
pubkey: "a".repeat(64),
author: "alice",
time: "1:00 AM",
body,
depth: 0,
kind: 9,
tags: [],
};
}
const history = Array.from({ length: 20 }, (_, index) => message(index + 1));
const sent = message(21, "my own send");
let root;
let container;
afterEach(() => {
act(() => root?.unmount());
container?.remove();
});
async function mountAtFloor() {
const router = createRouter({
routeTree: createRootRoute(),
history: createMemoryHistory({ initialEntries: ["/"] }),
});
const queryClient = new QueryClient({
defaultOptions: { queries: { enabled: false } },
});
const timeline = React.createRef();
const render = (messages) =>
act(async () =>
root.render(
React.createElement(
RouterContextProvider,
{ router },
React.createElement(
QueryClientProvider,
{ client: queryClient },
React.createElement(MessageTimeline, {
channelId: "channel",
messages,
ref: timeline,
}),
),
),
),
);
container = document.createElement("div");
document.body.appendChild(container);
root = createRoot(container);
await render(history);
const scroller = container.querySelector('[data-testid="message-timeline"]');
act(() => {
for (const observer of resizeObservers) {
observer.callback(
[...observer.targets].map((target) => ({
target,
contentRect: {
width: 600,
height: target === scroller ? VIEWPORT : ROW,
},
})),
);
}
});
flushFrames();
readerScrollsTo(scroller, scroller.scrollHeight);
flushFrames();
return { render, scroller, timeline };
}
function readerScrollsTo(scroller, offset) {
act(() => {
scroller.scrollTop = offset;
scroller.dispatchEvent(new window.Event("scroll"));
});
}
// The guard cannot tell Virtua's stale gap from a real one, so a real 200px
// gap stands in for it. Either way, the send pulls the view to its row.
for (const { name, order } of [
{ name: "after the send", order: ["send", "gap"] },
{ name: "with its freeze still queued at the send", order: ["gap", "send"] },
]) {
test(`an own send reaches the floor when Virtua reports a gap ${name}`, async () => {
const { render, scroller, timeline } = await mountAtFloor();
assert.equal(
container.querySelector('[data-testid="message-scroll-to-latest"]'),
null,
);
for (const step of order) {
if (step === "send") {
act(() => timeline.current.scrollToBottomOnNextUpdate());
} else {
readerScrollsTo(scroller, scroller.scrollTop - 200);
}
}
flushFrames();
await render([...history, sent]);
flushFrames();
assert.ok(container.querySelector(`[data-message-id="${sent.id}"]`));
assert.equal(
container.querySelector('[data-testid="message-scroll-to-latest"]'),
null,
);
assert.equal(scroller.scrollTop, scroller.scrollHeight - VIEWPORT);
});
}

View File

@@ -358,6 +358,7 @@ const MessageTimelineBase = React.forwardRef<
onScroll, onScroll,
scrollToBottom, scrollToBottom,
scrollToBottomOnNextUpdate, scrollToBottomOnNextUpdate,
isBottomPinArmed,
scrollToMessage, scrollToMessage,
onVirtualizerAtBottomStateChange, onVirtualizerAtBottomStateChange,
} = useAnchoredScroll({ } = useAnchoredScroll({
@@ -424,13 +425,17 @@ const MessageTimelineBase = React.forwardRef<
} }
} else if (hasConfirmedVirtualizerBottomRef.current) { } else if (hasConfirmedVirtualizerBottomRef.current) {
onVirtualizerAtBottomStateChange(false); onVirtualizerAtBottomStateChange(false);
if (semanticAtBottomRef.current) { // An armed own-send pin owns the next append. Freezing the tail first
// would buffer the sent row, and the pin would never see it arrive.
// Virtua reports a stale gap here when a discarded render already
// counted the sent row in its store.
if (semanticAtBottomRef.current && !isBottomPinArmed()) {
suppressNextSemanticBottomRef.current = true; suppressNextSemanticBottomRef.current = true;
queueSemanticBottom(false); queueSemanticBottom(false);
} }
} }
}, },
[onVirtualizerAtBottomStateChange, queueSemanticBottom], [isBottomPinArmed, onVirtualizerAtBottomStateChange, queueSemanticBottom],
); );
const hasPendingLiveRows = bufferedTimeline.pendingCount > 0; const hasPendingLiveRows = bufferedTimeline.pendingCount > 0;
React.useEffect(() => { React.useEffect(() => {
@@ -480,7 +485,13 @@ const MessageTimelineBase = React.forwardRef<
const prepareForOwnMessage = React.useCallback(() => { const prepareForOwnMessage = React.useCallback(() => {
// The user's own send is the deliberate Zulip exception: release buffered // The user's own send is the deliberate Zulip exception: release buffered
// output before arming the next-append bottom pin so the sent row can enter // output before arming the next-append bottom pin so the sent row can enter
// Virtua's model and become the new physical floor. // Virtua's model and become the new physical floor. Drop a freeze queued
// before the send, or it would land after this release.
if (semanticBottomRafRef.current !== null) {
window.cancelAnimationFrame(semanticBottomRafRef.current);
semanticBottomRafRef.current = null;
}
semanticAtBottomRef.current = true;
setIsSemanticallyAtBottom(true); setIsSemanticallyAtBottom(true);
scrollToBottomOnNextUpdate(); scrollToBottomOnNextUpdate();
}, [scrollToBottomOnNextUpdate]); }, [scrollToBottomOnNextUpdate]);

View File

@@ -88,6 +88,8 @@ type UseAnchoredScrollResult = {
/** Arm a one-shot scroll-to-bottom that fires on the next appended message /** Arm a one-shot scroll-to-bottom that fires on the next appended message
* (used by the composer's send flow). */ * (used by the composer's send flow). */
scrollToBottomOnNextUpdate: () => void; scrollToBottomOnNextUpdate: () => void;
/** True while that one-shot is armed and its append has not landed. */
isBottomPinArmed: () => boolean;
/** Imperative: scroll a specific message into view; optionally pulse it. /** Imperative: scroll a specific message into view; optionally pulse it.
* Returns true if the row was found and scrolled, false otherwise. */ * Returns true if the row was found and scrolled, false otherwise. */
scrollToMessage: ( scrollToMessage: (
@@ -445,6 +447,10 @@ export function useAnchoredScroll({
const scrollToBottomOnNextUpdate = React.useCallback(() => { const scrollToBottomOnNextUpdate = React.useCallback(() => {
forceBottomOnNextAppendRef.current = true; forceBottomOnNextAppendRef.current = true;
}, []); }, []);
const isBottomPinArmed = React.useCallback(
() => forceBottomOnNextAppendRef.current,
[],
);
const settleAtBottomAfterLayout = React.useCallback(() => { const settleAtBottomAfterLayout = React.useCallback(() => {
const container = scrollContainerRef.current; const container = scrollContainerRef.current;
@@ -1059,6 +1065,7 @@ export function useAnchoredScroll({
scrollToBottom: scrollToBottomImperative, scrollToBottom: scrollToBottomImperative,
settleAtBottomAfterLayout, settleAtBottomAfterLayout,
scrollToBottomOnNextUpdate, scrollToBottomOnNextUpdate,
isBottomPinArmed,
scrollToMessage: scrollToMessageImperative, scrollToMessage: scrollToMessageImperative,
onVirtualizerAtBottomStateChange, onVirtualizerAtBottomStateChange,
}; };