From 167e3e495f5e2d125d7b9bdf9b8d0f18067547e2 Mon Sep 17 00:00:00 2001 From: efiten Date: Wed, 30 Sep 2026 18:40:50 +0200 Subject: [PATCH 1/6] fix(packets): resolve path hops with the observer that heard them (#2097) Every 1-byte path prefix on the network is shared: measured, 254 prefixes cover all 2043 nodes, nine of them on a single prefix. HopResolver already reports such a hop as ambiguous with its candidate list, and HopDisplay already renders a warning badge with a candidate popover. Neither reached the packets page. HopResolver.resolve() takes six parameters. packets.js passed one, so observerId was undefined, packetIata was null, nodeInRegion() never ran, no candidate was flagged regional, globalFallback stayed false, and HopDisplay computed a badge count of 0. One name was printed as certain out of nine candidates, chosen without the geographic filter that was already written. Observed on live data: a hop resolved to a repeater 126 km from the transmitter and 185 km from the observer, while the only candidate with a recorded edge to the next hop in the path sat at the transmitter's own location. resolveHops() now takes the observer and caches under the per-observer key that renderHop() has always read and nothing ever wrote. resolveHopsForPackets() groups a multi-packet call by observer so each group gets its own filtering, and the six multi-packet call sites use it. One site was the bug in its purest form: it wrote the result under hopNameCache[k + ':' + pkt.observer_id] after resolving without that observer, so the key promised something the value was not. The structural guard fails if any call passes the hops alone; verified by reverting one call, which names it in the failure. No browser validation yet; deploying to staging next. --- public/packets.js | 89 +++++++----- test-all.sh | 1 + .../test-issue-2097-hop-ambiguity-badge.js | 135 ++++++++++++++++++ 3 files changed, 188 insertions(+), 37 deletions(-) create mode 100644 tests/unit/test-issue-2097-hop-ambiguity-badge.js diff --git a/public/packets.js b/public/packets.js index 5abee67ac..9764ef37c 100644 --- a/public/packets.js +++ b/public/packets.js @@ -969,14 +969,46 @@ } // 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; }); + // #2097 — 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. + function hopCacheKey(h, observerId) { + return observerId ? h + ':' + observerId : h; + } + + // #2097 — 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 resolved = HopResolver.resolve(unknown, null, null, null, null, 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); } } @@ -1005,7 +1037,7 @@ function renderHop(h, observerId) { // 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 }); } @@ -1306,9 +1338,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, obsTarget); initPanelResize(); @@ -1364,7 +1394,7 @@ } try { hops.forEach(h => { if (!(h in hopNameCache)) newHops.add(h); }); } catch {} } - (newHops.size ? resolveHops([...newHops]) : Promise.resolve()).then(() => { + (newHops.size ? resolveHopsForPackets(filtered) : Promise.resolve()).then(() => { if (groupByHash) { // Update existing groups or create new ones for (const p of filtered) { @@ -1565,11 +1595,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) { @@ -2136,11 +2162,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(); }); @@ -3268,9 +3290,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'); @@ -3360,7 +3380,10 @@ resolved = HopResolver.resolveFromServer(pathHops, serverResolved); } else { await ensureHopResolver(); - resolved = HopResolver.resolve(pathHops); + // #2097 — 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. + resolved = HopResolver.resolve(pathHops, null, null, null, null, pkt.observer_id); } if (resolved) { for (const [k, v] of Object.entries(resolved)) { @@ -4142,12 +4165,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 @@ -4231,10 +4249,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/test-all.sh b/test-all.sh index 889327026..06f220749 100755 --- a/test-all.sh +++ b/test-all.sh @@ -139,6 +139,7 @@ node tests/unit/test-issue-1997-distance-building.js node tests/unit/test-issue-2042-recent-adverts-label.js node tests/unit/test-issue-2001-map-scope-state.js node tests/unit/test-issue-2012-clear-filters-selection.js +node tests/unit/test-issue-2097-hop-ambiguity-badge.js node tests/unit/test-live-anims.js node tests/unit/test-live-dt-cap-1524.js node tests/unit/test-live-legend-helper.js diff --git a/tests/unit/test-issue-2097-hop-ambiguity-badge.js b/tests/unit/test-issue-2097-hop-ambiguity-badge.js new file mode 100644 index 000000000..703384214 --- /dev/null +++ b/tests/unit/test-issue-2097-hop-ambiguity-badge.js @@ -0,0 +1,135 @@ +/** + * #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 = require('path').resolve(__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${passed} passed, ${failed} failed`); +process.exit(failed ? 1 : 0); From ec6bbca653617b8d25daab5cbd2f4f64fcccdbaa Mon Sep 17 00:00:00 2001 From: efiten Date: Wed, 30 Sep 2026 20:00:13 +0200 Subject: [PATCH 2/6] fix(packets): tie each badge to its pill, and summarise the path in the list Both found by looking at the previous commit on staging rather than at its tests. The badge is a sibling of its pill, so a path read "A [8] -> B [6] -> C" and nothing said which name the 8 qualified. It qualifies the one on its left. Pill and badge now render inside a single nowrap .hop-group, which puts the separator outside the pair. Only a hop that carries a badge is wrapped: wrapping every hop would change the layout of every path to no purpose. The markup predates this work, but the previous commit is what made it visible: before it, the badge essentially never appeared on the packets page. In the list, every 1-byte hop getting its own badge turned a row into a line of warning triangles. renderPath now takes { summary: true } for the three list call sites, which renders the names with no per-hop badge and appends one indicator reading "N of M hops have more than one candidate". The two detail-pane call sites are unchanged and keep a badge per hop, because that is where the candidate list actually gets read. HopDisplay gains opts.badge === false for that, and the hop keeps its hop-ambiguous class either way so CSS can still mark it. The new .hop-path-warn mirrors the .hop-conflict-btn rule directly above it, including its var(--status-yellow, #f59e0b) fallback form. Suite green. Browser check on staging next. --- public/hop-display.js | 18 ++++++-- public/packets.js | 33 ++++++++++--- public/style.css | 14 ++++++ .../test-issue-2097-hop-ambiguity-badge.js | 46 +++++++++++++++++++ 4 files changed, 99 insertions(+), 12 deletions(-) diff --git a/public/hop-display.js b/public/hop-display.js index 1f7751943..7e02a9e79 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; + // #2097 — 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', @@ -97,10 +99,16 @@ window.HopDisplay = (function() { globalFallback ? 'hop-global-fallback' : '', ].filter(Boolean).join(' '); - if (opts.link !== false) { - return `${display}${warnBadge}`; - } - return `${display}${warnBadge}`; + const pill = opts.link !== false + ? `${display}` + : `${display}`; + + // #2097 — 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/packets.js b/public/packets.js index 9764ef37c..006b2d8ee 100644 --- a/public/packets.js +++ b/public/packets.js @@ -1035,14 +1035,18 @@ } } - function renderHop(h, observerId) { + 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]; - return HopDisplay.renderHop(h, entry, { hexMode: showHexHashes }); + return HopDisplay.renderHop(h, entry, Object.assign({ hexMode: showHexHashes }, opts || {})); } - function renderPath(hops, observerId) { + // #2097 — 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 @@ -1051,7 +1055,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; @@ -2419,7 +2438,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; @@ -2470,7 +2489,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}` : '—'} @@ -2505,7 +2524,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; diff --git a/public/style.css b/public/style.css index 7b95eda7b..5f7ce042c 100644 --- a/public/style.css +++ b/public/style.css @@ -3082,6 +3082,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); } + +/* #2097 — 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, #f59e0b); color: #000; 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/tests/unit/test-issue-2097-hop-ambiguity-badge.js b/tests/unit/test-issue-2097-hop-ambiguity-badge.js index 703384214..84ab5f149 100644 --- a/tests/unit/test-issue-2097-hop-ambiguity-badge.js +++ b/tests/unit/test-issue-2097-hop-ambiguity-badge.js @@ -131,5 +131,51 @@ console.log('\n=== #2097: packets.js never resolves a hop without its observer = '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'); +} + +console.log('\n=== #2097: the list summarises, the detail pane does not ==='); + +{ + const src = fs.readFileSync(REPO_ROOT + '/public/packets.js', 'utf8'); + assert(/function renderPath\(hops, observerId, opts\)/.test(src), + 'renderPath takes an options object'); + assert(/summary: true/.test(src), + 'at least one call site asks for the summarised form'); + assert(/renderPath\(pathHops, effectivePkt\.observer_id\)/.test(src), + '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'); +} + console.log(`\n${passed} passed, ${failed} failed`); process.exit(failed ? 1 : 0); From 2dbacce1ab715c29e4032c584a82819d0e83c718 Mon Sep 17 00:00:00 2001 From: efiten Date: Wed, 30 Sep 2026 22:12:57 +0200 Subject: [PATCH 3/6] fix(packets): anchor hop resolution on the observer position (#2097) The geographic filter in HopResolver has never worked on this network, and the previous commits only made that visible rather than causing it. nodeInRegion() looks the observer's IATA code up in /api/iata-coords. Measured on the live deployment: 0 of 42 observers have their code in that table. All ten codes in use are missing (ANR BRU GNE HEP KJK LGG MST NRW OBL OST) while the 54 entries it does hold are AMS, APC, ATL, AUS and the like. So every candidate came back with regional=false and distKm=undefined, no candidate was ever excluded, and the displayed name was simply the first in index order. 27 of 42 observers report their own lat/lon, and resolve() has always taken observerLat/observerLon as the anchor at the receiving end: when nothing later in the path is resolved, the observer is the next known position, which is what pickByAffinity needs to score candidates. resolveHops now looks the observer up in the existing observerMap and passes it. The test pins the behaviour with iataCoords deliberately empty, which is the live state: with FAR-AWAY listed first, the resolver picks it without an anchor and rules it out with one. It stays reported as ambiguous with all candidates listed, because a better pick is not a certain one. ensureHopResolver() seeds observerMap from the observer list it already fetches, so the anchor is available if a render precedes loadObservers(). The broken IATA table is a separate defect and gets its own issue: it is a network-wide lookup that resolves nothing, and hop resolution may not be its only consumer. --- public/packets.js | 18 +++++++- .../test-issue-2097-hop-ambiguity-badge.js | 41 +++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/public/packets.js b/public/packets.js index 006b2d8ee..33e196e83 100644 --- a/public/packets.js +++ b/public/packets.js @@ -956,10 +956,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 || {}, }); + // #2097 — 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. @@ -986,7 +992,15 @@ const unknown = hops.filter(h => !(hopCacheKey(h, observerId) in hopNameCache)); if (!unknown.length) return; await ensureHopResolver(); - const resolved = HopResolver.resolve(unknown, null, null, null, null, observerId) || {}; + // #2097 — the observer's own position, as the anchor at the receiving end. + // The IATA path is dead weight here: measured on the live deployment, none + // of the 42 observers has its code in /api/iata-coords, so nodeInRegion() + // always returns null and the 300 km filter narrows nothing. The lat/lon + // the observer already reports does work, and resolve() has always taken it. + const obs = observerId && observerMap ? observerMap.get(observerId) : null; + const obsLat = obs && Number.isFinite(Number(obs.lat)) ? Number(obs.lat) : null; + const obsLon = obs && Number.isFinite(Number(obs.lon)) ? Number(obs.lon) : null; + 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; diff --git a/tests/unit/test-issue-2097-hop-ambiguity-badge.js b/tests/unit/test-issue-2097-hop-ambiguity-badge.js index 84ab5f149..3bffd3f11 100644 --- a/tests/unit/test-issue-2097-hop-ambiguity-badge.js +++ b/tests/unit/test-issue-2097-hop-ambiguity-badge.js @@ -177,5 +177,46 @@ console.log('\n=== #2097: the list summarises, the detail pane does not ==='); 'the summarised form emits a single per-path indicator'); } +console.log('\n=== #2097: the observer position anchors the pick ==='); + +{ + // Measured on the live deployment: 0 of 42 observers have their IATA code in + // /api/iata-coords, so nodeInRegion() always returns null and the 300 km + // filter narrows nothing. 27 of 42 do carry their own lat/lon, and resolve() + // already accepts them as the backward anchor for pickByAffinity. + // + // FAR-AWAY is listed first on purpose: with no anchor the resolver has no + // context and keeps candidate order, which is the shape of the live bug. + const far = { public_key: 'efbf0eeacc', name: 'FAR-AWAY', role: 'repeater', lat: 50.87, lon: 5.52 }; + 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 }; + HopResolver.init([far, near1, near2], { + observers: [{ id: OBSERVER_ID, iata: IATA }], + iataCoords: {}, // deliberately empty: this is the live state + }); + + const OBS_LAT = 51.21, OBS_LON = 3.44; // the observer that heard it + + const without = HopResolver.resolve(['ef'], null, null, null, null, OBSERVER_ID)['ef']; + const withPos = HopResolver.resolve(['ef'], null, null, OBS_LAT, OBS_LON, OBSERVER_ID)['ef']; + + assert(without.name === 'FAR-AWAY', + 'without an anchor the resolver keeps candidate order (got ' + without.name + ')'); + assert(withPos.name !== 'FAR-AWAY', + 'the observer position rules out the distant candidate (got ' + withPos.name + ')'); + assert(withPos.ambiguous === true, + 'it is still reported as ambiguous: a better pick is not a certain one'); + assert((withPos.conflicts || []).length === 3, + 'every candidate is still listed for the reader'); +} + +{ + const src = fs.readFileSync(REPO_ROOT + '/public/packets.js', 'utf8'); + assert(/HopResolver\.resolve\(unknown, null, null, obsLat, obsLon, observerId\)/.test(src), + 'resolveHops passes the observer position as the anchor'); + assert(/observerMap/.test(src.slice(src.indexOf('async function resolveHops'), src.indexOf('async function resolveHopsForPackets'))), + 'resolveHops looks the observer up to find its position'); +} + console.log(`\n${passed} passed, ${failed} failed`); process.exit(failed ? 1 : 0); From 3532cede0e3ad242dee8d348f9ee80c1a78b8d9f Mon Sep 17 00:00:00 2001 From: efiten Date: Wed, 30 Sep 2026 22:29:01 +0200 Subject: [PATCH 4/6] fix(packets): give the detail pane the anchor too, via one helper Caught on staging, not in the tests: the detail pane resolves through its own HopResolver.resolve call, and the previous commit gave it the observer id while still passing null for the position. So the path that shows the candidate badges was the one path with no anchor, and it kept picking the first candidate. observerPosition() is now the single lookup and both resolve paths use it. The new guard fails if any call passes an observer id and drops its position; verified by reverting the detail call, which names it in the failure. That is the check that would have caught this before the deploy. --- public/packets.js | 23 +++++++++++-------- .../test-issue-2097-hop-ambiguity-badge.js | 7 ++++-- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/public/packets.js b/public/packets.js index 33e196e83..6bf73ca3d 100644 --- a/public/packets.js +++ b/public/packets.js @@ -978,6 +978,17 @@ // #2097 — 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. + // #2097 — 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; } @@ -992,14 +1003,7 @@ const unknown = hops.filter(h => !(hopCacheKey(h, observerId) in hopNameCache)); if (!unknown.length) return; await ensureHopResolver(); - // #2097 — the observer's own position, as the anchor at the receiving end. - // The IATA path is dead weight here: measured on the live deployment, none - // of the 42 observers has its code in /api/iata-coords, so nodeInRegion() - // always returns null and the 300 km filter narrows nothing. The lat/lon - // the observer already reports does work, and resolve() has always taken it. - const obs = observerId && observerMap ? observerMap.get(observerId) : null; - const obsLat = obs && Number.isFinite(Number(obs.lat)) ? Number(obs.lat) : null; - const obsLon = obs && Number.isFinite(Number(obs.lon)) ? Number(obs.lon) : null; + 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; @@ -3416,7 +3420,8 @@ // #2097 — 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. - resolved = HopResolver.resolve(pathHops, null, null, null, null, pkt.observer_id); + 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)) { diff --git a/tests/unit/test-issue-2097-hop-ambiguity-badge.js b/tests/unit/test-issue-2097-hop-ambiguity-badge.js index 3bffd3f11..138af9954 100644 --- a/tests/unit/test-issue-2097-hop-ambiguity-badge.js +++ b/tests/unit/test-issue-2097-hop-ambiguity-badge.js @@ -214,8 +214,11 @@ console.log('\n=== #2097: the observer position anchors the pick ==='); const src = fs.readFileSync(REPO_ROOT + '/public/packets.js', 'utf8'); assert(/HopResolver\.resolve\(unknown, null, null, obsLat, obsLon, observerId\)/.test(src), 'resolveHops passes the observer position as the anchor'); - assert(/observerMap/.test(src.slice(src.indexOf('async function resolveHops'), src.indexOf('async function resolveHopsForPackets'))), - 'resolveHops looks the observer up to find its position'); + const withId = src.match(/HopResolver\.resolve\([^)]*observer_id[^)]*\)/g) || []; + assert(withId.every(c => !/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`); From 4ca1ef879a27a2019a5395497c2a760f223ed828 Mon Sep 17 00:00:00 2001 From: efiten Date: Sat, 3 Oct 2026 12:22:40 +0200 Subject: [PATCH 5/6] fix(nodes): mark the current hop on its pill, not on the badge wrapper Since #2097 renderHop wraps a hop that carries a badge in a `.hop-group` span. The node full view marks the current node by replacing the first `class="` in that HTML, which now hit the wrapper, so the selected hop lost its outline whenever it had a conflict or unreliable badge. Match `class="hop ` so only the pill is marked. The #1146 E2E counted every nested button as a non-hop child of the chain, so the two wrapped hops read as 16 of 18. Count only direct button children. Locally the #1146 suite went from 7 passed / 4 failed to 11 / 0. Co-Authored-By: Claude Opus 5.5 --- public/nodes.js | 2 +- tests/e2e/test-issue-1146-path-link-contrast-e2e.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/public/nodes.js b/public/nodes.js index 0af9abab5..a4dc1fd5f 100644 --- a/public/nodes.js +++ b/public/nodes.js @@ -1004,7 +1004,7 @@ 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 isThis ? html.replace('class="hop ', 'class="hop-current hop ') : html; } const name = escapeHtml(h.name || h.prefix); const link = h.pubkey ? `${name}` : `${name}`; diff --git a/tests/e2e/test-issue-1146-path-link-contrast-e2e.js b/tests/e2e/test-issue-1146-path-link-contrast-e2e.js index 7c496611b..3698ea893 100644 --- a/tests/e2e/test-issue-1146-path-link-contrast-e2e.js +++ b/tests/e2e/test-issue-1146-path-link-contrast-e2e.js @@ -199,7 +199,7 @@ 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, + hopCount: chain.children.length - chain.querySelectorAll(':scope > button').length, }; }, { selector, targetHopKey, siblingKey }); assert(result.hopCount === 18, 'Expected all 18 hops in the rendered chain'); From 2557894d2a2a2fb7131d97a99978b5fa6f59a5a5 Mon Sep 17 00:00:00 2001 From: efiten Date: Sat, 3 Oct 2026 12:59:56 +0200 Subject: [PATCH 6/6] fix(packets): lead the path with its warning so the clip edge never hides it .path-hops clips at its right edge. The per-path ambiguity indicator was appended after the last hop, so in a narrow path column it was the thing clipped: every hop still fitted, no +N pill was added, and the warning was invisible. With #2090's packed columns this made the #1128 "every overflowing .path-hops has a +N pill" E2E fail. The indicator now leads the hops and does not shrink, so the edge clips hops, which the +N pill counts. The +N popover leaves it out of the hop list, as it already did with the pill itself. Locally, against master merged in: #1128 packets layout fails 4/1 without this change and passes 5/0 with it; #1146 11/0, #1128 multi-viewport 15/0. Co-Authored-By: Claude Opus 5.5 --- public/packets.js | 7 +++++-- public/style.css | 2 +- tests/unit/test-issue-2097-hop-ambiguity-badge.js | 8 ++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/public/packets.js b/public/packets.js index d6bbe3ad7..3d7edc6e7 100644 --- a/public/packets.js +++ b/public/packets.js @@ -1111,9 +1111,12 @@ } if (!uncertain) return body; const label = uncertain + ' of ' + filtered.length + ' hops have more than one candidate'; - return body + ' ' + uncertain + ''; + return warn + body; } let directPacketId = null; @@ -2907,7 +2910,7 @@ var kids = Array.prototype.slice.call(host.children); for (var i = 0; i < kids.length; i++) { var k = kids[i]; - if (k.classList.contains('path-overflow-pill')) continue; + if (k.classList.contains('path-overflow-pill') || k.classList.contains('hop-path-warn')) continue; if (k.classList.contains('arrow')) { if (current) segments.push(current); current = ''; diff --git a/public/style.css b/public/style.css index aa0270839..64a9a61ee 100644 --- a/public/style.css +++ b/public/style.css @@ -3119,7 +3119,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. */ -.hop-path-warn { display: inline-flex; align-items: center; gap: 2px; margin-left: 6px; +.hop-path-warn { display: inline-flex; align-items: center; gap: 2px; margin-right: 4px; flex: 0 0 auto; background: var(--status-yellow, #f59e0b); color: #000; 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/tests/unit/test-issue-2097-hop-ambiguity-badge.js b/tests/unit/test-issue-2097-hop-ambiguity-badge.js index 138af9954..4753e911f 100644 --- a/tests/unit/test-issue-2097-hop-ambiguity-badge.js +++ b/tests/unit/test-issue-2097-hop-ambiguity-badge.js @@ -175,6 +175,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'); + // .path-hops clips at its edge. Trailing the hops, the indicator was what + // got clipped: every hop fitted, so no +N pill appeared and the warning + // was invisible (#1128 Bug 1 E2E once the path column narrowed). + assert(/return warn \+ body;/.test(src), + 'the indicator leads the path, so the edge clips hops, never the indicator'); + const seg = (src.match(/function _pathHopSegments\(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 ===');