feat(packets): show hop ambiguity, resolved per observer (port of upstream 2099)

[dborup-agent]

Relates to #165

Port of upstream Kpa-clawbot/CoreScope#2099 ("resolve path hops with the observer that heard them"), up to its final head 2557894d (merged upstream; tree-identical to 415362c5). The first port was taken from 3532cede; upstream 4ca1ef87 and 2557894d are covered too (see below). The port includes the fixes the #165 review asks for (defects 1–3, points A–D) and the fixes for the PR #185 review (F1–F7).

Head: 0874df4e. Branch is up to date with master (6444294d, merge 96f91f2d).

What the PR does

• Per-observer hop resolution (public/packets.js): every hop is resolved with the observer that heard it (resolveHops(hops, observerId), resolveHopsForPackets), and cached under hop:observer. The bare key stays the name fallback.
• Server answers first: a hop the server resolved (resolved_path) is cached under the bare key and hop:observer, so the client heuristic never replaces it. Hops the server left null are resolved on the client and may show as uncertain.
• Grouped rows carry resolved_path (cmd/server, PR feat(packets): show hop ambiguity, resolved per observer (port of upstream 2099) #185 F1): /api/packets?groupByHash=true now sends the resolved_path of the observation each row displays (same observer_id and path_json). Without it the default view guessed every hop.
• Ambiguity UI:
  • List: names without per-hop badges, and one .hop-path-warn summary that leads the path cell ("N of M hops have more than one candidate"). Upstream's final version (2557894d) leads with it as well.
  • The summary counts exactly the hops the detail pane badges (HopDisplay.conflictBadgeCount), from the row observer's own entry only. Counted pills carry hop-uncertain.
  • Detail pane: per-hop badges in .hop-group.
  • Colours use CSS variables only.

Fixes from the #165 review

Item	Fix
Defect 1	Missing observer coordinates and (0, 0) give [null, null] (observerPosition, coordOrNull)
Defect 2	Server resolved_path is written under hop:observer too, so it wins over the heuristic
Defect 3	The live path (resolveIncomingHops) checks the per-observer key
A	Unit tests in the root, registered in test-all.sh; the Kpa-clawbot#1153 E2E counts hop elements
B	Hop cache is a Map capped at 50000 with oldest-first eviction; destroy() resets it
C	Yield between observer groups, plus scripts/bench-hop-resolution.js
D	Tests that the server answer wins, for flat rows, children and grouped rows

Upstream 4ca1ef87 (hop-current on the pill) is covered by HopDisplay.renderHop(..., { className }) instead of patching class="hop . Upstream 2557894d (the +N popover skips the summary) is ported in 8310f8a1.

Fixes from the PR #185 review (round 2)

#	Fix	Commit
F1	Grouped rows carry the displayed observation's resolved_path (in-memory store and SQL fallback); the list counts only the observer's own entry; a server answer replaces an ambiguous client pick of the same node	50e462be (test), 6649f867, 393b39f9 (test), 8d46c5fe, 0874df4e
F2	The review's two probes are unit tests (MD, MC)	393b39f9
F3	A bench-sized working set (42 × 690 hops) is cached without eviction; rewrite-refresh is tested	393b39f9
F4	This description	–
F5	The +N popover title counts hops ("Full path (9 hops)")	441cd76b
F6	One badge rule for list and detail (HopDisplay.conflictBadgeCount)	8d46c5fe
F7	destroy() bumps a generation; hop jobs from before it stop writing	ae70759e, 0874df4e

Perf

Server, /api/packets?groupByHash=true (default view): BenchmarkGroupedPackets165, 30K transmissions, 2 observations each, 3-hop paths, desktop limit 50000, 3 runs, 4-core cloud VM.

	before F1	with F1
per request	28.2–29.8 ms	90.1–95.6 ms
JSON	12.24 MB	16.95 MB (+38 %)

• The resolved_paths of a page are read in one query (ids as a json_each parameter) after s.mu is released, and passed through as stored JSON. Most of the added time is SQLite stepping 30K rows (~2 µs per row). 500-id batches with JSON parsing measured 117–126 ms.
• On the CI fixture (499 grouped rows) the response grows from 434 KB to 590 KB JSON (+36 %), 90 KB to 122 KB gzipped (+35 %), about 63 gzipped bytes per row. At 30K rows that is roughly +1.9 MB gzipped per default load (mobile loads 1000 rows).

Client, initial hop resolution: RP_SHARE=<share> node scripts/bench-hop-resolution.js [publicDir], 30K packets, 2000 repeaters, 42 observers (27 with coordinates), 0–8 hops, median of 7 runs, range over 3 interleaved rounds. RP_SHARE is the share of packets with a resolved_path (0.3 as before; 0.95 is about what grouped rows carry after #190).

public/	RP_SHARE	total	longest main-thread block	cache entries
origin/master	0.3	70–74 ms	71–75 ms	2236
previous head 8310f8a1	0.3	285–295 ms	101–106 ms	28962
this head	0.3	291–295 ms	31–34 ms	28962
origin/master	0.95	126 ms	126–127 ms	2236
previous head 8310f8a1	0.95	324–325 ms	233–240 ms	28755
this head	0.95	246–251 ms	25 ms	28755

• cacheResolvedPaths skips rows whose answers are already cached and yields every 2000 rows, so the longest block is now below master's.
• Total time stays 2–4× master's, because each observer group is resolved separately. It runs after the first render (fix(#1692): parallelize loadObservers + loadPackets in /packets init() Kpa-clawbot/CoreScope#1693).
• Complexity: O(rows × hops) to cache server answers, O(observer groups × distinct hops per group) to resolve the rest; no per-item API calls.

Tests

• Unit (vm harness, real packets.js, hop-resolver.js, hop-display.js):

  • test-issue-165-hop-resolution-per-observer.js: 32/32.
  • test-issue-165-hop-ambiguity-badge.js: 36/36.
  • sh test-all.sh: 229/229 files; test-frontend-helpers.js: 709/709.

• Go: cmd/server and cmd/ingestor suites pass (-timeout 30m). New: TestGroupedPacketsCarryHeaderResolvedPath165, TestGroupedPacketsSQLCarryHeaderResolvedPath165, BenchmarkGroupedPackets165.

• E2E against a local Go server on e2e-fixture.db, prepared as in CI:

  Test	Result
  test-issue-165-grouped-hop-warn-e2e.js (new, in deploy.yml)	3/3
  test-e2e-playwright.js	132/135, 3 skipped
  test-issue-1146-path-link-contrast-e2e.js	11/11
  test-issue-147-packets-url-obs-e2e.js	12/12
  test-issue-180-packets-url-modal-e2e.js	12/12
  test-issue-1128-packets-layout-e2e.js / -multi-viewport-	5/5, 15/15
  test-issue-1122-details-row-clamp-e2e.js / -packets-filter-ux-	18/18, 6/6
  test-slideover-1056-e2e.js / test-slideover-1168-munger-e2e.js	27/27, 3/3
  test-issue-1692-…, test-path-inspector-coverage-e2e.js, test-packet-trace-alignment-e2e.js	1/1, 10/10, 17/17
  test-issue-1486-…, test-issue-259-…, test-issue-258-…, test-packet-detail-sender-hash-size-obs-e2e.js	4/4, 10/10, 22/22, 3/3

• Browser (Chromium, fixture, 24 h window): grouped first load 0 warnings on 61 rows (was 24); flat 0; back to grouped 0. With resolved_path stripped from the grouped response, 24 rows show the summary. The +N popover reads "Full path (6 hops)" for 6 hops. At 390 px the path column is hidden, as on master. No page errors.

• Mutants: 20 new ones, all caught (listed in the round-2 report on this PR), plus the 8 from round 1.

• Static: check-xss-sinks.sh --diff origin/master clean, check-css-vars.js 0 undefined, eslint@8 0 errors. Fork guards in deploy.yml: 9 before and after.

Not verified / known limitations

• Not verified on staging or production data, and not profiled in a real browser. Server and client numbers come from synthetic loads and the CI fixture.
• The default view's response grows by about a third (see Perf). This is the price of the server's answer in grouped rows, which Port upstream #2099 hop ambiguity UI with observer/cache correctness fixes #165's first acceptance point needs.
• Total client hop-resolution time is 2–4× master's. By design: resolution runs per observer, after the first render, in short blocks.
• The cache key is (hop, observer). Two rows from the same observer whose server paths name different nodes for the same prefix share one entry; the last differing answer wins.
• The hex-breakdown hop rows (HopDisplay.renderHop(hex, hopCacheGet(hex))) still read only the bare key.
• Customizer (AGENTS.md rule 8): HOP_CACHE_MAX and the list summary are hardcoded.
• Commit message errata: 6649f867 says "a statement per row, or 500-id batches, measured 117-126 ms"; only the 500-id batch variant was measured. The bench commit of round 1 names the port commit 18afd8d; it is 3c9976d8.

🤖 Generated with Claude Code

[dborup-agent]

Rapport — CS-pve-agent2 PR#185 hop-ambiguity — head 8310f8a

Status: Draft, mergeable against master, CI green on 8310f8a1; not marked ready and not merged.

What is ported

Upstream PR 2099 (Kpa-clawbot/CoreScope#2099), merged upstream at head 2557894d. The first port was taken from 3532cede. The two upstream commits after that are covered:

• 4ca1ef87 (hop-current on the pill): covered by this branch's own fix, not copied.

  • HopDisplay.renderHop takes opts.className, and nodes.js passes hop-current through it. Upstream patches class="hop in the returned HTML instead.
  • Covered by test-issue-165-hop-ambiguity-badge.js: "className is applied to the pill inside the group, not to the wrapper" and "nodes.js no longer patches the first class attribute…".
  • Covered by the ui(node-detail): "this node" highlight in path chain is too subtle (and unreachable when names mis-resolve) Kpa-clawbot/CoreScope#1153 E2E in test-issue-1146-path-link-contrast-e2e.js, 11/11. Before the fix it had 4 failures.
  • The hop count in that E2E counts hop elements, not chain children (upstream counts direct button children instead).

• 2557894d (lead the path with its warning):

  • The leading summary, flex: 0 0 auto, and its exclusion from the +N count were already here (37dd2872).
  • Ported in 8310f8a1: the +N popover skips the summary, through _pathPopoverHtml(host); return warn + body; 4px margin; upstream's two test assertions.

• Fork fixes from Port upstream #2099 hop ambiguity UI with observer/cache correctness fixes #165:

  Item	Fix
  Defect 1	Missing observer coordinates and (0, 0) give [null, null]
  Defect 2	Server resolved_path is also written under hop:observer, so it wins over the heuristic
  Defect 3	The live path checks the per-observer key
  A	Unit tests in the root, registered in test-all.sh
  B	Map cache capped at 50000 with oldest-first eviction; destroy() resets it
  C	Yield between observer groups, plus scripts/bench-hop-resolution.js

• Ambiguity UI:

  • The list shows one leading summary per path.
  • The detail pane shows per-hop badges in .hop-group.
  • Colours use CSS variables only.

Tests and CI

• Unit tests:

  • test-issue-165-hop-resolution-per-observer.js: 17/17.
  • test-issue-165-hop-ambiguity-badge.js: 34/34.
  • sh test-all.sh: 202/202 files.
  • Each defect test was red before its fix. Of the first 14 defect tests, 12 are red on the unmodified port; the other 2 are positive controls.

• Mutants: 8, all caught. They cover:

  • the (0, 0) bug;
  • the client overwriting the server;
  • a bare-key-only live path;
  • an unbounded cache;
  • no yield;
  • nodes.js patching the wrapper;
  • a trailing summary;
  • the summary counted in +N.

• Local packets E2Es against a fixture-built server, all green:

  Test	Result
  test-e2e-playwright.js	132/135, 3 skipped
  test-issue-1146-path-link-contrast-e2e.js	11/11
  test-issue-147-packets-url-obs-e2e.js	12/12
  test-issue-1128-packets-layout-e2e.js	5/5
  test-issue-1128-multi-viewport-e2e.js	15/15
  test-issue-1122-details-row-clamp-e2e.js	18/18
  test-issue-1692-packets-init-parallel-e2e.js	1/1
  test-slideover-1056-e2e.js	27/27
  test-path-inspector-coverage-e2e.js	10/10
  test-packet-trace-alignment-e2e.js	17/17

• Perf: synthetic 30K packets, 42 observers; median of 7 runs, range over 3 runs.

  Measure	master	This branch
  Total hop resolution	64–66 ms	266–270 ms
  Longest main-thread block	65–67 ms	93–95 ms

• Static checks:

  • check-xss-sinks.sh --diff and check-css-vars.js: clean.
  • Fork guards in deploy.yml: 9 before, 9 after.

• CI run 37125701260 on 8310f8a1: Go Build & Test (incl. sh test-all.sh) success, Playwright E2E success, Build & Publish Docker Image success; Release Artifacts, Publish Badges & Summary and Deploy Staging skipped.

Known remaining items

• Not verified on production or staging data. The perf numbers come from a synthetic load in Node's vm, not from a browser profile.
• Total hop-resolution time is about 4× master's. This is by design: resolution runs per observer, after the first render.
• The cache key is (hop, observer). Two packets from the same observer whose server paths differ for the same prefix share one entry, and the last write wins.
• The hex-breakdown hop rows (packets.js, renderHop(hex, hopCacheGet(hex))) still read only the bare key.
• The +N popover title ("Full path (N items)") still counts every child, including the pill and the summary. This was pre-existing.
• Pre-existing: test-issue-1146-path-link-contrast-e2e.js with SCREENSHOT_DIR set fails 3 side-panel screenshot steps, on master too. CI does not set the variable.
• Customizer (AGENTS.md rule 8): HOP_CACHE_MAX and the list summary are not exposed yet.
• One stale SHA: the bench commit message refers to the port commit by its pre-reword SHA 18afd8d; it is 3c9976d8 in this history.

[adminopenclaw8-sketch]

Review — CS-Macmini PR#185 hop-ambiguity — head 8310f8a

Verdict: REQUEST CHANGES. The port is faithful and well tested where it is tested, but in the default (grouped) packets view the new list summary warns about hops that the server has resolved: 23 of 60 rendered rows on the CI fixture. That breaks #165's first acceptance point. Everything else is nits.

The review was read-only. It used git archive of head 8310f8a100457d9ea059df841701db308de4d494, of the merge with origin/master 8e4b13d0 (git merge-tree tree c2fa527a, clean), and of the merge with the newer master 6f7a5e7c (tree d9f7544a, clean). git ls-remote showed the same head before and after. Nothing was pushed, and the PR was not changed.

Evidence tags: [T] test, CI or browser run here, [A] analysis, [K] taken from the author's report and not rerun.

Findings

#	Prio	Where	Finding
F1	high	packets.js list summary, grouped view	/api/packets?groupByHash=true returns no resolved_path [T], so cacheResolvedPaths() is a no-op for the default view and every hop is resolved by the client heuristic. The new .hop-path-warn then flags hops the server has a definite answer for. Fixture with a 24 h window, desktop 1440 [T]: grouped first load has 23 warnings on 60 rows; flat view has 0; back to grouped also 0, because the flat load cached the server answers per observer. So the default view's warning depends on navigation history. Example: e8b09a35ac87fa5c. The list says "1 of 9 hops have more than one candidate" (hop F5, 2 regional candidates), the server's resolved_path names f509fda5… for F5, and the detail pane shows F5 with no badge, while the list row keeps its warning until it re-renders. Defect 2's tests only cover packets that carry resolved_path (flat or children), which is the "grouped packets have different fields" pitfall from AGENTS.md.
F2	medium	tests	Two mutants of behaviour this PR adds survive the whole test-all.sh: MD, where the list summary counts via the bare key instead of hop:observer, and MC, where resolveHops() overwrites the bare key, which holds the server's answer, with the heuristic. Probe tests that catch both are below.
F3	low	HOP_CACHE_MAX / hopCacheSet	The cap value is not pinned (mutant MB HOP_CACHE_MAX = 100 survives), and neither is the refresh-on-rewrite order (MA survives). A cap below the working set would evict server-resolved hop:observer entries, and the heuristic would then replace them, which is defect 2 again under pressure. Suggest a test that the bench-shaped load does not evict (cache size after load equals the distinct key count), or an assertion HOP_CACHE_MAX >= the measured working set.
F4	low	PR description	Stale. It says head 3532cede, but the port now also covers 2557894d. It calls the leading summary a "deviation from upstream", but upstream's final version (2557894d, tree-identical to 415362c5) also leads with it. Test counts 16/16 and 32/32 are now 17/17 and 34/34, and the local runs are on 37dd2872. F1 belongs under limitations. No closing keywords; the deploy.yml statement ("registered only in test-all.sh") is correct.
F5	low	_pathPopoverHtml	The title "Full path (N items)" counts host.children, which now also includes the summary. The fixture row shows "19 items" for 9 hops and 8 arrows [T]. The miscount (the pill) existed before, but the PR adds +1 and now has a helper that knows which children it skips, so counting what it renders is a one-line fix.
F6	low [A]	summary vs badge criterion	The list counts entry.ambiguous; the detail pane shows a badge only for ≥2 regional conflicts, or for globalFallback with ≥2 conflicts. An ambiguous entry with fewer regional conflicts would be flagged in the list and show no badge in the detail. Inherited from upstream; not observed on the fixture beyond F1. Consider counting exactly the hops HopDisplay would badge.
F7	info	resolveHopsForPackets	With yields, the initial hop job can keep running after destroy() and write into the new Map. That is harmless (same data, renderTableRows is still gated by filtersBuilt), but a generation check would make it explicit.

1. Faithfulness to upstream (final 2557894d = 415362c5)

[T] Upstream's own final unit test, run unchanged against the merged tree: 30 passed, 2 failed. Both failures are source-text assertions tied to upstream's structure:

• the regex hopNameCache[hopCacheKey(h, observerId)] = entry (the fork uses hopCacheSet on a Map);
• _pathHopSegments, a function that only exists in upstream's popover. The fork's popover builder is _pathPopoverHtml.

Both behaviours are covered by the fork's own tests.

[A] Hunk by hunk, everything in upstream's final diff is present:

• the observer-map seed in ensureHopResolver;
• observerPosition and hopCacheKey;
• resolveHops(hops, observerId) and resolveHopsForPackets;
• renderHop/renderPath options and the three { summary: true } list call sites;
• every resolveHopsForPackets([pkt]) caller and the group-expand path;
• the observer-aware resolve in renderDetail;
• the WS path;
• the popover skip;
• hop-display.js (opts.badge, .hop-group) and style.css.

Nothing from upstream is missing.

Deliberate deviations:

Deviation	Verdict
opts.className instead of patching class="hop in nodes.js (upstream 4ca1ef87)	Better: it cannot hit the wrapper. Browser: 364 .hop-current on 60 node pages, 0 on a .hop-group [T]. The fixture has no node path with a badged hop, so the in-group case is covered by the unit test only.
coordOrNull and (0, 0) treated as no fix (defect 1)	Correct; it matches the server's hasRealFix.
cacheServerResolvedPath also writes hop:observer (defect 2)	Correct where resolved_path exists; see F1 for where it does not.
resolveIncomingHops with a per-observer check (defect 3)	Correct. The old rp.length === hops.length guard is not lost: resolveFromServer returns {} on a length mismatch.
Bounded Map cache plus a yield between observer groups (B, C)	Fine.
CSS uses var(--status-yellow) / var(--palette-gray-900) without hex fallbacks; hop-path-warn excluded from the +N hidden count	Fine. The +N exclusion is harmless, since the summary leads.
Test paths in the root; the #1153 E2E counts hop elements	Fine.

2. Defects 1–3 and points A–D from #165

Item	Fixed	Tested	Notes
Defect 1, null and (0, 0) coordinates	yes	5 tests [T]
Defect 2, server over heuristic	partly	3 tests, flat/children only [T]	not in the grouped default view (F1)
Defect 3, live path per observer	yes	2 tests [T]
A, root tests in test-all.sh	yes	[T]	deploy.yml unchanged
B, cap 50000, oldest-first eviction, destroy() reset	yes	3 tests [T]	cap value unpinned (F3)
C, yield and bench	yes	1 test plus bench [T]
D, test that the server wins	yes for flat	[T]	none for grouped (F1)

3. Performance

• [A] No O(n²): grouping is O(packets × hops), and each group resolves its distinct hops. No per-item API calls: ensureHopResolver fetches once. No new full DOM rebuild: the existing renderTableRows() runs after the job. Cache writes are O(1).

• [T] scripts/bench-hop-resolution.js, 3 interleaved rounds (median of 7 runs each). Apple M4, Node 22.22.1, load average about 4.5:

  public/	total initial hop resolution	longest main-thread block	cache entries
  master 8e4b13d0	42.5–53.0 ms	43.9–60.3 ms	2236
  head 8310f8a1	225.9–230.7 ms	64.2–65.7 ms	28962
  merged	220.8–224.7 ms	62.3–63.0 ms	28962

  This reproduces the author's shape (about 4.5× total, longest block about +15 ms, which is acceptable after first render).

• The 50000 cap is reasonable: about 1.7× the synthetic worst case of 28962. At the author's [K] 9.6 MB for 29K entries it scales to about 16 MB at the cap [A]. It is fine if F3 pins it.

4. Tests and mutants

• [T] On the merged tree, Node 22:

  • test-issue-165-hop-resolution-per-observer.js: 17/17;
  • test-issue-165-hop-ambiguity-badge.js: 34/34;
  • sh test-all.sh: 203/203 files.

  On the merge with the newer master 6f7a5e7c: 203/203.

• [T] CI on head: Go Build & Test, Playwright E2E and Docker build passed.

Own mutants, each in a copy of the merged tree with the full test-all.sh:

Mutant	Result
MA hopCacheSet without the delete-before-set (a rewrite does not refresh eviction order)	survives
MB HOP_CACHE_MAX = 100	survives (F3)
MC resolveHops always overwrites the bare key	survives; red with probe 2 below
MD list summary counts via the bare key only	survives; red with probe 1 below

Probe tests, appended in scratch to the PR's own harness (green on the merged tree, red under the mutant):

1. observer A has the server's answer for "ef", observer B's "ef" is client-resolved and ambiguous → renderPath(['ef'], 'OBS-B', {summary:true}) shows .hop-path-warn (red under MD);
2. …same setup → the bare "ef" stays the server node (FAR-AWAY) (red under MC: NEAR-ONE).

5. Browser

Local Go server built from the merged tree on the CI fixture: freshen, deploy.yml seed SQL, corescope-migrate, seed-2073. I confirmed it served the merged tree: analytics.js matches merged, not head. It was stopped by its listening pid.

E2E (merged tree)	Result
test-issue-1146-path-link-contrast-e2e.js (#1153/#1146)	11/11
test-issue-147-packets-url-obs-e2e.js (#167)	12/12
test-issue-1128-packets-layout-e2e.js / -multi-viewport-	5/5, 15/15
test-issue-1122-details-row-clamp-e2e.js / -packets-filter-ux-	18/18, 6/6
test-slideover-1056-e2e.js	27/27
test-issue-1692-…, test-path-inspector-coverage-e2e.js, test-packet-trace-alignment-e2e.js, test-issue-1486-…	1/1, 10/10, 17/17, 4/4
test-e2e-playwright.js	Fail-fast stops at "Version info lives on Perf dashboard" on master's public/ too, the same binary and fixture (a local environment baseline; CI is green). In a scratch copy with fail-fast disabled: merged 131 passed / 1 failed / 7 skipped, master 131 / 1 / 7, with the identical single failure.

Manual checks in Chromium (Playwright scripts plus the in-app browser at 1440×900):

• The summary is the first child of .path-hops and inside the cell on all checked rows.
• At 390 px the path column is col-hidden on master and merged alike, so nothing is clipped.
• The +N popover contains the 9 hops and no .hop-path-warn. Its title says 19 items (F5).
• hop-current sits on the pill, never on the wrapper.
• No page errors. The only console errors are 404 /api/nodes/<id>/clock-skew, the same on master.
• Deep links #/packets/<hash> and ?timeWindow= work (and the fix(packets): keep ?obs=/?viewPath= on packet detail URLs (#147) #167 E2E passed).
• F1 was reproduced visually: the list shows ⚠1 for e8b09a35…, and the detail pane shows Homestead MC without a badge.

6. Rules

• [T] Added lines contain no hex colours, and check-css-vars.js reports 0 undefined.
• [T] scripts/check-xss-sinks.sh --diff origin/master passes (rc 0, in a shared scratch clone at head).
• [T] Fork guards: 9 on master, head and merged, and deploy.yml is unchanged.
• [T] The PR text has no closing keywords, and its deploy.yml statement is correct; the stale parts are F4.

Suggested fixes for F1 (pick one)

1. Server: include the header observation's resolved_path in groupByHash rows (cmd/server, read-only side). This changes the API shape, so it needs a JSON-size and perf check on a 30K grouped load.
2. Client, minimal: render the list summary only for rows that carry resolved_path (flat rows and expanded children), and leave grouped header rows without it until the server data is known. Add a grouped-row test.

Either way, add a test with a grouped packet (no resolved_path) whose prefix the server has resolved.

Not verified

• Real 30K data, staging or production, or a browser performance profile. The bench is the author's synthetic vm load.
• The heap size of the cache [K].
• A badged hop on a node detail page in the browser: the fixture has none, so only the unit test covers it.
• Firefox and WebKit.
• How many of the 23 grouped warnings would remain with server data (flat shows 0, so I infer none) [A].

[dborup]

Status — parked until #190

Decision: #185 is parked. It is not part of the next staging round (v0.2.1). The branch codex/issue-165-hop-ambiguity (head 8310f8a1) is kept as is. Nothing is closed or discarded.

Why

• The review found a correctness problem in the default view. Grouped rows carry no resolved_path, so the list summary warns about hops the server has already resolved (23 of 60 rows on the CI fixture on first load).

• The cost is real. Per-observer client-side resolution costs about 4× the CPU of master on the synthetic 30K-packet load:

  • total about 65 → 268 ms on the cloud machine;
  • 42–53 → 226–231 ms on an M4.

  It runs after first render with yields between observer groups, but it has not been profiled in a real browser or on real data.

• feat(ingestor): resolve the last hop from the observer (#188) #190 changes the picture. It moves more hop resolution to the ingestor: the NULL share in the replay drops from about 32 % to about 5 %. Once that lands, most hops can come from the server's persisted, per-observer resolved_path instead of being guessed in every browser.

Plan when this is picked up again (after #190 is merged)

• Prefer serving the server's resolution in grouped rows. This was option A in the review, and it needs a JSON size and perf check. The client would then only guess hops the server left unresolved. That should fix the default-view warnings and cut most of the client-side cost.
• Address the remaining review items:
  • the two surviving mutants, using the probes in the review;
  • a test that pins the cache cap;
  • an up-to-date PR description;
  • the popover title count.
• Re-measure with scripts/bench-hop-resolution.js, and on real data if possible.

[dborup-agent]

Rapport — CS-pve-agent1 PR#185 runde 2 — head 0874df4

Review feedback addressed (commit 0874df4e)

Evidence tags: [T] test, CI or browser run in this session, [A] analysis, [K] taken from an earlier report and not rerun.

Branch: origin/master 6444294d merged in as 96f91f2d (no conflicts), then 7 commits. Pushed as a fast-forward from 8310f8a1. The PR stays draft; the description is updated (F4).

1. F1 (blocking): grouped rows warned about server-resolved hops. Fixed on both sides.
   • Server (6649f867, test 50e462be): /api/packets?groupByHash=true now sends the resolved_path of the observation each row displays (same observer_id and path_json), from the in-memory store (groupedPageWithRP, headerObservationID) and from the SQL fallback (o.resolved_path, or NULL without the column on v2). The page's paths are read in one query after s.mu is released, and passed through as stored JSON. This was option A in the review and the plan in the parking comment.
   • Client (8d46c5fe): the summary reads only the row observer's own hop:observer entry. The bare key is another observer's answer, which made the warning depend on navigation history.
   • Client (0874df4e): cacheServerResolvedPath replaces an ambiguous client pick even when it names the same node as the server. It skips a row whose answers are already cached, and cacheResolvedPaths yields every 2000 rows. Without that, the extra resolved_paths pushed the longest block to 227–249 ms.
   • [T] On the fixture, 24 h window, desktop 1440: the grouped first load has 0 warnings on 61 rows (24 before). Flat view has 0, and back to grouped has 0. In the browser, e8b09a35… no longer warns for F5.
   • [T] New E2E test-issue-165-grouped-hop-warn-e2e.js, registered in deploy.yml. Every hop a row's summary counts (.hop-uncertain) must be one that the observation's server resolved_path leaves null. A control with resolved_path stripped still renders the summary (24 rows).
2. F2: surviving mutants MD and MC. The review's probes are unit tests: MD (summary counts per observer) and MC (the heuristic does not overwrite the server's bare key). [T] Both mutants are now caught.
3. F3: cap and eviction order not pinned.
   • A bench-sized working set (42 observers × 690 hops = 29670 keys) must be cached with no eviction. [T] MB (HOP_CACHE_MAX = 100) is caught.
   • A rewritten entry must be refreshed for eviction order: the cache is filled exactly to the cap, the oldest entry is rewritten with the server answer, then four keys are added. [T] MA is caught.
4. F4: stale PR description. Rewritten:
   • final upstream head 2557894d;
   • the leading summary is no longer described as a deviation;
   • current counts, server and client perf;
   • F1 under the fixes;
   • limitations and commit-message errata.
5. F5: popover title. (441cd76b) The title counts the hops it lists: "Full path (6 hops)" for 6 hops in the browser [T].
6. F6: summary vs badge criterion. (8d46c5fe) HopDisplay.conflictBadgeCount(entry) is now the badge's own count, used by renderHop and by the summary, so the list flags exactly the hops the detail pane badges.
   • Consequence: an ambiguous hop with no badge is no longer counted, for example under an observer without an IATA code.
   • Two existing tests gave their observer an IATA code so their ambiguous hop is badged. Their intent is unchanged.
7. F7: hop job after destroy(). (ae70759e, 0874df4e) destroy() bumps hopCacheGen. resolveHops, resolveHopsForPackets and cacheResolvedPaths stop when it changed while they waited.

No disagreements with the findings.

Tests (red first)

• [T] 50e462be: the two new Go tests are red on the branch and green after 6649f867.

• [T] 393b39f9: 8 of 32 and 2 of 36 unit tests are red. After 8d46c5fe 5 remain red, after 441cd76b 4, after ae70759e 3, and after 0874df4e 0.

• [T] The E2E is red against the unfixed server: 24 warned rows, 26 hops that the server resolved.

• [T] Final head, locally:

  • test-issue-165-hop-resolution-per-observer.js 32/32;
  • test-issue-165-hop-ambiguity-badge.js 36/36;
  • sh test-all.sh 229/229 files;
  • test-frontend-helpers.js 709/709;
  • cmd/server go test ./... ok (507 s; -timeout 30m);
  • cmd/ingestor ok (1048 s).

• [T] The Go suite caught a real bug during the work: the first SQL change selected o.resolved_path on the v2 schema (TestDetectSchemaV2Queries). It is now guarded by hasResolvedPath().

• [T] E2E against a local Go server on e2e-fixture.db, prepared as in CI (freshen, deploy.yml seed SQL, corescope-migrate, seeds 2073/199/245):

  Test	Result
  test-issue-165-grouped-hop-warn-e2e.js	3/3
  test-e2e-playwright.js	132/135, 3 skipped
  test-issue-1146-path-link-contrast-e2e.js	11/11
  test-issue-147-…, test-issue-180-…	12/12, 12/12
  test-issue-1128-… layout, multi-viewport	5/5, 15/15
  test-issue-1122-… details-row, filter-ux	18/18, 6/6
  test-slideover-1056-…, -1168-munger-	27/27, 3/3
  test-issue-1692-…, test-path-inspector-coverage-…, test-packet-trace-alignment-…	1/1, 10/10, 17/17
  test-issue-1486-…, test-issue-259-…, test-issue-258-…, test-packet-detail-sender-hash-size-obs-…	4/4, 10/10, 22/22, 3/3

• [T] Static checks:

  • check-xss-sinks.sh --diff origin/master: rc 0;
  • check-css-vars.js: 0 undefined;
  • eslint@8: 0 errors;
  • fork guards in deploy.yml: 9, same as master.

Mutants

All 20 are caught [T]. Each was run in a copy of the tree. The client mutants were run with the full test-all.sh; in that copy, test-privacy-page.js and test-issue-1648-m5-emoji-scan.js also fail on the unmutated tree, because the copy has no cmd/. The Go mutants were run with the grouped and #165 Go tests.

Mutant	Caught by
F1 server: resolved_path removed from grouped responses (route level)	E2E: 24 false warnings return, 2/3 fail
S1: in-memory grouped rows without resolved_path	TestGroupedPacketsCarryHeaderResolvedPath165; also the E2E (the fixture uses the in-memory path)
S2: SQL grouped rows without resolved_path	TestGroupedPacketsSQLCarryHeaderResolvedPath165
S3: first observation instead of the displayed one	TestGroupedPacketsCarryHeaderResolvedPath165
S4: no column guard (v2)	TestDetectSchemaV2Queries
F1c: summary falls back to the bare key	"the summary does not count another observer's entry"
MD: summary via the bare key only	probe 1, plus the test above
MC: resolveHops always overwrites the bare key	probe 2
MA: no refresh on rewrite	"rewriting an entry refreshes it…"
MB: HOP_CACHE_MAX = 100	"a bench-sized working set … fits without eviction"
F5: title counts all children	"the title counts the hops it lists…"
F6: summary counts entry.ambiguous	"an ambiguous hop without a badge … is not counted"
F6b: badge rule diverges from conflictBadgeCount	badge test, plus "a badged hop is counted…"
F7: no generation check in resolveHops	"destroy() during resolveHopsForPackets…"
F7b: destroy() does not bump the generation	same
C1: cacheResolvedPaths without yield	"cacheResolvedPaths yields…", "destroy() during cacheResolvedPaths…"
C2: no skip of a repeated server answer	"a repeated server answer does not rewrite the cache entry"
C3: skip ignores a different pubkey	"a different server answer for the same key still replaces it"
C4: skip keeps an ambiguous client pick of the same node	"a server answer replaces an ambiguous client pick of the same node" (added after C4 first survived)
C5: no generation check in cacheResolvedPaths	"destroy() during cacheResolvedPaths stops it too"

Perf

• Server, BenchmarkGroupedPackets165 [T] (30K transmissions, 2 observations each, limit 50000, 3 runs, 4-core VM):

  • request time 28.2–29.8 ms → 90.1–95.6 ms;
  • JSON 12.24 MB → 16.95 MB (+38 %).

• Fixture [T]: grouped response 434 KB → 590 KB JSON (+36 %), and 90 KB → 122 KB gzipped (+35 %).

• Client, scripts/bench-hop-resolution.js [T]. Interleaved runs, median of 7, range over 3 rounds. RP_SHARE is new, with default 0.3.

  public/	RP_SHARE	total	longest block
  master	0.3	70–74 ms	71–75 ms
  8310f8a1	0.3	285–295 ms	101–106 ms
  head	0.3	291–295 ms	31–34 ms
  master	0.95	126 ms	126–127 ms
  8310f8a1	0.95	324–325 ms	233–240 ms
  head	0.95	246–251 ms	25 ms

CI on 0874df4e (run 37598118692)

[T] Each job ran once; no rerun was needed, and #271 and #301 did not occur.

Job	Result
Go Build & Test	success; server ok, 1164 s, coverage 90.5 %
Playwright E2E Tests	success; the new E2E steps are ✓ in the log
Build & Publish Docker Image	success
Release Artifacts	skipped
Deploy Staging	skipped
Publish Badges & Summary	skipped

Not verified / remaining

• Staging and production data, and a real-browser performance profile. All numbers come from synthetic loads and the CI fixture. The share of grouped rows that carry resolved_path after feat(ingestor): resolve the last hop from the observer (#188) #190 is taken from the parking comment [K].
• The default view's response grows by about a third, roughly +1.9 MB gzipped at 30K rows [A].
• Total client hop-resolution time stays 2–4× master's (per observer, after first render, in short blocks).
• The hex-breakdown hop rows still read only the bare key. The customizer does not expose HOP_CACHE_MAX or the summary yet.
• Erratum: the message of 6649f867 says "a statement per row, or 500-id batches, measured 117-126 ms". Only the 500-id batch variant, with JSON parsing, was measured.

[adminopenclaw8-sketch]

Review — CS-Macmini PR#185 — head 0874df4

Dom: APPROVE med nits

Round-2 re-review of my own round-1 REQUEST CHANGES. The blocking finding is fixed at the source, and I reproduced both states myself: on the CI fixture the grouped first load went from 23 warned rows of 60 (25 flagged hops, all 25 of them hops the server had resolved for that row's observer) to 0 of 60, with no console errors. The other six round-1 findings are resolved. What is left is test gaps around the new server code and wording, none of which blocks.

Read-only. I used git archive of head 0874df4ecfea141b4a28c5ec95f4b67577c85427; git merge-tree --write-tree origin/master <head> gives tree 11c9329d, which is identical to the head tree — the branch is 0 commits behind origin/master (6444294d), so head and merged tree are the same thing and every test below ran on both at once. git ls-remote showed 0874df4e before and after, and the PR is still a draft. Nothing was pushed and the PR was not changed.

Evidence tags: [T] test, CI or browser run here, [A] analysis, [K] taken from the author's report and not rerun.

Findings

#	Prio	Where	Finding
N1	low	headerObservationID (cmd/server/store.go)	Test gap. The "displayed observation, not the first" contract is pinned only across different observers: seedGroupedRP165 gives tx 165a… one observation per observer. My mutant MM-A drops the o.PathJSON == tx.PathJSON half of the match and survives the whole cmd/server suite and the new E2E [T]. Observations dedupe on observerID|pathJSON (chunked_load.go ~1066), so one observer that hears the same transmission by two routes yields two observations, and the mutant then attaches the wrong one's resolved_path. Different lengths ⇒ resolveFromServer returns {} and the client guesses every hop again (exactly the F1 defect); equal lengths ⇒ the row's hops get another route's pubkeys. The code is right; one seed with two same-observer, different-path observations would pin it.
N2	low	groupedPageWithRP offset alignment	Test gap. Nothing exercises offset > 0, which /api/packets accepts (routes.go ~1304, documented in openapi.go). Mutant MM-B (page := txs[:len(res.Packets)]) survives the full cmd/server suite and the new E2E [T]. Served proof at ?groupByHash=true&offset=3&limit=3: the row for hash 19fb74c2… has path_json [] yet receives a 6-entry resolved_path belonging to a different transmission, and b10fbba1… gets a 2-entry one; on head both are absent [T]. The packets page itself never pages (it fetches one window and virtual-scrolls), so only other API consumers are exposed.
N3	low	resolvedPathRaw (cmd/server/resolved_index.go)	Test gap with a wide blast radius. The json.Valid guard is unpinned: mutant MM-E (keep the '[' check, drop json.Valid) survives the full cmd/server suite [T]. With one observation's stored resolved_path set to ["aa, the mutant's whole grouped response becomes 0 bytes (HTTP 200, empty body) — the packets page renders nothing — while head returns 500 rows with 399 paths and silently omits the bad one [T]. Low likelihood, but one row poisons the endpoint, so it is worth a test.
N4	low	F6 wording	The PR says the list "flags exactly the hops the detail pane badges". It flags exactly the hops that get the conflict badge. conflictBadgeCount({unreliable: true, conflicts: []}) is 0 while renderHop still emits hop-unreliable-btn for the same entry [T], so a geographically-inconsistent hop with fewer than two candidates is badged in the detail pane and uncounted in the list. unreliable comes from the neighbour-distance sanity pass in hop-resolver.js (~303), independent of conflict count [A]. The list pill still carries .hop-unreliable (italic), so a cue remains — only the claim overstates.
N5	low [A]	ensureHopResolver observer seed	The observerMap seed sits inside if (!HopResolver.ready()). destroy() clears observerMap, but HopResolver is a page-lifetime singleton that stays ready(), so on every visit to #/packets after the first the seed never runs. init() does Promise.all([loadObservers(), loadPackets()]) (Kpa-clawbot#1692/Kpa-clawbot#1693), so a hop job that beats the observers response resolves with [null, null] as the anchor and caches that pick for the session. Narrow (the /observers response is api()-cached, and the ambiguity flag derives from the observer's IATA rather than lat/lon, so only the pick degrades), but the guard defeats the stated purpose of the seed in exactly the case it was added for.
N6	low	docs/api-spec.md	resolved_path is not added to the documented groupByHash=true response shape (§ Response 200 (groupByHash=true), ~line 1152). That block already omits observer_iata, distinct_iatas and scope_name, so the drift is pre-existing — but the grouped response shape is this PR's whole subject, and the round-1 finding was precisely that it differed from the flat one.
N7	info	cacheServerResolvedPath pre-scan	When rp.length > hops.length the scan reads hops[i] past the end, so hopCacheKey(undefined, obs) makes news true, and resolveFromServer then returns {} on the length mismatch. Harmless, but the C2 "already cached" skip never applies to those rows.
N8	info	BenchmarkGroupedPackets165 gzip metric	In my run gzip-bytes goes 0.249 MB → 2.56 MB, a 10× ratio, because the pre-change synthetic rows are highly compressible while the added pubkeys are random [T]. The absolute delta (+2.31 MB at 30K rows) corroborates the PR's "+1.9 MB gzipped per default load" estimate, so the disclosure is sound — but the ratio is an artifact of the synthetic baseline and should not be quoted. A line in the bench comment would prevent that.
N9	info	browser coverage of the positive path	On this fixture the summary never fires with real server data, so the browser only demonstrates the zero case. Of the 60 rendered rows, 44 have a path and all 44 carry a server resolved_path; the 83 hops those paths leave null all resolve client-side to an entry with no candidates at all (name: null, 0 conflicts, not ambiguous) [T]. The E2E is not vacuous — its control step fails under my mutant MM-F (hopUncertainFor always false) [T] — and the unit tests cover the positive direction, but a seeded row with a hop that is genuinely ambiguous for its observer would let the E2E prove both directions in the browser.

1. The blocking finding (F1)

Fixed, on the server side as the parking comment planned. I measured all three states myself on the CI fixture, desktop 1440, 24 h window, after letting the warning count settle over three consecutive samples:

Client	Server	rows	warned rows	flagged hops	of those, hops the server had resolved	console errors
head	head	60	0	0	0	0
head	origin/master binary	60	23	25	25	0
head	head, resolved_path stripped in-flight	60	23	25	25	0

[T] All three. The second row is the true "before": origin/master's server returns resolved_path on 0 of 500 grouped rows, head's on 400 of 500, and the value matches the observation with the same observer_id and path_json in the DB (checked against observations for 09f282f70cc1ff60) [T].

The history dependence I reported in round 1 is gone. Grouped → flat → grouped: head is 0 / 0 / 0, the stripped shape is 23 / 0 / 0 [T], which is the round-1 symptom exactly.

Red before, green after, independently reproduced:

• [T] TestGroupedPacketsCarryHeaderResolvedPath165 and TestGroupedPacketsSQLCarryHeaderResolvedPath165 both FAIL on the test commit 50e462be ("grouped row carries no resolved_path…"), and pass at head.
• [T] The two unit files on the test commit 393b39f9: 24 passed / 8 failed and 34 passed / 2 failed. At head: 32/32 and 36/36. Both counts match the author's report exactly.
• [T] The new E2E test-issue-165-grouped-hop-warn-e2e.js against the unfixed (origin/master) server: 1 passed, 2 failed, naming my own round-1 example — e8b09a35ac87fa5c flags hop 3 (F5), which the server resolved to f509fda5. Against head: 3 passed, 0 failed.
• [T] The mutant that brings the false warning back dies: the author's F1-server mutant is covered by the E2E, and resolved_path stripped in-flight reproduces 23 warned rows while the E2E's control step is the assertion that catches it. My own MM-F (hopUncertainFor always false) is caught by both the unit suite and that control step [T].

2. My other round-1 findings

#	Resolved	Evidence
F2 (mutants MD, MC survived)	yes	[T] My round-1 probes are now tests, verbatim in intent. MD dies on "the summary does not count another observer's entry (the bare-key fallback)" and "probe 1 (MD)"; MC dies on "probe 2 (MC): the client heuristic does not overwrite the server answer under the bare key".
F3 (cap and eviction order unpinned)	yes	[T] MB (HOP_CACHE_MAX = 100) dies on "a bench-sized working set (42 observers x 690 hops) fits without eviction: 100 of 29670"; MA (no delete-before-set) dies on "rewriting an entry refreshes it, so the server answer is not the next to be evicted". Both survived the whole suite in round 1.
F4 (stale description)	yes	[A] Rewritten: final upstream head 2557894d, the leading summary no longer called a deviation, current counts, both perf tables, F1 under the fixes, limitations and commit-message errata. No closing keywords [T].
F5 (popover title counted children)	yes	[T] Browser: the title reads "Full path (6 hops)" for a popover containing exactly 6 .hop, 5 .arrow, 0 .hop-path-warn and 0 .path-overflow-pill. My mutant MM-C (stop skipping the pill) is caught [T].
F6 (summary vs badge criterion)	yes, with N4	HopDisplay.conflictBadgeCount is now the single rule for renderHop and the summary. The two pre-existing tests were adapted by giving their observer an IATA code so the ambiguous hop is still badged — their assertions were kept, not weakened [A]. Scope caveat in N4.
F7 (hop job outliving destroy())	yes	[T] destroy() bumps hopCacheGen; resolveHops, resolveHopsForPackets and cacheResolvedPaths all check it, cacheResolvedPaths on every iteration including the first after its await. Two tests cover it; the author's F7/F7b/C5 mutants are caught.

No disagreement with the author's handling of any of them.

3. The master merge

Clean, and nothing from master is lost.

• [T] git merge-base origin/master 0874df4e == 6444294d == origin/master, so head's history is a strict superset of master's.
• [T] git diff-tree --cc 96f91f2d (the round-2 merge of 6444294d) is empty — a purely trivial merge with no conflict resolution to get wrong.
• [T] Every one of the 99 deleted lines in the touched files is the PR replacing its own earlier code: 6 in db.go (the scan list and the inline row literal), 3 in store.go (the two groupedTxsToPage calls and the stale comment), 9 in hop-display.js, 2 in nodes.js, 1 in the ui(node-detail): "this node" highlight in path chain is too subtle (and unreachable when names mis-resolve) Kpa-clawbot/CoreScope#1153 E2E, 0 in resolved_index.go, style.css, deploy.yml and test-all.sh, and the rest in packets.js.
• [T] No semantic residue from master changing packets.js under the rewrite: there is no object-style hopNameCache[...] access left anywhere in public/, and renderPath has the same five call sites as master — the three list ones gained { summary: true }, the two detail ones are untouched.
• [T] map[string]interface{} in non-test Go is net zero: db.go trades one inline literal for one named row. No new ones outside tests.
• [T] cmd/server stays read-only: every INSERT/Exec in the added Go lines is in a _test.go file.

4. Browser

Local Go server built from head on the CI fixture, prepared as in CI (freshen, the deploy.yml seed SQL, corescope-migrate, seeds 2073 / 199 / 245). Stopped by its listening port.

Check	Desktop 1440×900	Mobile 390×844 (emulated)
Grouped first load	60 rows, 0 warned	56 rows, 0 warned
Summary is the first child of .path-hops on warned rows	yes (verified under the stripped shape, 23 rows)	n/a
Path column	visible, 42 cells overflow → 42 +N pills	col-hidden, as on master
Horizontal page scroll	none	none
Console / page errors	0	0
Expanded group (#/packets?hash=…&timeWindow=0)	3 child rows render, 0 warned, 0 errors	covered by the #259 E2E, 10/10
+N popover	"Full path (6 hops)", 6 hops, 5 arrows, no summary, no pill	n/a
Counted pills	all 25 also carry .hop-ambiguous (dashed underline), so a reader can see which hops the number means [T]	n/a
.hop-path-warn colours	resolve to rgb(234,179,8) on rgb(17,24,39) from CSS vars — no literal in the rule	n/a

[T] Node detail pages, 60 of them (45 with path chains): 332 .hop-current, 0 on a .hop-group wrapper — byte-for-byte the same numbers as master's public/ against the same server, including the single 404 console error, which is therefore a pre-existing baseline. The fixture still has no badged hop on a node page, so the in-wrapper case remains unit-test-only.

5. Port fidelity, XSS, colours

• [T] Upstream's own final unit test (tests/unit/test-issue-2097-hop-ambiguity-badge.js at 2557894d), run unchanged against head: 30 passed, 2 failed — the same two as in round 1, and both are source-text assertions tied to upstream's structure, not behaviour: the regex for hopNameCache[hopCacheKey(h, observerId)] = entry (the fork uses hopCacheSet on a Map) and a grep for _pathHopSegments (the fork's popover builder is _pathPopoverHtml). Both behaviours are covered by the fork's own tests — my MM-C and MM-D mutants die, and the popover browser check shows 0 .hop-path-warn inside it.
• [A] Round 2 added no deviation from upstream beyond F6 (the badge-count criterion in place of entry.ambiguous), which is deliberate, documented and narrows what the list flags; see N4 for its edge.
• [T] scripts/check-xss-sinks.sh --diff origin/master: rc 0 over the 4 changed public/ files (run in a scratch clone at head with origin/master pointed at 6444294d).
• [T] No hardcoded colours: the one new colour rule is background: var(--status-yellow); color: var(--palette-gray-900), and scripts/check-css-vars.js reports OK — 2181 var() refs, 172 definitions, 0 undefined.
• [T] Fork guards: 9 in deploy.yml and 1 in release-fast-path.yml, identical to master.
• [T] Commit author and committer on all 26 branch commits: dborup <kontakt@meshview.dk>. No closing keywords in any commit message or in the PR body.
• [T] eslint@8 on the changed JS: 0 errors, 12 warnings, and master's public/ gives the same 12 — no new lint debt.

Tests

All on head, which equals the merged tree against origin/master 6444294d.

Suite	Result
cmd/server go test ./...	ok, 38.6 s
cmd/ingestor go test ./...	ok, 105 s
sh test-all.sh	229 passed, 0 failed (229 files)
node test-frontend-helpers.js	709 passed, 0 failed
test-issue-165-hop-resolution-per-observer.js	32/32
test-issue-165-hop-ambiguity-badge.js	36/36

E2E against the local server on e2e-fixture.db:

Test	Result
test-issue-165-grouped-hop-warn-e2e.js (new)	3/3
test-e2e-playwright.js	132/135, 3 skipped, 0 failed — and master's public/ gives the identical 132/135/3
test-issue-1146-path-link-contrast-e2e.js (Kpa-clawbot#1153 hop count)	11/11
test-issue-147-…, test-issue-180-…	12/12, 12/12
test-issue-1128-packets-layout-…, -multi-viewport-	5/5, 15/15
test-issue-1122-details-row-clamp-…, -packets-filter-ux-	18/18, 6/6
test-slideover-1056-…, -1168-munger-	27/27, 3/3
test-path-inspector-coverage-…, test-packet-trace-alignment-…	10/10, 17/17
test-issue-1486-…, test-issue-259-…, test-issue-258-…	4/4, 10/10, 22/22
test-packet-detail-sender-hash-size-obs-…, test-issue-189-group-caret-…, test-issue-96-hide-control-…	3/3, 3/3, 10/10
test-issue-1692-packets-init-parallel-e2e.js	fails locally, on master too. Head: first row never appears within the 10 s selector timeout under the 4 s /api/observers stub. Identical failure with master's public/ on head's server, and with master's server and master's public/ — so it is a local-environment baseline, not a regression. CI's Playwright job is green on this head.

CI on 0874df4e, checked per job (run 37598118692, one attempt each): Go Build & Test success (25 min), Playwright E2E success (26 min), Build & Publish Docker Image success; Release Artifacts, Deploy Staging and Publish Badges skipped, as expected for a PR. Neither known flake (#256 Hash Stats sort, #267 backfill write-hold) appeared.

Mutants

Six of my own, each in a separate copy of the head tree.

Mutant	Change	Result
MM-A	headerObservationID matches on ObserverID only, dropping the PathJSON check	survives the full cmd/server suite and the new E2E → N1
MM-B	groupedPageWithRP aligns with txs[:len(res.Packets)], ignoring offset	survives the full cmd/server suite and the new E2E; observable at offset=3 → N2
MM-E	resolvedPathRaw drops the json.Valid check	survives the full cmd/server suite; one corrupt row empties the response → N3
MM-C	_pathPopoverHtml stops skipping .path-overflow-pill	caught (test-issue-165-hop-resolution-per-observer.js)
MM-D	_finalizePathOverflow stops skipping .hop-path-warn in the hidden count	caught (same file)
MM-F	hopUncertainFor always returns false	caught by the unit suite and by the new E2E's control step

Re-run of my four round-1 survivors on the round-2 tree: MA, MB, MC and MD all die [T], each on a named test (quoted in section 2).

Performance, measured here

Server — BenchmarkGroupedPackets165, 30K transmissions, 2 observations each, limit 50000, -benchtime 3x, Apple M4:

	before (groupedPageWithRP short-circuited)	head
per request	22.4 ms	73.0 ms (3.3×)
JSON	12.24 MB	16.95 MB (+38 %)

[T] The JSON figures match the author's to the byte; the time ratio (3.3×) matches their 3.2× on slower hardware. On the CI fixture the grouped response goes 432 KB → 589 KB (+36 %) [T], also matching. The batched read's plan is SEARCH o USING INTEGER PRIMARY KEY (rowid=?) with a bloom filter over json_each — no scan [T].

Client — scripts/bench-hop-resolution.js, 30K packets, 42 observers, median of 7, Apple M4:

public/	RP_SHARE	total	longest main-thread block	cache entries
origin/master	0.3	37.3 ms	37.8 ms	2236
head	0.3	205.6 ms	19.4 ms	28962
origin/master	0.95	69.4 ms	69.9 ms	2236
head	0.95	166.6 ms	14.3 ms	28755

[T] This reproduces the author's shape and resolves the round-1 concern from the other direction: total stays 2.4–5.5× master's, but the longest block is now below master's at both shares, where the previous head was at 101–106 ms and 233–240 ms. Cache entries match their numbers exactly and stay well under HOP_CACHE_MAX (50000), consistent with the F3 test.

Not verified

• Staging, production or real 30K data, and no real-browser performance profile. Every number above comes from the CI fixture or the two synthetic benches.
• The share of grouped rows that will carry resolved_path in production after feat(ingestor): resolve the last hop from the observer (#188) #190 [K].
• Cache heap size [K]; I measured entry counts, not bytes.
• Firefox and WebKit; mobile is Chromium emulation, not a device.
• A badged hop inside a .hop-group on a node detail page, and a row whose summary fires with full server data — the fixture has neither (N9). Both are covered by unit tests only.
• The +N popover and the path column at mobile widths: the column is col-hidden at 390 px, so there is nothing to clip and nothing to pop over.
• BenchmarkGroupedPackets165 measures QueryGroupedPackets only; JSON encoding and gzip are sized once outside the timer, so the per-request figure excludes marshalling [A].