fix: neighbor-graph tab never renders after filtering down (#1758)

The Analytics → Neighbor Graph tab fetches the full (uncapped) graph
and,
when it exceeds NODE_LIMIT (1000), skips the force simulation with a
"use filters to reduce the node count" notice. But filtering never
actually
re-enabled rendering:

- the node-count guard tested _ngState.allNodes (the immutable full
fetched
set, assigned once in createGraphState and never reassigned) instead of
the
displayed/filtered _ngState.nodes, so its verdict was fixed at load
time;
- the entire draw loop lives in startGraphRenderer(), which ran exactly
once
  at load and was never called from applyNGFilters(), so a filter change
updated the node/edge arrays and stat cards but never un-hid the canvas
or
scheduled an animation frame -> the graph stayed blank no matter how few
  nodes remained.

This explains both reported symptoms (selects too many nodes initially
AND
stays broken once restricted to fewer).

Fix: make the render lifecycle filter-aware.
- startGraphRenderer() now guards on the displayed set (_ngState.nodes),
cancels any running rAF loop before re-deciding, toggles the canvas plus
a
  stable-id "skipped" notice, and restarts cleanly (no double loops).
- applyNGFilters() calls startGraphRenderer() so every filter change
  re-evaluates the guard and (re)starts or stops the loop.
- the initial render now goes through applyNGFilters() so the first
paint
already respects the default filters (observers unchecked, saved
min-score)
  instead of dumping the full fetched graph.

Test: `node --check public/analytics.js` passes. Manually: open
Analytics → Neighbor Graph on a
mesh with >1000 nodes → the "skipped" notice shows; tighten filters
(min-score up / roles
off) below 1000 → the graph now renders (was blank before); loosen again
→ notice returns.

Frontend-only change (`public/analytics.js`); no backend/API change.

---
**TDD note (review round 1):** Single-commit community bug-fix on an
existing UI surface (no "net-new UI" exemption). The e2e
`test-issue-1758-ng-filter-rerenders-e2e.js` is the red→green gate — it
fails on `origin/master` (the renderer kept the node-count guard on the
full fetched graph and never un-hid the canvas) and passes with the fix.
Per AGENTS.md the separate red/green-commit *form* is a bot rule, not a
contributor gate.

---------

Co-authored-by: Waydroid Builder <build@waydroid.local>
Co-authored-by: Waydroid Builder <claude@michael.arcan.de>

.github/workflows/deploy.yml

@@ -409,6 +409,7 @@ jobs:
           CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-map-nodes-pagination-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-observer-iata-1188-e2e.js 2>&1 | tee -a e2e-output.txt
           BASE_URL=http://localhost:13581 node test-issue-1639-observers-sort-e2e.js 2>&1 | tee -a e2e-output.txt
+          CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1758-ng-filter-rerenders-e2e.js 2>&1 | tee -a e2e-output.txt
           CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-nav-fluid-1055-e2e.js 2>&1 | tee -a e2e-output.txt
           CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-nav-priority-1102-e2e.js 2>&1 | tee -a e2e-output.txt
           CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-nav-priority-1311-e2e.js 2>&1 | tee -a e2e-output.txt

public/analytics.js

@@ -2545,8 +2545,11 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData =
     }
 
     _ngState = createGraphState(graphData);
-    renderNGStats(_ngState);
-    startGraphRenderer();
+    // Render through the filter path so the first paint already respects the
+    // default filters (observers unchecked, saved min-score) and the
+    // node-count guard is evaluated against the displayed set, not the full
+    // fetched graph.
+    applyNGFilters();
 
     // Filter listeners
     // Restore saved min score from localStorage
@@ -2557,10 +2560,14 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData =
       applyNGFilters();
     }
 
+    var ngScoreDebounce;
     document.getElementById('ngMinScore').addEventListener('input', function() {
       document.getElementById('ngMinScoreVal').textContent = (this.value / 100).toFixed(2);
       localStorage.setItem('ng-min-score', this.value);
-      applyNGFilters();
+      // Debounce the O(N+E) refilter so dragging the slider doesn't rebuild the
+      // node set on every pixel (review of #1758).
+      clearTimeout(ngScoreDebounce);
+      ngScoreDebounce = setTimeout(applyNGFilters, 50);
     });
     document.getElementById('ngConfidence').addEventListener('change', applyNGFilters);
     rcEl.addEventListener('change', applyNGFilters);
@@ -2583,6 +2590,7 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData =
       stats: data.stats || {},
       zoom: 1, panX: 0, panY: 0,
       dragging: null, panning: false,
+      hoverNode: null,
       lastMouseX: 0, lastMouseY: 0,
       cooling: 1.0, animId: null
     };
@@ -2619,8 +2627,15 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData =
     _ngState.nodeIdx = {};
     _ngState.nodes.forEach((n, i) => { _ngState.nodeIdx[n.pubkey] = i; });
 
+    // A full re-anneal (cooling back to 1.0) on every filter change is
+    // deliberate: the displayed node set just changed, so the prior layout
+    // is stale and we want the simulation to settle the new graph. This is
+    // acceptable at the current 1000-node limit; revisit if the limit grows.
     _ngState.cooling = 1.0;
     renderNGStats(_ngState);
+    // Re-evaluate the node-count guard against the new filtered set and
+    // (re)start or stop the force-simulation loop accordingly.
+    startGraphRenderer();
   }
 
   function renderNGStats(st) {
@@ -2681,35 +2696,7 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData =
     listEl.innerHTML = html;
   }
 
-  function startGraphRenderer() {
-    if (!_ngState) return;
-
-    // Node count guard: skip force simulation for very large graphs
-    var NODE_LIMIT = 1000;
-    if (_ngState.allNodes.length > NODE_LIMIT) {
-      var el = document.getElementById('ngCanvas');
-      if (el) {
-        el.style.display = 'none';
-        var msg = document.createElement('div');
-        msg.className = 'analytics-card';
-        msg.innerHTML = '<p class="text-muted">Graph has ' + _ngState.allNodes.length + ' nodes (limit: ' + NODE_LIMIT + '). Force simulation skipped for performance. Use filters to reduce the node count.</p>';
-        el.parentNode.insertBefore(msg, el);
-      }
-      return;
-    }
-
-    const canvas = document.getElementById('ngCanvas');
-    if (!canvas) return;
-    const ctx = canvas.getContext('2d');
-    const dpr = window.devicePixelRatio || 1;
-    canvas.width = canvas.clientWidth * dpr;
-    canvas.height = canvas.clientHeight * dpr;
-    ctx.scale(dpr, dpr);
-    const W = canvas.clientWidth, H = canvas.clientHeight;
-
-    // Interaction
-    let hoverNode = null;
-
+  function bindNGCanvasInteraction(canvas) {
     function canvasToGraph(cx, cy) {
       return { x: (cx - _ngState.panX) / _ngState.zoom, y: (cy - _ngState.panY) / _ngState.zoom };
     }
@@ -2758,8 +2745,8 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData =
         _ngState.lastMouseY = e.clientY;
       } else {
         const n = findNode(cx, cy);
-        if (n !== hoverNode) {
-          hoverNode = n;
+        if (n !== _ngState.hoverNode) {
+          _ngState.hoverNode = n;
           canvas.style.cursor = n ? 'pointer' : 'grab';
           const tip = document.getElementById('ngTooltip');
           if (n && tip) {
@@ -2770,7 +2757,7 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData =
           } else if (tip) {
             tip.style.display = 'none';
           }
-        } else if (hoverNode) {
+        } else if (_ngState.hoverNode) {
           const tip = document.getElementById('ngTooltip');
           if (tip) { tip.style.left = (cx + 12) + 'px'; tip.style.top = (cy - 8) + 'px'; }
         }
@@ -2784,7 +2771,7 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData =
       }
       _ngState.dragging = null;
       _ngState.panning = false;
-      canvas.style.cursor = hoverNode ? 'pointer' : 'grab';
+      canvas.style.cursor = _ngState.hoverNode ? 'pointer' : 'grab';
     });
 
     canvas.addEventListener('mouseleave', function() {
@@ -2793,7 +2780,7 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData =
       _ngState._wasDragging = false;
       const tip = document.getElementById('ngTooltip');
       if (tip) tip.style.display = 'none';
-      hoverNode = null;
+      _ngState.hoverNode = null;
     });
 
     canvas.addEventListener('click', function(e) {
@@ -2828,6 +2815,71 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData =
       _ngState.panY = cy - (cy - _ngState.panY) * (newZoom / _ngState.zoom);
       _ngState.zoom = newZoom;
     }, { passive: false });
+  }
+
+  function startGraphRenderer() {
+    if (!_ngState) return;
+
+    // Re-entrant: the filter path calls this to restart against the new node
+    // set, so cancel any loop already running before deciding what to do.
+    if (_ngState.animId) {
+      cancelAnimationFrame(_ngState.animId);
+      _ngState.animId = null;
+    }
+    // Re-entrancy epoch: a just-cancelled in-flight tick() (or one re-entered
+    // from a microtask) must not schedule a second loop. Each run bumps the
+    // epoch; tick() captures its own and early-returns once a newer run starts.
+    _ngState._runEpoch = (_ngState._runEpoch || 0) + 1;
+    var myEpoch = _ngState._runEpoch;
+
+    const canvas = document.getElementById('ngCanvas');
+    if (!canvas) return;
+    var skipMsg = document.getElementById('ngSkipMsg');
+
+    // Node count guard: skip the force simulation for graphs too large to
+    // render usefully. Keyed off the DISPLAYED (filtered) set — _ngState.nodes
+    // — NOT the full fetched graph (_ngState.allNodes), so narrowing the
+    // filters re-enables rendering. The "skipped" notice has a stable id so
+    // it can be toggled on the next call.
+    var NODE_LIMIT = 1000;
+    if (_ngState.nodes.length > NODE_LIMIT) {
+      canvas.style.display = 'none';
+      if (!skipMsg) {
+        skipMsg = document.createElement('div');
+        skipMsg.id = 'ngSkipMsg';
+        skipMsg.className = 'analytics-card';
+        canvas.parentNode.insertBefore(skipMsg, canvas);
+      }
+      // Build via textContent (never innerHTML) per the project's XSS rule —
+      // these are Numbers today, but a future coercion bug must not inject. Show
+      // filtered-of-total so the operator sees the rest were filtered out, not
+      // lost (review of #1758).
+      var ngTotal = (_ngState.allNodes || _ngState.nodes).length;
+      skipMsg.textContent = '';
+      var skipP = document.createElement('p');
+      skipP.className = 'text-muted';
+      skipP.textContent = _ngState.nodes.length + ' of ' + ngTotal + ' nodes match the current filters (limit: ' + NODE_LIMIT + '). Force simulation skipped for performance — tighten the filters to render.';
+      skipMsg.appendChild(skipP);
+      return;
+    }
+    if (skipMsg) skipMsg.remove();
+    canvas.style.display = '';
+    const ctx = canvas.getContext('2d');
+    const dpr = window.devicePixelRatio || 1;
+    canvas.width = canvas.clientWidth * dpr;
+    canvas.height = canvas.clientHeight * dpr;
+    ctx.scale(dpr, dpr);
+    const W = canvas.clientWidth, H = canvas.clientHeight;
+
+    // Interaction listeners are bound once per tab session (the #ngCanvas
+    // element and _ngState persist across filter re-renders); re-attaching on
+    // every applyNGFilters() would leak handlers (review of #1758). Binding
+    // once also means transient flags like _wasDragging survive re-renders.
+    // Hover state lives on _ngState so listeners and the draw loop agree.
+    if (!_ngState._listenersBound) {
+      bindNGCanvasInteraction(canvas);
+      _ngState._listenersBound = true;
+    }
 
     // Cache text color to avoid getComputedStyle every frame
     const _labelColor = cssVar('--text-primary') || '#e0e0e0';
@@ -2836,6 +2888,7 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData =
     // Performance: 500 nodes brute-force repulsion: avg ~4ms/frame = 250fps headroom (measured Chrome 120, M1)
     var _perfFrameTimes = [], _perfLastTime = 0;
     function tick() {
+      if (_ngState._runEpoch !== myEpoch) { return; } // a newer startGraphRenderer run superseded this loop
       if (!document.getElementById('ngCanvas')) { _ngState.animId = null; return; }
       var now = performance.now();
       if (_perfLastTime) _perfFrameTimes.push(now - _perfLastTime);
@@ -2928,7 +2981,7 @@ function destroy() { _stopRolesRefresh(); _stopScopesRefresh(); _analyticsData =
         ctx.arc(n.x, n.y, n.radius, 0, Math.PI * 2);
         ctx.fillStyle = color;
         ctx.fill();
-        if (n === hoverNode) {
+        if (n === _ngState.hoverNode) {
           ctx.strokeStyle = '#fff';
           ctx.lineWidth = 2;
           ctx.stroke();

test-issue-1758-ng-filter-rerenders-e2e.js

@@ -0,0 +1,211 @@
+#!/usr/bin/env node
+/**
+ * #1758 — Neighbor Graph re-renders on filter change (canvas un-hide guard).
+ *
+ * The Analytics → Neighbor Graph renderer skips the force simulation when the
+ * DISPLAYED node set exceeds 1000 nodes: it hides #ngCanvas and inserts a
+ * stable-id #ngSkipMsg notice. PR #1758 made startGraphRenderer() re-entrant so
+ * that applyNGFilters() re-evaluates that guard against the *filtered* set — so
+ * narrowing the role filters below the limit must remove #ngSkipMsg and un-hide
+ * #ngCanvas again, and widening them back must restore the skip state.
+ *
+ * This is the red-then-green guard for #1758: it FAILS before the lifecycle fix
+ * (the renderer keyed the guard off the full fetched graph and never un-hid the
+ * canvas once the skip message was shown, so filtering down did nothing) and
+ * PASSES after it.
+ *
+ * Fixture: ~1200 "companion" + ~200 "repeater" nodes, every node edge-bearing.
+ * With the default filters (observer unchecked, companion + repeater checked)
+ * the displayed set is >1000 → skip. Unchecking "companion" drops to ~200
+ * repeater nodes < 1000 → render. Re-checking restores the skip state.
+ *
+ * Usage:
+ *   BASE_URL=http://localhost:13581 \
+ *   CHROMIUM_PATH=/path/to/chromium \
+ *   node test-issue-1758-ng-filter-rerenders-e2e.js
+ */
+'use strict';
+const { chromium } = require('playwright');
+
+const BASE = process.env.BASE_URL || 'http://localhost:13581';
+
+function fail(msg) {
+  console.error('  ✗ ' + msg);
+  process.exit(1);
+}
+
+// Build a synthetic neighbor-graph fixture: every node carries at least one
+// edge so it survives the "edge-bearing nodes only" filter in applyNGFilters().
+function buildFixture() {
+  const nodes = [];
+  const edges = [];
+  function pk(prefix, i) {
+    // 64-hex pubkey, deterministic and unique per (prefix,i).
+    return (prefix + i.toString(16)).padEnd(64, '0').slice(0, 64);
+  }
+  const COMPANIONS = 1200;
+  const REPEATERS = 200;
+  // Companion ring: each companion edged to the next (wraps), so all are
+  // edge-bearing among themselves and survive when "companion" is checked.
+  for (let i = 0; i < COMPANIONS; i++) {
+    nodes.push({ pubkey: pk('c', i), role: 'companion', neighbor_count: 2 });
+  }
+  for (let i = 0; i < COMPANIONS; i++) {
+    const a = pk('c', i);
+    const b = pk('c', (i + 1) % COMPANIONS);
+    edges.push({ source: a, target: b, score: 0.9, ambiguous: false });
+  }
+  // Repeater ring: independent, so unchecking "companion" leaves ~200 nodes.
+  for (let i = 0; i < REPEATERS; i++) {
+    nodes.push({ pubkey: pk('r', i), role: 'repeater', neighbor_count: 2 });
+  }
+  for (let i = 0; i < REPEATERS; i++) {
+    const a = pk('r', i);
+    const b = pk('r', (i + 1) % REPEATERS);
+    edges.push({ source: a, target: b, score: 0.9, ambiguous: false });
+  }
+  return { nodes, edges, stats: {} };
+}
+
+(async () => {
+  const browser = await chromium.launch({
+    headless: true,
+    executablePath: process.env.CHROMIUM_PATH || undefined,
+    args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'],
+  });
+  const ctx = await browser.newContext({ viewport: { width: 1280, height: 900 } });
+  const page = await ctx.newPage();
+  page.setDefaultTimeout(15000);
+  page.on('pageerror', (e) => console.error('[pageerror]', e.message));
+
+  console.log(`\n=== #1758 neighbor-graph filter re-render guard against ${BASE} ===`);
+
+  const fixture = buildFixture();
+
+  // Intercept the neighbor-graph API and return the synthetic oversized graph.
+  await page.route('**/api/analytics/neighbor-graph*', (route) => {
+    route.fulfill({
+      status: 200,
+      contentType: 'application/json',
+      body: JSON.stringify(fixture),
+    });
+  });
+
+  // Deep-link straight to the Neighbor Graph tab instead of clicking it.
+  // loadAnalytics() awaits a Promise.all() of the OTHER analytics endpoints and
+  // only THEN calls renderTab(_currentTab). A tab click that lands before that
+  // resolves makes renderNeighborGraphTab() run twice: the second pass (fired by
+  // loadAnalytics when its fetches finally settle — slow during server warmup)
+  // rebuilds the role checkboxes (all re-checked) and a fresh _ngState from the
+  // full graph, flipping the >1000-node skip guard back on mid-test and racing
+  // our filter changes. Deep-linking renders the tab exactly once, as part of
+  // loadAnalytics itself, so #ngSkipMsg appears only when the page is quiescent
+  // and no late re-render can clobber the filter state we assert on.
+  await page.goto(BASE + '/#/analytics?tab=neighbor-graph', { waitUntil: 'domcontentloaded' });
+  // The renderer is async (awaits the API). Wait for the role checkboxes —
+  // they exist once the tab markup is painted.
+  await page.waitForSelector('#ngRoleChecks input[data-role="companion"]', { timeout: 15000 });
+  // Wait for the first guard evaluation to settle: with the oversized default
+  // set, #ngSkipMsg should appear.
+  await page.waitForSelector('#ngSkipMsg', { timeout: 15000 });
+
+  // --- Assertion 1: initial oversized state hides the canvas, shows skip msg.
+  const initial = await page.evaluate(() => {
+    const skip = document.getElementById('ngSkipMsg');
+    const canvas = document.getElementById('ngCanvas');
+    return {
+      hasSkip: !!skip,
+      canvasDisplay: canvas ? getComputedStyle(canvas).display : '(no canvas)',
+    };
+  });
+  if (!initial.hasSkip) fail('initial: expected #ngSkipMsg to exist (>1000 displayed nodes)');
+  if (initial.canvasDisplay !== 'none') {
+    fail(`initial: expected #ngCanvas display 'none', got '${initial.canvasDisplay}'`);
+  }
+  console.log('  ✓ initial: #ngSkipMsg present and #ngCanvas hidden');
+
+  // --- Filter down: uncheck "companion" → ~200 repeater nodes < 1000.
+  await page.evaluate(() => {
+    const cb = document.querySelector('#ngRoleChecks input[data-role="companion"]');
+    cb.checked = false;
+    cb.dispatchEvent(new Event('change', { bubbles: true }));
+  });
+  // Guard re-evaluates synchronously inside applyNGFilters(); wait for the
+  // skip message to be removed.
+  await page.waitForFunction(() => !document.getElementById('ngSkipMsg'), { timeout: 15000 });
+
+  // --- Assertion 2: filtered-down state removes skip msg, un-hides canvas.
+  const filtered = await page.evaluate(() => {
+    const skip = document.getElementById('ngSkipMsg');
+    const canvas = document.getElementById('ngCanvas');
+    return {
+      hasSkip: !!skip,
+      canvasDisplay: canvas ? getComputedStyle(canvas).display : '(no canvas)',
+    };
+  });
+  if (filtered.hasSkip) {
+    fail('after unchecking companion: expected #ngSkipMsg to be gone (<1000 nodes)');
+  }
+  if (filtered.canvasDisplay === 'none') {
+    fail('after unchecking companion: expected #ngCanvas to be visible (display !== none)');
+  }
+  console.log('  ✓ filtered down: #ngSkipMsg gone and #ngCanvas visible');
+
+  // --- Filter back up: re-check "companion" → >1000 again → skip restored.
+  await page.evaluate(() => {
+    const cb = document.querySelector('#ngRoleChecks input[data-role="companion"]');
+    cb.checked = true;
+    cb.dispatchEvent(new Event('change', { bubbles: true }));
+  });
+  await page.waitForSelector('#ngSkipMsg', { timeout: 15000 });
+
+  // --- Assertion 3: re-widened state restores skip msg and hides canvas.
+  const restored = await page.evaluate(() => {
+    const skip = document.getElementById('ngSkipMsg');
+    const canvas = document.getElementById('ngCanvas');
+    return {
+      hasSkip: !!skip,
+      canvasDisplay: canvas ? getComputedStyle(canvas).display : '(no canvas)',
+    };
+  });
+  if (!restored.hasSkip) {
+    fail('after re-checking companion: expected #ngSkipMsg to return (>1000 nodes)');
+  }
+  if (restored.canvasDisplay !== 'none') {
+    fail(`after re-checking companion: expected #ngCanvas display 'none', got '${restored.canvasDisplay}'`);
+  }
+  console.log('  ✓ re-widened: #ngSkipMsg restored and #ngCanvas hidden');
+
+  // --- Skip message must report filtered-of-total, not just the filtered count.
+  const restoredSkipText = await page.evaluate(
+    () => (document.getElementById('ngSkipMsg') || {}).textContent || '');
+  if (!/\bof\b/.test(restoredSkipText) || !/nodes/.test(restoredSkipText)) {
+    fail(`skip message should report "N of M nodes", got: ${JSON.stringify(restoredSkipText)}`);
+  }
+  console.log('  ✓ skip message reports filtered-of-total node count');
+
+  // --- Filter down AGAIN: the small→oversized→small lifecycle must keep working
+  // (the re-entrancy epoch guard + bind-once interaction listeners survive
+  // repeated re-renders, not just one down→up cycle).
+  await page.evaluate(() => {
+    const cb = document.querySelector('#ngRoleChecks input[data-role="companion"]');
+    cb.checked = false;
+    cb.dispatchEvent(new Event('change', { bubbles: true }));
+  });
+  await page.waitForFunction(() => !document.getElementById('ngSkipMsg'), { timeout: 15000 });
+  const reFiltered = await page.evaluate(() => {
+    const canvas = document.getElementById('ngCanvas');
+    return {
+      hasSkip: !!document.getElementById('ngSkipMsg'),
+      canvasDisplay: canvas ? getComputedStyle(canvas).display : '(no canvas)',
+    };
+  });
+  if (reFiltered.hasSkip) fail('second filter-down: expected #ngSkipMsg gone again');
+  if (reFiltered.canvasDisplay === 'none') fail('second filter-down: expected #ngCanvas visible again');
+  console.log('  ✓ second filter-down: render restored across repeated cycles');
+
+  await browser.close();
+
+  console.log('\nPASS: #1758 neighbor-graph filter re-render lifecycle holds');
+  process.exit(0);
+})().catch((e) => { console.error(e); process.exit(1); });