Add testing harness and fix up a few niggling bugs

frontend/src/App.tsx

@@ -27,7 +27,13 @@ import { RichPayloadProvider } from './contexts/RichPayloadContext';
 import { usePush } from './contexts/PushSubscriptionContext';
 import { messageContainsMention } from './utils/messageParser';
 import { getStateKey } from './utils/conversationState';
-import type { BulkCreateHashtagChannelsResult, Channel, Conversation, RawPacket } from './types';
+import type {
+  BulkCreateHashtagChannelsResult,
+  Channel,
+  Conversation,
+  Message,
+  RawPacket,
+} from './types';
 import { CONTACT_TYPE_REPEATER, CONTACT_TYPE_ROOM } from './types';
 import { shouldAutoFocusInput } from './utils/autoFocusInput';
 
@@ -43,6 +49,34 @@ interface NewMessagePrefillRequest {
   nonce: number;
 }
 
+/**
+ * Which message the unread divider should sit on.
+ *
+ * Normally the server's first-unread id. The exception is a channel that has
+ * never been read: its true boundary is the first message ever sent there, so
+ * offering to jump would haul the reader to the start of history for no gain.
+ * Everything loaded is unread in that case, so the divider belongs at the top of
+ * the window — which is what the pre-id behaviour did, and it is genuinely the
+ * more useful answer.
+ */
+export function resolveUnreadMarkerId(
+  boundaryId: number | null,
+  lastReadAt: number | null,
+  messages: Message[]
+): number | null {
+  if (boundaryId === null) return null;
+  if (lastReadAt !== null) return boundaryId;
+  if (messages.length === 0) return boundaryId;
+  if (messages.some((msg) => msg.id === boundaryId)) return boundaryId;
+
+  const oldestLoaded = messages.reduce((oldest, msg) => {
+    if (msg.received_at < oldest.received_at) return msg;
+    if (msg.received_at === oldest.received_at && msg.id < oldest.id) return msg;
+    return oldest;
+  }, messages[0]);
+  return oldestLoaded.id;
+}
+
 export function App() {
   const quoteSearchOperatorValue = useCallback((value: string) => {
     return `"${value.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"`;
@@ -311,6 +345,7 @@ export function App() {
     unreadCounts,
     mentions,
     lastMessageTimes,
+    unreadLastReadAts,
     firstUnreadIds,
     recordMessageEvent,
     renameConversationState,
@@ -344,17 +379,23 @@ export function App() {
     const activeChannelId = activeConversation.id;
     const activeChannelUnreadCount = unreadCounts[getStateKey('channel', activeChannelId)] ?? 0;
 
+    const boundaryId = firstUnreadIds[getStateKey('channel', activeChannelId)] ?? null;
+
     setChannelUnreadMarker((prev) => {
       if (prev?.channelId === activeChannelId) {
+        // Same channel: hold the marker steady so it does not move under the
+        // reader, except to fill in a boundary we did not have yet. A marker
+        // created before /unreads resolved would otherwise stay blank for as long
+        // as the user stays put.
+        if (prev.messageId === null && boundaryId !== null) {
+          return { channelId: activeChannelId, messageId: boundaryId };
+        }
         return prev;
       }
       if (activeChannelUnreadCount <= 0) {
         return null;
       }
-      return {
-        channelId: activeChannelId,
-        messageId: firstUnreadIds[getStateKey('channel', activeChannelId)] ?? null,
-      };
+      return { channelId: activeChannelId, messageId: boundaryId };
     });
   }, [activeConversation, unreadCounts, firstUnreadIds]);
 
@@ -539,7 +580,11 @@ export function App() {
     unreadMarkerMessageId:
       activeConversation?.type === 'channel' &&
       channelUnreadMarker?.channelId === activeConversation.id
-        ? channelUnreadMarker.messageId
+        ? resolveUnreadMarkerId(
+            channelUnreadMarker.messageId,
+            unreadLastReadAts[getStateKey('channel', activeConversation.id)] ?? null,
+            messages
+          )
         : undefined,
     onNavigateToUnread: (messageId: number) => setTargetMessageId(messageId),
     targetMessageId,

frontend/src/components/MessageList.tsx

@@ -422,6 +422,11 @@ export function MessageList({
   // fired once and marked done.
   const pendingBottomScrollRef = useRef(false);
   const [bottomScrollNonce, setBottomScrollNonce] = useState(0);
+  const virtualSpacerRef = useRef<HTMLDivElement>(null);
+  // Distance from the scroll container's content origin down to the first row.
+  // Non-zero because the container carries p-4 and can show a loading/older
+  // banner above the rows; see the scrollMargin note on the virtualizer.
+  const [scrollMargin, setScrollMargin] = useState(0);
   const [showScrollToBottom, setShowScrollToBottom] = useState(false);
   const [selectedPath, setSelectedPath] = useState<{
     paths: MessagePath[];
@@ -528,7 +533,16 @@ export function MessageList({
     count: sortedMessages.length,
     getScrollElement: () => listRef.current,
     estimateSize: () => ESTIMATED_MESSAGE_HEIGHT,
-    getItemKey: (index) => sortedMessages[index]?.id ?? index,
+    // Rows do not start at the scroll container's origin: the container has p-4
+    // padding and may render an "older messages" banner above them. Without this
+    // the virtualizer's offsets are short by that distance, so every
+    // scrollToIndex with 'start'/'center' lands high by 16-48px — and the error
+    // moves as the banner appears and disappears during pagination.
+    scrollMargin,
+    // String sentinel for the transient window past the end of a shrunken list:
+    // a bare index would share the keyspace with message ids and poison the
+    // measurement cache for whichever message happens to have that id.
+    getItemKey: (index) => sortedMessages[index]?.id ?? `__idx:${index}`,
     overscan: 8,
     // A row that measures zero has not really been laid out yet (hidden pane, images
     // still loading). Keep the estimate instead, or the window balloons to compensate.
@@ -551,6 +565,20 @@ export function MessageList({
   });
   const virtualRows = virtualizer.getVirtualItems();
 
+  // Re-measured whenever something above the rows can change height.
+  useLayoutEffect(() => {
+    const spacer = virtualSpacerRef.current;
+    const list = listRef.current;
+    if (!spacer || !list) return;
+    // Relative to the scroll container's *content* origin, so it is independent
+    // of the current scroll position. offsetTop is not usable here: the two
+    // elements can resolve to different offsetParents.
+    const next = Math.round(
+      spacer.getBoundingClientRect().top - list.getBoundingClientRect().top + list.scrollTop
+    );
+    setScrollMargin((prev) => (prev === next ? prev : next));
+  }, [loadingOlder, hasOlderMessages, messages.length]);
+
   const scrollToIndex = useCallback(
     (index: number, align: 'start' | 'center' | 'end') => {
       if (index < 0) return;
@@ -613,7 +641,15 @@ export function MessageList({
     if ((isInitialLoadRef.current || conversationChanged) && messages.length > 0) {
       // Initial load or conversation switch - pin to the newest message. Requested
       // rather than performed here; see pendingBottomScrollRef.
-      requestBottomScroll();
+      //
+      // Unless we are loading *at* a specific message: jump-to-message and
+      // jump-to-unread clear the list before fetching a window around their
+      // target, which trips both the initial-load and conversation-changed
+      // branches. Pinning to the bottom here would then discard the target scroll
+      // a frame later, stranding the user at the newest message instead.
+      if (!targetMessageId) {
+        requestBottomScroll();
+      }
       isInitialLoadRef.current = false;
     } else if (messagesAdded > 0 && prevMessagesLengthRef.current > 0) {
       if (scrollStateRef.current.wasNearTop) {
@@ -629,7 +665,7 @@ export function MessageList({
     }
 
     prevMessagesLengthRef.current = messages.length;
-  }, [messages, sortedMessages.length, scrollToIndex, requestBottomScroll]);
+  }, [messages, sortedMessages.length, scrollToIndex, requestBottomScroll, targetMessageId]);
 
   // Scroll to target message and highlight it
   useLayoutEffect(() => {
@@ -637,8 +673,10 @@ export function MessageList({
     const targetIndex = sortedMessages.findIndex((msg) => msg.id === targetMessageId);
     if (targetIndex === -1) return;
 
-    // Prevent the initial-load layout effect from overriding our scroll
+    // Prevent the initial-load layout effect from overriding our scroll, and drop
+    // any bottom pin already queued by an earlier pass over the same commit.
     isInitialLoadRef.current = false;
+    pendingBottomScrollRef.current = false;
     scrollToIndex(targetIndex, 'center');
     setHighlightedMessageId(targetMessageId);
     targetScrolledRef.current = true;
@@ -1036,6 +1074,7 @@ export function MessageList({
           </div>
         )}
         <div
+          ref={virtualSpacerRef}
           className="relative w-full flex-shrink-0"
           style={{ height: virtualizer.getTotalSize() }}
         >
@@ -1146,7 +1185,10 @@ export function MessageList({
                 data-index={index}
                 ref={virtualizer.measureElement}
                 className="absolute left-0 top-0 flex w-full flex-col pb-0.5"
-                style={{ transform: `translateY(${virtualRow.start}px)` }}
+                // start is measured from the scroll container's origin, which
+                // scrollMargin accounts for; the spacer already sits that far
+                // down, so subtract it back out when positioning within it.
+                style={{ transform: `translateY(${virtualRow.start - scrollMargin}px)` }}
               >
                 {unreadMarkerIndex === index &&
                   (onDismissUnreadMarker ? (

frontend/src/hooks/useUnreadCounts.ts

@@ -164,18 +164,29 @@ export function useUnreadCounts(
     }
   }, [activeConversation]);
 
-  const incrementUnread = useCallback((stateKey: string, hasMention?: boolean) => {
-    setUnreadCounts((prev) => ({
-      ...prev,
-      [stateKey]: (prev[stateKey] || 0) + 1,
-    }));
-    if (hasMention) {
-      setMentions((prev) => ({
+  const incrementUnread = useCallback(
+    (stateKey: string, messageId: number, hasMention?: boolean) => {
+      setUnreadCounts((prev) => ({
         ...prev,
-        [stateKey]: true,
+        [stateKey]: (prev[stateKey] || 0) + 1,
       }));
-    }
-  }, []);
+      // Counts move live over the socket, but first_unread_ids only arrives with a
+      // full /unreads fetch. Without seeding it here, a conversation that goes from
+      // read to unread while the app is open has a count but no boundary, and the
+      // divider silently never renders. Only the transition matters: once a
+      // boundary exists, later messages are not the *first* unread.
+      setFirstUnreadIds((prev) =>
+        prev[stateKey] != null ? prev : { ...prev, [stateKey]: messageId }
+      );
+      if (hasMention) {
+        setMentions((prev) => ({
+          ...prev,
+          [stateKey]: true,
+        }));
+      }
+    },
+    []
+  );
 
   const recordMessageEvent = useCallback(
     ({
@@ -205,7 +216,7 @@ export function useUnreadCounts(
       setLastMessageTimes(updated);
 
       if (!isActiveConversation && !msg.outgoing && isNewMessage) {
-        incrementUnread(stateKey, hasMention);
+        incrementUnread(stateKey, msg.id, hasMention);
       }
     },
     [incrementUnread]
@@ -230,6 +241,14 @@ export function useUnreadCounts(
       return next;
     });
 
+    setFirstUnreadIds((prev) => {
+      if (!(oldStateKey in prev)) return prev;
+      const next = { ...prev };
+      next[newStateKey] = next[newStateKey] ?? next[oldStateKey];
+      delete next[oldStateKey];
+      return next;
+    });
+
     setLastMessageTimes(renameConversationTimeKey(oldStateKey, newStateKey));
   }, []);
 
@@ -246,6 +265,12 @@ export function useUnreadCounts(
       delete next[stateKey];
       return next;
     });
+    setFirstUnreadIds((prev) => {
+      if (!(stateKey in prev)) return prev;
+      const next = { ...prev };
+      delete next[stateKey];
+      return next;
+    });
     setUnreadLastReadAts((prev) => {
       if (!(stateKey in prev)) return prev;
       const next = { ...prev };
@@ -261,6 +286,7 @@ export function useUnreadCounts(
     setUnreadCounts({});
     setMentions({});
     setUnreadLastReadAts({});
+    setFirstUnreadIds({});
 
     // Persist to server with single bulk request
     api.markAllRead().catch((err) => {

frontend/src/test/appFavorites.test.tsx

@@ -104,6 +104,7 @@ vi.mock('../hooks', async (importOriginal) => {
       mentions: {},
       lastMessageTimes: {},
       unreadLastReadAts: {},
+      firstUnreadIds: {},
       recordMessageEvent: mocks.hookFns.recordMessageEvent,
       renameConversationState: vi.fn(),
       markAllRead: mocks.hookFns.markAllRead,

frontend/src/test/appSearchJump.test.tsx

@@ -53,6 +53,7 @@ vi.mock('../hooks', async (importOriginal) => {
       mentions: {},
       lastMessageTimes: {},
       unreadLastReadAts: {},
+      firstUnreadIds: {},
       recordMessageEvent: vi.fn(),
       renameConversationState: vi.fn(),
       markAllRead: vi.fn(),

frontend/src/test/appStartupHash.test.tsx

@@ -47,6 +47,7 @@ vi.mock('../hooks', async (importOriginal) => {
       mentions: {},
       lastMessageTimes: {},
       unreadLastReadAts: {},
+      firstUnreadIds: {},
       recordMessageEvent: vi.fn(),
       renameConversationState: vi.fn(),
       markAllRead: vi.fn(),

frontend/src/test/unreadMarkerResolution.test.ts

@@ -0,0 +1,56 @@
+/**
+ * The unread divider is anchored to a message id from the server. These cover the
+ * one case where that id is deliberately overridden: a channel that has never
+ * been read, whose true boundary is the start of history and therefore a useless
+ * jump target.
+ */
+import { describe, expect, it } from 'vitest';
+
+import { resolveUnreadMarkerId } from '../App';
+import type { Message } from '../types';
+
+function msg(id: number, receivedAt: number): Message {
+  return {
+    id,
+    type: 'CHAN',
+    conversation_key: 'CHAN1',
+    text: `Alice: m${id}`,
+    sender_timestamp: receivedAt,
+    received_at: receivedAt,
+    paths: null,
+    txt_type: 0,
+    signature: null,
+    sender_key: null,
+    outgoing: false,
+    acked: 0,
+    sender_name: 'Alice',
+  };
+}
+
+describe('resolveUnreadMarkerId', () => {
+  const loaded = [msg(50, 1700000050), msg(51, 1700000051), msg(52, 1700000052)];
+
+  it('uses the server boundary when the channel has been read before', () => {
+    expect(resolveUnreadMarkerId(9, 1700000000, loaded)).toBe(9);
+  });
+
+  it('uses the server boundary when it is inside the loaded window', () => {
+    expect(resolveUnreadMarkerId(51, null, loaded)).toBe(51);
+  });
+
+  it('anchors a never-read channel to the top of the loaded window', () => {
+    // Boundary 1 is the first message ever sent; jumping there would dump the
+    // reader at the start of history. Everything loaded is unread, so the top of
+    // the window is both true and useful.
+    expect(resolveUnreadMarkerId(1, null, loaded)).toBe(50);
+  });
+
+  it('picks the oldest loaded message regardless of array order', () => {
+    expect(resolveUnreadMarkerId(1, null, [loaded[2], loaded[0], loaded[1]])).toBe(50);
+  });
+
+  it('passes through when there is no boundary or no messages', () => {
+    expect(resolveUnreadMarkerId(null, null, loaded)).toBeNull();
+    expect(resolveUnreadMarkerId(7, null, [])).toBe(7);
+  });
+});

frontend/src/test/useUnreadCounts.test.ts

@@ -479,4 +479,74 @@ describe('useUnreadCounts', () => {
     expect(result.current.lastMessageTimes[getStateKey('contact', CONTACT_KEY)]).toBe(1700002000);
     expect(result.current.lastMessageTimes[getStateKey('channel', CHANNEL_KEY)]).toBe(1700002001);
   });
+
+  it('seeds the first-unread boundary when a conversation goes unread over the socket', async () => {
+    // Counts move live over WS but first_unread_ids only arrives with a full
+    // fetch. Without seeding here, a channel that goes unread while the app is
+    // open has a count but no boundary, so the divider never renders.
+    const mocks = await getMockedApi();
+    mocks.getUnreads.mockResolvedValue({
+      counts: {},
+      mentions: {},
+      last_message_times: {},
+      first_unread_ids: {},
+      last_read_ats: {},
+    });
+
+    const { result } = renderWith({ channels: [makeChannel(CHANNEL_KEY, 'Test')] });
+    await act(async () => {
+      await vi.waitFor(() => expect(mocks.getUnreads).toHaveBeenCalled());
+    });
+
+    const key = getStateKey('channel', CHANNEL_KEY);
+    act(() => {
+      result.current.recordMessageEvent({
+        msg: makeMessage({ id: 4711, type: 'CHAN', conversation_key: CHANNEL_KEY }),
+        activeConversation: false,
+        isNewMessage: true,
+      });
+    });
+    expect(result.current.firstUnreadIds[key]).toBe(4711);
+
+    // A later message must not move the boundary — it is not the *first* unread.
+    act(() => {
+      result.current.recordMessageEvent({
+        msg: makeMessage({ id: 4712, type: 'CHAN', conversation_key: CHANNEL_KEY }),
+        activeConversation: false,
+        isNewMessage: true,
+      });
+    });
+    expect(result.current.firstUnreadIds[key]).toBe(4711);
+    expect(result.current.unreadCounts[key]).toBe(2);
+  });
+
+  it('drops first-unread boundaries on mark-all-read', async () => {
+    const mocks = await getMockedApi();
+    mocks.getUnreads.mockResolvedValue({
+      counts: {},
+      mentions: {},
+      last_message_times: {},
+      first_unread_ids: {},
+      last_read_ats: {},
+    });
+
+    const { result } = renderWith({ channels: [makeChannel(CHANNEL_KEY, 'Test')] });
+    await act(async () => {
+      await vi.waitFor(() => expect(mocks.getUnreads).toHaveBeenCalled());
+    });
+
+    act(() => {
+      result.current.recordMessageEvent({
+        msg: makeMessage({ id: 99, type: 'CHAN', conversation_key: CHANNEL_KEY }),
+        activeConversation: false,
+        isNewMessage: true,
+      });
+    });
+    expect(result.current.firstUnreadIds[getStateKey('channel', CHANNEL_KEY)]).toBe(99);
+
+    await act(async () => {
+      await result.current.markAllRead();
+    });
+    expect(result.current.firstUnreadIds).toEqual({});
+  });
 });