diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 42733f05..e41ae498 100644 --- a/.github/workflows/deploy.yml +++ b/.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 diff --git a/public/analytics.js b/public/analytics.js index e7b0ad52..1130fb5c 100644 --- a/public/analytics.js +++ b/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 = '
Graph has ' + _ngState.allNodes.length + ' nodes (limit: ' + NODE_LIMIT + '). Force simulation skipped for performance. Use filters to reduce the node count.
'; - 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(); diff --git a/test-issue-1758-ng-filter-rerenders-e2e.js b/test-issue-1758-ng-filter-rerenders-e2e.js new file mode 100644 index 00000000..1a75fdb4 --- /dev/null +++ b/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); });