mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-10-15 03:59:56 +00:00
Fixes #2097. Every 1-byte path prefix on this network is shared. Measured on the live deployment: 254 prefixes cover all 2043 nodes, nine of them on a single prefix, and not one node has a prefix to itself. The packets page printed one of those nine as a certainty. It could already have said otherwise. `hop-resolver.js` marks such a hop `ambiguous` and returns its full candidate list; `hop-display.js` renders a warning badge with a count and a candidate popover; `renderHop(h, observerId)` reads a per-observer cache key. None of it fired, because `HopResolver.resolve()` takes six parameters and `packets.js` passed one. Without `observerId`, `packetIata` is null, `nodeInRegion()` never runs, no candidate is flagged `regional`, `globalFallback` stays false, and `hop-display.js` computes a badge count of 0. The whole chain stayed silent while the data said it was a guess. ## What this changes **Resolution carries the observer.** `resolveHops()` takes it and writes the per-observer cache key that `renderHop()` has always read and nothing ever wrote. `resolveHopsForPackets()` groups a multi-packet call by observer so each group is filtered on its own. One site was the bug in miniature: it wrote its result under `hopNameCache[k + ':' + pkt.observer_id]` after resolving without that observer, so the key promised something the value was not. **The observer's position is the anchor.** `resolve()` has always accepted `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. `observerPosition()` is the single lookup, used by both resolve paths. This matters more than the IATA route, which turns out to be dead on this network. `nodeInRegion()` looks the observer's code up in `/api/iata-coords`; **0 of 42 observers have their code in that table** (`ANR BRU GNE HEP KJK LGG MST NRW OBL OST` are all missing, while its 54 entries are `AMS`, `APC`, `ATL` and the like). So the 300 km filter has never excluded anything here. 27 of 42 observers report lat/lon directly, and that works. Filed separately as #2098, since it is a network-wide lookup that resolves nothing and hop resolution may not be its only consumer. **Each badge belongs to its pill.** The badge is a sibling after the pill, so a path read `A [8] → B [6] → C` with nothing to say which name the 8 qualified. Pill and badge now render inside one `white-space: nowrap` `.hop-group`, leaving the separator outside the pair. Only a hop that carries a badge is wrapped; wrapping every hop would change the layout of every path for nothing. That markup predates this work, but these commits are what made it visible — before them the badge essentially never appeared on this page. **The list summarises, the detail pane does not.** Every 1-byte hop getting its own badge turned a table row into a line of warning triangles. `renderPath()` takes `{ summary: true }` for the three list call sites: names with no per-hop badge, and one indicator reading "N of M hops have more than one candidate". The two detail-pane sites are unchanged and keep a badge per hop, because that is where the candidates actually get read. `HopDisplay` gains `opts.badge === false` for it, and the hop keeps its `hop-ambiguous` class either way so CSS can still mark it. ## Scope Deliberately not included: chaining the next resolved hop to narrow a candidate set to one. `pickByAffinity` already scores on graph edges and would do it; it needs neighbouring context this call does not yet supply, and that belongs in its own change to `hop-resolver.js`. What is here makes the uncertainty visible and picks a defensible candidate rather than the first in index order. ## Verification 30 unit tests in `tests/unit/test-issue-2097-hop-ambiguity-badge.js`, registered in `test-all.sh`. Full frontend suite exits 0. Four of them are structural guards over `packets.js`, and each was mutation-checked by reverting the change it protects: - no `HopResolver.resolve()` call passes the hops alone - no call passes an observer id and drops its position - one helper does the observer lookup - the detail pane calls `renderPath` without `summary` The behavioural test for the anchor runs with `iataCoords` deliberately empty, which is the live state, and lists the distant candidate first: without an anchor the resolver keeps candidate order and picks it, with one it does not, and the hop stays reported as ambiguous with all candidates listed. **Browser validated** on a staging deployment of this branch, against live data: | | list | detail pane | |---|---|---| | per-hop badges | 0 | 24 | | path indicators | 1 | 0 | | badges outside a `.hop-group` | — | 0 | On the packet that prompted the issue, the first hop now resolves to a repeater 10.6 km from the transmitter instead of one 126 km away, and the second hop resolves to the only candidate that has a recorded neighbour edge matching the next hop in the path. ## Note for reviewers The last commit exists because staging disagreed with the tests. After the anchor landed, the detail pane still picked the distant candidate: it resolves through its own `HopResolver.resolve` call, which had the observer id but a null position. The guard that now fails on exactly that shape is what would have caught it before the deploy. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
efiten
co-authored by
Claude Opus 5.5
Claude Opus 5.5
parent
8933e2c703
commit
415362c5af
+13
-5
@@ -87,7 +87,9 @@ window.HopDisplay = (function() {
|
||||
const unreliableBadge = unreliable
|
||||
? ' <button class="hop-unreliable-btn status-warn" aria-label="Unreliable name resolution" title="Unreliable name resolution — this hash\u2192name match is geographically inconsistent with the surrounding path hops. The repeater itself may be fine; this specific hop assignment is uncertain."><svg class="ph-icon" aria-hidden="true"><use href="/icons/phosphor-sprite.svg#ph-warning"/></svg></button>'
|
||||
: '';
|
||||
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 `<a class="${cls} hop-link" href="#/nodes/${encodeURIComponent(pubkey)}" title="${escapeHtml(title)}" data-hop-link="true">${display}</a>${warnBadge}`;
|
||||
}
|
||||
return `<span class="${cls}" title="${escapeHtml(title)}">${display}</span>${warnBadge}`;
|
||||
const pill = opts.link !== false
|
||||
? `<a class="${cls} hop-link" href="#/nodes/${encodeURIComponent(pubkey)}" title="${escapeHtml(title)}" data-hop-link="true">${display}</a>`
|
||||
: `<span class="${cls}" title="${escapeHtml(title)}">${display}</span>`;
|
||||
|
||||
// #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 `<span class="hop-group">${pill}${warnBadge}</span>`;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+1
-1
@@ -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 ? `<a href="#/nodes/${encodeURIComponent(h.pubkey)}"${isThis ? ' class="hop-current"' : ''}>${name}</a>` : `<span>${name}</span>`;
|
||||
|
||||
+102
-46
@@ -981,10 +981,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.
|
||||
@@ -994,14 +1000,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; });
|
||||
// #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;
|
||||
}
|
||||
|
||||
// #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 [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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1028,14 +1078,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 = 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) {
|
||||
// #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
|
||||
@@ -1044,7 +1098,25 @@
|
||||
? window.MC_filterPathHops(hops)
|
||||
: hops;
|
||||
if (!filtered.length) return '— <span class="text-muted" title="All path hops were 1-byte and are hidden by the customizer toggle">(1-byte filtered)</span>';
|
||||
return filtered.map(h => renderHop(h, observerId)).join('<span class="arrow">→</span>');
|
||||
const summary = !!(opts && opts.summary);
|
||||
const body = filtered
|
||||
.map(h => renderHop(h, observerId, summary ? { badge: false } : null))
|
||||
.join('<span class="arrow">→</span>');
|
||||
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';
|
||||
// Leads the hops: .path-hops clips at its right edge, and a trailing
|
||||
// indicator was the one thing clipped, with no +N pill to show for it.
|
||||
const warn = '<span class="hop-path-warn status-warn" title="' + escapeHtml(label) +
|
||||
'"><svg class="ph-icon" aria-hidden="true"><use href="/icons/phosphor-sprite.svg#ph-warning"/></svg>' +
|
||||
uncertain + '</span>';
|
||||
return warn + body;
|
||||
}
|
||||
|
||||
let directPacketId = null;
|
||||
@@ -1340,9 +1412,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();
|
||||
@@ -1398,7 +1468,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) {
|
||||
@@ -1605,11 +1675,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) {
|
||||
@@ -2177,11 +2243,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();
|
||||
});
|
||||
|
||||
@@ -2465,7 +2527,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;
|
||||
@@ -2516,7 +2578,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 += `<tr class="group-child" data-id="${c.id}" data-hash="${c.hash || ''}" data-action="select-observation" data-value="${c.id}" data-parent-hash="${p.hash}" data-entry-idx="${entryIdx}" tabindex="0" role="row"${_childHashStripe ? ' style="' + _childHashStripe + '"' : ''}>
|
||||
<td class="col-expand"></td><td class="col-region">${childRegion ? `<span class="badge-region">${childRegion}</span>` : '—'}</td>
|
||||
@@ -2551,7 +2613,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;
|
||||
@@ -2848,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 = '';
|
||||
@@ -3446,9 +3508,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 ? '' : ('<div class="panel-resize-handle" id="pktResizeHandle"></div>' + PANEL_CLOSE_HTML));
|
||||
const content = document.createElement('div');
|
||||
@@ -3538,7 +3598,11 @@
|
||||
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.
|
||||
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)) {
|
||||
@@ -4320,12 +4384,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
|
||||
@@ -4410,10 +4469,7 @@
|
||||
await loadObservers();
|
||||
const data = await api(`/packets/${param}`);
|
||||
if (!data?.packet) { app.innerHTML = `<div style="max-width:800px;margin:0 auto;padding:40px;text-align:center"><h2>Packet not found</h2><p>Packet ${param} doesn't exist.</p><a href="#/packets">← Back to packets</a></div>`; 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 = `<div style="margin-bottom:16px"><a href="#/packets" style="color:var(--link-color);text-decoration:none">← Back to packets</a></div>`;
|
||||
|
||||
@@ -3109,6 +3109,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-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; }
|
||||
.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);
|
||||
|
||||
@@ -140,6 +140,7 @@ 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-2095-channels-client-state.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
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -0,0 +1,233 @@
|
||||
/**
|
||||
* #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=== #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(/<span class="hop-group">([\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');
|
||||
// .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 ===');
|
||||
|
||||
{
|
||||
// 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');
|
||||
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`);
|
||||
process.exit(failed ? 1 : 0);
|
||||
Reference in New Issue
Block a user