From 951d2900f75ce636a1647e9c7181f66f07d9c62e Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 07:46:48 +0000 Subject: [PATCH 01/24] test(e2e): count hop elements, not chain children, in the #1153 check (#165) The full-chain assertion derived the hop count from chain.children.length minus buttons. Once a hop and its badge are wrapped together (the ambiguity UI ported for #165), a wrapped hop is one child holding a pill and a button, so the count drops even though every hop is rendered. Count the hop pills themselves (links and spans, excluding the wrapper and anything inside a badge button). Green on master: 11 passed against a local fixture server. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- test-issue-1146-path-link-contrast-e2e.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test-issue-1146-path-link-contrast-e2e.js b/test-issue-1146-path-link-contrast-e2e.js index 43f24cd8c..3e236eec5 100644 --- a/test-issue-1146-path-link-contrast-e2e.js +++ b/test-issue-1146-path-link-contrast-e2e.js @@ -204,7 +204,11 @@ async function effectiveBgFor(page, selector) { warning: !!chain.querySelector('.hop-unreliable-btn'), conflict: !!chain.querySelector('.hop-conflict-btn'), siblingAmbiguous: sibling.classList.contains('hop-ambiguous'), - hopCount: chain.children.length - chain.querySelectorAll('button').length, + // Count the hop elements themselves, not the chain's children: a hop + // with a badge is wrapped in a .hop-group together with that badge + // (#165), so child count says nothing about how many hops rendered. + hopCount: Array.from(chain.querySelectorAll('a, span')) + .filter(el => !el.classList.contains('hop-group') && !el.closest('button')).length, }; }, { selector, targetHopKey, siblingKey }); assert(result.hopCount === 18, 'Expected all 18 hops in the rendered chain'); From 3c9976d892dc56e6324b6b4151943976ba4e15ef Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 08:11:50 +0000 Subject: [PATCH 02/24] feat(packets): port hop ambiguity UI, resolved per observer (#165) Port of upstream PR 2099 (Kpa-clawbot/CoreScope, head 3532cede): - resolveHops() resolves with the observer that heard the packet and its position, and caches under hop:observer; resolveHopsForPackets() groups multi-packet call sites by observer. - HopDisplay wraps a pill and its badge in .hop-group, and opts.badge === false drops the per-hop badge for the packets list, which shows one .hop-path-warn summary per path instead. - The new styles use CSS variables only (no hex fallbacks). Fork-specific adjustments: - The unit test lives in the repo root as test-issue-165-hop-ambiguity-badge.js and is registered in test-all.sh and the deploy.yml JS unit step. - HopDisplay.renderHop takes opts.className, and nodes.js uses it for hop-current. nodes.js patched the first class="" of the returned HTML, which is the .hop-group wrapper once a hop has a badge, so the #1153 selected-hop marker was lost (4 E2E failures in test-issue-1146-path-link-contrast-e2e.js before this change). - The incremental (WS/poll) hop resolution is extracted unchanged into resolveIncomingHops(), and the resolve helpers are exposed on _packetsTestAPI for vm-harness tests. The defects listed in #165 (null coordinates, client overriding the server's resolved_path, the incremental path, an unbounded cache) are carried over as-is here and are fixed test-first in the following commits. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- .github/workflows/deploy.yml | 1 + public/hop-display.js | 22 ++- public/nodes.js | 3 +- public/packets.js | 178 +++++++++++++------ public/style.css | 14 ++ test-all.sh | 1 + test-issue-165-hop-ambiguity-badge.js | 239 ++++++++++++++++++++++++++ 7 files changed, 395 insertions(+), 63 deletions(-) create mode 100644 test-issue-165-hop-ambiguity-badge.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 4472efec9..12f21a612 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -214,6 +214,7 @@ jobs: node test-issue-1697-mqtt-mobile-e2e.js node test-warmup-banner.js node test-issue-1633-hide-1byte-hops.js + node test-issue-165-hop-ambiguity-badge.js node test-issue-1668-m4-per-route.js node test-a11y-axe-1668-selftest.js node test-a11y-1716-rf-range-btn-active.js diff --git a/public/hop-display.js b/public/hop-display.js index 1f7751943..68b5aed51 100644 --- a/public/hop-display.js +++ b/public/hop-display.js @@ -87,7 +87,9 @@ window.HopDisplay = (function() { const unreliableBadge = unreliable ? ' ' : ''; - const warnBadge = conflictBadge + unreliableBadge; + // #165 (port of upstream 2099) — opts.badge === false renders the name without its badge, for the + // packets list, which summarises per path instead of per hop. + const warnBadge = opts.badge === false ? '' : (conflictBadge + unreliableBadge); const cls = [ 'hop', @@ -95,12 +97,22 @@ window.HopDisplay = (function() { ambiguous ? 'hop-ambiguous' : '', unreliable ? 'hop-unreliable' : '', globalFallback ? 'hop-global-fallback' : '', + // #165 — callers mark the pill itself (e.g. nodes.js 'hop-current'). + // Patching the first class="" of the returned HTML would hit the + // .hop-group wrapper whenever the hop carries a badge. + opts.className || '', ].filter(Boolean).join(' '); - if (opts.link !== false) { - return `${display}${warnBadge}`; - } - return `${display}${warnBadge}`; + const pill = opts.link !== false + ? `${display}` + : `${display}`; + + // #165 (port of upstream 2099) — a badge is a sibling of its pill, so in "A [8] → B [6]" nothing + // said which name the 8 belonged to. Wrapping the pair keeps them together + // and puts the separator clearly outside. Only when there is a badge: + // wrapping every hop would change the layout of every path for nothing. + if (!warnBadge) return pill; + return `${pill}${warnBadge}`; } /** diff --git a/public/nodes.js b/public/nodes.js index c91de427e..9d4bddbab 100644 --- a/public/nodes.js +++ b/public/nodes.js @@ -1094,8 +1094,7 @@ const isThis = h.pubkey && h.pubkey.toLowerCase() === currentPubkey; if (window.HopDisplay) { const entry = { name: h.name, pubkey: h.pubkey, ambiguous: h.ambiguous, conflicts: h.conflicts, totalGlobal: h.totalGlobal, totalRegional: h.totalRegional, globalFallback: h.globalFallback, unreliable: h.unreliable }; - const html = HopDisplay.renderHop(h.prefix, entry); - return isThis ? html.replace('class="', 'class="hop-current ') : html; + return HopDisplay.renderHop(h.prefix, entry, isThis ? { className: 'hop-current' } : undefined); } const name = escapeHtml(h.name || h.prefix); const link = h.pubkey ? `${name}` : `${name}`; diff --git a/public/packets.js b/public/packets.js index aa86bb18e..402d046d2 100644 --- a/public/packets.js +++ b/public/packets.js @@ -978,10 +978,16 @@ api('/observers', { ttl: 60000 }), api('/iata-coords', { ttl: 300000 }).catch(() => ({ coords: {} })), ]); + const obsList = obsData.observers || obsData || []; HopResolver.init(nodeData.nodes || [], { - observers: obsData.observers || obsData || [], + observers: obsList, iataCoords: coordData.coords || {}, }); + // #165 (upstream 2099) — seed the lookup resolveHops() needs for the anchor, in case a + // render gets here before loadObservers() has run. + if (!observerMap || !observerMap.size) { + observerMap = new Map(obsList.map(o => [o.id, o])); + } } catch (e) { // Non-fatal: hops will render as unresolved hex prefixes until a later // call succeeds. Log so a paginated /api/nodes failure isn't silent. @@ -991,14 +997,58 @@ } // Resolve hop hex prefixes to node names (cached, client-side) - async function resolveHops(hops) { - const unknown = hops.filter(h => !(h in hopNameCache)); - if (unknown.length) { - await ensureHopResolver(); - const resolved = HopResolver.resolve(unknown); - Object.assign(hopNameCache, resolved || {}); - // Cache misses as null so we don't re-query - unknown.forEach(h => { if (!(h in hopNameCache)) hopNameCache[h] = null; }); + // #165 (upstream 2099) — the cache key carries the observer, because an ambiguous hop + // resolves differently depending on who heard it. renderHop() has always + // looked for this key; nothing ever wrote it. + // #165 (upstream 2099) — the observer's own position, used as the anchor at the receiving + // end of the path. The IATA route is dead weight: measured on the live + // deployment, none of the 42 observers has its code in /api/iata-coords, so + // nodeInRegion() always returns null. lat/lon is reported directly and works. + function observerPosition(observerId) { + const o = observerId && observerMap ? observerMap.get(observerId) : null; + const lat = o && Number.isFinite(Number(o.lat)) ? Number(o.lat) : null; + const lon = o && Number.isFinite(Number(o.lon)) ? Number(o.lon) : null; + return (lat === null || lon === null) ? [null, null] : [lat, lon]; + } + + function hopCacheKey(h, observerId) { + return observerId ? h + ':' + observerId : h; + } + + // #165 (upstream 2099) — resolve WITH the observer. Every 1-byte prefix on the network is + // shared by several nodes, and the observer is what lets HopResolver filter + // candidates by region and report the rest as conflicts. Called with the hops + // alone, it returns the first candidate with no signal that it guessed, which + // is how a repeater 126 km outside the observer's region ended up displayed + // as a certainty. + async function resolveHops(hops, observerId) { + const unknown = hops.filter(h => !(hopCacheKey(h, observerId) in hopNameCache)); + if (!unknown.length) return; + await ensureHopResolver(); + const [obsLat, obsLon] = observerPosition(observerId); + const resolved = HopResolver.resolve(unknown, null, null, obsLat, obsLon, observerId) || {}; + for (const h of unknown) { + const entry = resolved[h] || null; + hopNameCache[hopCacheKey(h, observerId)] = entry; + // Bare key as a fallback for any render that has no observer in hand. + if (!(h in hopNameCache)) hopNameCache[h] = entry; + } + } + + // Resolve every hop of every packet, grouped by the observer that heard it, + // so each group gets its own regional filtering. + async function resolveHopsForPackets(packets) { + const groups = new Map(); + for (const p of packets || []) { + const obs = (p && p.observer_id) ? String(p.observer_id) : ''; + let set = groups.get(obs); + if (!set) { set = new Set(); groups.set(obs, set); } + try { getParsedPath(p).forEach(h => set.add(h)); } catch {} + } + // ensureHopResolver() is idempotent and awaited once inside resolveHops; + // the resolve itself is local computation, so the loop costs no requests. + for (const [obs, set] of groups) { + if (set.size) await resolveHops([...set], obs || undefined); } } @@ -1025,14 +1075,34 @@ } } - function renderHop(h, observerId) { + // Incremental path (WS/poll): pre-populate from server-side resolved_path, + // then fall back to the client resolver for the remaining hops. + function resolveIncomingHops(pkts) { + const newHops = new Set(); + for (const p of pkts) { + const rp = getResolvedPath(p); + const hops = getParsedPath(p); + if (rp && rp.length === hops.length && window.HopResolver && HopResolver.ready()) { + const resolved = HopResolver.resolveFromServer(hops, rp); + Object.assign(hopNameCache, resolved); + } + try { hops.forEach(h => { if (!(h in hopNameCache)) newHops.add(h); }); } catch {} + } + return newHops.size ? resolveHopsForPackets(pkts) : Promise.resolve(); + } + + function renderHop(h, observerId, opts) { // Use per-packet cache key if observer context available (ambiguous hops differ by region) - const cacheKey = observerId ? h + ':' + observerId : h; + const cacheKey = hopCacheKey(h, observerId); const entry = hopNameCache[cacheKey] || hopNameCache[h]; - return HopDisplay.renderHop(h, entry, { hexMode: showHexHashes }); + return HopDisplay.renderHop(h, entry, Object.assign({ hexMode: showHexHashes }, opts || {})); } - function renderPath(hops, observerId) { + // #165 (upstream 2099) — opts.summary renders the list form: names without a badge on every + // hop, and one indicator for the whole path. A row with five 1-byte hops was + // five warning triangles, which is noise in a table; the detail pane keeps + // the per-hop badges, because that is where the question gets answered. + function renderPath(hops, observerId, opts) { if (!hops || !hops.length) return '—'; // #1633 — render-time filter (default OFF). Applies at every consumer // because every site funnels through this function (group header, child @@ -1041,7 +1111,22 @@ ? window.MC_filterPathHops(hops) : hops; if (!filtered.length) return '— (1-byte filtered)'; - return filtered.map(h => renderHop(h, observerId)).join('→'); + const summary = !!(opts && opts.summary); + const body = filtered + .map(h => renderHop(h, observerId, summary ? { badge: false } : null)) + .join('→'); + if (!summary) return body; + + let uncertain = 0; + for (const h of filtered) { + const entry = hopNameCache[hopCacheKey(h, observerId)] || hopNameCache[h]; + if (entry && entry.ambiguous) uncertain++; + } + if (!uncertain) return body; + const label = uncertain + ' of ' + filtered.length + ' hops have more than one candidate'; + return body + ' ' + + uncertain + ''; } let directPacketId = null; @@ -1249,9 +1334,7 @@ panel.appendChild(content); const pkt = data.packet; try { - const hops = getParsedPath(pkt); - const newHops = hops.filter(h => !(h in hopNameCache)); - if (newHops.length) await resolveHops(newHops); + await resolveHopsForPackets([pkt]); } catch {} await renderDetail(content, data); initPanelResize(); @@ -1296,18 +1379,7 @@ if (!filtered.length) return; // Resolve any new hops, then update and re-render - // Pre-populate from server-side resolved_path, then fall back for remaining - const newHops = new Set(); - for (const p of filtered) { - const rp = getResolvedPath(p); - const hops = getParsedPath(p); - if (rp && rp.length === hops.length && window.HopResolver && HopResolver.ready()) { - const resolved = HopResolver.resolveFromServer(hops, rp); - Object.assign(hopNameCache, resolved); - } - try { hops.forEach(h => { if (!(h in hopNameCache)) newHops.add(h); }); } catch {} - } - (newHops.size ? resolveHops([...newHops]) : Promise.resolve()).then(() => { + resolveIncomingHops(filtered).then(() => { if (groupByHash) { // Update existing groups or create new ones for (const p of filtered) { @@ -1508,11 +1580,7 @@ const hopJob = (async () => { try { await cacheResolvedPaths(packets); - const allHops = new Set(); - for (const p of packets) { - try { getParsedPath(p).forEach(h => allHops.add(h)); } catch {} - } - if (allHops.size) await resolveHops([...allHops]); + await resolveHopsForPackets(packets); // Re-render rows so resolved hop names replace hex prefixes. if (filtersBuilt) renderTableRows(); } catch (e) { @@ -2048,11 +2116,7 @@ if (p._children) sortGroupChildren(p); } // Resolve any new hops from updated header paths - const newHops = new Set(); - for (const p of packets) { - try { getParsedPath(p).forEach(h => { if (!(h in hopNameCache)) newHops.add(h); }); } catch {} - } - if (newHops.size) await resolveHops([...newHops]); + await resolveHopsForPackets(packets); renderTableRows(); }); @@ -2295,7 +2359,7 @@ const groupRegion = headerObserverId ? (observerMap.get(headerObserverId)?.iata || '') : ''; let groupPath = []; try { groupPath = JSON.parse(headerPathJson || '[]'); } catch {} - const groupPathStr = renderPath(groupPath, headerObserverId); + const groupPathStr = renderPath(groupPath, headerObserverId, { summary: true }); const groupTypeName = payloadTypeName(p.payload_type); const groupTypeClass = payloadTypeColor(p.payload_type); const groupSize = p.raw_hex ? Math.floor(p.raw_hex.length / 2) : 0; @@ -2346,7 +2410,7 @@ : (childPath.length > 0 ? childPath[0].length / 2 : 0)); const _cHashSizeTitle = _cIsTrace ? ' title="TRACE path bytes are SNR readings, not hash prefixes — see sidebar decoder for actual hop count"' : ''; const childRegion = c.observer_id ? (observerMap.get(c.observer_id)?.iata || '') : ''; - const childPathStr = renderPath(childPath, c.observer_id); + const childPathStr = renderPath(childPath, c.observer_id, { summary: true }); const _childHashStripe = _hashStripeStyle(c.hash || p.hash); html += ` ${childRegion ? `${childRegion}` : '—'} @@ -2381,7 +2445,7 @@ const _flatIsTrace = p.payload_type === 9; const hashBytes = _flatIsTrace ? '—' : (((parseInt(p.raw_hex?.slice(_flatPlOff * 2, _flatPlOff * 2 + 2), 16) || 0) >> 6) + 1); const _flatHashSizeTitle = _flatIsTrace ? ' title="TRACE path bytes are SNR readings, not hash prefixes — see sidebar decoder for actual hop count"' : ''; - const pathStr = renderPath(pathHops, p.observer_id); + const pathStr = renderPath(pathHops, p.observer_id, { summary: true }); const detail = getDetailPreview(decoded); const _flatHashStripe = _hashStripeStyle(p.hash); const _flatStyle = _flatHashStripe + _chanStyle; @@ -3207,9 +3271,7 @@ // Resolve path hops for detail view const pkt = data.packet; try { - const hops = getParsedPath(pkt); - const newHops = hops.filter(h => !(h in hopNameCache)); - if (newHops.length) await resolveHops(newHops); + await resolveHopsForPackets([pkt]); } catch {} panel.innerHTML = isMobileNow ? '' : (useSlideOver ? '' : ('
' + PANEL_CLOSE_HTML)); const content = document.createElement('div'); @@ -3344,7 +3406,11 @@ resolved = HopResolver.resolveFromServer(pathHops, serverResolved); } else { await ensureHopResolver(); - resolved = HopResolver.resolve(pathHops); + // #165 (upstream 2099) — with the observer: the cache write below stores this under + // the per-observer key, so it has to have been resolved for that + // observer or the key promises something the value is not. + const [dLat, dLon] = observerPosition(pkt.observer_id); + resolved = HopResolver.resolve(pathHops, null, null, dLat, dLon, pkt.observer_id); } if (resolved) { for (const [k, v] of Object.entries(resolved)) { @@ -4083,12 +4149,7 @@ } // Resolve hops from children: prefer server-side resolved_path await cacheResolvedPaths(group?._children || []); - const childHops = new Set(); - for (const c of (group?._children || [])) { - try { getParsedPath(c).forEach(h => childHops.add(h)); } catch {} - } - const newHops = [...childHops].filter(h => !(h in hopNameCache)); - if (newHops.length) await resolveHops(newHops); + await resolveHopsForPackets(group?._children || []); expandedHashes.add(hash); renderTableRows(); // Also open detail panel — no extra fetch needed @@ -4150,6 +4211,14 @@ fieldRow, renderTimestampCell, renderPath, + observerPosition, + resolveHops, + resolveHopsForPackets, + resolveIncomingHops, + cacheResolvedPaths, + _hopCacheGet: function(k) { return hopNameCache[k]; }, + _hopCacheSize: function() { return Object.keys(hopNameCache).length; }, + _destroy: destroy, _getRowCount, _cumulativeRowOffsets, _invalidateRowCounts, @@ -4174,10 +4243,7 @@ await loadObservers(); const data = await api(`/packets/${param}`); if (!data?.packet) { app.innerHTML = `

Packet not found

Packet ${param} doesn't exist.

← Back to packets
`; return; } - const hops = []; - try { hops.push(...getParsedPath(data.packet)); } catch {} - const newHops = hops.filter(h => !(h in hopNameCache)); - if (newHops.length) await resolveHops(newHops); + await resolveHopsForPackets([data.packet]); const container = document.createElement('div'); container.style.cssText = 'max-width:800px;margin:0 auto;padding:20px'; container.innerHTML = `
← Back to packets
`; diff --git a/public/style.css b/public/style.css index 5b02ae965..342455158 100644 --- a/public/style.css +++ b/public/style.css @@ -3153,6 +3153,20 @@ button.ch-item.ch-item-encrypted .ch-badge { filter: grayscale(0.6); } .hop-conflict-btn { background: var(--status-yellow, #f59e0b); color: #000; border: none; border-radius: 4px; font-size: 11px; font-weight: 700; padding: 1px 5px; cursor: pointer; vertical-align: middle; margin-left: 3px; line-height: 1.2; } .hop-conflict-btn:hover { background: var(--status-yellow, #d97706); filter: brightness(0.85); } + +/* #165 (port of upstream 2099) — a badge is a sibling of its pill, so + "A [8] -> B [6]" gave no clue which name the 8 belonged to. The group ties the pair together and keeps the + separator outside it. Only hops that carry a badge are wrapped. */ +.hop-group { display: inline-flex; align-items: center; white-space: nowrap; } +.hop-group .hop-conflict-btn, +.hop-group .hop-unreliable-btn { margin-left: 2px; } + +/* One indicator for a whole path, used by the packets list. The detail pane + keeps a badge per hop, because that is where the candidates get read. */ +.hop-path-warn { display: inline-flex; align-items: center; gap: 2px; margin-left: 6px; + background: var(--status-yellow); color: var(--palette-gray-900); border-radius: 4px; + font-size: 11px; font-weight: 700; padding: 1px 5px; line-height: 1.2; + vertical-align: middle; white-space: nowrap; } .hop-conflict-popover { position: absolute; z-index: var(--z-popover); background: var(--surface-1); border: 1px solid var(--border); border-radius: 8px; box-shadow: 0 8px 24px rgba(0,0,0,0.25); width: 260px; max-height: 300px; overflow-y: auto; } .hop-conflict-header { padding: 10px 12px; font-size: 12px; font-weight: 700; border-bottom: 1px solid var(--border); diff --git a/test-all.sh b/test-all.sh index 181872c38..654bb2c76 100755 --- a/test-all.sh +++ b/test-all.sh @@ -76,6 +76,7 @@ node test-issue-1418-edge-weights.js node test-issue-1418-cb-preset-ramp.js node test-issue-1418-spider-fan.js node test-issue-1418-deeplink-hops-channels.js +node test-issue-165-hop-ambiguity-badge.js node test-issue-1418-polish-review.js node test-issue-1420-tile-providers.js node test-issue-1614-tile-url-function.js diff --git a/test-issue-165-hop-ambiguity-badge.js b/test-issue-165-hop-ambiguity-badge.js new file mode 100644 index 000000000..94a6d33ee --- /dev/null +++ b/test-issue-165-hop-ambiguity-badge.js @@ -0,0 +1,239 @@ +/** + * #165 (port of upstream `Kpa-clawbot/CoreScope#2099`, upstream issue 2097) — + * an ambiguous path hop was shown as a single certain name. + * + * Every node on the network shares its 1-byte path prefix with at least one + * other: measured, 254 prefixes cover all 2043 nodes. HopResolver already marks + * such a hop `ambiguous` with a populated `conflicts` list, and HopDisplay + * already renders a warning badge for it. Neither was reaching the packets page, + * because resolve() takes six parameters and packets.js passed one. + * + * Without the observer, `packetIata` is null, so nodeInRegion() never runs, no + * candidate is flagged `regional`, `globalFallback` stays false, and + * HopDisplay's badgeCount computes to 0. The whole chain is silent while the + * data says the name is a guess. + * + * These tests pin the chain end to end: resolver output, then the markup + * HopDisplay produces from it. + */ +'use strict'; +const REPO_ROOT = __dirname; +const fs = require('fs'); +const vm = require('vm'); + +function load(file, extra) { + const sandbox = Object.assign({ + window: {}, document: { addEventListener() {}, getElementById: () => null, querySelector: () => null, createElement: () => ({ style: {}, classList: { add() {}, remove() {} }, appendChild() {}, setAttribute() {} }), body: { appendChild() {}, removeChild() {} } }, + console, Math, Object, Array, Number, Date, Map, Set, JSON, String, Boolean, + parseInt, parseFloat, isNaN, encodeURIComponent, decodeURIComponent, + setTimeout, clearTimeout, + }, extra || {}); + vm.createContext(sandbox); + vm.runInContext(fs.readFileSync(REPO_ROOT + '/public/' + file, 'utf8'), sandbox); + return sandbox; +} + +const rs = load('hop-resolver.js'); +const HopResolver = rs.window.HopResolver; +const ds = load('hop-display.js'); +const HopDisplay = ds.window.HopDisplay; + +let passed = 0, failed = 0; +function assert(cond, msg) { + if (cond) { passed++; console.log(' PASS ' + msg); } + else { failed++; console.log(' FAIL ' + msg); } +} + +// Three nodes share the 1-byte prefix "ef", as nine real ones do today. Two sit +// near the observer, one is 130 km away. +const near1 = { public_key: 'ef0069c0aa', name: 'NEAR-ONE', role: 'repeater', lat: 51.08, lon: 3.78 }; +const near2 = { public_key: 'ef86f6a5bb', name: 'NEAR-TWO', role: 'repeater', lat: 51.15, lon: 3.70 }; +const far = { public_key: 'efbf0eeacc', name: 'FAR-AWAY', role: 'repeater', lat: 50.87, lon: 5.52 }; +const other = { public_key: 'e7aaaaaadd', name: 'NEXT-HOP', role: 'repeater', lat: 51.10, lon: 3.80 }; +const NODES = [near1, near2, far, other]; + +// The observer that heard the packet, and its IATA centre. +const OBSERVER_ID = 'A943DC76B109'; +const IATA = 'OST'; + +console.log('\n=== #2097: the resolver marks an ambiguous hop ==='); + +HopResolver.init(NODES, { + observers: [{ id: OBSERVER_ID, iata: IATA }], + iataCoords: { [IATA]: { lat: 51.23, lon: 2.92 } }, +}); + +{ + const r = HopResolver.resolve(['ef']); + const e = r['ef']; + assert(e && e.ambiguous === true, 'three candidates for "ef" are reported as ambiguous'); + assert(e && Array.isArray(e.conflicts) && e.conflicts.length === 3, 'all three candidates are listed in conflicts'); +} + +console.log('\n=== #2097: HopDisplay renders a badge only when the count survives ==='); + +{ + // This is the regression itself: resolve() without the observer produces + // conflicts that carry no regional flag, and HopDisplay then computes + // badgeCount = 0 and renders nothing. + const noCtx = HopResolver.resolve(['ef'])['ef']; + const withCtx = HopResolver.resolve(['ef'], null, null, null, null, OBSERVER_ID)['ef']; + + const flaggedNo = (noCtx.conflicts || []).filter(c => c.regional).length; + const flaggedYes = (withCtx.conflicts || []).filter(c => c.regional).length; + assert(flaggedYes > flaggedNo, + `observer context flags regional candidates (${flaggedNo} without, ${flaggedYes} with)`); + + const html = HopDisplay.renderHop('ef', withCtx, {}); + assert(/hop-conflict-btn/.test(html), + 'a hop with several regional candidates renders the conflict badge'); + assert(/ph-warning/.test(html), 'the badge carries a warning icon, not colour alone'); + assert(/data-conflict=/.test(html), 'the badge carries the candidate list for the popover'); +} + +{ + // A hop with exactly one candidate must stay clean: the badge has to mean + // something, and 2-byte hops are unique 98.5% of the time. + const one = HopResolver.resolve(['e7aa'], null, null, null, null, OBSERVER_ID)['e7aa']; + const html = HopDisplay.renderHop('e7aa', one, {}); + assert(!/hop-conflict-btn/.test(html), 'an unambiguous hop renders no badge'); + assert(/NEXT-HOP/.test(html), 'an unambiguous hop still shows its name'); +} + +{ + // Geography must actually narrow the field: the node 130 km outside the + // observer's region should not be the one presented as the answer. + const withCtx = HopResolver.resolve(['ef'], null, null, null, null, OBSERVER_ID)['ef']; + assert(withCtx.name !== 'FAR-AWAY', + 'the candidate outside the observer region is not the one displayed (got ' + withCtx.name + ')'); +} + + +console.log('\n=== #2097: packets.js never resolves a hop without its observer ==='); + +{ + // Structural guard. The resolver and the display could always report the + // ambiguity; the regression was one caller dropping five of six arguments, + // and nothing failed when it did. + const src = fs.readFileSync(REPO_ROOT + '/public/packets.js', 'utf8'); + + const bare = src.match(/HopResolver\.resolve\(\s*[A-Za-z_$][\w$]*\s*\)/g) || []; + assert(bare.length === 0, + 'no HopResolver.resolve() call passes the hops alone' + + (bare.length ? ': ' + bare.join(', ') : '')); + + assert(/function hopCacheKey\(/.test(src), + 'the per-observer cache key is computed in one place'); + assert(/async function resolveHopsForPackets\(/.test(src), + 'multi-packet call sites group their hops by observer'); + + // renderHop reads the per-observer key; something has to write it. + assert(/hopNameCache\[hopCacheKey\(h, observerId\)\] = entry/.test(src), + 'resolveHops writes the per-observer cache key that renderHop reads'); +} + +console.log('\n=== #2097: a badge belongs to the pill on its left ==='); + +{ + // The badge is a sibling after the pill, so in "A [8] -> B [6]" a reader + // cannot tell which name the 8 qualifies. Pill and badge must render as one + // unbreakable group, with the arrow clearly outside it. + const withCtx = HopResolver.resolve(['ef'], null, null, null, null, OBSERVER_ID)['ef']; + const html = HopDisplay.renderHop('ef', withCtx, {}); + assert(/class="hop-group"/.test(html), 'pill and badge are wrapped in one group'); + const m = html.match(/([\s\S]*)<\/span>$/); + assert(m && /hop-link/.test(m[1]) && /hop-conflict-btn/.test(m[1]), + 'the group contains both the pill and its badge'); +} + +{ + // An unambiguous hop needs no wrapper: the group exists to tie a badge to its + // pill, and wrapping every hop would change layout for nothing. + const one = HopResolver.resolve(['e7aa'], null, null, null, null, OBSERVER_ID)['e7aa']; + const html = HopDisplay.renderHop('e7aa', one, {}); + assert(!/hop-group/.test(html), 'a hop with no badge is not wrapped'); +} + +{ + // The packets list wants the names without a badge on every hop. + const withCtx = HopResolver.resolve(['ef'], null, null, null, null, OBSERVER_ID)['ef']; + const plain = HopDisplay.renderHop('ef', withCtx, { badge: false }); + assert(!/hop-conflict-btn/.test(plain), 'badge:false suppresses the badge'); + assert(/hop-ambiguous/.test(plain), + 'the hop keeps its ambiguous class, so CSS can still mark it'); + assert(/NEAR-|FAR-/.test(plain), 'the name is still rendered'); +} + +{ + // #165 — nodes.js marks the selected hop. It used to patch the first + // class="" of the returned HTML, which is the .hop-group wrapper once the + // hop has a badge, so the marker missed the pill. + const withCtx = HopResolver.resolve(['ef'], null, null, null, null, OBSERVER_ID)['ef']; + const html = HopDisplay.renderHop('ef', withCtx, { className: 'hop-current' }); + assert(/^ !/null, null, null, null/.test(c)), + 'no call passes an observer id but drops its position: ' + withId.join(' | ')); + assert(/function observerPosition\(/.test(src) && /observerPosition\(observerId\)/.test(src), + 'one helper looks the observer up, used by every resolve path'); +} + +console.log(`\n${passed} passed, ${failed} failed`); +process.exit(failed ? 1 : 0); From 489a310d0e0112abf5746adfd92ab8fe2527779e Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 07:49:41 +0000 Subject: [PATCH 03/24] test(packets): reproduce missing observer coordinates becoming (0, 0) (#165) observerPosition() runs Number() on lat/lon, and Number(null) is 0, which is finite. An observer that /api/observers returns with lat: null, lon: null therefore anchors hop resolution at (0, 0), and a same-prefix candidate near Null Island wins over the candidate order. The new vm-harness test loads the real packets.js, hop-resolver.js and hop-display.js and asserts: - null, half-null or empty coordinates give [null, null]; - (0, 0) counts as no fix, as on the server (routes.go hasRealFix) and in HopResolver; - real coordinates still pass through; - an observer without coordinates does not change the pick. Red on this commit: 4 failed, 1 passed. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- .github/workflows/deploy.yml | 1 + test-all.sh | 1 + test-issue-165-hop-resolution-per-observer.js | 119 ++++++++++++++++++ 3 files changed, 121 insertions(+) create mode 100644 test-issue-165-hop-resolution-per-observer.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 12f21a612..531f20746 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -215,6 +215,7 @@ jobs: node test-warmup-banner.js node test-issue-1633-hide-1byte-hops.js node test-issue-165-hop-ambiguity-badge.js + node test-issue-165-hop-resolution-per-observer.js node test-issue-1668-m4-per-route.js node test-a11y-axe-1668-selftest.js node test-a11y-1716-rf-range-btn-active.js diff --git a/test-all.sh b/test-all.sh index 654bb2c76..1d3e0ba06 100755 --- a/test-all.sh +++ b/test-all.sh @@ -77,6 +77,7 @@ node test-issue-1418-cb-preset-ramp.js node test-issue-1418-spider-fan.js node test-issue-1418-deeplink-hops-channels.js node test-issue-165-hop-ambiguity-badge.js +node test-issue-165-hop-resolution-per-observer.js node test-issue-1418-polish-review.js node test-issue-1420-tile-providers.js node test-issue-1614-tile-url-function.js diff --git a/test-issue-165-hop-resolution-per-observer.js b/test-issue-165-hop-resolution-per-observer.js new file mode 100644 index 000000000..65c1cbe69 --- /dev/null +++ b/test-issue-165-hop-resolution-per-observer.js @@ -0,0 +1,119 @@ +/** + * #165 — hop resolution per observer on the packets page. + * + * The port of upstream `Kpa-clawbot/CoreScope#2099` resolves every path hop + * with the observer that heard it and caches the result under `hop:observer`. + * Review of that port found defects which these tests pin, each against the + * real public/packets.js, public/hop-resolver.js and public/hop-display.js + * loaded into a vm sandbox (no copies of production code): + * + * 1. Missing observer coordinates must not become a (0, 0) anchor. + * + * Run: node test-issue-165-hop-resolution-per-observer.js + */ +'use strict'; +const fs = require('fs'); +const path = require('path'); +const vm = require('vm'); + +let passed = 0, failed = 0; +const pending = []; +function test(name, fn) { pending.push({ name, fn }); } +function assert(cond, msg) { if (!cond) throw new Error(msg); } + +function escapeHtml(s) { + return String(s == null ? '' : s).replace(/[&<>"']/g, c => ({ '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' }[c])); +} + +// Load the real packets page into a fresh sandbox. `nodes` and `observers` +// are what /api/nodes and /api/observers return to ensureHopResolver(). +function loadPackets(nodes, observers) { + const store = {}; + const ctx = { + window: { addEventListener() {}, dispatchEvent() {}, innerWidth: 1280 }, + document: { + readyState: 'complete', + createElement: () => ({ style: {}, dataset: {}, classList: { add() {}, remove() {} }, appendChild() {}, setAttribute() {} }), + head: { appendChild() {} }, body: { appendChild() {}, removeChild() {} }, + getElementById: () => null, querySelector: () => null, querySelectorAll: () => [], + addEventListener() {}, removeEventListener() {}, + }, + localStorage: { getItem: k => (k in store ? store[k] : null), setItem: (k, v) => { store[k] = String(v); }, removeItem: k => { delete store[k]; } }, + console, Math, Object, Array, Number, Date, Map, Set, JSON, String, Boolean, RegExp, Error, TypeError, Promise, + parseInt, parseFloat, isNaN, isFinite, encodeURIComponent, decodeURIComponent, URLSearchParams, + setTimeout, clearTimeout, setInterval: () => 0, clearInterval() {}, + requestAnimationFrame: () => 0, cancelAnimationFrame() {}, + performance: { now: () => Date.now() }, + location: { hash: '' }, + escapeHtml, + registerPage() {}, onWS() {}, offWS() {}, debouncedOnWS: fn => fn, + CLIENT_TTL: {}, + fetchAllNodes: () => Promise.resolve({ nodes }), + api: (p) => Promise.resolve(p.startsWith('/observers') ? { observers } : p.startsWith('/iata-coords') ? { coords: {} } : {}), + invalidateApiCache() {}, + }; + ctx.window.localStorage = ctx.localStorage; + vm.createContext(ctx); + for (const f of ['payload-labels.js', 'packet-helpers.js', 'hop-resolver.js', 'hop-display.js', 'packets.js']) { + vm.runInContext(fs.readFileSync(path.join(__dirname, 'public', f), 'utf8'), ctx, { filename: f }); + for (const k of Object.keys(ctx.window)) ctx[k] = ctx.window[k]; + } + return ctx.window._packetsTestAPI; +} + +// Three repeaters share the 1-byte prefix "ef". FAR-AWAY is listed first: +// with no anchor the resolver keeps candidate order, so it is the default pick. +// NULL-ISLAND sits next to (0, 0), so a false (0, 0) anchor selects it. +const FAR = { public_key: 'efbf0eeacc', name: 'FAR-AWAY', role: 'repeater', lat: 50.87, lon: 5.52 }; +const NULL_ISLAND = { public_key: 'ef00000011', name: 'NULL-ISLAND', role: 'repeater', lat: 0.2, lon: 0.2 }; +const NEAR = { public_key: 'ef0069c0aa', name: 'NEAR-ONE', role: 'repeater', lat: 51.08, lon: 3.78 }; + +console.log('\n=== #165 defect 1: missing observer coordinates are not (0, 0) ==='); + +test('observerPosition() returns [null, null] for null coordinates', async () => { + const T = loadPackets([FAR, NULL_ISLAND], [{ id: 'OBS-NULL', lat: null, lon: null }]); + await T.resolveHops(['ef'], 'OBS-NULL'); // seeds observerMap via ensureHopResolver + const pos = T.observerPosition('OBS-NULL'); + assert(pos[0] === null && pos[1] === null, 'expected [null, null], got ' + JSON.stringify(pos)); +}); + +test('observerPosition() returns [null, null] when only one coordinate is valid', async () => { + const T = loadPackets([FAR], [{ id: 'OBS-HALF', lat: 51.2, lon: null }, { id: 'OBS-EMPTY', lat: '', lon: '' }]); + await T.resolveHops(['ef'], 'OBS-HALF'); + const half = T.observerPosition('OBS-HALF'); + const empty = T.observerPosition('OBS-EMPTY'); + assert(half[0] === null && half[1] === null, 'lat only: expected [null, null], got ' + JSON.stringify(half)); + assert(empty[0] === null && empty[1] === null, 'empty strings: expected [null, null], got ' + JSON.stringify(empty)); +}); + +test('observerPosition() treats (0, 0) as no fix, like the server and HopResolver do', async () => { + const T = loadPackets([FAR], [{ id: 'OBS-ZERO', lat: 0, lon: 0 }]); + await T.resolveHops(['ef'], 'OBS-ZERO'); + const pos = T.observerPosition('OBS-ZERO'); + assert(pos[0] === null && pos[1] === null, 'expected [null, null], got ' + JSON.stringify(pos)); +}); + +test('observerPosition() still returns real coordinates', async () => { + const T = loadPackets([FAR], [{ id: 'OBS-OK', lat: 51.21, lon: '3.44' }]); + await T.resolveHops(['ef'], 'OBS-OK'); + const pos = T.observerPosition('OBS-OK'); + assert(pos[0] === 51.21 && pos[1] === 3.44, 'expected [51.21, 3.44], got ' + JSON.stringify(pos)); +}); + +test('an observer without coordinates does not steer the candidate choice', async () => { + const T = loadPackets([FAR, NULL_ISLAND, NEAR], [{ id: 'OBS-NULL', lat: null, lon: null }]); + await T.resolveHops(['ef'], 'OBS-NULL'); + const entry = T._hopCacheGet('ef:OBS-NULL'); + assert(entry && entry.ambiguous, 'the hop is still reported as ambiguous'); + assert(entry.name === 'FAR-AWAY', + 'with no anchor the resolver keeps candidate order (FAR-AWAY); got ' + (entry && entry.name)); +}); + +(async () => { + for (const t of pending) { + try { await t.fn(); passed++; console.log(' ✅ ' + t.name); } + catch (e) { failed++; console.log(' ❌ ' + t.name + ': ' + e.message); } + } + console.log(`\n${passed} passed, ${failed} failed`); + process.exit(failed ? 1 : 0); +})(); From 9edd86cf11ff641bad070f07b49322a62d0dbb26 Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 07:49:52 +0000 Subject: [PATCH 04/24] fix(packets): never turn missing observer coordinates into (0, 0) (#165) observerPosition() now accepts a coordinate only when it is a real number, so null, undefined, '' and non-numeric values all count as missing. Both lat and lon must be present, and (0, 0) is treated as no fix, as the server (routes.go hasRealFix) and HopResolver already do. Anything else gives [null, null], so hop resolution runs without an observer anchor instead of anchoring at Null Island. test-issue-165-hop-resolution-per-observer.js: 5 passed, 0 failed. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- public/packets.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/public/packets.js b/public/packets.js index 402d046d2..f8ed66100 100644 --- a/public/packets.js +++ b/public/packets.js @@ -1004,11 +1004,21 @@ // end of the path. The IATA route is dead weight: measured on the live // deployment, none of the 42 observers has its code in /api/iata-coords, so // nodeInRegion() always returns null. lat/lon is reported directly and works. + // #165 — /api/observers sends lat/lon as null when no node location is + // known, and Number(null) is 0. Only a pair of real numbers is an anchor; + // (0, 0) means "no fix" here, as on the server and in HopResolver. + function coordOrNull(v) { + if (v === null || v === undefined || v === '') return null; + const n = Number(v); + return Number.isFinite(n) ? n : null; + } + function observerPosition(observerId) { const o = observerId && observerMap ? observerMap.get(observerId) : null; - const lat = o && Number.isFinite(Number(o.lat)) ? Number(o.lat) : null; - const lon = o && Number.isFinite(Number(o.lon)) ? Number(o.lon) : null; - return (lat === null || lon === null) ? [null, null] : [lat, lon]; + const lat = o ? coordOrNull(o.lat) : null; + const lon = o ? coordOrNull(o.lon) : null; + if (lat === null || lon === null || (lat === 0 && lon === 0)) return [null, null]; + return [lat, lon]; } function hopCacheKey(h, observerId) { From 015841d893a0aeefa398f487e3a0effcb502ad83 Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 07:50:32 +0000 Subject: [PATCH 05/24] test(packets): reproduce the client heuristic replacing resolved_path (#165) cacheResolvedPaths() writes the server's resolved_path entries under bare hop keys only. resolveHopsForPackets() then finds no hop:observer key, re-resolves the same hop on the client and writes that key, and renderHop() prefers it. The heuristic's pick (next to the observer) replaces the node the server confirmed. The incremental path writes the server's entries bare in the same way. Three tests: - the initial-load sequence keeps the server node, with a precondition showing that the heuristic would pick another; - a hop the server left null is still client-resolved, and the list summary counts only that hop as uncertain; - resolveIncomingHops() keeps the server node. Red on this commit: 3 failed, 5 passed. Section headers now print in order with their tests. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- test-issue-165-hop-resolution-per-observer.js | 59 ++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/test-issue-165-hop-resolution-per-observer.js b/test-issue-165-hop-resolution-per-observer.js index 65c1cbe69..ae027e025 100644 --- a/test-issue-165-hop-resolution-per-observer.js +++ b/test-issue-165-hop-resolution-per-observer.js @@ -8,6 +8,8 @@ * loaded into a vm sandbox (no copies of production code): * * 1. Missing observer coordinates must not become a (0, 0) anchor. + * 2. A hop the server resolved (resolved_path) keeps the server's node; the + * client heuristic must not overwrite it under the same observer. * * Run: node test-issue-165-hop-resolution-per-observer.js */ @@ -19,6 +21,7 @@ const vm = require('vm'); let passed = 0, failed = 0; const pending = []; function test(name, fn) { pending.push({ name, fn }); } +function section(title) { pending.push({ title }); } function assert(cond, msg) { if (!cond) throw new Error(msg); } function escapeHtml(s) { @@ -68,7 +71,7 @@ const FAR = { public_key: 'efbf0eeacc', name: 'FAR-AWAY', role: 'repeater', lat: const NULL_ISLAND = { public_key: 'ef00000011', name: 'NULL-ISLAND', role: 'repeater', lat: 0.2, lon: 0.2 }; const NEAR = { public_key: 'ef0069c0aa', name: 'NEAR-ONE', role: 'repeater', lat: 51.08, lon: 3.78 }; -console.log('\n=== #165 defect 1: missing observer coordinates are not (0, 0) ==='); +section('#165 defect 1: missing observer coordinates are not (0, 0)'); test('observerPosition() returns [null, null] for null coordinates', async () => { const T = loadPackets([FAR, NULL_ISLAND], [{ id: 'OBS-NULL', lat: null, lon: null }]); @@ -109,8 +112,62 @@ test('an observer without coordinates does not steer the candidate choice', asyn 'with no anchor the resolver keeps candidate order (FAR-AWAY); got ' + (entry && entry.name)); }); +section('#165 defect 2: the server resolved_path wins over the heuristic'); + +// OBS-A sits next to NEAR-ONE, so the client heuristic picks NEAR-ONE for +// "ef". The server, which has the neighbour graph, says FAR-AWAY. "c1" is +// left unresolved by the server (null) and has two candidates: that one is +// genuinely uncertain and must still be shown as such. +const C1A = { public_key: 'c1aaaa0001', name: 'C1-ALPHA', role: 'repeater', lat: 51.10, lon: 3.70 }; +const C1B = { public_key: 'c1bbbb0002', name: 'C1-BRAVO', role: 'repeater', lat: 51.12, lon: 3.72 }; +const OBS_A = { id: 'OBS-A', lat: 51.21, lon: 3.44 }; +function serverPacket() { + return { id: 1, hash: 'h1', observer_id: 'OBS-A', path_json: '["ef","c1"]', + resolved_path: JSON.stringify([FAR.public_key, null]) }; +} + +test('initial load: cacheResolvedPaths + resolveHopsForPackets keep the server node', async () => { + const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_A]); + // Precondition: without the server's answer the heuristic picks NEAR-ONE, + // so this test can only pass if the server's entry is the one kept. + const T0 = loadPackets([FAR, NEAR, C1A, C1B], [OBS_A]); + await T0.resolveHops(['ef'], 'OBS-A'); + assert(T0._hopCacheGet('ef:OBS-A').name === 'NEAR-ONE', 'precondition: heuristic picks NEAR-ONE'); + + const pkt = serverPacket(); + await T.cacheResolvedPaths([pkt]); + await T.resolveHopsForPackets([pkt]); + const entry = T._hopCacheGet('ef:OBS-A'); + assert(entry && entry.pubkey === FAR.public_key, + 'ef:OBS-A must be the server node FAR-AWAY; got ' + (entry && entry.name)); + const html = T.renderPath(['ef'], 'OBS-A'); + assert(/FAR-AWAY/.test(html) && !/NEAR-ONE/.test(html), 'the rendered hop shows FAR-AWAY: ' + html); +}); + +test('a hop the server left unresolved is still resolved and flagged', async () => { + const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_A]); + const pkt = serverPacket(); + await T.cacheResolvedPaths([pkt]); + await T.resolveHopsForPackets([pkt]); + const c1 = T._hopCacheGet('c1:OBS-A'); + assert(c1 && c1.ambiguous, 'c1 (server null) is client-resolved and ambiguous'); + const list = T.renderPath(['ef', 'c1'], 'OBS-A', { summary: true }); + const m = list.match(/hop-path-warn[^>]*>[\s\S]*?<\/svg>(\d+)<\/span>/); + assert(m && m[1] === '1', 'the list summary counts exactly the one uncertain hop (c1); got ' + (m ? m[1] : 'no summary')); +}); + +test('incremental path: resolveIncomingHops keeps the server node too', async () => { + const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_A]); + await T.resolveHops(['00'], 'OBS-A'); // HopResolver ready, as after the initial load + await T.resolveIncomingHops([serverPacket()]); + const entry = T._hopCacheGet('ef:OBS-A'); + assert(entry && entry.pubkey === FAR.public_key, + 'ef:OBS-A must be the server node FAR-AWAY; got ' + (entry && entry.name)); +}); + (async () => { for (const t of pending) { + if (t.title) { console.log('\n=== ' + t.title.trim() + ' ==='); continue; } try { await t.fn(); passed++; console.log(' ✅ ' + t.name); } catch (e) { failed++; console.log(' ❌ ' + t.name + ': ' + e.message); } } From f93dc2562e8c58afcf0d61dc3e50be1f794f87b8 Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 07:50:49 +0000 Subject: [PATCH 06/24] fix(packets): keep the server's resolved_path over the client heuristic (#165) Chosen shape (option 1 in the #165 review, point D): the server's entries are written under hop:observer as well as the bare key, by one helper, cacheServerResolvedPath(), which cacheResolvedPaths() and the incremental resolveIncomingHops() now share. Why this option and not a skip in resolveHops(): renderHop(), renderPath()'s summary and resolveHops() all read the per-observer key first. Storing the server's answer where those readers look makes it win everywhere at once. resolveHops() works on a set of prefixes per observer and does not know which packet a prefix came from, so it could not tell which hops to skip. renderDetail() already wrote both keys the same way. Hops the server leaves null are not written. The client still resolves them, so a genuinely unresolved hop keeps its ambiguity badge. test-issue-165-hop-resolution-per-observer.js: 8 passed, 0 failed. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- public/packets.js | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/public/packets.js b/public/packets.js index f8ed66100..45cc6e5b0 100644 --- a/public/packets.js +++ b/public/packets.js @@ -1076,12 +1076,24 @@ } if (!needsInit) return; await ensureHopResolver(); - for (const p of packets) { - const rp = getResolvedPath(p); - if (!rp) continue; - const hops = getParsedPath(p); - const resolved = HopResolver.resolveFromServer(hops, rp); - Object.assign(hopNameCache, resolved); + for (const p of packets) cacheServerResolvedPath(p); + } + + // #165 — store the server's resolved_path entries under the bare key AND + // under hop:observer. renderHop() and resolveHops() both look at the + // per-observer key first, so a server answer stored only under the bare key + // was re-resolved by the client heuristic and replaced. Writing it where + // the readers look makes the server win without resolveHops() needing to + // know which packet a prefix came from. Hops the server left null are not + // written, so the client still resolves them and can show the ambiguity. + // Caller must have initialised HopResolver. + function cacheServerResolvedPath(p) { + const rp = getResolvedPath(p); + if (!rp) return; + const resolved = HopResolver.resolveFromServer(getParsedPath(p), rp); + for (const h of Object.keys(resolved)) { + hopNameCache[h] = resolved[h]; + if (p.observer_id) hopNameCache[hopCacheKey(h, String(p.observer_id))] = resolved[h]; } } @@ -1090,12 +1102,8 @@ function resolveIncomingHops(pkts) { const newHops = new Set(); for (const p of pkts) { - const rp = getResolvedPath(p); const hops = getParsedPath(p); - if (rp && rp.length === hops.length && window.HopResolver && HopResolver.ready()) { - const resolved = HopResolver.resolveFromServer(hops, rp); - Object.assign(hopNameCache, resolved); - } + if (window.HopResolver && HopResolver.ready()) cacheServerResolvedPath(p); try { hops.forEach(h => { if (!(h in hopNameCache)) newHops.add(h); }); } catch {} } return newHops.size ? resolveHopsForPackets(pkts) : Promise.resolve(); From 741f3c5be514ce8d76394965e49a0a1d70579e33 Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 07:51:06 +0000 Subject: [PATCH 07/24] test(packets): reproduce live packets inheriting another observer's hop (#165) resolveIncomingHops() decides whether there is anything to resolve by looking only at the bare hop key. Once observer A has resolved a prefix, a packet from observer B with the same prefix finds the bare key, resolves nothing, and renders A's pick through the bare-key fallback. Two tests: - A, then B, in separate incremental batches; - an initial load for A, then one incremental batch holding both A and B. Red on this commit: 2 failed, 8 passed. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- test-issue-165-hop-resolution-per-observer.js | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test-issue-165-hop-resolution-per-observer.js b/test-issue-165-hop-resolution-per-observer.js index ae027e025..e8e484639 100644 --- a/test-issue-165-hop-resolution-per-observer.js +++ b/test-issue-165-hop-resolution-per-observer.js @@ -10,6 +10,8 @@ * 1. Missing observer coordinates must not become a (0, 0) anchor. * 2. A hop the server resolved (resolved_path) keeps the server's node; the * client heuristic must not overwrite it under the same observer. + * 3. The incremental path (WS/poll) resolves the same prefix separately for + * each observer instead of reusing another observer's bare-key entry. * * Run: node test-issue-165-hop-resolution-per-observer.js */ @@ -165,6 +167,33 @@ test('incremental path: resolveIncomingHops keeps the server node too', async () 'ef:OBS-A must be the server node FAR-AWAY; got ' + (entry && entry.name)); }); +section('#165 defect 3: the incremental path resolves per observer'); + +// OBS-A is next to NEAR-ONE, OBS-B next to FAR-AWAY. The same 1-byte prefix +// heard by each must resolve to that observer's neighbour. +const OBS_B = { id: 'OBS-B', lat: 50.88, lon: 5.50 }; +function livePacket(id, obs) { return { id, hash: 'live' + id, observer_id: obs, path_json: '["ef"]' }; } + +test('same prefix from observer A, then B, via resolveIncomingHops: each gets its own name', async () => { + const T = loadPackets([FAR, NEAR], [OBS_A, OBS_B]); + await T.resolveIncomingHops([livePacket(1, 'OBS-A')]); + await T.resolveIncomingHops([livePacket(2, 'OBS-B')]); + const a = T._hopCacheGet('ef:OBS-A'); + const b = T._hopCacheGet('ef:OBS-B'); + assert(a && a.name === 'NEAR-ONE', 'ef:OBS-A is NEAR-ONE; got ' + (a && a.name)); + assert(b && b.name === 'FAR-AWAY', 'ef:OBS-B is resolved for B (FAR-AWAY); got ' + (b ? b.name : 'no entry')); + const html = T.renderPath(['ef'], 'OBS-B'); + assert(/FAR-AWAY/.test(html) && !/NEAR-ONE/.test(html), 'B does not inherit A\'s name: ' + html); +}); + +test('one incremental batch with both observers resolves each separately', async () => { + const T = loadPackets([FAR, NEAR], [OBS_A, OBS_B]); + await T.resolveHopsForPackets([livePacket(1, 'OBS-A')]); // initial load saw only A + await T.resolveIncomingHops([livePacket(2, 'OBS-A'), livePacket(3, 'OBS-B')]); + const b = T._hopCacheGet('ef:OBS-B'); + assert(b && b.name === 'FAR-AWAY', 'ef:OBS-B is resolved for B; got ' + (b ? b.name : 'no entry')); +}); + (async () => { for (const t of pending) { if (t.title) { console.log('\n=== ' + t.title.trim() + ' ==='); continue; } From 4a98067f12c97992ca8722d361b977e4fdbf1a39 Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 07:51:18 +0000 Subject: [PATCH 08/24] fix(packets): resolve live packets' hops per observer (#165) resolveIncomingHops() now checks the hop:observer key for each packet's own observer before deciding whether anything needs resolving. It no longer checks the bare key that any observer may have filled. resolveHopsForPackets() already groups by observer and skips keys that are present, so a known (hop, observer) pair still costs nothing. test-issue-165-hop-resolution-per-observer.js: 10 passed, 0 failed. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- public/packets.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/public/packets.js b/public/packets.js index 45cc6e5b0..b9c3c7c3f 100644 --- a/public/packets.js +++ b/public/packets.js @@ -1099,14 +1099,16 @@ // Incremental path (WS/poll): pre-populate from server-side resolved_path, // then fall back to the client resolver for the remaining hops. + // #165 — "already resolved" means resolved for THIS packet's observer. A + // bare-key check let a second observer reuse the first one's pick. function resolveIncomingHops(pkts) { - const newHops = new Set(); + let missing = false; for (const p of pkts) { - const hops = getParsedPath(p); if (window.HopResolver && HopResolver.ready()) cacheServerResolvedPath(p); - try { hops.forEach(h => { if (!(h in hopNameCache)) newHops.add(h); }); } catch {} + const obs = p.observer_id ? String(p.observer_id) : undefined; + try { if (getParsedPath(p).some(h => !(hopCacheKey(h, obs) in hopNameCache))) missing = true; } catch {} } - return newHops.size ? resolveHopsForPackets(pkts) : Promise.resolve(); + return missing ? resolveHopsForPackets(pkts) : Promise.resolve(); } function renderHop(h, observerId, opts) { From f6195ea07ba3599121ef27a996b65c7f6e5fed9a Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 07:51:55 +0000 Subject: [PATCH 09/24] test(packets): require a bounded hop:observer cache (#165) hopNameCache gains a key per (prefix, observer) pair and is only cleared in destroy(). AGENTS.md requires a size limit or eviction for every map. The tests require: - an exposed HOP_CACHE_MAX that the cache never exceeds; - eviction that drops the oldest entries and keeps the newest; - destroy(), the existing reset point, emptying the cache. Red on this commit: 2 failed (no cap exists), 11 passed. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- test-issue-165-hop-resolution-per-observer.js | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test-issue-165-hop-resolution-per-observer.js b/test-issue-165-hop-resolution-per-observer.js index e8e484639..ac5df3c9b 100644 --- a/test-issue-165-hop-resolution-per-observer.js +++ b/test-issue-165-hop-resolution-per-observer.js @@ -12,6 +12,8 @@ * client heuristic must not overwrite it under the same observer. * 3. The incremental path (WS/poll) resolves the same prefix separately for * each observer instead of reusing another observer's bare-key entry. + * B. The hop:observer cache is bounded (AGENTS.md: no unbounded maps) and is + * cleared at the page's existing reset point, destroy(). * * Run: node test-issue-165-hop-resolution-per-observer.js */ @@ -194,6 +196,41 @@ test('one incremental batch with both observers resolves each separately', async assert(b && b.name === 'FAR-AWAY', 'ef:OBS-B is resolved for B; got ' + (b ? b.name : 'no entry')); }); +section('#165 B: the hop cache is bounded'); + +test('the cache never holds more than HOP_CACHE_MAX entries', async () => { + const T = loadPackets([FAR, NEAR], [OBS_A]); + const max = T.HOP_CACHE_MAX; + assert(Number.isInteger(max) && max > 0, 'HOP_CACHE_MAX is a positive integer; got ' + max); + // Distinct 3-byte prefixes no node has: each writes hop:OBS-A and hop. + const hops = []; + for (let i = 0; hops.length < max; i++) hops.push((0x100000 + i).toString(16)); + await T.resolveHops(hops, 'OBS-A'); + assert(T._hopCacheSize() <= max, 'size ' + T._hopCacheSize() + ' exceeds the cap ' + max); +}); + +test('eviction drops the oldest entries and keeps the newest', async () => { + const T = loadPackets([FAR, NEAR], [OBS_A, OBS_B]); + const max = T.HOP_CACHE_MAX; + await T.resolveHops(['ef'], 'OBS-A'); // oldest + const hops = []; + for (let i = 0; hops.length < max; i++) hops.push((0x100000 + i).toString(16)); + await T.resolveHops(hops, 'OBS-B'); + await T.resolveHops(['ef'], 'OBS-B'); // newest + assert(T._hopCacheGet('ef:OBS-A') === undefined, 'the oldest entry (ef:OBS-A) was evicted'); + const b = T._hopCacheGet('ef:OBS-B'); + assert(b && b.name === 'FAR-AWAY', 'the newest entry (ef:OBS-B) is kept; got ' + (b ? b.name : 'no entry')); + assert(T._hopCacheSize() <= max, 'size ' + T._hopCacheSize() + ' exceeds the cap ' + max); +}); + +test('destroy() empties the cache', async () => { + const T = loadPackets([FAR, NEAR], [OBS_A]); + await T.resolveHops(['ef'], 'OBS-A'); + assert(T._hopCacheSize() > 0, 'precondition: the cache has entries'); + T._destroy(); + assert(T._hopCacheSize() === 0, 'destroy() leaves ' + T._hopCacheSize() + ' entries'); +}); + (async () => { for (const t of pending) { if (t.title) { console.log('\n=== ' + t.title.trim() + ' ==='); continue; } From fad570e1b44156530f4f1cb288dc4d5021293993 Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 07:52:20 +0000 Subject: [PATCH 10/24] fix(packets): bound the hop name cache (#165) hopNameCache becomes a Map read and written only through hopCacheHas(), hopCacheGet() and hopCacheSet(). - Past HOP_CACHE_MAX (20000 entries) the oldest entry is evicted. A Map keeps insertion order, so this costs O(1) per write, and a rewrite moves the key to the newest end. - destroy(), the existing reset point, still replaces the whole cache. - An evicted hop:observer entry falls back to the bare key in renderHop(), and the next packet from that observer re-resolves it. 20000 is above the per-observer working set seen here (prefixes x ~42 observers), and its worst-case footprint stays in the tens of MB. It is hardcoded for now and should move to the customizer later (AGENTS.md rule 8). The ported structural assertion in test-issue-165-hop-ambiguity-badge.js now looks for the hopCacheSet(hopCacheKey(...)) write. test-issue-165-hop-resolution-per-observer.js: 13 passed, 0 failed. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- public/packets.js | 43 +++++++++++++++++---------- test-issue-165-hop-ambiguity-badge.js | 3 +- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/public/packets.js b/public/packets.js index b9c3c7c3f..09f10fcaf 100644 --- a/public/packets.js +++ b/public/packets.js @@ -761,7 +761,19 @@ if (isMobile && savedTimeWindowMin > 180) savedTimeWindowMin = 15; let totalCount = 0; let expandedHashes = new Set(); - let hopNameCache = {}; + // #165 — resolved hop names, keyed by hop and by hop:observer. The + // per-observer keys grow with every observer that hears a prefix, so the + // cache is bounded: past HOP_CACHE_MAX the oldest entries are evicted + // (Map keeps insertion order), and destroy() clears it. + const HOP_CACHE_MAX = 20000; + let hopNameCache = new Map(); + function hopCacheHas(k) { return hopNameCache.has(k); } + function hopCacheGet(k) { return hopNameCache.get(k); } + function hopCacheSet(k, v) { + hopNameCache.delete(k); // a rewrite counts as new, for eviction order + hopNameCache.set(k, v); + if (hopNameCache.size > HOP_CACHE_MAX) hopNameCache.delete(hopNameCache.keys().next().value); + } let _tableSortInstance = null; let _packetSortColumn = null; let _packetSortDirection = 'desc'; @@ -1032,16 +1044,16 @@ // is how a repeater 126 km outside the observer's region ended up displayed // as a certainty. async function resolveHops(hops, observerId) { - const unknown = hops.filter(h => !(hopCacheKey(h, observerId) in hopNameCache)); + const unknown = hops.filter(h => !hopCacheHas(hopCacheKey(h, observerId))); if (!unknown.length) return; await ensureHopResolver(); const [obsLat, obsLon] = observerPosition(observerId); const resolved = HopResolver.resolve(unknown, null, null, obsLat, obsLon, observerId) || {}; for (const h of unknown) { const entry = resolved[h] || null; - hopNameCache[hopCacheKey(h, observerId)] = entry; + hopCacheSet(hopCacheKey(h, observerId), entry); // Bare key as a fallback for any render that has no observer in hand. - if (!(h in hopNameCache)) hopNameCache[h] = entry; + if (!hopCacheHas(h)) hopCacheSet(h, entry); } } @@ -1092,8 +1104,8 @@ if (!rp) return; const resolved = HopResolver.resolveFromServer(getParsedPath(p), rp); for (const h of Object.keys(resolved)) { - hopNameCache[h] = resolved[h]; - if (p.observer_id) hopNameCache[hopCacheKey(h, String(p.observer_id))] = resolved[h]; + hopCacheSet(h, resolved[h]); + if (p.observer_id) hopCacheSet(hopCacheKey(h, String(p.observer_id)), resolved[h]); } } @@ -1106,7 +1118,7 @@ for (const p of pkts) { if (window.HopResolver && HopResolver.ready()) cacheServerResolvedPath(p); const obs = p.observer_id ? String(p.observer_id) : undefined; - try { if (getParsedPath(p).some(h => !(hopCacheKey(h, obs) in hopNameCache))) missing = true; } catch {} + try { if (getParsedPath(p).some(h => !hopCacheHas(hopCacheKey(h, obs)))) missing = true; } catch {} } return missing ? resolveHopsForPackets(pkts) : Promise.resolve(); } @@ -1114,7 +1126,7 @@ function renderHop(h, observerId, opts) { // Use per-packet cache key if observer context available (ambiguous hops differ by region) const cacheKey = hopCacheKey(h, observerId); - const entry = hopNameCache[cacheKey] || hopNameCache[h]; + const entry = hopCacheGet(cacheKey) || hopCacheGet(h); return HopDisplay.renderHop(h, entry, Object.assign({ hexMode: showHexHashes }, opts || {})); } @@ -1139,7 +1151,7 @@ let uncertain = 0; for (const h of filtered) { - const entry = hopNameCache[hopCacheKey(h, observerId)] || hopNameCache[h]; + const entry = hopCacheGet(hopCacheKey(h, observerId)) || hopCacheGet(h); if (entry && entry.ambiguous) uncertain++; } if (!uncertain) return body; @@ -1490,7 +1502,7 @@ filtersBuilt = false; delete filters.node; expandedHashes = new Set(); - hopNameCache = {}; + hopNameCache = new Map(); totalCount = 0; observers = []; observerMap = new Map(); @@ -3434,8 +3446,8 @@ } if (resolved) { for (const [k, v] of Object.entries(resolved)) { - hopNameCache[k] = v; - if (pkt.observer_id) hopNameCache[k + ':' + pkt.observer_id] = v; + hopCacheSet(k, v); + if (pkt.observer_id) hopCacheSet(hopCacheKey(k, String(pkt.observer_id)), v); } } } catch {} @@ -3806,7 +3818,7 @@ for (let i = 0; i < pathHops.length; i++) { const hopOff = off + i * hashSize; const hex = String(pathHops[i] || '').toUpperCase(); - const hopHtml = HopDisplay.renderHop(hex, hopNameCache[hex]); + const hopHtml = HopDisplay.renderHop(hex, hopCacheGet(hex)); const label = `Hop ${i} — ${hopHtml}`; rows += fieldRow(hopOff, label, hex, ''); } @@ -4236,8 +4248,9 @@ resolveHopsForPackets, resolveIncomingHops, cacheResolvedPaths, - _hopCacheGet: function(k) { return hopNameCache[k]; }, - _hopCacheSize: function() { return Object.keys(hopNameCache).length; }, + HOP_CACHE_MAX, + _hopCacheGet: hopCacheGet, + _hopCacheSize: function() { return hopNameCache.size; }, _destroy: destroy, _getRowCount, _cumulativeRowOffsets, diff --git a/test-issue-165-hop-ambiguity-badge.js b/test-issue-165-hop-ambiguity-badge.js index 94a6d33ee..af97e571d 100644 --- a/test-issue-165-hop-ambiguity-badge.js +++ b/test-issue-165-hop-ambiguity-badge.js @@ -128,7 +128,8 @@ console.log('\n=== #2097: packets.js never resolves a hop without its observer = 'multi-packet call sites group their hops by observer'); // renderHop reads the per-observer key; something has to write it. - assert(/hopNameCache\[hopCacheKey\(h, observerId\)\] = entry/.test(src), + // #165: the cache is a bounded Map written through hopCacheSet(). + assert(/hopCacheSet\(hopCacheKey\(h, observerId\), entry\)/.test(src), 'resolveHops writes the per-observer cache key that renderHop reads'); } From 277a25c8048ea837186bbb6b774883d6f92e44ec Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 07:53:56 +0000 Subject: [PATCH 11/24] fix(packets): size the hop cache from a measured working set (#165) The previous commit set HOP_CACHE_MAX to 20000 and claimed that this was above the working set. That claim was not measured, and it is wrong. Measured with the real packets.js in a vm sandbox, on a synthetic 30K packet load: - 2000 repeaters in three regions, 42 observers (27 with coordinates) and 0-8 hops per packet (85% 1-byte, 30% with resolved_path); - the initial load leaves 28962 entries (bare plus hop:observer), using 9.6 MB of heap; - at 20000 the cache evicts during the initial load itself, which costs about 80 ms extra (309 ms vs 229 ms median of 7 runs). 50000 holds that load with headroom. At the cap the heap stays under about 17 MB at the same per-entry size. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- public/packets.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/packets.js b/public/packets.js index 09f10fcaf..88325aaba 100644 --- a/public/packets.js +++ b/public/packets.js @@ -765,7 +765,7 @@ // per-observer keys grow with every observer that hears a prefix, so the // cache is bounded: past HOP_CACHE_MAX the oldest entries are evicted // (Map keeps insertion order), and destroy() clears it. - const HOP_CACHE_MAX = 20000; + const HOP_CACHE_MAX = 50000; let hopNameCache = new Map(); function hopCacheHas(k) { return hopNameCache.has(k); } function hopCacheGet(k) { return hopNameCache.get(k); } From 371928c5ef54925965651d46a3d705617a85fe63 Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 07:54:29 +0000 Subject: [PATCH 12/24] test(packets): require resolveHopsForPackets to yield between observers (#165) Resolving per observer does more work than master's single resolve. On the synthetic 30K load (see the previous commit), the initial hop resolution takes 217 ms against master's 67 ms, all in one main-thread block, because the observer groups run back to back on microtasks. The test schedules a timer and requires it to fire after the first observer group and before the last. Red on this commit: 1 failed, 13 passed. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- test-issue-165-hop-resolution-per-observer.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test-issue-165-hop-resolution-per-observer.js b/test-issue-165-hop-resolution-per-observer.js index ac5df3c9b..6957be3b7 100644 --- a/test-issue-165-hop-resolution-per-observer.js +++ b/test-issue-165-hop-resolution-per-observer.js @@ -14,6 +14,9 @@ * each observer instead of reusing another observer's bare-key entry. * B. The hop:observer cache is bounded (AGENTS.md: no unbounded maps) and is * cleared at the page's existing reset point, destroy(). + * C. Resolving per observer costs more than one global resolve, so + * resolveHopsForPackets() yields to the event loop between observer + * groups instead of blocking the page for the whole initial load. * * Run: node test-issue-165-hop-resolution-per-observer.js */ @@ -231,6 +234,21 @@ test('destroy() empties the cache', async () => { assert(T._hopCacheSize() === 0, 'destroy() leaves ' + T._hopCacheSize() + ' entries'); }); +section('#165 C: resolveHopsForPackets does not block for the whole load'); + +test('a timer runs between two observer groups', async () => { + const T = loadPackets([FAR, NEAR], [OBS_A, OBS_B]); + await T.resolveHops(['00'], 'OBS-A'); // HopResolver ready, as after the first render + const before = T._hopCacheSize(); + let seenAt = null; + setTimeout(() => { seenAt = T._hopCacheSize(); }, 0); + await T.resolveHopsForPackets([livePacket(1, 'OBS-A'), livePacket(2, 'OBS-B')]); + const after = T._hopCacheSize(); + assert(seenAt !== null && seenAt > before && seenAt < after, + 'the timer should fire after the first group and before the last (cache sizes: before ' + before + + ', at timer ' + seenAt + ', after ' + after + ')'); +}); + (async () => { for (const t of pending) { if (t.title) { console.log('\n=== ' + t.title.trim() + ' ==='); continue; } From 3b908c50da1a923bc6415a31a2b5b1096156f934 Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 07:54:58 +0000 Subject: [PATCH 13/24] perf(packets): yield between observer groups when resolving hops (#165) resolveHopsForPackets() now awaits a macrotask (setTimeout 0) before every observer group after the first. Input and paint can then get in during the initial load, instead of waiting for the whole per-observer resolve. Synthetic 30K load (2000 repeaters, 42 observers), real packets.js in vm, median of 7 runs: | measure | master | before (per observer) | after (with yield) | |------------------------------|--------|-----------------------|--------------------| | total hop resolution | 67 ms | 217 ms | 271 ms | | longest main-thread block | 70 ms | 220 ms | 92 ms | The remaining longest block is cacheResolvedPaths() over the whole load (~40 ms) plus grouping and the first observer group. The job still runs off the critical path: rows render with hex hops first and are upgraded when it finishes (#1693). Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- public/packets.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/public/packets.js b/public/packets.js index 88325aaba..177cbd7c2 100644 --- a/public/packets.js +++ b/public/packets.js @@ -1069,8 +1069,14 @@ } // ensureHopResolver() is idempotent and awaited once inside resolveHops; // the resolve itself is local computation, so the loop costs no requests. + // #165 — but it is CPU: ~42 groups on a 30K load took one ~220 ms block. + // Yield to the event loop between groups so input and paint get through. + let first = true; for (const [obs, set] of groups) { - if (set.size) await resolveHops([...set], obs || undefined); + if (!set.size) continue; + if (!first) await new Promise(r => setTimeout(r, 0)); + first = false; + await resolveHops([...set], obs || undefined); } } From 2fd4e65c8bcd90cd8ad492a469969d53abda6c4f Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 07:55:51 +0000 Subject: [PATCH 14/24] test(perf): add a reproducible bench for packets-page hop resolution (#165) scripts/bench-hop-resolution.js times the initial-load hop resolution of the real packets.js on a deterministic synthetic 30K packet load: - 2000 repeaters in three regions; - 42 observers, 27 of them with coordinates; - 0-8 hops per packet, 30% with resolved_path. It reports, as medians of 7 runs, the total time, the longest main-thread block and the cache size. Point it at another checkout's public/ for a baseline: packets.js from before #165 runs the old single resolveHops() sequence. | public/ from | total | longest block | cache entries | |-------------------------------|--------|---------------|---------------| | origin/master | 60 ms | 61 ms | 2236 | | upstream port as-is (18afd8d) | 396 ms | 396 ms | 28962 | | this branch | 251 ms | 87 ms | 28962 | Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- scripts/bench-hop-resolution.js | 134 ++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 scripts/bench-hop-resolution.js diff --git a/scripts/bench-hop-resolution.js b/scripts/bench-hop-resolution.js new file mode 100644 index 000000000..70b4e086a --- /dev/null +++ b/scripts/bench-hop-resolution.js @@ -0,0 +1,134 @@ +#!/usr/bin/env node +/* + * scripts/bench-hop-resolution.js — #165 perf proof for the packets page. + * + * Times the initial-load hop resolution of the REAL public/packets.js on a + * synthetic ~30K packet load, in a vm sandbox (network stubbed): + * - 2000 repeaters in three regions, 42 observers (27 with coordinates), + * - 0-8 hops per packet (85% 1-byte, 15% 2-byte), 30% with resolved_path. + * + * Reports the median of 7 runs of: + * - total time from cacheResolvedPaths() to the last resolved hop, + * - the longest main-thread block (max gap of a setTimeout(0) chain), + * - the cache size left behind. + * + * Works on any checkout. Point it at a public/ dir from before #165 + * (e.g. `git worktree add /tmp/m origin/master`) for the baseline: when + * packets.js has no resolveHopsForPackets() it runs the old sequence + * (one resolveHops() over all hops) instead. + * + * Usage: node scripts/bench-hop-resolution.js [publicDir] (default: public) + */ +'use strict'; +const fs = require('fs'); +const path = require('path'); +const vm = require('vm'); + +const PUBLIC = path.resolve(process.argv[2] || path.join(__dirname, '..', 'public')); +const RUNS = 7; + +// Deterministic data (mulberry32). +function rng(a) { + return () => { + a |= 0; a = a + 0x6D2B79F5 | 0; + let t = Math.imul(a ^ a >>> 15, 1 | a); + t = t + Math.imul(t ^ t >>> 7, 61 | t) ^ t; + return ((t ^ t >>> 14) >>> 0) / 4294967296; + }; +} +const r = rng(42); +const hex = n => Array.from({ length: n }, () => Math.floor(r() * 256).toString(16).padStart(2, '0')).join(''); +const centers = [[51.1, 3.7], [50.85, 4.35], [51.2, 4.4]]; +const nodes = Array.from({ length: 2000 }, (_, i) => { + const c = centers[i % 3]; + return { public_key: hex(32), name: 'N' + i, role: 'repeater', lat: c[0] + (r() - 0.5), lon: c[1] + (r() - 0.5) }; +}); +const observers = Array.from({ length: 42 }, (_, i) => { + const c = centers[i % 3]; + return i < 27 + ? { id: 'OBS' + i, iata: 'X' + (i % 3), lat: c[0] + (r() - 0.5) * 0.5, lon: c[1] + (r() - 0.5) * 0.5 } + : { id: 'OBS' + i, iata: 'X' + (i % 3), lat: null, lon: null }; +}); +const iataCoords = { X0: { lat: 51.1, lon: 3.7 }, X1: { lat: 50.85, lon: 4.35 }, X2: { lat: 51.2, lon: 4.4 } }; +const packets = Array.from({ length: 30000 }, (_, i) => { + const len = Math.floor(r() * 9), bytes = r() < 0.85 ? 1 : 2; + const hops = Array.from({ length: len }, () => nodes[Math.floor(r() * nodes.length)].public_key.slice(0, bytes * 2).toUpperCase()); + const p = { id: i, hash: 'h' + i, observer_id: observers[Math.floor(r() * observers.length)].id, path_json: JSON.stringify(hops) }; + if (r() < 0.3) { + p.resolved_path = JSON.stringify(hops.map(h => (r() < 0.8 + ? ((nodes.find(n => n.public_key.toUpperCase().startsWith(h)) || {}).public_key || null) : null))); + } + return p; +}); + +function load() { + const store = {}; + const ctx = { + window: { addEventListener() {}, dispatchEvent() {}, innerWidth: 1280 }, + document: { + readyState: 'complete', + createElement: () => ({ style: {}, dataset: {}, classList: { add() {}, remove() {} }, appendChild() {}, setAttribute() {} }), + head: { appendChild() {} }, body: { appendChild() {}, removeChild() {} }, + getElementById: () => null, querySelector: () => null, querySelectorAll: () => [], + addEventListener() {}, removeEventListener() {}, + }, + localStorage: { getItem: k => (k in store ? store[k] : null), setItem: (k, v) => { store[k] = String(v); }, removeItem() {} }, + console: { log() {}, warn() {}, error: console.error }, + Math, Object, Array, Number, Date, Map, Set, JSON, String, Boolean, RegExp, Error, TypeError, Promise, + parseInt, parseFloat, isNaN, isFinite, encodeURIComponent, decodeURIComponent, URLSearchParams, + setTimeout, clearTimeout, setInterval: () => 0, clearInterval() {}, + requestAnimationFrame: () => 0, cancelAnimationFrame() {}, performance, location: { hash: '' }, + escapeHtml: s => String(s), + registerPage() {}, onWS() {}, offWS() {}, debouncedOnWS: f => f, CLIENT_TTL: {}, + fetchAllNodes: () => Promise.resolve({ nodes }), + api: p => Promise.resolve(p.startsWith('/observers') ? { observers } : p.startsWith('/iata-coords') ? { coords: iataCoords } : {}), + }; + ctx.window.localStorage = ctx.localStorage; + vm.createContext(ctx); + let legacy = false; + for (const f of ['payload-labels.js', 'packet-helpers.js', 'hop-resolver.js', 'hop-display.js', 'packets.js']) { + let src = fs.readFileSync(path.join(PUBLIC, f), 'utf8'); + if (f === 'packets.js' && !/async function resolveHopsForPackets\(/.test(src)) { + // Before #165: expose the two functions the old initial load used. + legacy = true; + src = src.replace('window._packetsTestAPI = {', + 'window._packetsTestAPI = { resolveHops, cacheResolvedPaths, _hopCacheSize: () => Object.keys(hopNameCache).length,'); + } + vm.runInContext(src, ctx, { filename: f }); + for (const k of Object.keys(ctx.window)) ctx[k] = ctx.window[k]; + } + return { T: ctx.window._packetsTestAPI, ctx, legacy }; +} + +(async () => { + const times = [], blocks = []; + let size = 0, legacyMode = false; + for (let run = 0; run < RUNS; run++) { + const { T, ctx, legacy } = load(); + legacyMode = legacy; + const pk = packets.map(p => ({ ...p })); + await T.cacheResolvedPaths(pk.slice(0, 1)); // HopResolver.init outside the timing + let maxGap = 0, last = performance.now(), on = true; + (function tick() { const n = performance.now(); maxGap = Math.max(maxGap, n - last); last = n; if (on) setTimeout(tick, 0); })(); + const t0 = performance.now(); + await T.cacheResolvedPaths(pk); + if (legacy) { + const all = new Set(); + for (const p of pk) { try { ctx.getParsedPath(p).forEach(h => all.add(h)); } catch (e) { /* skip */ } } + if (all.size) await T.resolveHops([...all]); + } else { + await T.resolveHopsForPackets(pk); + } + const t1 = performance.now(); + await new Promise(res => setTimeout(res, 0)); // let the last gap be recorded + on = false; + times.push(t1 - t0); blocks.push(maxGap); size = T._hopCacheSize(); + } + const median = a => a.slice().sort((x, y) => x - y)[Math.floor(a.length / 2)]; + console.log(JSON.stringify({ + publicDir: PUBLIC, mode: legacyMode ? 'single resolve (pre-#165)' : 'per observer (#165)', + packets: packets.length, observers: observers.length, nodes: nodes.length, runs: RUNS, + total_ms_median: +median(times).toFixed(1), longest_block_ms_median: +median(blocks).toFixed(1), + cache_entries: size, + })); +})(); From 9d98f03a2515ad272da9b90fa54d0717c2169258 Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 08:00:56 +0000 Subject: [PATCH 15/24] test(packets): keep the list's ambiguity summary visible in a clipped path (#165) Browser check of the port: the path cell in the packets list is narrow (overflow: hidden). The .hop-path-warn summary is appended after the last hop, so on any path that overflows it is clipped. It is exactly the long paths where the warning matters. It is also a child of .path-hops, so _finalizePathOverflow() counts it as a hidden hop and the "+N more hops" pill reads one too many. The tests require: - the summary precedes the first hop in list form; - the detail form keeps per-hop badges and no summary; - the overflow pill counts hops only (fake DOM: one clipped hop and a clipped summary give "+1"). _finalizePathOverflow is exposed on _packetsTestAPI for this. Red on this commit: 2 failed, 14 passed. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- public/packets.js | 1 + test-issue-165-hop-resolution-per-observer.js | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/public/packets.js b/public/packets.js index 177cbd7c2..342856a6d 100644 --- a/public/packets.js +++ b/public/packets.js @@ -4254,6 +4254,7 @@ resolveHopsForPackets, resolveIncomingHops, cacheResolvedPaths, + _finalizePathOverflow, HOP_CACHE_MAX, _hopCacheGet: hopCacheGet, _hopCacheSize: function() { return hopNameCache.size; }, diff --git a/test-issue-165-hop-resolution-per-observer.js b/test-issue-165-hop-resolution-per-observer.js index 6957be3b7..18eb0120f 100644 --- a/test-issue-165-hop-resolution-per-observer.js +++ b/test-issue-165-hop-resolution-per-observer.js @@ -17,6 +17,8 @@ * C. Resolving per observer costs more than one global resolve, so * resolveHopsForPackets() yields to the event loop between observer * groups instead of blocking the page for the whole initial load. + * UI. The list's one-per-path ambiguity indicator stays visible when the + * path overflows its cell, and is not counted as a hidden hop. * * Run: node test-issue-165-hop-resolution-per-observer.js */ @@ -249,6 +251,34 @@ test('a timer runs between two observer groups', async () => { ', at timer ' + seenAt + ', after ' + after + ')'); }); +section('#165 UI: the list summary survives a clipped path cell'); + +test('the summary indicator comes before the hops, so overflow cannot clip it', async () => { + const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_A]); + await T.resolveHops(['ef', 'c1'], 'OBS-A'); + const html = T.renderPath(['c1', 'ef'], 'OBS-A', { summary: true }); + const warn = html.indexOf('hop-path-warn'), firstHop = html.indexOf('class="hop'); + assert(warn !== -1, 'the summary indicator is rendered: ' + html); + assert(warn < firstHop, 'the indicator precedes the first hop: ' + html); + const detail = T.renderPath(['c1', 'ef'], 'OBS-A'); + assert(detail.indexOf('hop-path-warn') === -1 && /hop-conflict-btn/.test(detail), + 'the detail form keeps per-hop badges and no summary'); +}); + +test('the overflow pill counts hops only, not the summary indicator', async () => { + const T = loadPackets([FAR], []); + // Fake .path-hops host 100px wide: [warn past the edge] [hop inside] [arrow] [hop past the edge]. + const el = (cls, left, right) => ({ classList: { contains: c => cls.split(' ').includes(c) }, getBoundingClientRect: () => ({ left, right }) }); + const appended = []; + const host = { + dataset: {}, children: [el('hop-path-warn status-warn', 120, 140), el('hop', 0, 50), el('arrow', 50, 60), el('hop', 60, 160)], + querySelector: () => null, getBoundingClientRect: () => ({ right: 100 }), appendChild: c => appended.push(c), + }; + T._finalizePathOverflow({ querySelectorAll: () => [host] }); + assert(appended.length === 1 && appended[0].textContent === '+1', + 'expected one "+1" pill for the one clipped hop; got ' + JSON.stringify(appended.map(a => a.textContent))); +}); + (async () => { for (const t of pending) { if (t.title) { console.log('\n=== ' + t.title.trim() + ' ==='); continue; } From 37dd2872a236d335d3190f9acfa9e7f7df0b7150 Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 08:02:22 +0000 Subject: [PATCH 16/24] fix(packets): lead the path with its ambiguity summary in the list (#165) renderPath(..., { summary: true }) now puts .hop-path-warn before the hops instead of after them, and the overflow pill skips it when counting hidden hops. The list's path cell clips overflow (#1122). With the summary last, a path long enough to overflow hid it, so the warning vanished on exactly the paths with the most hops. In the browser (desktop 1440, fixture packet with three genuinely ambiguous, server-null hops) the "warning 3" indicator was clipped before this change and is visible after it. The overflow pill stays a count of hops. Test fixes in the same commit, the test intent unchanged: - the first-hop search matched class="hop-path-warn" itself; it now matches the hop class exactly; - the detail-form check asserted a conflict badge, which needs an IATA region this sandbox does not set; it now asserts no summary plus hop-ambiguous. Badge rendering is covered in test-issue-165-hop-ambiguity-badge.js. Both tests are still red on the previous commit. test-issue-165-hop-resolution-per-observer.js: 16 passed, 0 failed. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- public/packets.js | 9 ++++++--- public/style.css | 5 +++-- test-issue-165-hop-resolution-per-observer.js | 7 ++++--- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/public/packets.js b/public/packets.js index 342856a6d..753f2dd19 100644 --- a/public/packets.js +++ b/public/packets.js @@ -1162,9 +1162,11 @@ } if (!uncertain) return body; const label = uncertain + ' of ' + filtered.length + ' hops have more than one candidate'; - return body + ' ' + - uncertain + ''; + uncertain + '' + body; } let directPacketId = null; @@ -2700,7 +2702,8 @@ // is past the host's right edge. for (var j = 0; j < children.length; j++) { var ch = children[j]; - if (ch.classList.contains('arrow')) continue; + // #165 — the path's ambiguity summary is not a hop. + if (ch.classList.contains('arrow') || ch.classList.contains('hop-path-warn')) continue; var r = ch.getBoundingClientRect(); if (r.left >= hostRight || r.right > hostRight + 0.5) hidden++; } diff --git a/public/style.css b/public/style.css index 342455158..0c1bc760c 100644 --- a/public/style.css +++ b/public/style.css @@ -3162,8 +3162,9 @@ button.ch-item.ch-item-encrypted .ch-badge { filter: grayscale(0.6); } .hop-group .hop-unreliable-btn { margin-left: 2px; } /* One indicator for a whole path, used by the packets list. The detail pane - keeps a badge per hop, because that is where the candidates get read. */ -.hop-path-warn { display: inline-flex; align-items: center; gap: 2px; margin-left: 6px; + keeps a badge per hop, because that is where the candidates get read. It + leads the path so a clipped (overflowing) path cell cannot hide it. */ +.hop-path-warn { display: inline-flex; align-items: center; gap: 2px; margin-right: 6px; flex: 0 0 auto; background: var(--status-yellow); color: var(--palette-gray-900); border-radius: 4px; font-size: 11px; font-weight: 700; padding: 1px 5px; line-height: 1.2; vertical-align: middle; white-space: nowrap; } diff --git a/test-issue-165-hop-resolution-per-observer.js b/test-issue-165-hop-resolution-per-observer.js index 18eb0120f..a91148c52 100644 --- a/test-issue-165-hop-resolution-per-observer.js +++ b/test-issue-165-hop-resolution-per-observer.js @@ -257,12 +257,13 @@ test('the summary indicator comes before the hops, so overflow cannot clip it', const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_A]); await T.resolveHops(['ef', 'c1'], 'OBS-A'); const html = T.renderPath(['c1', 'ef'], 'OBS-A', { summary: true }); - const warn = html.indexOf('hop-path-warn'), firstHop = html.indexOf('class="hop'); + const warn = html.indexOf('hop-path-warn'), firstHop = html.search(/class="hop[ "]/); assert(warn !== -1, 'the summary indicator is rendered: ' + html); assert(warn < firstHop, 'the indicator precedes the first hop: ' + html); const detail = T.renderPath(['c1', 'ef'], 'OBS-A'); - assert(detail.indexOf('hop-path-warn') === -1 && /hop-conflict-btn/.test(detail), - 'the detail form keeps per-hop badges and no summary'); + // (Per-hop badge rendering itself is covered in test-issue-165-hop-ambiguity-badge.js.) + assert(detail.indexOf('hop-path-warn') === -1 && /hop-ambiguous/.test(detail), + 'the detail form has no path summary and keeps the per-hop ambiguity: ' + detail); }); test('the overflow pill counts hops only, not the summary indicator', async () => { From 8310f8a100457d9ea059df841701db308de4d494 Mon Sep 17 00:00:00 2001 From: dborup Date: Sat, 3 Oct 2026 13:17:58 +0000 Subject: [PATCH 17/24] fix(packets): keep the path summary out of the +N popover (upstream 2557894d) (#165) Ports upstream commit 2557894d ("lead the path with its warning so the clip edge never hides it"), from upstream PR 2099's final head. Most of it was already on this branch: 37dd2872 puts .hop-path-warn before the hops, with flex: 0 0 auto, and leaves it out of the +N count. What remained: - the +N popover still listed the summary as a path item. Its body is now built by _pathPopoverHtml(host), which skips the pill and .hop-path-warn (upstream does the same in _pathHopSegments, a helper this fork does not have); - renderPath() returns `warn + body`, upstream's shape, so the ported structural assertion matches; - margin-right 4px, as upstream. Tests (written first, red before this change, 1 + 2 failures): - test-issue-165-hop-resolution-per-observer.js: the popover lists hops and arrows, not the summary or the pill (fake host). 17/17. - test-issue-165-hop-ambiguity-badge.js: upstream's two assertions, adapted to _pathPopoverHtml. 34/34. sh test-all.sh: 202/202 files. The packets E2Es are green locally: #1128 layout 5/5, #1128 multi-viewport 15/15, #1146 11/11. Relates to #165 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ANjbSkzh7MxgT51Dzd7xeZ --- public/packets.js | 29 ++++++++++++------- public/style.css | 2 +- test-issue-165-hop-ambiguity-badge.js | 8 +++++ test-issue-165-hop-resolution-per-observer.js | 15 ++++++++++ 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/public/packets.js b/public/packets.js index ec5aff9e0..70fb6d4b9 100644 --- a/public/packets.js +++ b/public/packets.js @@ -1210,9 +1210,10 @@ const label = uncertain + ' of ' + filtered.length + ' hops have more than one candidate'; // #165 — first, not last: the list's path cell clips overflow, and the // long paths that overflow are the ones the warning matters most for. - return '' + - uncertain + '' + body; + uncertain + ''; + return warn + body; } let directPacketId = null; @@ -2806,6 +2807,19 @@ } } + // Popover body for a .path-hops host: clone all children except the pill + // and (#165, upstream 2557894d) the path's ambiguity summary, preserving + // rendered chips/arrows. + function _pathPopoverHtml(host) { + var inner = '
Full path (' + (host.children.length) + ' items)
'; + var kids = Array.prototype.slice.call(host.children); + for (var i = 0; i < kids.length; i++) { + if (kids[i].classList.contains('path-overflow-pill') || kids[i].classList.contains('hop-path-warn')) continue; + inner += kids[i].outerHTML; + } + return inner + '
'; + } + // Delegated click for path overflow pills — show popover of full path. function _wirePathOverflowPopover() { if (window.__pathOverflowWired) return; @@ -2829,15 +2843,7 @@ dismiss(); var pop = document.createElement('div'); pop.className = 'path-popover'; - // Clone all children except the pill, preserving rendered chips/arrows. - var inner = '
Full path (' + (host.children.length) + ' items)
'; - var kids = Array.prototype.slice.call(host.children); - for (var i = 0; i < kids.length; i++) { - if (kids[i].classList.contains('path-overflow-pill')) continue; - inner += kids[i].outerHTML; - } - inner += '
'; - pop.innerHTML = inner; + pop.innerHTML = _pathPopoverHtml(host); document.body.appendChild(pop); var r = pill.getBoundingClientRect(); // #1128 (Bug 2): position below by default, but flip ABOVE when there @@ -4300,6 +4306,7 @@ resolveIncomingHops, cacheResolvedPaths, _finalizePathOverflow, + _pathPopoverHtml, HOP_CACHE_MAX, _hopCacheGet: hopCacheGet, _hopCacheSize: function() { return hopNameCache.size; }, diff --git a/public/style.css b/public/style.css index 0c1bc760c..a616033b6 100644 --- a/public/style.css +++ b/public/style.css @@ -3164,7 +3164,7 @@ button.ch-item.ch-item-encrypted .ch-badge { filter: grayscale(0.6); } /* One indicator for a whole path, used by the packets list. The detail pane keeps a badge per hop, because that is where the candidates get read. It leads the path so a clipped (overflowing) path cell cannot hide it. */ -.hop-path-warn { display: inline-flex; align-items: center; gap: 2px; margin-right: 6px; flex: 0 0 auto; +.hop-path-warn { display: inline-flex; align-items: center; gap: 2px; margin-right: 4px; flex: 0 0 auto; background: var(--status-yellow); color: var(--palette-gray-900); border-radius: 4px; font-size: 11px; font-weight: 700; padding: 1px 5px; line-height: 1.2; vertical-align: middle; white-space: nowrap; } diff --git a/test-issue-165-hop-ambiguity-badge.js b/test-issue-165-hop-ambiguity-badge.js index af97e571d..ab59a626d 100644 --- a/test-issue-165-hop-ambiguity-badge.js +++ b/test-issue-165-hop-ambiguity-badge.js @@ -190,6 +190,14 @@ console.log('\n=== #2097: the list summarises, the detail pane does not ==='); 'the detail pane calls renderPath without summary, so it keeps per-hop badges'); assert(/hop-path-warn/.test(src), 'the summarised form emits a single per-path indicator'); + // Upstream 2557894d: .path-hops clips at its edge. Trailing the hops, the + // indicator was what got clipped, so it leads the path, and the +N popover + // leaves it out of the hop list. + assert(/return warn \+ body;/.test(src), + 'the indicator leads the path, so the edge clips hops, never the indicator'); + const seg = (src.match(/function _pathPopoverHtml\(host\) \{[\s\S]*?\n \}/) || [''])[0]; + assert(/hop-path-warn/.test(seg), + 'the +N popover leaves the indicator out of the hop list'); } console.log('\n=== #2097: the observer position anchors the pick ==='); diff --git a/test-issue-165-hop-resolution-per-observer.js b/test-issue-165-hop-resolution-per-observer.js index a91148c52..1efb5cc4e 100644 --- a/test-issue-165-hop-resolution-per-observer.js +++ b/test-issue-165-hop-resolution-per-observer.js @@ -280,6 +280,21 @@ test('the overflow pill counts hops only, not the summary indicator', async () = 'expected one "+1" pill for the one clipped hop; got ' + JSON.stringify(appended.map(a => a.textContent))); }); +test('the +N popover lists the hops, not the summary indicator (upstream 2557894d)', async () => { + const T = loadPackets([FAR], []); + assert(typeof T._pathPopoverHtml === 'function', '_pathPopoverHtml is exposed'); + const kid = (cls, html) => ({ classList: { contains: c => cls.split(' ').includes(c) }, outerHTML: html }); + const host = { children: [ + kid('hop-path-warn status-warn', '!2'), + kid('hop hop-named', 'A'), kid('arrow', '→'), + kid('hop hop-named', 'B'), kid('path-overflow-pill', '+1'), + ] }; + const html = T._pathPopoverHtml(host); + assert(!/hop-path-warn/.test(html), 'the summary indicator is left out: ' + html); + assert(!/path-overflow-pill/.test(html), 'the pill is left out: ' + html); + assert(/>AB { for (const t of pending) { if (t.title) { console.log('\n=== ' + t.title.trim() + ' ==='); continue; } From 50e462bee335f72dc2a2876afa393630434ce592 Mon Sep 17 00:00:00 2001 From: dborup Date: Wed, 7 Oct 2026 08:30:15 +0000 Subject: [PATCH 18/24] test(server): reproduce grouped rows without resolved_path (#165) PR #185 review F1: /api/packets?groupByHash=true sends no resolved_path, so the packets page's default view guesses every hop on the client and its list summary flags hops the server resolved (23 of 60 rows on the CI fixture). The tests seed a transmission heard by two observers with different paths and resolved_paths, and require the grouped row to carry the resolved_path of the observation it displays (same observer_id and path_json), from the in-memory store and from the SQL fallback. A row whose observation has none omits the key. BenchmarkGroupedPackets165 measures the desktop page (limit 50000) on 30K transmissions and reports its JSON and gzip size, for the cost check #165 asked for. Red on the branch: both tests fail, the grouped rows have no resolved_path. Co-Authored-By: Claude Opus 5.5 --- .../grouped_resolved_path_165_bench_test.go | 74 +++++++++++++ cmd/server/grouped_resolved_path_165_test.go | 102 ++++++++++++++++++ 2 files changed, 176 insertions(+) create mode 100644 cmd/server/grouped_resolved_path_165_bench_test.go create mode 100644 cmd/server/grouped_resolved_path_165_test.go diff --git a/cmd/server/grouped_resolved_path_165_bench_test.go b/cmd/server/grouped_resolved_path_165_bench_test.go new file mode 100644 index 000000000..efafae177 --- /dev/null +++ b/cmd/server/grouped_resolved_path_165_bench_test.go @@ -0,0 +1,74 @@ +package main + +import ( + "bytes" + "compress/gzip" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "testing" + "time" +) + +// BenchmarkGroupedPackets165 measures the default packets-page request +// (/api/packets?groupByHash=true, desktop limit 50000) on 30K transmissions +// with two observations each and a 3-hop resolved_path, the shape #165 +// asked to be checked before the grouped rows carry resolved_path. It +// reports the JSON size of the page as json-bytes, and gzipped (the server +// compresses responses, compress.go) as gzip-bytes. +// +// go test -run '^$' -bench GroupedPackets165 -benchtime 20x . +func BenchmarkGroupedPackets165(b *testing.B) { + const nTx = 30000 + db := setupTestDB(b) + defer db.Close() + now := time.Now().UTC() + ts := now.Format(time.RFC3339) + epoch := now.Add(-time.Minute).Unix() + tx, err := db.conn.Begin() + if err != nil { + b.Fatal(err) + } + for i := 1; i <= 2; i++ { + tx.Exec(`INSERT INTO observers (id, name, iata, last_seen, first_seen, packet_count) VALUES (?, ?, 'SJC', ?, ?, 1)`, + fmt.Sprintf("OBS%02d", i), fmt.Sprintf("O%d", i), ts, ts) + } + // Pubkeys look random, as real ones do, so gzip-bytes is not flattered. + pk := func(i, h int) string { + sum := sha256.Sum256([]byte(fmt.Sprint(i, ":", h))) + return fmt.Sprintf("%02x", (i+h)%256) + hex.EncodeToString(sum[:])[2:] + } + for i := 1; i <= nTx; i++ { + tx.Exec(`INSERT INTO transmissions (id, raw_hex, hash, first_seen, route_type, payload_type, decoded_json) VALUES (?, ?, ?, ?, 1, 4, '{}')`, + i, "11223344556677889900aabbccddeeff", fmt.Sprintf("%016x", i), ts) + path := fmt.Sprintf(`["%02x","%02x","%02x"]`, (i+0)%256, (i+1)%256, (i+2)%256) + rp := fmt.Sprintf(`["%s","%s",null]`, pk(i, 0), pk(i, 1)) + tx.Exec(`INSERT INTO observations (transmission_id, observer_idx, snr, rssi, path_json, timestamp, resolved_path) VALUES (?, 1, 5, -90, ?, ?, ?)`, i, path, epoch-int64(i), rp) + tx.Exec(`INSERT INTO observations (transmission_id, observer_idx, snr, rssi, path_json, timestamp, resolved_path) VALUES (?, 2, 5, -90, ?, ?, ?)`, + i, fmt.Sprintf(`["%02x"]`, (i+2)%256), epoch-int64(i), fmt.Sprintf(`["%s"]`, pk(i, 2))) + } + if err := tx.Commit(); err != nil { + b.Fatal(err) + } + store := NewPacketStore(db, nil) + if err := store.Load(); err != nil { + b.Fatal(err) + } + q := PacketQuery{Limit: 50000} + r := store.QueryGroupedPackets(q) + if r.Total != nTx { + b.Fatalf("total = %d, want %d", r.Total, nTx) + } + body, _ := json.Marshal(r) + b.ResetTimer() + for i := 0; i < b.N; i++ { + store.QueryGroupedPackets(q) + } + var gz bytes.Buffer + zw := gzip.NewWriter(&gz) + zw.Write(body) + zw.Close() + b.ReportMetric(float64(len(body)), "json-bytes") + b.ReportMetric(float64(gz.Len()), "gzip-bytes") +} diff --git a/cmd/server/grouped_resolved_path_165_test.go b/cmd/server/grouped_resolved_path_165_test.go new file mode 100644 index 000000000..7df873da5 --- /dev/null +++ b/cmd/server/grouped_resolved_path_165_test.go @@ -0,0 +1,102 @@ +package main + +import ( + "encoding/json" + "strings" + "testing" + "time" +) + +// #165 — the packets page's default (grouped) view resolves hop names from +// the grouped row. Without the server's resolved_path on that row, the client +// heuristic guessed every hop and the list flagged hops the server had a +// definite answer for (23 of 60 rows on the CI fixture). The grouped row must +// carry the resolved_path of the observation it displays: the same observer +// and path_json as the row's observer_id/path_json, since the client caches +// it under hop:observer. + +const ( + rp165A = "aa11111111111111111111111111111111111111111111111111111111111111" + rp165B = "bb22222222222222222222222222222222222222222222222222222222222222" + rp165X = "aa99999999999999999999999999999999999999999999999999999999999999" +) + +// seedGroupedRP165 inserts two transmissions: +// - "165a…": observer A hears path [aa,bb] with resolved_path [A, null] +// (the header: longest path); observer B hears [aa] resolved to X. +// - "165b…": one observation with no resolved_path at all. +func seedGroupedRP165(t *testing.T, db *DB) { + t.Helper() + now := time.Now().UTC() + ts := now.Format(time.RFC3339) + epoch := now.Add(-time.Minute).Unix() + mustExec := func(q string, args ...interface{}) { + t.Helper() + if _, err := db.conn.Exec(q, args...); err != nil { + t.Fatalf("seed: %v\n%s", err, q) + } + } + mustExec(`INSERT INTO observers (id, name, iata, last_seen, first_seen, packet_count) VALUES ('OBSA', 'A', 'SJC', ?, ?, 1)`, ts, ts) + mustExec(`INSERT INTO observers (id, name, iata, last_seen, first_seen, packet_count) VALUES ('OBSB', 'B', 'SFO', ?, ?, 1)`, ts, ts) + mustExec(`INSERT INTO transmissions (id, raw_hex, hash, first_seen, route_type, payload_type, decoded_json) VALUES (1, 'AABB', '165a000000000001', ?, 1, 4, '{}')`, ts) + mustExec(`INSERT INTO transmissions (id, raw_hex, hash, first_seen, route_type, payload_type, decoded_json) VALUES (2, 'CCDD', '165b000000000002', ?, 1, 4, '{}')`, ts) + mustExec(`INSERT INTO observations (transmission_id, observer_idx, snr, rssi, path_json, timestamp, resolved_path) VALUES (1, 2, 5, -90, '["aa"]', ?, ?)`, epoch, `["`+rp165X+`"]`) + mustExec(`INSERT INTO observations (transmission_id, observer_idx, snr, rssi, path_json, timestamp, resolved_path) VALUES (1, 1, 6, -88, '["aa","bb"]', ?, ?)`, epoch, `["`+rp165A+`",null]`) + mustExec(`INSERT INTO observations (transmission_id, observer_idx, snr, rssi, path_json, timestamp) VALUES (2, 1, 7, -80, '["cc"]', ?)`, epoch) +} + +func groupedRowsByHash165(t *testing.T, r *PacketResult) map[string]map[string]interface{} { + t.Helper() + out := map[string]map[string]interface{}{} + for _, p := range r.Packets { + h, _ := p["hash"].(string) + out[h] = p + } + if len(out) != 2 { + t.Fatalf("expected 2 grouped rows, got %d: %#v", len(out), r.Packets) + } + return out +} + +func assertGroupedRP165(t *testing.T, rows map[string]map[string]interface{}) { + t.Helper() + a := rows["165a000000000001"] + if got := strings.ToUpper(a["observer_id"].(string)); got != "OBSA" { + t.Fatalf("header observer = %q, want OBSA (longest path)", got) + } + rp, ok := a["resolved_path"] + if !ok { + t.Fatalf("grouped row carries no resolved_path; the client then guesses hops the server resolved (#165): %#v", a) + } + b, _ := json.Marshal(rp) + if want := `["` + rp165A + `",null]`; string(b) != want { + t.Fatalf("grouped resolved_path = %s, want the header observation's %s", b, want) + } + if _, ok := rows["165b000000000002"]["resolved_path"]; ok { + t.Fatalf("a row whose observation has no resolved_path must omit the key, got %#v", rows["165b000000000002"]["resolved_path"]) + } +} + +func TestGroupedPacketsCarryHeaderResolvedPath165(t *testing.T) { + db := setupTestDB(t) + defer db.Close() + seedGroupedRP165(t, db) + store := NewPacketStore(db, nil) + if err := store.Load(); err != nil { + t.Fatal(err) + } + assertGroupedRP165(t, groupedRowsByHash165(t, store.QueryGroupedPackets(PacketQuery{Limit: 50}))) + // Second call is served from the grouped sort cache; same contract. + assertGroupedRP165(t, groupedRowsByHash165(t, store.QueryGroupedPackets(PacketQuery{Limit: 50}))) +} + +func TestGroupedPacketsSQLCarryHeaderResolvedPath165(t *testing.T) { + db := setupTestDB(t) + defer db.Close() + seedGroupedRP165(t, db) + r, err := db.QueryGroupedPackets(PacketQuery{Limit: 50}) + if err != nil { + t.Fatal(err) + } + assertGroupedRP165(t, groupedRowsByHash165(t, r)) +} From 6649f867c4de3dd436f582a2535c8e33e8ae2e4b Mon Sep 17 00:00:00 2001 From: dborup Date: Wed, 7 Oct 2026 08:30:26 +0000 Subject: [PATCH 19/24] fix(server): send the displayed observation's resolved_path in grouped rows (#165) PR #185 review F1. The packets page resolves hop names from the grouped row, under hop:observer. Without the server's answer there, the client heuristic guessed every hop of the default view, and the list summary flagged hops the server had resolved. - In-memory store: groupedPageWithRP adds, per page row, the resolved_path of the observation pickBestObservation copied onto the tx (same observer and path, headerObservationID). The ids are read under s.mu; the SQL runs after it, as ONE query for the whole page (ids as a json_each parameter, no bound-variable limit). It bypasses the resolved-path LRU, which a 50K page would flush. - SQL fallback: the grouped query selects o.resolved_path of the row it already joins, or NULL on a schema without the column (v2). - The stored JSON is passed through as json.RawMessage (checked with json.Valid), not parsed and re-encoded. Cost, BenchmarkGroupedPackets165 (30K tx, 2 observations each, 3-hop paths, random-looking pubkeys; 3 runs, 4-core cloud VM): per request 28.2-29.8 ms -> 90.1-95.6 ms JSON 12.24 MB -> 16.95 MB (+38 %) gzip 0.25 MB -> 2.56 MB (synthetic raw_hex/decoded_json compress far better than real ones) Most of the added time is modernc/sqlite stepping 30K rows (~2 us per row); a statement per row, or 500-id batches, measured 117-126 ms. Co-Authored-By: Claude Opus 5.5 --- cmd/server/db.go | 26 ++++++++++++---- cmd/server/resolved_index.go | 60 ++++++++++++++++++++++++++++++++++++ cmd/server/store.go | 47 ++++++++++++++++++++++++++-- 3 files changed, 124 insertions(+), 9 deletions(-) diff --git a/cmd/server/db.go b/cmd/server/db.go index 5c8a88737..a5aa80027 100644 --- a/cmd/server/db.go +++ b/cmd/server/db.go @@ -792,6 +792,12 @@ func (db *DB) QueryGroupedPackets(q PacketQuery) (*PacketResult, error) { if db.hasScopeName() { groupedScopeCol = ", t.scope_name" } + // #165 — the displayed observation's resolved_path. Always one column + // (NULL without the column), so the Scan below does not depend on it. + groupedRPCol := "NULL" + if db.hasResolvedPath() { + groupedRPCol = "o.resolved_path" + } var querySQL string if db.isV3() { querySQL = fmt.Sprintf(`SELECT t.hash, t.first_seen, t.raw_hex, t.decoded_json, t.payload_type, t.route_type, @@ -799,7 +805,7 @@ func (db *DB) QueryGroupedPackets(q PacketQuery) (*PacketResult, error) { COALESCE((SELECT COUNT(DISTINCT oi.observer_idx) FROM observations oi WHERE oi.transmission_id = t.id), 0) AS observer_count, COALESCE((SELECT MAX(strftime('%%Y-%%m-%%dT%%H:%%M:%%fZ', oi.timestamp, 'unixepoch')) FROM observations oi WHERE oi.transmission_id = t.id), t.first_seen) AS latest, obs.id AS observer_id, obs.name AS observer_name, COALESCE(obs.iata, '') AS observer_iata, - o.snr, o.rssi, o.path_json, + o.snr, o.rssi, o.path_json, `+groupedRPCol+`, COALESCE((SELECT GROUP_CONCAT(DISTINCT obi.iata) FROM observations oi JOIN observers obi ON obi.rowid = oi.observer_idx WHERE oi.transmission_id = t.id AND obi.iata IS NOT NULL AND obi.iata != ''), '') AS distinct_iatas`+groupedScopeCol+` FROM transmissions t LEFT JOIN observations o ON o.id = ( @@ -814,7 +820,7 @@ func (db *DB) QueryGroupedPackets(q PacketQuery) (*PacketResult, error) { COALESCE((SELECT COUNT(DISTINCT oi.observer_id) FROM observations oi WHERE oi.transmission_id = t.id), 0) AS observer_count, COALESCE((SELECT MAX(oi.timestamp) FROM observations oi WHERE oi.transmission_id = t.id), t.first_seen) AS latest, o.observer_id, o.observer_name, COALESCE(obs2.iata, '') AS observer_iata, - o.snr, o.rssi, o.path_json, + o.snr, o.rssi, o.path_json, `+groupedRPCol+`, COALESCE((SELECT GROUP_CONCAT(DISTINCT obi.iata) FROM observations oi JOIN observers obi ON obi.id = oi.observer_id WHERE oi.transmission_id = t.id AND obi.iata IS NOT NULL AND obi.iata != ''), '') AS distinct_iatas`+groupedScopeCol+` FROM transmissions t LEFT JOIN observations o ON o.id = ( @@ -837,7 +843,7 @@ func (db *DB) QueryGroupedPackets(q PacketQuery) (*PacketResult, error) { packets := make([]map[string]interface{}, 0) for rows.Next() { - var hash, firstSeen, rawHex, decodedJSON, latest, observerID, observerName, observerIATA, pathJSON, distinctIatasCSV sql.NullString + var hash, firstSeen, rawHex, decodedJSON, latest, observerID, observerName, observerIATA, pathJSON, resolvedPath, distinctIatasCSV sql.NullString var payloadType, routeType sql.NullInt64 var count, observerCount int var snr, rssi sql.NullFloat64 @@ -845,7 +851,7 @@ func (db *DB) QueryGroupedPackets(q PacketQuery) (*PacketResult, error) { scanArgs := []interface{}{&hash, &firstSeen, &rawHex, &decodedJSON, &payloadType, &routeType, &count, &observerCount, &latest, - &observerID, &observerName, &observerIATA, &snr, &rssi, &pathJSON, &distinctIatasCSV} + &observerID, &observerName, &observerIATA, &snr, &rssi, &pathJSON, &resolvedPath, &distinctIatasCSV} if db.hasScopeName() { scanArgs = append(scanArgs, &scopeName) } @@ -853,7 +859,7 @@ func (db *DB) QueryGroupedPackets(q PacketQuery) (*PacketResult, error) { continue } - packets = append(packets, map[string]interface{}{ + row := map[string]interface{}{ "hash": nullStr(hash), "first_seen": nullStr(firstSeen), "count": count, @@ -872,7 +878,15 @@ func (db *DB) QueryGroupedPackets(q PacketQuery) (*PacketResult, error) { "snr": nullFloat(snr), "rssi": nullFloat(rssi), "scope_name": nullStr(scopeName), - }) + } + // #165 — the displayed observation's resolved_path, as the in-memory + // store sends it (groupedPageWithRP). + if resolvedPath.Valid { + if rp := resolvedPathRaw(resolvedPath.String); rp != nil { + row["resolved_path"] = rp + } + } + packets = append(packets, row) } return &PacketResult{Packets: packets, Total: total}, nil diff --git a/cmd/server/resolved_index.go b/cmd/server/resolved_index.go index 8dc76e97c..8fd195b7b 100644 --- a/cmd/server/resolved_index.go +++ b/cmd/server/resolved_index.go @@ -11,8 +11,10 @@ package main import ( "database/sql" + "encoding/json" "hash/fnv" "log" + "strconv" "strings" "time" ) @@ -198,6 +200,64 @@ func (s *PacketStore) fetchResolvedPathsForTx(txID int) map[int][]*string { return result } +// fetchResolvedPathsForObsIDs reads resolved_path for many observations in +// one query (#165, grouped packet pages of up to 50K rows): the ids go in as +// one JSON array parameter, so there is no bound-variable limit and one +// statement to prepare. Values come back as the stored JSON, unparsed +// (resolvedPathRaw), because the caller only re-encodes them. Id 0 is +// skipped. It bypasses the LRU on purpose: a page is far larger than +// lruMaxSize and would evict every entry other endpoints rely on. +func (s *PacketStore) fetchResolvedPathsForObsIDs(ids []int) map[int]json.RawMessage { + out := make(map[int]json.RawMessage) + if s.db == nil || s.db.conn == nil || !s.db.hasResolvedPath() || len(ids) == 0 { + return out + } + var b strings.Builder + b.Grow(len(ids) * 8) + b.WriteByte('[') + n := 0 + for _, id := range ids { + if id == 0 { + continue + } + if n > 0 { + b.WriteByte(',') + } + b.WriteString(strconv.Itoa(id)) + n++ + } + b.WriteByte(']') + if n == 0 { + return out + } + rows, err := s.db.conn.Query(`SELECT o.id, o.resolved_path FROM observations o + WHERE o.id IN (SELECT value FROM json_each(?)) AND o.resolved_path IS NOT NULL`, b.String()) + if err != nil { + return out + } + defer rows.Close() + for rows.Next() { + var id int + var rpJSON sql.NullString + if rows.Scan(&id, &rpJSON) == nil && rpJSON.Valid { + if rp := resolvedPathRaw(rpJSON.String); rp != nil { + out[id] = rp + } + } + } + return out +} + +// resolvedPathRaw returns a stored resolved_path as JSON to embed in a +// response, or nil when it is empty or not a JSON array (an invalid +// RawMessage would fail the whole response's encoding). +func resolvedPathRaw(s string) json.RawMessage { + if len(s) < 2 || s[0] != '[' || !json.Valid([]byte(s)) { + return nil + } + return json.RawMessage(s) +} + // fetchResolvedPathForObs fetches resolved_path for a single observation, // using the LRU cache. func (s *PacketStore) fetchResolvedPathForObs(obsID int) []*string { diff --git a/cmd/server/store.go b/cmd/server/store.go index 7ad0237ad..2b1cdadad 100644 --- a/cmd/server/store.go +++ b/cmd/server/store.go @@ -2360,7 +2360,7 @@ func (s *PacketStore) QueryGroupedPackets(q PacketQuery) *PacketResult { cachedTxs := s.groupedCacheTxs cachedTotal := s.groupedCacheTotal s.groupedCacheMu.Unlock() - return groupedTxsToPage(cachedTxs, cachedTotal, q.Offset, q.Limit) + return s.groupedPageWithRP(cachedTxs, cachedTotal, q.Offset, q.Limit) } s.groupedCacheMu.Unlock() @@ -2386,7 +2386,48 @@ func (s *PacketStore) QueryGroupedPackets(q PacketQuery) *PacketResult { s.groupedCacheExp = time.Now().Add(3 * time.Second) s.groupedCacheMu.Unlock() - return groupedTxsToPage(txs, total, q.Offset, q.Limit) + return s.groupedPageWithRP(txs, total, q.Offset, q.Limit) +} + +// groupedPageWithRP is groupedTxsToPage plus each row's resolved_path (#165). +// The packets page resolves hop names from the grouped row, keyed by hop and +// row observer; without the server's answer it guessed every hop and flagged +// hops the server had resolved. The path is the one of the observation the +// row displays (headerObservationID), not the tx's longest-resolved one, so +// it matches the row's observer_id and path_json. Observation ids are read +// under s.mu; the SQL runs after it is released, as one batched read per +// page (fetchResolvedPathsForObsIDs) rather than one per row. +func (s *PacketStore) groupedPageWithRP(txs []*StoreTx, total, offset, limit int) *PacketResult { + res := groupedTxsToPage(txs, total, offset, limit) + if len(res.Packets) == 0 { + return res + } + page := txs[offset : offset+len(res.Packets)] + ids := make([]int, len(page)) + s.mu.RLock() + for i, tx := range page { + ids[i] = headerObservationID(tx) + } + s.mu.RUnlock() + rps := s.fetchResolvedPathsForObsIDs(ids) + for i, id := range ids { + if rp := rps[id]; rp != nil { + res.Packets[i]["resolved_path"] = rp + } + } + return res +} + +// headerObservationID returns the id of the observation pickBestObservation +// copied onto tx (same observer and path), or 0 when none matches. +// Caller holds s.mu. +func headerObservationID(tx *StoreTx) int { + for _, o := range tx.Observations { + if o != nil && o.ObserverID == tx.ObserverID && o.PathJSON == tx.PathJSON { + return o.ID + } + } + return 0 } // pagePacketResult returns a window of a PacketResult without re-allocating the slice. @@ -2441,7 +2482,7 @@ func groupedTxsToPage(txs []*StoreTx, total, offset, limit int) *PacketResult { "rssi": floatPtrOrNil(tx.RSSI), "scope_name": strOrNil(tx.ScopeName), } - // resolved_path omitted for grouped view (cold path, not worth SQL round-trip) + // resolved_path is added by groupedPageWithRP (#165). packets[i] = m } From 393b39f9f7609eba23973daee32e6528b4a3cf32 Mon Sep 17 00:00:00 2001 From: dborup Date: Wed, 7 Oct 2026 08:31:02 +0000 Subject: [PATCH 20/24] test(packets): pin the PR #185 review findings (#165) Unit tests (test-issue-165-hop-resolution-per-observer.js, test-issue-165-hop-ambiguity-badge.js), against the real modules: - F1: a grouped row as the server now sends it is not flagged for its server-resolved hop; the summary does not count another observer's bare-key entry; a server answer replaces an ambiguous client pick of the same node; cacheResolvedPaths yields on a large page and skips a repeated answer, and a different answer still replaces it. - F2: the review's two probes (MD: the summary counts per observer; MC: the heuristic does not overwrite the server's bare key). - F3: a bench-sized working set (42 observers x 690 hops) is cached without eviction; a rewritten entry is refreshed for eviction order. - F5: the +N popover title counts the hops it lists. - F6: the list counts exactly the hops the detail pane badges (HopDisplay.conflictBadgeCount), and marks them hop-uncertain. - F7: destroy() during resolveHopsForPackets or cacheResolvedPaths leaves the new cache empty. Two existing tests now give their observer an IATA code, so the ambiguous hop they expect in the summary is badged. E2E test-issue-165-grouped-hop-warn-e2e.js (registered in deploy.yml): on a cold grouped load of the fixture, every grouped row carries its observation's resolved_path, and every hop a row's summary counts is one that resolved_path leaves null; a control with resolved_path stripped still renders the summary. Red on the branch: 8 of 32 and 2 of 36 unit tests. The E2E is red against the unfixed server (24 warned rows, 26 hops the server resolved). Co-Authored-By: Claude Opus 5.5 --- .github/workflows/deploy.yml | 1 + test-issue-165-grouped-hop-warn-e2e.js | 129 +++++++++++ test-issue-165-hop-ambiguity-badge.js | 20 ++ test-issue-165-hop-resolution-per-observer.js | 208 +++++++++++++++++- 4 files changed, 356 insertions(+), 2 deletions(-) create mode 100644 test-issue-165-grouped-hop-warn-e2e.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 229aa62b7..f6f3d2e07 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -441,6 +441,7 @@ jobs: BASE_URL=http://localhost:13581 node test-slideover-1056-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1692-packets-init-parallel-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-147-packets-url-obs-e2e.js 2>&1 | tee -a e2e-output.txt + BASE_URL=http://localhost:13581 node test-issue-165-grouped-hop-warn-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-packet-detail-sender-hash-size-obs-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-96-hide-control-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-180-packets-url-modal-e2e.js 2>&1 | tee -a e2e-output.txt diff --git a/test-issue-165-grouped-hop-warn-e2e.js b/test-issue-165-grouped-hop-warn-e2e.js new file mode 100644 index 000000000..2bda5adef --- /dev/null +++ b/test-issue-165-grouped-hop-warn-e2e.js @@ -0,0 +1,129 @@ +/** + * E2E (#165, PR #185 review F1): in the default grouped packets view, the + * list's path summary (.hop-path-warn) may only count hops the server left + * unresolved for the row's observer. Before the fix the grouped rows carried + * no resolved_path, the client guessed every hop, and the list flagged hops + * the server had a definite answer for (23 of 60 rows on this fixture), while + * the flat view showed none. + * + * Checks, on a cold load of #/packets (grouped, 24 h window): + * - every grouped row carries the header observation's resolved_path; + * - every hop a row's summary counts (.hop-uncertain, as many as the + * summary's number) is one that observation's server resolved_path + * leaves null; + * - control: with resolved_path stripped from the grouped response (the + * server knows nothing), the summary still appears, so the check above + * is not passing because the summary never renders. + * + * Usage: BASE_URL=http://localhost:13581 node test-issue-165-grouped-hop-warn-e2e.js + */ +'use strict'; +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:13581'; +const LIST = BASE + '/#/packets?timeWindow=1440'; + +let passed = 0, failed = 0; +async function step(name, fn) { + try { await fn(); passed++; console.log(' ✓ ' + name); } + catch (e) { failed++; console.error(' ✗ ' + name + ': ' + e.message); } +} +function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); } + +// Rows render with hex prefixes first; hop names arrive from a background +// job. Wait for names, then for the warning count to settle. +async function loadSettled(page) { + await page.goto(LIST, { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#pktBody tr[data-hash]', { timeout: 20000 }); + await page.waitForFunction(() => document.querySelector('#pktBody .hop-named'), null, { timeout: 20000 }); + let last = -1; + for (let i = 0; i < 20; i++) { + const n = await page.evaluate(() => document.querySelectorAll('#pktBody .hop-path-warn').length); + if (n === last) break; + last = n; + await page.waitForTimeout(300); + } +} + +// Per warned row: the summary's number and the path index of every pill it +// counted (.hop-uncertain), in path order. +function warnedRows(page) { + return page.evaluate(() => [...document.querySelectorAll('#pktBody tr[data-hash]')] + .filter(tr => tr.querySelector('.path-hops .hop-path-warn')) + .map(tr => { + const pills = [...tr.querySelectorAll('.path-hops .hop')]; + return { + hash: tr.dataset.hash, + count: parseInt(tr.querySelector('.path-hops .hop-path-warn').textContent, 10), + flagged: pills.map((p, i) => (p.classList.contains('hop-uncertain') ? i : -1)).filter(i => i >= 0), + }; + })); +} + +(async () => { + const browser = await chromium.launch({ + executablePath: process.env.CHROMIUM_PATH || undefined, + args: ['--no-sandbox'], + }); + + await step('grouped rows carry the header observation\'s resolved_path', async () => { + const res = await (await fetch(BASE + '/api/packets?groupByHash=true&limit=200&since=' + + encodeURIComponent(new Date(Date.now() - 1440 * 60000).toISOString()))).json(); + const rows = res.packets || []; + assert(rows.length > 20, 'fixture returned only ' + rows.length + ' grouped rows'); + let checked = 0; + for (const g of rows) { + const path = JSON.parse(g.path_json || '[]'); + if (!path.length) continue; + const d = await (await fetch(BASE + '/api/packets/' + g.hash)).json(); + const obs = (d.observations || []).find(o => o.observer_id === g.observer_id && o.path_json === g.path_json); + if (!obs || !obs.resolved_path) continue; + checked++; + assert(JSON.stringify(g.resolved_path) === JSON.stringify(obs.resolved_path), + g.hash + ': grouped resolved_path ' + JSON.stringify(g.resolved_path) + ' != observation ' + JSON.stringify(obs.resolved_path)); + } + assert(checked > 10, 'only ' + checked + ' grouped rows had a server resolved_path to compare'); + }); + + await step('desktop 1440: no grouped row warns about a hop the server resolved for its observer', async () => { + const ctx = await browser.newContext({ viewport: { width: 1440, height: 900 } }); + const page = await ctx.newPage(); + await loadSettled(page); + const named = await page.evaluate(() => document.querySelectorAll('#pktBody .hop-named').length); + assert(named > 20, 'only ' + named + ' named hops rendered'); + const warned = await warnedRows(page); + const wrong = []; + for (const w of warned) { + if (w.flagged.length !== w.count) { wrong.push(w.hash + ' summary says ' + w.count + ' but marks ' + w.flagged.length + ' hops'); continue; } + const g = (await (await fetch(BASE + '/api/packets?groupByHash=true&hash=' + w.hash + '&limit=1')).json()).packets[0]; + const d = await (await fetch(BASE + '/api/packets/' + w.hash)).json(); + const obs = (d.observations || []).find(o => o.observer_id === g.observer_id && o.path_json === g.path_json); + const rp = obs && Array.isArray(obs.resolved_path) ? obs.resolved_path : []; + const path = JSON.parse(g.path_json || '[]'); + for (const i of w.flagged) { + if (rp[i]) wrong.push(w.hash + ' flags hop ' + i + ' (' + path[i] + '), which the server resolved to ' + rp[i].slice(0, 8)); + } + } + assert(!wrong.length, wrong.length + ' problems in ' + warned.length + ' warned rows: ' + wrong.slice(0, 5).join('; ')); + await ctx.close(); + }); + + await step('control: with resolved_path stripped from the grouped rows, the summary still renders', async () => { + const ctx = await browser.newContext({ viewport: { width: 1440, height: 900 } }); + await ctx.route(/\/api\/packets\?.*groupByHash=true/, async (route) => { + const resp = await route.fetch(); + const body = await resp.json(); + for (const p of body.packets || []) delete p.resolved_path; + await route.fulfill({ response: resp, json: body }); + }); + const page = await ctx.newPage(); + await loadSettled(page); + const warned = await warnedRows(page); + assert(warned.length > 0, 'no .hop-path-warn rendered even without server answers'); + await ctx.close(); + }); + + await browser.close(); + console.log(`\n${passed} passed, ${failed} failed`); + process.exit(failed ? 1 : 0); +})().catch((e) => { console.error(e); process.exit(1); }); diff --git a/test-issue-165-hop-ambiguity-badge.js b/test-issue-165-hop-ambiguity-badge.js index ab59a626d..6ba9ddd9c 100644 --- a/test-issue-165-hop-ambiguity-badge.js +++ b/test-issue-165-hop-ambiguity-badge.js @@ -244,5 +244,25 @@ console.log('\n=== #2097: the observer position anchors the pick ==='); 'one helper looks the observer up, used by every resolve path'); } +console.log('\n=== PR #185 review F6: one badge rule for the detail and the list ==='); + +{ + assert(typeof HopDisplay.conflictBadgeCount === 'function', 'HopDisplay exposes conflictBadgeCount'); + const cases = [ + HopResolver.resolve(['ef'], null, null, null, null, OBSERVER_ID)['ef'], + HopResolver.resolve(['ef'])['ef'], + HopResolver.resolve(['e7aa'], null, null, null, null, OBSERVER_ID)['e7aa'], + { name: 'X', ambiguous: true, conflicts: [{ regional: true }], globalFallback: false }, + { name: 'Y', ambiguous: true, conflicts: [{}, {}, {}], globalFallback: true }, + { name: 'Z', ambiguous: true, conflicts: [{}, {}], globalFallback: false }, + null, + ]; + const agree = cases.every(e => { + const n = typeof HopDisplay.conflictBadgeCount === 'function' ? HopDisplay.conflictBadgeCount(e) : NaN; + return (n > 1) === /hop-conflict-btn/.test(HopDisplay.renderHop('ef', e, {})); + }); + assert(agree, 'conflictBadgeCount(entry) > 1 exactly when renderHop draws the conflict badge'); +} + console.log(`\n${passed} passed, ${failed} failed`); process.exit(failed ? 1 : 0); diff --git a/test-issue-165-hop-resolution-per-observer.js b/test-issue-165-hop-resolution-per-observer.js index 1efb5cc4e..842c703f8 100644 --- a/test-issue-165-hop-resolution-per-observer.js +++ b/test-issue-165-hop-resolution-per-observer.js @@ -154,7 +154,9 @@ test('initial load: cacheResolvedPaths + resolveHopsForPackets keep the server n }); test('a hop the server left unresolved is still resolved and flagged', async () => { - const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_A]); + // With an IATA code, so the ambiguous hop gets a badge (globalFallback) and + // is counted; PR #185 F6 counts only badged hops. + const T = loadPackets([FAR, NEAR, C1A, C1B], [Object.assign({ iata: 'XYZ' }, OBS_A)]); const pkt = serverPacket(); await T.cacheResolvedPaths([pkt]); await T.resolveHopsForPackets([pkt]); @@ -254,7 +256,7 @@ test('a timer runs between two observer groups', async () => { section('#165 UI: the list summary survives a clipped path cell'); test('the summary indicator comes before the hops, so overflow cannot clip it', async () => { - const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_A]); + const T = loadPackets([FAR, NEAR, C1A, C1B], [Object.assign({ iata: 'XYZ' }, OBS_A)]); await T.resolveHops(['ef', 'c1'], 'OBS-A'); const html = T.renderPath(['c1', 'ef'], 'OBS-A', { summary: true }); const warn = html.indexOf('hop-path-warn'), firstHop = html.search(/class="hop[ "]/); @@ -295,6 +297,208 @@ test('the +N popover lists the hops, not the summary indicator (upstream 2557894 assert(/>AB]*>[\s\S]*?<\/svg>(\d+)<\/span>/); + return m ? Number(m[1]) : 0; +} + +test('a grouped row as the server sends it (header resolved_path) is not flagged for the server-resolved hop', async () => { + const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_I]); + // groupByHash row: no observations, resolved_path of the displayed observation. + const row = { hash: 'g1', observer_id: 'OBS-I', path_json: '["ef","c1"]', count: 3, + resolved_path: [FAR.public_key, C1A.public_key] }; + await T.cacheResolvedPaths([row]); + await T.resolveHopsForPackets([row]); + const html = T.renderPath(['ef', 'c1'], 'OBS-I', { summary: true }); + assert(warnCount(html) === 0, 'both hops are server-resolved; got a summary: ' + html); + assert(/FAR-AWAY/.test(html) && /C1-ALPHA/.test(html), 'the server names are shown: ' + html); +}); + +test('the summary does not count another observer\'s entry (the bare-key fallback)', async () => { + const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_I, OBS_B]); + // OBS-I resolved "c1" on the client: ambiguous, badged. That also seeds the + // bare "c1". OBS-B has not resolved "c1" yet. + await T.resolveHops(['c1'], 'OBS-I'); + assert(warnCount(T.renderPath(['c1'], 'OBS-I', { summary: true })) === 1, 'precondition: flagged for OBS-I'); + const html = T.renderPath(['c1'], 'OBS-B', { summary: true }); + assert(warnCount(html) === 0, 'OBS-B has no answer of its own yet, so nothing is flagged for it: ' + html); +}); + +section('PR #185 review F1: grouped rows now bring resolved_path for the whole page'); + +function serverRows(n, obsIds) { + return Array.from({ length: n }, (_, i) => ({ id: i, hash: 'r' + i, observer_id: obsIds[i % obsIds.length], + path_json: '["ef","c1"]', resolved_path: [FAR.public_key, null] })); +} + +test('cacheResolvedPaths yields while it walks a large page', async () => { + const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_A, OBS_B]); + await T.resolveHops(['00'], 'OBS-A'); // HopResolver ready + let ranDuring = false, done = false; + setTimeout(() => { ranDuring = !done; }, 0); + await T.cacheResolvedPaths(serverRows(5000, ['OBS-A', 'OBS-B'])); + done = true; + assert(ranDuring, 'a timer queued before the call must run before it finishes'); +}); + +test('a repeated server answer does not rewrite the cache entry', async () => { + const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_A]); + await T.cacheResolvedPaths(serverRows(1, ['OBS-A'])); + const first = T._hopCacheGet('ef:OBS-A'), bare = T._hopCacheGet('ef'); + assert(first && first.pubkey === FAR.public_key, 'precondition: the server answer is cached'); + await T.cacheResolvedPaths(serverRows(50, ['OBS-A'])); + assert(T._hopCacheGet('ef:OBS-A') === first && T._hopCacheGet('ef') === bare, + 'the same answer for the same key is skipped, not written again per row'); +}); + +test('a different server answer for the same key still replaces it', async () => { + const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_A]); + await T.cacheResolvedPaths(serverRows(1, ['OBS-A'])); + await T.cacheResolvedPaths([{ id: 7, hash: 'x7', observer_id: 'OBS-A', path_json: '["ef"]', resolved_path: [NEAR.public_key] }]); + const e = T._hopCacheGet('ef:OBS-A'); + assert(e && e.pubkey === NEAR.public_key, 'ef:OBS-A follows the newer server answer; got ' + (e && e.name)); +}); + +test('a server answer replaces an ambiguous client pick of the same node', async () => { + // The heuristic already picked NEAR-ONE for OBS-I, flagged as ambiguous; + // the server then confirms NEAR-ONE. Same pubkey, but the entry must + // become the server's, or the list keeps warning about a resolved hop. + const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_I]); + await T.resolveHops(['ef'], 'OBS-I'); + const pick = T._hopCacheGet('ef:OBS-I'); + assert(pick && pick.ambiguous && pick.pubkey === NEAR.public_key, 'precondition: ambiguous client pick NEAR-ONE; got ' + (pick && pick.name)); + await T.cacheResolvedPaths([{ id: 8, hash: 'x8', observer_id: 'OBS-I', path_json: '["ef"]', resolved_path: [NEAR.public_key] }]); + const e = T._hopCacheGet('ef:OBS-I'); + assert(e && !e.ambiguous, 'ef:OBS-I is the server answer now, not the ambiguous pick'); + assert(warnCount(T.renderPath(['ef'], 'OBS-I', { summary: true })) === 0, 'and the list no longer flags it'); +}); + +section('PR #185 review F2: probes for the two surviving mutants'); + +// OBS-A's packet carries the server's answer for "ef" (FAR-AWAY); OBS-I's +// "ef" is client-resolved and ambiguous. +async function serverThenClient() { + const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_A, OBS_I]); + const a = { id: 1, hash: 'h1', observer_id: 'OBS-A', path_json: '["ef"]', resolved_path: JSON.stringify([FAR.public_key]) }; + const b = { id: 2, hash: 'h2', observer_id: 'OBS-I', path_json: '["ef"]' }; + await T.cacheResolvedPaths([a, b]); + await T.resolveHopsForPackets([a, b]); + return T; +} + +test('probe 1 (MD): the summary counts per observer, so OBS-I\'s ambiguous "ef" is flagged', async () => { + const T = await serverThenClient(); + assert(T._hopCacheGet('ef:OBS-I') && T._hopCacheGet('ef:OBS-I').ambiguous, 'precondition: ef:OBS-I is ambiguous'); + const html = T.renderPath(['ef'], 'OBS-I', { summary: true }); + assert(warnCount(html) === 1, 'expected one flagged hop for OBS-I: ' + html); + assert(warnCount(T.renderPath(['ef'], 'OBS-A', { summary: true })) === 0, 'and none for OBS-A, which has the server answer'); +}); + +test('probe 2 (MC): the client heuristic does not overwrite the server answer under the bare key', async () => { + const T = await serverThenClient(); + const bare = T._hopCacheGet('ef'); + assert(bare && bare.pubkey === FAR.public_key, 'bare "ef" stays the server node FAR-AWAY; got ' + (bare && bare.name)); +}); + +section('PR #185 review F3: the cache cap and its eviction order are pinned'); + +test('a bench-sized working set (42 observers x 690 hops) fits without eviction', async () => { + const obs = Array.from({ length: 42 }, (_, i) => ({ id: 'W' + i, iata: 'XYZ', lat: 51 + i / 100, lon: 3.5 })); + const T = loadPackets([FAR, NEAR], obs); + const hops = Array.from({ length: 690 }, (_, i) => (0x200000 + i).toString(16)); + // One packet per observer carrying all 690 hops: 42 x 690 hop:observer keys + // plus 690 bare keys = 29670, about the 28962 the bench leaves behind. + const pkts = obs.map((o, i) => ({ id: i, hash: 'w' + i, observer_id: o.id, path_json: JSON.stringify(hops) })); + await T.resolveHopsForPackets(pkts); + assert(T._hopCacheSize() === 42 * 690 + 690, 'every key is still cached: ' + T._hopCacheSize() + ' of ' + (42 * 690 + 690)); + assert(T._hopCacheGet(hops[0] + ':W0') !== undefined, 'the first key written survives the load'); +}); + +test('rewriting an entry refreshes it, so the server answer is not the next to be evicted', async () => { + const T = loadPackets([FAR, NEAR], [OBS_A, OBS_B]); + const max = T.HOP_CACHE_MAX; + await T.resolveHops(['ef'], 'OBS-A'); // oldest: ef:OBS-A and ef + const fill = []; + for (let i = 0; fill.length < (max - 2) / 2; i++) fill.push((0x100000 + i).toString(16)); + await T.resolveHops(fill, 'OBS-B'); // two keys each: the cache is now exactly full + assert(T._hopCacheSize() === max, 'precondition: cache full, nothing evicted (' + T._hopCacheSize() + ')'); + // The server's answer for OBS-A arrives and rewrites both oldest keys. + await T.cacheResolvedPaths([{ id: 9, hash: 'h9', observer_id: 'OBS-A', path_json: '["ef"]', resolved_path: JSON.stringify([FAR.public_key]) }]); + await T.resolveHops(['abcdef', 'abcdf0'], 'OBS-B'); // four new keys evict four + const e = T._hopCacheGet('ef:OBS-A'); + assert(e && e.pubkey === FAR.public_key, 'ef:OBS-A (rewritten last) survives; got ' + (e ? e.name : 'evicted')); + assert(T._hopCacheGet(fill[0] + ':OBS-B') === undefined, 'the oldest untouched entry is the one evicted'); +}); + +section('PR #185 review F5: the +N popover title counts hops'); + +test('the title counts the hops it lists, not the pill, summary or arrows', async () => { + const T = loadPackets([FAR], []); + const kid = (cls, html) => ({ classList: { contains: c => cls.split(' ').includes(c) }, outerHTML: html }); + const host = { children: [ + kid('hop-path-warn status-warn', '!1'), + kid('hop hop-named', 'A'), kid('arrow', '→'), + kid('hop-group', 'B'), + kid('arrow', '→'), kid('hop', 'C'), + kid('path-overflow-pill', '+1'), + ] }; + const html = T._pathPopoverHtml(host); + assert(/Full path \(3 hops\)/.test(html), 'expected "Full path (3 hops)": ' + html); +}); + +section('PR #185 review F6: the list counts exactly the hops the detail pane badges'); + +test('an ambiguous hop without a badge (observer has no IATA) is not counted', async () => { + const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_N]); + await T.resolveHops(['c1'], 'OBS-N'); + const e = T._hopCacheGet('c1:OBS-N'); + assert(e && e.ambiguous, 'precondition: c1 is ambiguous for OBS-N'); + const detail = T.renderPath(['c1'], 'OBS-N'); + assert(!/hop-conflict-btn/.test(detail), 'precondition: the detail form shows no badge: ' + detail); + const list = T.renderPath(['c1'], 'OBS-N', { summary: true }); + assert(warnCount(list) === 0, 'the list must not flag a hop the detail does not: ' + list); +}); + +test('a badged hop is counted, and its pill is marked hop-uncertain', async () => { + const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_I]); + await T.resolveHops(['c1', 'ef'], 'OBS-I'); + const detail = T.renderPath(['c1', 'ef'], 'OBS-I'); + const badged = (detail.match(/hop-conflict-btn/g) || []).length; + assert(badged === 2, 'precondition: the detail badges both hops: ' + detail); + const list = T.renderPath(['c1', 'ef'], 'OBS-I', { summary: true }); + assert(warnCount(list) === 2, 'the summary counts the two badged hops: ' + list); + assert((list.match(/class="hop [^"]*hop-uncertain/g) || []).length === 2, 'both counted pills carry hop-uncertain: ' + list); + assert(!/hop-conflict-btn/.test(list), 'the list still has no per-hop badge: ' + list); +}); + +section('PR #185 review F7: a hop job outliving destroy() stops writing'); + +test('destroy() during resolveHopsForPackets leaves the new cache empty', async () => { + const T = loadPackets([FAR, NEAR], [OBS_A, OBS_B]); + await T.resolveHops(['00'], 'OBS-A'); // HopResolver ready + T._destroy(); + const job = T.resolveHopsForPackets([livePacket(1, 'OBS-A'), livePacket(2, 'OBS-B')]); + T._destroy(); // the page is left while the job is in flight + await job; + assert(T._hopCacheSize() === 0, 'the old job wrote ' + T._hopCacheSize() + ' entries into the new cache'); +}); + +test('destroy() during cacheResolvedPaths stops it too', async () => { + const T = loadPackets([FAR, NEAR, C1A, C1B], [OBS_A, OBS_B]); + await T.resolveHops(['00'], 'OBS-A'); // HopResolver ready + T._destroy(); + const job = T.cacheResolvedPaths(serverRows(5000, ['OBS-A', 'OBS-B'])); + setTimeout(() => T._destroy(), 0); // left during the first yield + await job; + assert(T._hopCacheSize() === 0, 'the old job wrote ' + T._hopCacheSize() + ' entries into the new cache'); +}); + (async () => { for (const t of pending) { if (t.title) { console.log('\n=== ' + t.title.trim() + ' ==='); continue; } From 8d46c5fef71996e22ca3c7b525f185a6538c090b Mon Sep 17 00:00:00 2001 From: dborup Date: Wed, 7 Oct 2026 08:31:12 +0000 Subject: [PATCH 21/24] fix(packets): flag only hops the detail badges, from the observer's own entry (#165) PR #185 review F1 and F6. The list summary counted entry.ambiguous and fell back to the bare key: - The bare key is another observer's answer (or the server's for another observation), so the warning depended on what had been resolved before. The summary now reads only the row observer's own entry; the bare key stays the name fallback. - entry.ambiguous is set for any hop with two candidates, also where the detail pane shows no badge (no regional conflicts and no global fallback, e.g. an observer without an IATA code). The badge's count is now HopDisplay.conflictBadgeCount(entry), used by renderHop and by the summary, so the list flags exactly the hops the detail badges. Counted pills carry the class hop-uncertain, so which hops a summary counts can be checked (the E2E does). Co-Authored-By: Claude Opus 5.5 --- public/hop-display.js | 19 +++++++++++++++---- public/packets.js | 26 +++++++++++++++++--------- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/public/hop-display.js b/public/hop-display.js index 68b5aed51..02fe7e52f 100644 --- a/public/hop-display.js +++ b/public/hop-display.js @@ -57,6 +57,19 @@ window.HopDisplay = (function() { popover.style.left = Math.max(8, Math.min(rect.left + window.scrollX - 60, window.innerWidth - 280)) + 'px'; } + /** + * The number on a hop's conflict badge; the badge is drawn when it is > 1. + * Only regional conflicts count, or every conflict when the resolver fell + * back to the global list. #165 — the packets list's path summary counts + * hops with this, so the list flags exactly the hops the detail badges. + */ + function conflictBadgeCount(entry) { + if (!entry || typeof entry !== 'object') return 0; + const conflicts = entry.conflicts || []; + const regional = conflicts.filter(c => c.regional).length; + return regional > 0 ? regional : (entry.globalFallback ? conflicts.length : 0); + } + /** * Render a hop prefix as HTML with conflict info. */ @@ -77,9 +90,7 @@ window.HopDisplay = (function() { let title = h; if (unreliable) title += ' — unreliable'; - // Badge — only count regional conflicts - const regionalConflicts = conflicts.filter(c => c.regional); - const badgeCount = regionalConflicts.length > 0 ? regionalConflicts.length : (globalFallback ? conflicts.length : 0); + const badgeCount = conflictBadgeCount(entry); const conflictData = escapeHtml(JSON.stringify({ h, conflicts, globalFallback })); const conflictBadge = badgeCount > 1 ? ` ` @@ -158,5 +169,5 @@ window.HopDisplay = (function() { '
    ' + items + '
'; } - return { renderHop, renderPath, _showFromBtn, PATH_SYMBOLS_LEGEND, renderPathSymbolsLegend }; + return { renderHop, renderPath, conflictBadgeCount, _showFromBtn, PATH_SYMBOLS_LEGEND, renderPathSymbolsLegend }; })(); diff --git a/public/packets.js b/public/packets.js index 6a1edbb5c..063340ec8 100644 --- a/public/packets.js +++ b/public/packets.js @@ -1265,6 +1265,16 @@ return HopDisplay.renderHop(h, entry, Object.assign({ hexMode: showHexHashes }, opts || {})); } + // #165 (PR #185 review F1, F6) — is this hop uncertain for this observer? + // Only the observer's own entry counts: the bare key renderHop() falls + // back to for the name is another observer's answer, or the server's for + // another observation. And "uncertain" is what the detail pane badges + // (HopDisplay.conflictBadgeCount), so list and detail never disagree. + function hopUncertainFor(h, observerId) { + const entry = observerId ? hopCacheGet(hopCacheKey(h, observerId)) : hopCacheGet(h); + return HopDisplay.conflictBadgeCount(entry) > 1; + } + // #165 (upstream 2099) — opts.summary renders the list form: names without a badge on every // hop, and one indicator for the whole path. A row with five 1-byte hops was // five warning triangles, which is noise in a table; the detail pane keeps @@ -1278,17 +1288,15 @@ ? window.MC_filterPathHops(hops) : hops; if (!filtered.length) return '— (1-byte filtered)'; - const summary = !!(opts && opts.summary); - const body = filtered - .map(h => renderHop(h, observerId, summary ? { badge: false } : null)) - .join('→'); - if (!summary) return body; + const arrow = '→'; + if (!(opts && opts.summary)) return filtered.map(h => renderHop(h, observerId)).join(arrow); let uncertain = 0; - for (const h of filtered) { - const entry = hopCacheGet(hopCacheKey(h, observerId)) || hopCacheGet(h); - if (entry && entry.ambiguous) uncertain++; - } + const body = filtered.map(h => { + const flag = hopUncertainFor(h, observerId); + if (flag) uncertain++; + return renderHop(h, observerId, flag ? { badge: false, className: 'hop-uncertain' } : { badge: false }); + }).join(arrow); if (!uncertain) return body; const label = uncertain + ' of ' + filtered.length + ' hops have more than one candidate'; // #165 — first, not last: the list's path cell clips overflow, and the From 441cd76b5cb67b37c6672c919af59a1c3bf62a05 Mon Sep 17 00:00:00 2001 From: dborup Date: Wed, 7 Oct 2026 08:31:20 +0000 Subject: [PATCH 22/24] fix(packets): count hops in the +N popover title (#165) PR #185 review F5. "Full path (N items)" counted host.children, which holds the pill and, since this PR, the path summary as well as the hops and arrows: 19 items for 9 hops on the fixture. _pathPopoverHtml now counts the hops it lists and says "Full path (9 hops)". Co-Authored-By: Claude Opus 5.5 --- public/packets.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/public/packets.js b/public/packets.js index 063340ec8..b0a7b118c 100644 --- a/public/packets.js +++ b/public/packets.js @@ -2976,15 +2976,17 @@ // Popover body for a .path-hops host: clone all children except the pill // and (#165, upstream 2557894d) the path's ambiguity summary, preserving - // rendered chips/arrows. + // rendered chips/arrows. The title counts the hops listed (PR #185 F5). function _pathPopoverHtml(host) { - var inner = '
Full path (' + (host.children.length) + ' items)
'; + var body = '', hops = 0; var kids = Array.prototype.slice.call(host.children); for (var i = 0; i < kids.length; i++) { - if (kids[i].classList.contains('path-overflow-pill') || kids[i].classList.contains('hop-path-warn')) continue; - inner += kids[i].outerHTML; + var cl = kids[i].classList; + if (cl.contains('path-overflow-pill') || cl.contains('hop-path-warn')) continue; + if (!cl.contains('arrow')) hops++; + body += kids[i].outerHTML; } - return inner + '
'; + return '
Full path (' + hops + (hops === 1 ? ' hop' : ' hops') + ')
' + body + '
'; } // Delegated click for path overflow pills — show popover of full path. From ae70759eb4e7d6aea43844e760a75a1719d1f4e9 Mon Sep 17 00:00:00 2001 From: dborup Date: Wed, 7 Oct 2026 08:31:21 +0000 Subject: [PATCH 23/24] fix(packets): stop a hop job that outlives destroy() (#165) PR #185 review F7. resolveHopsForPackets yields between observer groups, so the initial hop job could keep running after the page was left and write into the next page's cache. destroy() now bumps hopCacheGen; resolveHops() and resolveHopsForPackets() stop when it changed while they waited. Co-Authored-By: Claude Opus 5.5 --- public/packets.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/public/packets.js b/public/packets.js index b0a7b118c..548bd3872 100644 --- a/public/packets.js +++ b/public/packets.js @@ -835,6 +835,10 @@ // (Map keeps insertion order), and destroy() clears it. const HOP_CACHE_MAX = 50000; let hopNameCache = new Map(); + // #165 — bumped by destroy(). A hop job that started before it (the + // initial load yields between observer groups) stops instead of writing + // into the next page's cache. + let hopCacheGen = 0; function hopCacheHas(k) { return hopNameCache.has(k); } function hopCacheGet(k) { return hopNameCache.get(k); } function hopCacheSet(k, v) { @@ -1175,7 +1179,9 @@ async function resolveHops(hops, observerId) { const unknown = hops.filter(h => !hopCacheHas(hopCacheKey(h, observerId))); if (!unknown.length) return; + const gen = hopCacheGen; await ensureHopResolver(); + if (gen !== hopCacheGen) return; const [obsLat, obsLon] = observerPosition(observerId); const resolved = HopResolver.resolve(unknown, null, null, obsLat, obsLon, observerId) || {}; for (const h of unknown) { @@ -1200,10 +1206,12 @@ // the resolve itself is local computation, so the loop costs no requests. // #165 — but it is CPU: ~42 groups on a 30K load took one ~220 ms block. // Yield to the event loop between groups so input and paint get through. + const gen = hopCacheGen; let first = true; for (const [obs, set] of groups) { if (!set.size) continue; if (!first) await new Promise(r => setTimeout(r, 0)); + if (gen !== hopCacheGen) return; first = false; await resolveHops([...set], obs || undefined); } @@ -1668,6 +1676,7 @@ delete filters.node; expandedHashes = new Set(); hopNameCache = new Map(); + hopCacheGen++; totalCount = 0; observers = []; observerMap = new Map(); From 0874df4ecfea141b4a28c5ec95f4b67577c85427 Mon Sep 17 00:00:00 2001 From: dborup Date: Wed, 7 Oct 2026 08:31:32 +0000 Subject: [PATCH 24/24] perf(packets): skip repeated server answers and yield in cacheResolvedPaths (#165) Since grouped rows carry resolved_path (PR #185 F1), cacheResolvedPaths walks the whole page, up to 50K rows, and wrote two cache entries per hop per row in one synchronous loop. With 95 % of rows carrying a resolved_path, scripts/bench-hop-resolution.js measured the longest main-thread block at 227-249 ms. - cacheServerResolvedPath skips a row whose answers are already cached (holdsServerAnswer: same pubkey, and not a flagged client pick, which the server answer must still replace). - cacheResolvedPaths yields every 2000 rows and stops if destroy() ran meanwhile (hopCacheGen, as resolveHopsForPackets does). - The bench takes RP_SHARE (default 0.3, as before). Bench, 30K packets, 42 observers, median of 7 runs, 3 rounds each, 4-core cloud VM, Node 22: RP_SHARE total ms (before -> after) longest block ms 0.3 266-284 -> 279-283 91-99 -> 30-33 0.95 315-343 -> 234-240 227-249 -> 24-25 Co-Authored-By: Claude Opus 5.5 --- public/packets.js | 36 +++++++++++++++++++++++++++++---- scripts/bench-hop-resolution.js | 11 ++++++---- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/public/packets.js b/public/packets.js index 548bd3872..d1f137ec5 100644 --- a/public/packets.js +++ b/public/packets.js @@ -1230,8 +1230,24 @@ if (rp) { needsInit = true; break; } } if (!needsInit) return; + const gen = hopCacheGen; await ensureHopResolver(); - for (const p of packets) cacheServerResolvedPath(p); + // #165 — grouped rows carry resolved_path too since PR #185 F1, so this + // walks the whole page (up to 50K rows). Yield every 2000 rows so it is + // not one long main-thread block, and stop if destroy() ran meanwhile. + for (let i = 0; i < packets.length; i++) { + if (i && i % 2000 === 0) await new Promise(r => setTimeout(r, 0)); + if (gen !== hopCacheGen) return; + cacheServerResolvedPath(packets[i]); + } + } + + // Does k already hold exactly this server answer? A client pick of the same + // node that was flagged (ambiguous, unreliable, global fallback) does not + // count: the server's answer must replace it, or the list keeps warning. + function holdsServerAnswer(k, pubkey) { + const e = hopCacheGet(k); + return !!e && e.pubkey === pubkey && !e.ambiguous && !e.unreliable && !e.globalFallback; } // #165 — store the server's resolved_path entries under the bare key AND @@ -1242,13 +1258,25 @@ // know which packet a prefix came from. Hops the server left null are not // written, so the client still resolves them and can show the ambiguity. // Caller must have initialised HopResolver. + // PR #185 F1 — grouped rows carry resolved_path too, and most rows repeat + // an answer that is already cached (same prefix, observer and node). Those + // are skipped (holdsServerAnswer), so a 30K page does not rewrite the cache + // twice per hop per row. function cacheServerResolvedPath(p) { const rp = getResolvedPath(p); if (!rp) return; - const resolved = HopResolver.resolveFromServer(getParsedPath(p), rp); + const hops = getParsedPath(p); + const obs = p.observer_id ? String(p.observer_id) : ''; + let news = false; + for (let i = 0; i < rp.length && !news; i++) { + if (rp[i] && (!holdsServerAnswer(hops[i], rp[i]) || (obs && !holdsServerAnswer(hopCacheKey(hops[i], obs), rp[i])))) news = true; + } + if (!news) return; + const resolved = HopResolver.resolveFromServer(hops, rp); for (const h of Object.keys(resolved)) { - hopCacheSet(h, resolved[h]); - if (p.observer_id) hopCacheSet(hopCacheKey(h, String(p.observer_id)), resolved[h]); + const v = resolved[h]; + if (!holdsServerAnswer(h, v.pubkey)) hopCacheSet(h, v); + if (obs && !holdsServerAnswer(hopCacheKey(h, obs), v.pubkey)) hopCacheSet(hopCacheKey(h, obs), v); } } diff --git a/scripts/bench-hop-resolution.js b/scripts/bench-hop-resolution.js index 70b4e086a..40f54f29a 100644 --- a/scripts/bench-hop-resolution.js +++ b/scripts/bench-hop-resolution.js @@ -5,7 +5,9 @@ * Times the initial-load hop resolution of the REAL public/packets.js on a * synthetic ~30K packet load, in a vm sandbox (network stubbed): * - 2000 repeaters in three regions, 42 observers (27 with coordinates), - * - 0-8 hops per packet (85% 1-byte, 15% 2-byte), 30% with resolved_path. + * - 0-8 hops per packet (85% 1-byte, 15% 2-byte), 30% with resolved_path + * (RP_SHARE=0.95 for the share the grouped rows carry since PR #185 F1, + * after #190; 80% of a resolved_path's hops are non-null either way). * * Reports the median of 7 runs of: * - total time from cacheResolvedPaths() to the last resolved hop, @@ -17,7 +19,7 @@ * packets.js has no resolveHopsForPackets() it runs the old sequence * (one resolveHops() over all hops) instead. * - * Usage: node scripts/bench-hop-resolution.js [publicDir] (default: public) + * Usage: [RP_SHARE=0.3] node scripts/bench-hop-resolution.js [publicDir] (default: public) */ 'use strict'; const fs = require('fs'); @@ -26,6 +28,7 @@ const vm = require('vm'); const PUBLIC = path.resolve(process.argv[2] || path.join(__dirname, '..', 'public')); const RUNS = 7; +const RP_SHARE = Number(process.env.RP_SHARE || 0.3); // Deterministic data (mulberry32). function rng(a) { @@ -54,7 +57,7 @@ const packets = Array.from({ length: 30000 }, (_, i) => { const len = Math.floor(r() * 9), bytes = r() < 0.85 ? 1 : 2; const hops = Array.from({ length: len }, () => nodes[Math.floor(r() * nodes.length)].public_key.slice(0, bytes * 2).toUpperCase()); const p = { id: i, hash: 'h' + i, observer_id: observers[Math.floor(r() * observers.length)].id, path_json: JSON.stringify(hops) }; - if (r() < 0.3) { + if (r() < RP_SHARE) { p.resolved_path = JSON.stringify(hops.map(h => (r() < 0.8 ? ((nodes.find(n => n.public_key.toUpperCase().startsWith(h)) || {}).public_key || null) : null))); } @@ -127,7 +130,7 @@ function load() { const median = a => a.slice().sort((x, y) => x - y)[Math.floor(a.length / 2)]; console.log(JSON.stringify({ publicDir: PUBLIC, mode: legacyMode ? 'single resolve (pre-#165)' : 'per observer (#165)', - packets: packets.length, observers: observers.length, nodes: nodes.length, runs: RUNS, + packets: packets.length, observers: observers.length, nodes: nodes.length, runs: RUNS, rp_share: RP_SHARE, total_ms_median: +median(times).toFixed(1), longest_block_ms_median: +median(blocks).toFixed(1), cache_entries: size, }));