web: chat-log retention and vertical scroll fix

ACCEPTANCE.md

@@ -2341,3 +2341,92 @@ idempotent, `user_version`-gated), **FC-A2** (seed-then-delta — the warm delta
 contract is unchanged; only the backfill anchor moved), **PL-A1/PL-A2**
 (progressive load), and **B1**. No POST/GET/event contract change and no
 ingestor change, so **C2**, `CONTRACTS.md`, and the Python suite are unaffected.
+
+---
+
+## Bugfix: Chat-log entry retention, advert suppression, and chat vertical scroll
+
+Three independent chat-panel defects, all frontend-only (no API/DB/ingestor
+change, so the apex (I) and privacy (II) invariants are untouched):
+
+**(A1)** `rebuildNodeDerivedState` stored the *aggregated* snapshot arrays back
+into the raw accumulators (`allTelemetryEntries` / `allPositionEntries` /
+`allNeighbors`), which are also the merge targets for every refresh + backfill
+page. Re-aggregating an already-aggregated array is lossy (`aggregateSnapshots`
+clones with `{...snapshot}`, dropping the non-enumerable `snapshots` history, and
+merges oldest-last so the stalest reading's `rx_time`/`id` win), collapsing each
+node's history to `{stale-first, newest}` — so a telemetry/position Log entry
+appeared for one refresh tick and vanished on the next (no scrolling involved).
+The accumulators now stay **raw**; the aggregated forms are locals used only to
+enrich node records, so every packet keeps a stable, id-keyed Log entry.
+
+**(A2)** The advert-suppression claim key folded in `node_num` and required BOTH
+`node_id` and `node_num` to match. Specific events (telemetry/positions/
+neighbors) frequently carry only `node_id` (`node_num` is int|nil per CONTRACTS,
+commonly nil for MeshCore) while the node record carries a `node_num`, so the
+combined key failed to match and a redundant "Updated node info (advert)" line
+leaked alongside the specific entry (violating LV7/LV-A7). Suppression now keys on
+the canonical `!%08x` id alone (which `normaliseNodeId` derives from `node_num`
+when needed), so the id identifies a node across every event shape.
+
+**(B)** Every chat render force-scrolled the active panel to the bottom (in
+`setActiveTab`, plus a second `scrollActiveChatPanelToBottom` call), so a live
+update (40-80/hr in production) yanked the reader back to the bottom and made
+upward scrolling impossible. The prior LD-A2 fix preserved only the *horizontal*
+tab-list scroll. `renderChatTabs` now captures the active panel's vertical
+`scrollTop` before the subtree rebuild and restores it.
+
+### CL-A1 -- telemetry/position Log entries survive successive refreshes
+```bash
+( cd web && node --test public/assets/js/app/__tests__/main-log-snapshot-retention.test.js )
+```
+**Expected:** pass. After one node emits three telemetry packets across three
+refreshes, all three stay loaded (`getLoadedTelemetryCount() === 3`) and the
+rendered Log shows all three "Broadcasted telemetry" entries — the raw
+accumulator is no longer collapsed to a single per-node aggregate by the next
+tick's re-aggregation.
+
+### CL-A2 -- the advert is suppressed when a specific event omits `node_num`, and for encrypted-message hears
+```bash
+( cd web && node --test public/assets/js/app/__tests__/chat-log-tabs.test.js )
+```
+**Expected:** pass. When the node record carries a `node_num` but the telemetry/
+position rows carry only `node_id`, `buildChatTabModel(...).logEntries` still
+emits the telemetry and position entries and **no** redundant node-info (advert)
+entry. An id-less heard (no `node_id`, no derivable `node_num`) claims nothing and
+is never suppressed. An **encrypted message** (in either the `messages` or the
+`logOnlyMessages` feed) claims its sender's heard, so a node heard only via a
+`🔒 encrypted message on channel <id>` line shows **no** redundant
+`Updated node info (advert)` beneath it — the encrypted-message line is that
+heard's Log representation, mirroring how a decrypted message becomes a
+`(message)` node-info. Realises LV-A7 ("a position/telemetry/.../message
+suppresses a redundant advert line") across the `node_num`-nil and encrypted-
+message shapes that previously slipped through.
+
+### CL-A3 -- a passive chat re-render preserves the reader's vertical scroll
+```bash
+( cd web && node --test public/assets/js/app/__tests__/chat-tabs.test.js )
+```
+**Expected:** pass. `renderChatTabs` captures the active panel's `scrollTop`
+before the `replaceChildren` rebuild and restores it on the fresh panel: a reader
+scrolled up keeps their exact offset across a passive refresh, a bottom-pinned
+reader stays pinned to the new bottom (tail-follow), and an initial render (no
+prior panel) pins to the bottom. The per-render force-scroll (and the redundant
+`scrollActiveChatPanelToBottom`) are gone; panel scroll-to-bottom now fires only
+on an explicit tab switch (click/dropdown). Composes with the LD-A2 horizontal
+scroll preservation and the LV8 dropdown.
+
+### CL-R1 -- Regression: prior acceptance still holds
+```bash
+( cd web && npm test ) && ( cd web && bundle exec rspec )
+( . .venv/bin/activate && pytest -q tests/ )
+```
+**Expected:** all green. At risk and explicitly required to stay green: **LV-A7**
+(node-centric Log; the advert-suppression rule is strengthened, not weakened),
+**LD-A2** (horizontal tab scroll still preserved -- the new vertical-scroll
+preservation composes with it), **LV-A8** (the channel dropdown still jumps tabs),
+**VF-A6 / CR-A1** (an idle re-render still materialises 0 entries -- the scroll
+capture touches only already-built DOM), **CB-A1** (every bulk collection still
+backfills; the accumulators it merges into are raw, which is the shape the model
+already expects), and **B1** (all suites). Frontend-only: no POST/GET/event
+contract change, so `CONTRACTS.md` and the Python suite are unaffected.

web/public/assets/js/app/__tests__/chat-log-tabs.test.js

@@ -259,6 +259,71 @@ test('buildChatTabModel does not emit redundant node-info for telemetry/neighbor
   );
 });
 
+test('buildChatTabModel suppresses the advert when the specific event omits node_num (A2)', () => {
+  // Real-world shape: the node record carries a node_num, but the telemetry /
+  // position rows carry only node_id (node_num is int|nil per CONTRACTS, and is
+  // frequently nil — notably for MeshCore). The heard is the same node + ts, so
+  // the specific events must still claim it and suppress the redundant advert.
+  const model = buildChatTabModel({
+    nodes: [{ node_id: '!a', node_num: 10, last_heard: NOW - 5 }],
+    telemetry: [{ node_id: '!a', rx_time: NOW - 5, battery_level: 80 }],
+    positions: [{ node_id: '!a', rx_time: NOW - 5, latitude: 1, longitude: 2 }],
+    nowSeconds: NOW,
+    windowSeconds: WINDOW
+  });
+  // The specific events still render their own entries...
+  assert.ok(model.logEntries.some(e => e.type === CHAT_LOG_ENTRY_TYPES.TELEMETRY));
+  assert.ok(model.logEntries.some(e => e.type === CHAT_LOG_ENTRY_TYPES.POSITION));
+  // ...and no redundant "Updated node info (advert)" appears despite the
+  // node_num mismatch between the node record and the telemetry/position rows.
+  assert.equal(
+    model.logEntries.filter(e => e.type === CHAT_LOG_ENTRY_TYPES.NODE_INFO).length,
+    0
+  );
+});
+
+test('buildChatTabModel suppresses the advert when a node was heard via an encrypted message (A3, logOnly feed)', () => {
+  // Production shape: encrypted messages arrive via logOnlyMessages. The
+  // "🔒 encrypted message" line is the sender's Log representation for that heard,
+  // so the redundant "Updated node info (advert)" must be suppressed (LV7).
+  const model = buildChatTabModel({
+    nodes: [{ node_id: '!a', last_heard: NOW - 5 }],
+    logOnlyMessages: [{ id: 'e1', encrypted: true, from_id: '!a', channel: 78, rx_time: NOW - 5 }],
+    nowSeconds: NOW,
+    windowSeconds: WINDOW
+  });
+  assert.ok(model.logEntries.some(e => e.type === CHAT_LOG_ENTRY_TYPES.MESSAGE_ENCRYPTED));
+  assert.equal(model.logEntries.filter(e => e.type === CHAT_LOG_ENTRY_TYPES.NODE_INFO).length, 0);
+});
+
+test('buildChatTabModel suppresses the advert for an encrypted message in the messages feed (A3)', () => {
+  const model = buildChatTabModel({
+    nodes: [{ node_id: '!a', last_heard: NOW - 5 }],
+    messages: [{ id: 'e2', encrypted: true, from_id: '!a', channel: 78, rx_time: NOW - 5 }],
+    nowSeconds: NOW,
+    windowSeconds: WINDOW
+  });
+  assert.ok(model.logEntries.some(e => e.type === CHAT_LOG_ENTRY_TYPES.MESSAGE_ENCRYPTED));
+  assert.equal(model.logEntries.filter(e => e.type === CHAT_LOG_ENTRY_TYPES.NODE_INFO).length, 0);
+});
+
+test('buildChatTabModel keeps an id-less heard from claiming or being suppressed (A2 edge)', () => {
+  const model = buildChatTabModel({
+    // A node heard with neither node_id nor node_num: its advert still renders,
+    // and the unresolved (null) id is never used to suppress another heard.
+    nodes: [{ last_heard: NOW - 5 }],
+    // A telemetry row with no node_id/node_num: it renders its own entry but
+    // claims nothing — an id-less heard cannot stand in for any node's advert.
+    telemetry: [{ rx_time: NOW - 6, battery_level: 1 }],
+    nowSeconds: NOW,
+    windowSeconds: WINDOW
+  });
+  assert.ok(model.logEntries.some(e => e.type === CHAT_LOG_ENTRY_TYPES.TELEMETRY));
+  const adverts = model.logEntries.filter(e => e.type === CHAT_LOG_ENTRY_TYPES.NODE_INFO);
+  assert.equal(adverts.length, 1);
+  assert.equal(adverts[0].reason, 'advert');
+});
+
 test('buildChatTabModel attributes a message whose sender is absent from the nodes feed', () => {
   const model = buildChatTabModel({
     nodes: [],

web/public/assets/js/app/__tests__/chat-tabs.test.js

@@ -17,7 +17,7 @@
 import test from 'node:test';
 import assert from 'node:assert/strict';
 
-import { renderChatTabs } from '../chat-tabs.js';
+import { renderChatTabs, __test__ } from '../chat-tabs.js';
 
 class MockClassList {
   constructor() {
@@ -391,6 +391,114 @@ test('renderChatTabs does not scroll the active tab into view on a passive re-re
 });
 
 
+/** Return the single visible (active) panel within a rendered container. */
+function activePanel(container) {
+  const panelWrapper = container.children[1];
+  if (!panelWrapper || !Array.isArray(panelWrapper.children)) return null;
+  return panelWrapper.children.find(panel => panel && panel.hidden === false) || null;
+}
+
+test('renderChatTabs preserves the active panel vertical scroll across a passive re-render (bugfix B)', () => {
+  const document = createMockDocument();
+  const container = new MockElement('div');
+  const tabs = () => [
+    { id: 'log', label: 'Log', content: new MockElement('div') },
+    { id: 'c0', label: 'Default', content: new MockElement('div') }
+  ];
+  renderChatTabs({ document, container, tabs: tabs(), defaultActiveTabId: 'c0' });
+
+  // The user scrolls up to read history: the panel is no longer at the bottom.
+  const panel1 = activePanel(container);
+  panel1.scrollHeight = 1000;
+  panel1.clientHeight = 300;
+  panel1.scrollTop = 120; // 1000 - 120 - 300 = 580 px from the bottom → not pinned
+
+  // A passive live refresh (no tab switch) re-renders the same tabs.
+  renderChatTabs({ document, container, tabs: tabs(), defaultActiveTabId: 'c0' });
+  const panel2 = activePanel(container);
+
+  assert.notEqual(panel2, panel1); // a fresh panel element (full subtree rebuild)
+  assert.equal(
+    panel2.scrollTop,
+    120,
+    'a passive re-render must preserve the vertical scroll, not yank the reader to the bottom'
+  );
+});
+
+test('renderChatTabs keeps a bottom-pinned reader pinned across a passive re-render (tail-follow, bugfix B)', () => {
+  const document = createMockDocument();
+  const container = new MockElement('div');
+  const tabs = () => [
+    { id: 'log', label: 'Log', content: new MockElement('div') },
+    { id: 'c0', label: 'Default', content: new MockElement('div') }
+  ];
+  renderChatTabs({ document, container, tabs: tabs(), defaultActiveTabId: 'c0' });
+
+  // The user is at the bottom, reading the newest entries.
+  const panel1 = activePanel(container);
+  panel1.scrollHeight = 1000;
+  panel1.clientHeight = 300;
+  panel1.scrollTop = 700; // 1000 - 700 - 300 = 0 → pinned to the bottom
+
+  renderChatTabs({ document, container, tabs: tabs(), defaultActiveTabId: 'c0' });
+  const panel2 = activePanel(container);
+
+  // Still pinned: scrolled to the new bottom so freshly-arrived entries stay visible.
+  assert.equal(
+    panel2.scrollTop,
+    panel2.scrollHeight,
+    'a bottom-pinned reader must stay pinned to the new bottom (tail-follow)'
+  );
+});
+
+test('capturePreviousActivePanelScroll returns null when there is no prior panel (bugfix B)', () => {
+  const { capturePreviousActivePanelScroll } = __test__;
+  assert.equal(capturePreviousActivePanelScroll(null), null);
+  assert.equal(capturePreviousActivePanelScroll({}), null);
+  assert.equal(capturePreviousActivePanelScroll({ children: [] }), null);
+});
+
+test('capturePreviousActivePanelScroll returns null when every panel is hidden (bugfix B)', () => {
+  const { capturePreviousActivePanelScroll } = __test__;
+  const container = { children: [{}, { children: [null, { hidden: true }, { hidden: true }] }] };
+  assert.equal(capturePreviousActivePanelScroll(container), null);
+});
+
+test('capturePreviousActivePanelScroll reports the visible panel offset and pinned state (bugfix B)', () => {
+  const { capturePreviousActivePanelScroll, SCROLL_PIN_TOLERANCE_PX } = __test__;
+  assert.equal(SCROLL_PIN_TOLERANCE_PX, 4);
+
+  // A hidden panel precedes the visible one (mirrors the Log tab before a channel).
+  const pinnedPanel = { hidden: false, scrollTop: 700, scrollHeight: 1000, clientHeight: 300 };
+  const pinned = capturePreviousActivePanelScroll({ children: [{}, { children: [{ hidden: true }, pinnedPanel] }] });
+  assert.deepEqual(pinned, { top: 700, pinned: true }); // 1000 - 700 - 300 = 0 <= tol
+
+  const scrolledUp = { hidden: false, scrollTop: 100, scrollHeight: 1000, clientHeight: 300 };
+  const up = capturePreviousActivePanelScroll({ children: [{}, { children: [scrolledUp] }] });
+  assert.deepEqual(up, { top: 100, pinned: false }); // 600 px from the bottom > tol
+});
+
+test('applyActivePanelScroll pins, preserves, or no-ops on a missing panel (bugfix B)', () => {
+  const { applyActivePanelScroll } = __test__;
+  // No panel → no throw, nothing to do.
+  assert.doesNotThrow(() => applyActivePanelScroll(null, { top: 5, pinned: false }));
+
+  // No prior state (initial render) → pin to the bottom.
+  const initial = { scrollTop: 0, scrollHeight: 900 };
+  applyActivePanelScroll(initial, null);
+  assert.equal(initial.scrollTop, 900);
+
+  // Was pinned → pin to the new bottom (tail-follow).
+  const pinned = { scrollTop: 0, scrollHeight: 900 };
+  applyActivePanelScroll(pinned, { top: 42, pinned: true });
+  assert.equal(pinned.scrollTop, 900);
+
+  // Was scrolled up → restore the exact offset.
+  const preserved = { scrollTop: 0, scrollHeight: 900 };
+  applyActivePanelScroll(preserved, { top: 42, pinned: false });
+  assert.equal(preserved.scrollTop, 42);
+});
+
 test('renderChatTabs renders a channel dropdown selector that jumps to a tab (LV8)', () => {
   const document = createMockDocument();
   const container = new MockElement('div');