Test improvements and trimming

frontend/AGENTS.md

@@ -44,6 +44,8 @@ frontend/src/
 │   └── PushSubscriptionContext.tsx # Push subscription state context/provider
 ├── lib/
 │   └── utils.ts            # cn() — clsx + tailwind-merge helper
+├── stores/
+│   └── rawPacketStore.ts   # Overheard packet stream + session stats, outside React
 ├── hooks/
 │   ├── index.ts            # Central re-export of all hooks
 │   ├── useConversationActions.ts   # Send/resend/trace/block conversation actions
@@ -60,7 +62,6 @@ frontend/src/
 │   ├── useBrowserNotifications.ts  # Per-conversation browser notification preferences + dispatch
 │   ├── usePushSubscription.ts      # Web Push subscription lifecycle, per-conversation filters
 │   ├── useFaviconBadge.ts          # Browser tab unread badge state
-│   ├── useRawPacketStatsSession.ts # Session-scoped packet-feed stats history
 │   └── useRememberedServerPassword.ts # Browser-local repeater/room password persistence
 ├── components/
 │   ├── AppShell.tsx            # App-shell layout: status, sidebar, search/settings panes, cracker, modals, security warning
@@ -249,6 +250,10 @@ High-level state is delegated to hooks:
 
 `App.tsx` intentionally still does the final `AppShell` prop assembly. That composition layer is considered acceptable here because it keeps the shell contract visible in one place and avoids a prop-bundling hook with little original logic.
 
+**The overheard packet stream is the one piece of app state that deliberately does not live in React.** It is held in `stores/rawPacketStore.ts` and read through `useSyncExternalStore`, because it updates several times a second with every packet the node hears — far more often than anything else — and only four surfaces consume it (`MapView`, `VisualizerView`, `RawPacketFeedView`, `CrackerPanel`). Held in `App` state it re-rendered the entire tree, including `MessageList`, which is neither memoized nor cheap on a long history.
+
+That gives the store a load-bearing invariant: **no ancestor of `MessageList` may call `useRawPackets()` / `useRawPacketStatsSession()`.** Nothing about the prop signatures enforces it — an innocuous-looking subscription added to `App`, `AppShell`, or `ConversationPane` silently restores the original slowdown. `src/test/appPacketIsolation.test.tsx` pins it by mounting the real ancestor chain and asserting `MessageList` does not re-render when packets arrive; it carries a negative control so the assertion cannot pass vacuously. Reach for packets in a new view by subscribing in that view, never by lifting them up.
+
 `ConversationPane.tsx` owns the main active-conversation surface branching:
 - empty state
 - map view

frontend/src/hooks/useRealtimeAppState.ts

@@ -11,7 +11,7 @@ import { toast } from '../components/ui/sonner';
 import { getStateKey } from '../utils/conversationState';
 import { mergeContactIntoList } from '../utils/contactMerge';
 import { getContactDisplayName } from '../utils/pubkey';
-import { clearRawPackets, recordRawPacket } from '../stores/rawPacketStore';
+import { clearRawPackets, MAX_RAW_PACKETS, recordRawPacket } from '../stores/rawPacketStore';
 import { emitStatusDotPulse } from '../utils/statusDotPulse';
 import type {
   Channel,
@@ -57,6 +57,7 @@ interface UseRealtimeAppStateArgs {
     packetId?: number | null
   ) => void;
   notifyIncomingMessage?: (msg: Message) => void;
+  /** Buffer cap override. Defaults to the store's own cap; tests use it to force eviction. */
   maxRawPackets?: number;
 }
 
@@ -105,7 +106,7 @@ export function useRealtimeAppState({
   removeConversationMessages,
   receiveMessageAck,
   notifyIncomingMessage,
-  maxRawPackets = 500,
+  maxRawPackets = MAX_RAW_PACKETS,
 }: UseRealtimeAppStateArgs): UseWebSocketOptions {
   const mergeChannelIntoList = useCallback(
     (updated: Channel) => {

frontend/src/stores/rawPacketStore.ts

@@ -118,11 +118,19 @@ export function seedRawPacketStore(next: {
   packets?: RawPacket[];
   statsSession?: RawPacketStatsSessionState;
 }): void {
+  // Copy rather than alias. The whole store rests on "a snapshot is immutable, and its
+  // identity changes only when its contents do". Holding the caller's array would let
+  // them mutate the live snapshot in place, and because useSyncExternalStore compares
+  // snapshots with Object.is, React would then bail out of every subsequent render —
+  // leaving the UI permanently disagreeing with getRawPackets() and no way to tell why.
   if (next.packets) {
-    packets = next.packets;
+    packets = [...next.packets];
   }
   if (next.statsSession) {
-    statsSession = next.statsSession;
+    statsSession = {
+      ...next.statsSession,
+      observations: [...next.statsSession.observations],
+    };
   }
   emit();
 }

frontend/src/test/appPacketIsolation.test.tsx

@@ -0,0 +1,239 @@
+/**
+ * The invariant this store exists to protect: nothing on the chat render path may
+ * subscribe to the raw packet stream.
+ *
+ * The original bug was not that the chat view read packets — it never did. It was that
+ * the stream lived in `App` state, and `App` is an *ancestor* of `MessageList`. Nothing
+ * on that path is memoized, so every overheard packet re-rendered the whole message list
+ * regardless of which props it actually received.
+ *
+ * That means the regression can only be caught by mounting the real ancestor chain
+ * (`App` → `AppShell` → `ConversationPane` → `MessageList`). A test that renders
+ * `ConversationPane` on its own cannot see it: the offending subscription lives above.
+ */
+import React from 'react';
+import { act, render, screen, waitFor } from '@testing-library/react';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+const mocks = vi.hoisted(() => ({
+  messageList: vi.fn(() => <div data-testid="message-list" />),
+  api: {
+    getRadioConfig: vi.fn(),
+    getSettings: vi.fn(),
+    getUndecryptedPacketCount: vi.fn(),
+    getChannels: vi.fn(),
+    getContacts: vi.fn(),
+    getHealth: vi.fn(),
+  },
+  hookFns: {
+    observeMessage: vi.fn(() => ({ added: false, activeConversation: false })),
+    refreshUnreads: vi.fn(async () => {}),
+  },
+}));
+
+vi.mock('../api', () => ({ api: mocks.api }));
+
+vi.mock('../useWebSocket', () => ({ useWebSocket: vi.fn() }));
+
+vi.mock('../contexts/PushSubscriptionContext', () => ({
+  usePush: () => ({
+    isSupported: false,
+    isSubscribed: false,
+    currentSubscriptionId: null,
+    allSubscriptions: [],
+    pushConversations: [],
+    loading: false,
+    subscribe: vi.fn(async () => null),
+    unsubscribe: vi.fn(async () => {}),
+    toggleConversation: vi.fn(async () => {}),
+    isConversationPushEnabled: () => false,
+    deleteSubscription: vi.fn(async () => {}),
+    testPush: vi.fn(async () => {}),
+    refreshSubscriptions: vi.fn(async () => []),
+    refreshConversations: vi.fn(async () => {}),
+  }),
+}));
+
+vi.mock('../hooks', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('../hooks')>();
+  return {
+    ...actual,
+    useConversationMessages: () => ({
+      messages: [],
+      messagesLoading: false,
+      loadingOlder: false,
+      hasOlderMessages: false,
+      hasNewerMessages: false,
+      loadingNewer: false,
+      fetchOlderMessages: vi.fn(async () => {}),
+      fetchNewerMessages: vi.fn(async () => {}),
+      jumpToBottom: vi.fn(),
+      reloadCurrentConversation: vi.fn(),
+      observeMessage: mocks.hookFns.observeMessage,
+      receiveMessageAck: vi.fn(),
+      reconcileOnReconnect: vi.fn(),
+      renameConversationMessages: vi.fn(),
+      removeConversationMessages: vi.fn(),
+      clearConversationMessages: vi.fn(),
+    }),
+    useUnreadCounts: () => ({
+      unreadCounts: {},
+      mentions: {},
+      lastMessageTimes: {},
+      unreadLastReadAts: {},
+      recordMessageEvent: vi.fn(),
+      renameConversationState: vi.fn(),
+      removeConversationState: vi.fn(),
+      markAllRead: vi.fn(),
+      refreshUnreads: mocks.hookFns.refreshUnreads,
+    }),
+  };
+});
+
+// Mocked to keep the tree small and deterministic. None of them are mounted while a
+// chat conversation is active, so removing them cannot mask a chat-path subscription.
+vi.mock('../components/StatusBar', () => ({ StatusBar: () => <div data-testid="status-bar" /> }));
+vi.mock('../components/Sidebar', () => ({ Sidebar: () => <div data-testid="sidebar" /> }));
+vi.mock('../components/MessageList', () => ({ MessageList: mocks.messageList }));
+vi.mock('../components/MessageInput', () => ({
+  MessageInput: React.forwardRef((_props, ref) => {
+    React.useImperativeHandle(ref, () => ({ appendText: vi.fn(), focus: vi.fn() }));
+    return <div data-testid="message-input" />;
+  }),
+}));
+vi.mock('../components/NewMessageModal', () => ({ NewMessageModal: () => null }));
+vi.mock('../components/SettingsModal', () => ({
+  SettingsModal: () => null,
+  SETTINGS_SECTION_ORDER: ['radio'],
+  SETTINGS_SECTION_LABELS: { radio: 'Radio' },
+}));
+vi.mock('../components/MapView', () => ({ MapView: () => null }));
+vi.mock('../components/VisualizerView', () => ({ VisualizerView: () => null }));
+vi.mock('../components/CrackerPanel', () => ({ CrackerPanel: () => null }));
+vi.mock('../components/ui/sonner', () => ({
+  Toaster: () => null,
+  toast: { success: vi.fn(), error: vi.fn() },
+}));
+vi.mock('../utils/urlHash', () => ({
+  parseHashConversation: () => null,
+  parseHashSettingsSection: () => null,
+  updateUrlHash: vi.fn(),
+  pushUrlHash: vi.fn(),
+  updateSettingsHash: vi.fn(),
+  pushSettingsHash: vi.fn(),
+  getSettingsHash: (section: string) => `#settings/${section}`,
+  getMapFocusHash: () => '#map',
+}));
+
+import { App } from '../App';
+import {
+  getRawPackets,
+  recordRawPacket,
+  resetRawPacketStore,
+  useRawPackets,
+} from '../stores/rawPacketStore';
+import type { RawPacket } from '../types';
+
+function createPacket(overrides: Partial<RawPacket> = {}): RawPacket {
+  return {
+    id: 1,
+    observation_id: 1,
+    timestamp: 1700000000,
+    data: 'aabb',
+    payload_type: 'GROUP_TEXT',
+    snr: 7.5,
+    rssi: -80,
+    decrypted: false,
+    decrypted_info: null,
+    ...overrides,
+  };
+}
+
+const publicChannel = {
+  key: '8B3387E9C5CDEA6AC9E5EDBAA115CD72',
+  name: 'Public',
+  is_hashtag: false,
+  on_radio: false,
+  last_read_at: null,
+  favorite: false,
+  muted: false,
+};
+
+describe('overheard packets and the chat render path', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    resetRawPacketStore();
+    mocks.api.getRadioConfig.mockResolvedValue({
+      public_key: 'aa'.repeat(32),
+      name: 'TestNode',
+      lat: 0,
+      lon: 0,
+      tx_power: 17,
+      max_tx_power: 22,
+      radio: { freq: 910.525, bw: 62.5, sf: 7, cr: 5 },
+      path_hash_mode: 0,
+      path_hash_mode_supported: false,
+    });
+    mocks.api.getSettings.mockResolvedValue({
+      max_radio_contacts: 200,
+      auto_decrypt_dm_on_advert: false,
+      last_message_times: {},
+      advert_interval: 0,
+      last_advert_time: 0,
+      flood_scope: '',
+      known_regions: [],
+      blocked_keys: [],
+      blocked_names: [],
+    });
+    mocks.api.getUndecryptedPacketCount.mockResolvedValue({ count: 0 });
+    mocks.api.getChannels.mockResolvedValue([publicChannel]);
+    mocks.api.getContacts.mockResolvedValue([]);
+    mocks.api.getHealth.mockResolvedValue(null);
+  });
+
+  it('does not re-render the message list when packets arrive', async () => {
+    render(<App />);
+    await waitFor(() => {
+      expect(screen.getByTestId('message-list')).toBeInTheDocument();
+    });
+
+    const rendersBefore = mocks.messageList.mock.calls.length;
+    act(() => {
+      for (let i = 1; i <= 25; i++) {
+        recordRawPacket(createPacket({ id: i, observation_id: i }));
+      }
+    });
+
+    // Guards the assertion below against passing for the wrong reason
+    expect(getRawPackets()).toHaveLength(25);
+    expect(mocks.messageList.mock.calls.length).toBe(rendersBefore);
+  });
+
+  it('re-renders the message list once per batch if an ancestor subscribes', async () => {
+    // Negative control. Proves the assertion above can actually fail — without this, a
+    // render counter that never increments would look identical to a passing test.
+    //
+    // The ancestor has to create the <App /> element inside its own render. Taking it as
+    // a `children` prop would defeat the point: that element is built once by the caller,
+    // so its identity never changes and React bails out of the subtree — the test would
+    // pass while proving nothing.
+    function SubscribingAncestor() {
+      useRawPackets();
+      return <App />;
+    }
+
+    render(<SubscribingAncestor />);
+    await waitFor(() => {
+      expect(screen.getByTestId('message-list')).toBeInTheDocument();
+    });
+
+    const rendersBefore = mocks.messageList.mock.calls.length;
+    act(() => {
+      for (let i = 1; i <= 25; i++) {
+        recordRawPacket(createPacket({ id: i, observation_id: i }));
+      }
+    });
+
+    expect(mocks.messageList.mock.calls.length).toBeGreaterThan(rendersBefore);
+  });
+});

frontend/src/test/rawPacketStore.test.tsx

@@ -8,8 +8,10 @@ import {
   getRawPackets,
   recordRawPacket,
   resetRawPacketStore,
+  seedRawPacketStore,
   useRawPackets,
 } from '../stores/rawPacketStore';
+import { MAX_RAW_PACKET_STATS_OBSERVATIONS } from '../utils/rawPacketStats';
 import type {
   Channel,
   Contact,
@@ -48,6 +50,28 @@ function createPacket(overrides: Partial<RawPacket> = {}): RawPacket {
   };
 }
 
+/** A stats session already holding `count` distinct observations, for trim-boundary tests. */
+function sessionAtObservationCap(count: number): RawPacketStatsSessionState {
+  return {
+    sessionStartedAt: 1700000000000,
+    totalObservedPackets: count,
+    trimmedObservationCount: 0,
+    observations: Array.from({ length: count }, (_, i) => ({
+      observationKey: `seeded-${i}`,
+      timestamp: 1700000000 + i,
+      payloadType: 'GROUP_TEXT',
+      routeType: 'Flood',
+      decrypted: false,
+      rssi: null,
+      snr: null,
+      sourceKey: null,
+      sourceLabel: null,
+      pathTokenCount: 0,
+      pathSignature: null,
+    })),
+  };
+}
+
 const channel: Channel = {
   key: '8B3387E9C5CDEA6AC9E5EDBAA115CD72',
   name: 'Public',
@@ -204,6 +228,69 @@ describe('rawPacketStore', () => {
     expect(screen.getByTestId('count').textContent).toBe('1');
   });
 
+  /**
+   * Asserted through a mounted subscriber rather than getRawPackets(), because the
+   * failure mode is specifically a missing emit(): the module state would be correct
+   * while every view kept rendering packets that no longer exist. On a quiet mesh the
+   * next packet — and so the next repaint — can be minutes away.
+   */
+  it('notifies subscribed views when the buffer is cleared on reconnect', () => {
+    function PacketCount() {
+      return <span data-testid="count">{useRawPackets().length}</span>;
+    }
+    act(() => recordRawPacket(createPacket({ id: 1, observation_id: 1 })));
+    render(<PacketCount />);
+    expect(screen.getByTestId('count').textContent).toBe('1');
+
+    act(() => clearRawPackets());
+
+    expect(screen.getByTestId('count').textContent).toBe('0');
+  });
+
+  it('does not hand out a snapshot the seeding caller can still mutate', () => {
+    const fixture = [createPacket({ id: 1, observation_id: 1 })];
+    seedRawPacketStore({ packets: fixture });
+
+    fixture.push(createPacket({ id: 2, observation_id: 2 }));
+
+    // Aliasing the caller's array would mutate the live snapshot in place. Because
+    // useSyncExternalStore compares snapshots with Object.is, the identity would not
+    // change and React would bail out of every later render for good.
+    expect(getRawPackets()).toHaveLength(1);
+  });
+
+  /**
+   * MAX_RAW_PACKET_STATS_OBSERVATIONS is 20k, far past what a test can reach by
+   * recording, so the trim boundary is exercised by seeding a session that already
+   * sits on it. Without this, both the `<=` comparison and the trimmed-count
+   * arithmetic can be broken without any test noticing.
+   */
+  it('retains exactly the observation cap before trimming starts', () => {
+    seedRawPacketStore({
+      statsSession: sessionAtObservationCap(MAX_RAW_PACKET_STATS_OBSERVATIONS - 1),
+    });
+
+    recordRawPacket(createPacket({ id: 999999, observation_id: 999999 }));
+
+    const session = getRawPacketStatsSession();
+    expect(session.observations).toHaveLength(MAX_RAW_PACKET_STATS_OBSERVATIONS);
+    expect(session.trimmedObservationCount).toBe(0);
+  });
+
+  it('trims the oldest observation once the cap is exceeded', () => {
+    seedRawPacketStore({
+      statsSession: sessionAtObservationCap(MAX_RAW_PACKET_STATS_OBSERVATIONS),
+    });
+
+    recordRawPacket(createPacket({ id: 999999, observation_id: 999999 }));
+
+    const session = getRawPacketStatsSession();
+    expect(session.observations).toHaveLength(MAX_RAW_PACKET_STATS_OBSERVATIONS);
+    expect(session.trimmedObservationCount).toBe(1);
+    // The evicted entry is the oldest, not the newest
+    expect(session.observations[0].observationKey).not.toBe('seeded-0');
+  });
+
   /**
    * The reason this store exists: overheard traffic used to live in App state, so every
    * packet re-rendered the whole message list. That cost scales with history length and