Restore bottom positioning for ordinary routed threads

src/features/messages/ThreadPanel.test.tsx

@@ -16,6 +16,9 @@ import { MessageMarkdown } from "./MessageMarkdown";
 import { MediaAttachment } from "./MediaAttachment";
 import { MessageComposer } from "./MessageComposer";
 import type { RelaySession } from "../relay/session";
+import type { PageNavigation } from "../navigation/service";
+import { createNavigationController } from "../navigation/controller";
+import { createMemoryHistory } from "../navigation/history";
 import type { ThreadSnapshot, ThreadView } from "../relay/threads";
 import type { ChannelMessage } from "../relay/contracts";
 
@@ -129,7 +132,30 @@ const row: ChannelMessage = {
   reactions: [],
   replyCount: 2,
 };
-function setup(onOpenMediaReview?: ThreadPanelProps["onOpenMediaReview"]) {
+function ordinaryNavigation() {
+  return {
+    entryId: "thread-visit",
+    target: {
+      version: 1,
+      kind: "conversation",
+      channelId: "channel",
+      messageId: row.id,
+      threadRootId: row.id,
+      scope: {
+        viewer: row.authorId,
+        communityOrigin: "https://fixture.invalid",
+      },
+    },
+    signal: new AbortController().signal,
+    complete: vi.fn<PageNavigation["complete"]>(() => true),
+    resolve: vi.fn(() => true),
+    forSession: vi.fn<PageNavigation["forSession"]>(),
+  } satisfies PageNavigation;
+}
+function setup(
+  navigation?: PageNavigation,
+  onOpenMediaReview?: ThreadPanelProps["onOpenMediaReview"],
+) {
   const snapshot: ThreadSnapshot = {
     status: "ready",
     root: row,
@@ -165,6 +191,7 @@ function setup(onOpenMediaReview?: ThreadPanelProps["onOpenMediaReview"]) {
       channelName: "General",
       channelId: "channel",
       messageId: row.id,
+      navigation,
       close,
       onOpenLink: () => false,
       ...(onOpenMediaReview ? { onOpenMediaReview } : {}),
@@ -444,9 +471,10 @@ it("the actual message reply button opens its selected message and canonical thr
 });
 
 function messagesHarness(
+  navigation?: PageNavigation,
   onOpenMediaReview?: ThreadPanelProps["onOpenMediaReview"],
 ) {
-  const h = setup(onOpenMediaReview);
+  const h = setup(navigation, onOpenMediaReview);
   h.render();
   h.effects();
   const child = elements(h.render()).find(
@@ -568,7 +596,7 @@ it("preserves reading above the bottom through live updates and refresh, then re
 });
 it("routes media in replies through the resolved root review workspace", () => {
   const open = vi.fn();
-  const h = messagesHarness(open);
+  const h = messagesHarness(undefined, open);
   const root = { ...row, id: "resolved-root" };
   const attachment = { url: "https://safe/image.png", video: false };
   h.snapshot.root = root;
@@ -678,15 +706,25 @@ it("finishes automatic pages before initial positioning and preserves a reader
   h.effects();
   expect(h.element.scrollTop).toBe(4900);
 });
-it.each([false, true])(
-  "positions after automatic loading stops (limited=%s), without restarting pagination",
-  (limited) => {
-    const h = messagesHarness();
+it.each([
+  { limited: false, routed: false },
+  { limited: true, routed: false },
+  { limited: false, routed: true },
+  { limited: true, routed: true },
+])(
+  "positions after automatic loading stops (limited=$limited, routed=$routed), without restarting pagination",
+  ({ limited, routed }) => {
+    const navigation = routed ? ordinaryNavigation() : undefined;
+    const h = messagesHarness(navigation);
     h.snapshot.canLoadMore = true;
     h.render();
     h.effects();
     expect(h.element.scrollTop).toBe(0);
     expect(h.view.loadMore).toHaveBeenCalledTimes(1);
+    if (navigation)
+      expect(navigation.complete).toHaveBeenCalledExactlyOnceWith({
+        status: "opened",
+      });
     h.snapshot.status = "loading";
     h.render();
     h.effects();
@@ -700,12 +738,28 @@ it.each([false, true])(
     h.effects();
     expect(h.element.scrollTop).toBe(4200);
     expect(h.view.loadMore).toHaveBeenCalledTimes(1);
+    // A live update follows without completing the same visit twice.
+    h.snapshot.replies = [...h.snapshot.replies, { ...row, id: "live" }];
+    h.render();
+    h.effects();
+    if (navigation)
+      expect(navigation.complete).toHaveBeenCalledExactlyOnceWith({
+        status: "opened",
+      });
   },
 );
-it.each(["onWheel", "onTouchMove", "onPointerDown", "onKeyDown"])(
-  "a user %s gesture before the page completes wins over initial positioning",
-  (handler) => {
-    const h = messagesHarness();
+it.each(
+  [false, true].flatMap((routed) =>
+    ["onWheel", "onTouchMove", "onPointerDown", "onKeyDown"].map((handler) => ({
+      routed,
+      handler,
+    })),
+  ),
+)(
+  "a user $handler gesture before the page completes wins over initial positioning (routed=$routed)",
+  ({ routed, handler }) => {
+    const navigation = routed ? ordinaryNavigation() : undefined;
+    const h = messagesHarness(navigation);
     h.snapshot.status = "loading";
     const section = h.render();
     h.effects();
@@ -714,8 +768,85 @@ it.each(["onWheel", "onTouchMove", "onPointerDown", "onKeyDown"])(
     h.render();
     h.effects();
     expect(h.element.scrollTop).toBe(0);
+    if (navigation)
+      expect(navigation.complete).toHaveBeenCalledExactlyOnceWith({
+        status: "opened",
+      });
   },
 );
+it("ordinary routed loading failure completes as unavailable, never as an opened visit", () => {
+  const navigation = ordinaryNavigation();
+  const h = messagesHarness(navigation);
+  h.snapshot.status = "error";
+  h.render();
+  h.effects();
+  expect(h.element.scrollTop).toBe(0);
+  expect(navigation.complete).toHaveBeenCalledExactlyOnceWith({
+    status: "failed",
+    reason: "unavailable",
+  });
+});
+it("a presented ordinary thread survives the real navigation deadline while history is pending", async () => {
+  vi.useFakeTimers();
+  const controller = createNavigationController(createMemoryHistory());
+  let release = () => {};
+  try {
+    const navigation = ordinaryNavigation();
+    const result = controller.navigation.open(navigation.target);
+    const { attempt } = controller.navigation.snapshot();
+    navigation.signal = attempt.signal;
+    navigation.complete.mockImplementation((result) =>
+      controller.complete(attempt, result),
+    );
+    const h = messagesHarness(navigation);
+    const held = new Promise<void>((resolve) => {
+      release = resolve;
+    });
+    h.view.loadMore.mockImplementation(() => held);
+    h.snapshot.canLoadMore = true;
+    h.render();
+    h.effects();
+    expect(h.view.loadMore).toHaveBeenCalledTimes(1);
+    h.snapshot.status = "loading";
+    h.render();
+    h.effects();
+    await vi.advanceTimersByTimeAsync(16_000);
+    expect(controller.navigation.snapshot().status).toBe("opened");
+    expect(await result).toEqual({ status: "opened" });
+    expect(attempt.signal.aborted).toBe(false);
+    expect(h.view.dispose).not.toHaveBeenCalled();
+    expect(h.element.scrollTop).toBe(0);
+    release();
+    await held;
+    h.snapshot.status = "ready";
+    h.snapshot.canLoadMore = false;
+    h.render();
+    h.effects();
+    expect(h.element.scrollTop).toBe(3400);
+    expect(navigation.complete).toHaveBeenCalledTimes(1);
+    h.unmount();
+  } finally {
+    release();
+    controller.dispose();
+    vi.useRealTimers();
+  }
+});
+it("revoked ordinary presentation cannot position or complete after loading", () => {
+  const controller = new AbortController();
+  const navigation = { ...ordinaryNavigation(), signal: controller.signal };
+  const h = messagesHarness(navigation);
+  h.snapshot.status = "loading";
+  h.snapshot.root = undefined;
+  h.render();
+  h.effects();
+  controller.abort();
+  expect(h.view.dispose).toHaveBeenCalledTimes(1);
+  h.snapshot.status = "ready";
+  h.render();
+  h.effects();
+  expect(h.element.scrollTop).toBe(0);
+  expect(navigation.complete).not.toHaveBeenCalled();
+});
 it("shows bounded participant avatars on the real reply control, through the media boundary with fallback initials", () => {
   const participants = ["p1", "p2", "p3", "p4", "p5"];
   const media = vi.fn((url: string) =>

src/features/messages/ThreadPanel.tsx

@@ -244,10 +244,9 @@ function ThreadMessages({
     scroller,
     settled: positioned,
     messageId,
-    signal: navigation?.signal,
-    ready: rootTarget
-      ? snapshot.root?.id === messageId
-      : snapshot.targetStatus === "ready" && snapshot.target?.id === messageId,
+    signal: rootTarget ? undefined : navigation?.signal,
+    ready:
+      snapshot.targetStatus === "ready" && snapshot.target?.id === messageId,
     complete: completeTarget,
     prepare: prepareTarget,
   });
@@ -289,9 +288,20 @@ function ThreadMessages({
   // biome-ignore lint/correctness/useExhaustiveDependencies: Rendered rows/profiles change scroll height; sending is explicit navigation intent.
   useLayoutEffect(() => {
     const element = scroller.current;
+    if (!element || navigation?.signal.aborted) return;
+    // A mounted ordinary thread acknowledges the visit before slow history can
+    // exhaust navigation's deadline. Positioning still waits for bounded loading.
     if (
-      !element ||
-      (navigation && revealed.current !== navigation.signal) ||
+      rootTarget &&
+      snapshot.status !== "error" &&
+      snapshot.root?.id === messageId &&
+      revealed.current !== navigation.signal
+    ) {
+      revealed.current = navigation.signal;
+      navigation.complete({ status: "opened" });
+    }
+    if (
+      (navigation && !rootTarget && revealed.current !== navigation.signal) ||
       (!positioned.current &&
         (snapshot.status !== "ready" || snapshot.canLoadMore))
     )
@@ -313,10 +323,13 @@ function ThreadMessages({
   }, [
     snapshot.status,
     snapshot.canLoadMore,
+    snapshot.root,
+    messageId,
     rows,
     profiles,
     sent,
     navigation,
+    rootTarget,
     revealed,
     selectedRow,
   ]);

tests/browser/fixture.mjs

@@ -1083,7 +1083,7 @@ export const test = base.extend({
           targetEvents.push(event);
           relay.publish("primary", event);
         },
-        reply(rootId, own = false) {
+        reply(rootId, own = false, deliver = true) {
           const replies = threadReplies.get(rootId);
           if (!replies) throw new Error("Unknown fixture thread");
           const event = sign(
@@ -1097,7 +1097,7 @@ export const test = base.extend({
             replies.at(-1).created_at + 1,
           );
           replies.push(event);
-          relay.publish("primary", event);
+          if (deliver) relay.publish("primary", event);
           return event;
         },
         append(community, channel, content, deliver = true, own = true, root) {

tests/browser/layout.spec.mjs

@@ -5,7 +5,8 @@ const scroll = test.extend({ historyCounts: { alpha: 20, beta: 1 } });
 // Resize tests must not enter the fixture’s deliberately held paging path.
 const readingTest = test.extend({
   tallMessages: true,
-  historyCounts: { alpha: 20, beta: 1 },
+  // Keep the old restored row visible when wheel input selects another row.
+  historyCounts: { alpha: 24, beta: 1 },
 });
 async function expectNonPaging(page, app) {
   expect(
@@ -627,6 +628,11 @@ readingTest(
       reading = await anchor(page);
     }
     expect(reading.id).not.toBe(original.id);
+    // An offscreen restored row is ignored even if gesture() fails to clear it.
+    // Keep that row intersecting so the final assertion detects a stale anchor.
+    await expect(
+      history.locator(`[data-message-id="${original.id}"]`),
+    ).toBeInViewport();
     await button(page, "Close Bestie panel").click();
     await settle(page);
     await expectAnchor(page, reading);