fix(packets): one path-hash width helper for the hex breakdown (#322)

[dborup-agent]

Relates to #322

Follow-ups to #313 (#282), from the review of that PR. All four points, tests first then code (two commits). No behaviour change: points 1 and 7's output is identical for every well-formed frame — the width rules are unchanged, only their one home moves.

Plan (as built)

#	Point	Fix	Test	Mutant
1	Two copies of the path-hash width rules (DRY)	public/app.js gains pathHashSizeFromByte(pathByte, routeType, headerByte) — the one implementation of the rules. senderPathHashSize() and buildFieldTable's Path Length row both call it; each still reads its own path byte at its own offset (header-derived vs pkt.route_type), so there is one offset source per caller.	test-frontend-helpers.js senderPathHashSize + test-packets.js buildFieldTable both exercise the shared rule (route-3 0x00)	one helper mutant (`routeType === 2
2	TRANSPORT_DIRECT 0x00 marker untested	covered by the shared helper	test-packets.js: a buildFieldTable case for route 3 (17aabbccdd00), path byte at offset 5	the same mutant relabels the 0x00 as hash_size=1; the case kills it
3	test-issue-282-pktesc-listener.js header overstated the leak	header + the "second render" case reworded: renderLeft() returns at the filtersBuilt guard on filter/region changes, so the listener was added once per visit, not per change	test-issue-322-comment-guards.js point 3	re-introduce the "on every filter/region change" wording → red
4	public/channels.js #282 (8) comment wrong	the comment no longer says the sender badge uses normalizeObservedPathHashSizes(); renderSenderPathHashBadge() reads message.senderPathHashSize directly. Lists the real callers (union/merge, cached-message merge, packet → message mapping, dedup key).	test-issue-322-comment-guards.js point 4	re-introduce "the sender badge still use it" → red

The width rules were verified against the firmware: firmware/src/Packet.h (getPathHashSize() = (path_len >> 6) + 1, isRouteDirect() = routes 2/3, hasTransportCodes() = routes 0/3) and firmware/docs/packet_format.md (path_length bits 6-7 hash-size code, 0b11 reserved/invalid, 0x00 zero-hop marker).

Verification

• sh test-all.sh: 226/226 files pass (new comment-guard file registered).
• node test-frontend-helpers.js, node test-packets.js: green.
• Mutant (routeType === 2 || routeType === 3) → (routeType === 2) in the shared helper: kills both the senderPathHashSize assertion and the buildFieldTable route-3 case (single-source proof); reverted.
• scripts/check-xss-sinks.sh --diff origin/master: clean (scans the 3 changed public/ files).
• Frontend eslint no-undef: 0 errors (the new cross-file global is registered in .eslintrc.json).
• Fork-guards unchanged: 9 in deploy.yml, 1 in release-fast-path.yml. No new map[string]interface{} outside tests (no Go changes — cmd/server untouched). No hardcoded colours.
• Browser check against a local Go server on the CI-prepared e2e-fixture.db:
  • test-packet-detail-sender-hash-size-obs-e2e.js 3/3 (flood-route breakdown) and test-channels-observed-path-hash-size-e2e.js 8/8.
  • Transport-route hex breakdown rendered via buildFieldTable: TRANSPORT_FLOOD (route 0, byte-5 offset) → Path Length 0x40 → hash_size=2 bytes, hash_count=0; TRANSPORT_DIRECT (route 3) 0x00 → hash_count=0 (no encoded hash size); both with the Transport Codes section. No page errors.

Draft until review.

[dborup-agent]

Rapport — CS-MacBook PR#327 #322 — head 40f36f8

Status: All 4 follow-ups to #313 done on a draft PR; every point has a test and a mutant, all local suites are green, the browser check passes, and all three CI jobs are green on the first run (no re-runs).

Evidence tags: [T] test/run output I produced, [A] assessment/inference, [K] checked in code/diff/CI.

Branch

codex/issue-322-path-length-width-helper from origin/master b0b9843c. Two commits, tests before code, author+committer dborup <kontakt@meshview.dk>. Only explicit git add; no rebase/amend/force-push. No closing keywords (closingIssuesReferences = 0). [K]

• f6be268c tests
• 40f36f8a the helper + comment fixes

Requirements

#	Point	Fix	Test	Mutant	Result
1	Two copies of the path-hash width rules (DRY)	public/app.js pathHashSizeFromByte(pathByte, routeType, headerByte) is the one implementation; senderPathHashSize() and buildFieldTable's Path Length row both call it, each reading its own path byte at its own offset (header-derived vs pkt.route_type) → one offset source per caller	test-frontend-helpers.js senderPathHashSize (route-3 0x00) + test-packets.js buildFieldTable (route-3 0x00)	one helper mutant (routeType === 2 || routeType === 3) → (routeType === 2) turns both red → single-source proof [T]	✅ [T]
2	TRANSPORT_DIRECT 0x00 marker untested	covered by the shared helper	test-packets.js: buildFieldTable case for route 3 (17aabbccdd00), path byte at offset 5 → "no encoded hash size"	same mutant relabels the 0x00 as hash_size=1; the case kills it [T]	✅ [T]
3	test-issue-282-pktesc-listener.js header overstated the leak	header + the "second render" case reworded: renderLeft() returns at the filtersBuilt guard on filter/region changes, so the listener was added once per visit	test-issue-322-comment-guards.js p3	re-introduce the "on every filter/region change" wording → red [T]	✅ [T]
4	public/channels.js #282 (8) comment wrong	comment no longer says the sender badge uses normalizeObservedPathHashSizes(); renderSenderPathHashBadge() reads message.senderPathHashSize directly. Lists the real callers (union/merge, cached-message merge, packet → message mapping, dedup key)	test-issue-322-comment-guards.js p4	re-introduce "the sender badge still use it" → red [T]	✅ [T]

Firmware check [K]

Width rules verified against the firmware: firmware/src/Packet.h — getPathHashSize() = (path_len >> 6) + 1, isRouteDirect() = routes 2/3, hasTransportCodes() = routes 0/3 — and firmware/docs/packet_format.md path_length: bits 6-7 = hash-size code, 0b11 reserved/invalid, 0x00 = zero-hop marker.

Local checks [T]

• sh test-all.sh: 226/226 files pass (new test-issue-322-comment-guards.js registered; test-test-all.js registry/meta green).
• node test-packets.js (154) and node test-frontend-helpers.js (709): green.
• Mutant (routeType === 2 || routeType === 3) → (routeType === 2) in the shared helper: kills both the senderPathHashSize assertion and the buildFieldTable route-3 case; reverted.
• scripts/check-xss-sinks.sh --diff origin/master: clean (scans the 3 changed public/ files). [T]
• Frontend eslint no-undef (eslint@8, as CI): 0 errors (the new cross-file global is registered in .eslintrc.json); the 88 reports are pre-existing no-unused-vars warnings. [T]
• Fork-guards unchanged: 9 in deploy.yml, 1 in release-fast-path.yml. No new map[string]interface{} outside tests — no Go changes at all, cmd/server untouched. No hardcoded colours. [K]

Browser check [T] — local Go server (my build) on CI-prepared e2e-fixture.db (copy), port 13900

• test-packet-detail-sender-hash-size-obs-e2e.js 3/3 (flood-route breakdown through the refactored helper) and test-channels-observed-path-hash-size-e2e.js 8/8.
• Transport-route hex breakdown rendered via buildFieldTable (route-intercept injection):
  • TRANSPORT_FLOOD (route 0, path byte at byte-5 offset): Header 0x14, Transport Codes section present, Path Length 0x40 → hash_size=2 bytes, hash_count=0.
  • TRANSPORT_DIRECT (route 3), 0x00: Header 0x17, Transport Codes present, Path Length 0x00 → hash_count=0 (no encoded hash size).
  • No page errors.

CI [K] — run 37478839183, attempt 1, no re-runs; per job

• ✅ Go Build & Test: success (15m14s) — test-all.sh, XSS --diff gate, eslint no-undef, CSS-var lint, Go server + ingestor with -race, proto syntax.
• 🎭 Playwright E2E Tests: success (25m28s) — full E2E suite.
• 🏗️ Build & Publish Docker Image: success (53s).
• 📦 Release Artifacts / 🚀 Deploy Staging / 📝 Publish Badges: skipped (fork/branch-scoped, expected).

Neither known flake (#271) appeared; no re-run needed. PR stays a draft.

Rest / notes

• Points 3 and 4 are comment fixes, so their tests are source-text guards (red against the wrong wording, green once corrected); the behaviours they describe are covered by test-issue-282-pktesc-listener.js and test-channels-observed-path-hash-size.js. [A]
• Points 1 and 2 are a refactor + a test-gap fix: the width output is unchanged for every well-formed frame, so the new width tests are green on clean code and red only under the shared-helper mutant. [A]
• No Go suites were "relevant" to run (zero Go changes); relied on CI's Go job, which is green. [A]

[adminopenclaw8-sketch]

Review — CS-Macmini PR#327 — head 40f36f8

Dom: APPROVE med nits

Independent read-only review of the four #322 points (follow-ups to #313). Everything I could check against the firmware, the suites, the mutants and a real browser holds up. The nits below are all non-blocking; one is a documented behaviour delta on malformed input, two are residual DRY gaps outside the issue's stated scope.

Evidence tags: [T] test/run output I produced, [A] assessment/inference, [K] checked in code/diff/CI.

Findings

#	Sev	Where	Finding
N1	nit	public/app.js:36-37 vs master public/packets.js:3937	Behaviour delta on a malformed raw_hex. Master gated the whole width derivation on pathBytesAreHops = !isNaN(headerByte) && …, so an unparseable header byte suppressed the width. The new helper guards the TRACE test with typeof headerByte === 'number' && !isNaN(headerByte), so a NaN header now skips the TRACE check and a width is printed. Measured [T]: raw_hex zz40aabb, route 1 → master hash_count=0 (TRACE: path bytes are SNR, not a hash width), branch hash_size=2 bytes, hash_count=0. Unreachable through the normal pipeline — raw_hex is hex.EncodeToString output (cmd/ingestor/decoder.go) [K] — and master's own wording was wrong for a garbage header anyway, so neither output is right. Worth one line in the helper's comment, or a guard that mirrors master's isNaN suppression; no test pins either behaviour.
N2	nit	public/app.js:38, public/compare.js:38	The direct-route predicate is still duplicated. The helper inlines `(routeType === 2
N3	nit	public/packets.js:2500,2547,2584; public/hop-filter.js:57	Residual partial copies of the width rule on the user-visible list column. <td class="col-hashsize"> derives its value as ((pathByte || 0) >> 6) + 1 with a TRACE guard only — no 0b11 and no zero-hop guard. So a 0b11 frame prints 4 in the packets list while the detail panel says "not a valid hash size, 1-3 bytes only", and a direct zero-hop marker prints 1 while the detail panel says "no encoded hash size" [K][T]. hop-filter.js packetHashSize() has no TRACE guard either. All pre-existing and byte-identical on both trees [T], so not a regression — but it is the same drift class #322 point 1 is about, and now that the helper exists these are the obvious next callers.
N4	note	points 1 and 2	No test is red-before / green-after for points 1 and 2, by design. The new buildFieldTable route-3 case is green on unmodified master code [T] (master's inline copy already handled route 3), and point 1 is a pure refactor. The correct instrument is the mutant, and it does the job: it survives on master and dies in both suites on the branch [T]. Flagging so the "every criterion red before, green after" rule is not read as satisfied in the literal sense here. Points 3 and 4 do meet it literally [T].

The review points

1. DRY — one implementation of the width rules. Holds. pathHashSizeFromByte(pathByte, routeType, headerByte) in app.js is the single home; senderPathHashSize() (app.js:51) and buildFieldTable's Path Length row (packets.js:3939) are its only callers [K]. Each still reads its own path byte at its own offset — header-derived vs pkt.route_type — which preserves #282 point 7's "one offset source per caller" property [K]. The single-source claim is not just structural: one edit inside the helper turns both test-packets.js and test-frontend-helpers.js red [T].

Checked against the firmware [K]:

• getPathHashSize() { return (path_len >> 6) + 1; } — src/Packet.h:79 → matches (pathByte >> 6) + 1.
• isRouteDirect() = ROUTE_TYPE_DIRECT (0x02) ∥ ROUTE_TYPE_TRANSPORT_DIRECT (0x03) — src/Packet.h:65, :16-17 → matches (routeType === 2 || routeType === 3).
• sendZeroHop() sets path_len = 0 on both direct variants — src/Mesh.cpp:717-734 → confirms 0x00 is the zero-hop marker on routes 2 and 3, which is exactly the rule the mutant attacks.
• 0b11 reserved/unsupported — docs/packet_format.md:51-57 → matches size <= 3 ? size : null.
• TRACE path bytes as SNR — the path-type 9 guard matches the existing internal/packetpath/route.go PathBytesAreHops rationale [K].

Residual copies are N2/N3; neither is in the issue's stated scope.

2. Test gap — the TRANSPORT_DIRECT 0x00 mutant. Killed. Reproduced both directions myself [T]:

• On unmodified master, (pkt.route_type === 2 || pkt.route_type === 3) → (pkt.route_type === 2): test-packets.js 153 passed / 0 failed, test-frontend-helpers.js 709 passed / 0 failed — the mutant survives, confirming the issue's claim.
• On the branch, the same rule mutated in the shared helper: test-packets.js 153 / 1 failed (the new route-3 case) and test-frontend-helpers.js 708 / 1 failed (the senderPathHashSize assertion).

3. test-issue-282-pktesc-listener.js comment. Correct now. Verified in the source that renderLeft() returns at the filtersBuilt guard (packets.js:1723-1726) well before document.addEventListener('keydown', _pktEsc) (packets.js:2398), and that destroy() resets filtersBuilt = false (:1546) — so the stacking really was once per visit, not per filter/region change [K]. The reworded header and the renamed case both say that.

4. public/channels.js comment. Correct now, and precise. renderSenderPathHashBadge() reads Number(message && message.senderPathHashSize) and never calls normalizeObservedPathHashSizes() [K]. All four callers the new comment lists exist: union/merge :40, cached-message merge :110, packet → message mapping :1008/:1030, dedup key :2713 [K]. The only other reference is the test export at :2851, which the surrounding #282 comment already covers.

5. Browser check — no regression in the hex breakdown or the path display. Confirmed against a local Go server (my own build, CI-prepared e2e-fixture.db copy, loopback port) [T]. I ran the same probe against the branch's public/ and against unmodified master's public/ on the same DB and binary, so the only variable is the frontend diff.

Hex breakdown, 8 frames — output byte-identical on master and branch, no page errors:

Frame	Header	Path Length	Transport Codes	Hash Size summary
FLOOD route 1, 0x40	0x11	hash_size=2 bytes, hash_count=0	—	2 bytes
FLOOD route 1, 0x00	0x11	hash_size=1 byte, hash_count=0	—	1 byte
DIRECT route 2, 0x00	0x12	hash_count=0 (no encoded hash size)	—	none
TRANSPORT_FLOOD route 0, 0x40	0x14	hash_size=2 bytes, hash_count=0	next=aabb last=ccdd	2 bytes
TRANSPORT_DIRECT route 3, 0x00	0x17	hash_count=0 (no encoded hash size)	next=aabb last=ccdd	none
TRANSPORT_DIRECT route 3, 0x80	0x17	hash_size=3 bytes, hash_count=0	next=aabb last=ccdd	3 bytes
FLOOD route 1, 0xC0	0x11	hash_count=0 (width bits 7-6 = 3: not a valid hash size, 1-3 bytes only)	—	none
TRACE route 1, 0x40	0x25	hash_count=0 (TRACE: path bytes are SNR, not a hash width)	—	none

Path display with real hops — also byte-identical on both trees [T]: FLOOD 2 hops 0x42 (path length at byte 1, hops at 2 and 4), TRANSPORT_FLOOD 2 hops 0x42 (path length at byte 5, hops at 6 and 8), TRANSPORT_DIRECT 3 hops 0x03 (hops at 6, 7, 8, with hop names resolved). Section ordering, hop-row count and byte offsets all unchanged. A real fixture frame (route 2, path byte 0x00, one of 101 such rows) renders hash_count=0 (no encoded hash size) identically on both trees [T].

The PR's own E2E, same server: test-packet-detail-sender-hash-size-obs-e2e.js 3/3 and test-channels-observed-path-hash-size-e2e.js 8/8 [T].

No other behaviour change. cmd/ is untouched entirely, so cmd/server stays read-only [K]. 0 new map[string]interface{} [K]. No colour or style lines added at all — the diff adds no CSS and no inline style [K]. scripts/check-xss-sinks.sh --diff origin/master clean, scanning all three changed public/ files [T]. Fork-guards 9 in deploy.yml and 1 in release-fast-path.yml, unchanged from master, and no workflow file is touched [K]. Commit author and committer are dborup <kontakt@meshview.dk> on both commits [K]. No closing keywords on the PR [K]. app.js loads before packets.js and channels.js in index.html, so the new cross-file global resolves at runtime [K].

Tests I ran [T]

On the merged tree (git merge-tree --write-tree origin/master <head> against origin/master bf3151a4, clean merge):

• sh test-all.sh — 226/226 files pass. The registry grows 225 → 226, so the new file is wired in and test-test-all.js accepts it.
• node test-packets.js 154/0, node test-frontend-helpers.js 709/0.
• cd cmd/server && go test ./... — ok (41.1 s). cd cmd/ingestor && go test ./... — ok (105.1 s). Repo is multi-module; no Go source changed.
• npx eslint public/*.js with eslint@8 as CI does — 0 errors, 88 pre-existing no-unused-vars warnings.
• Red-before / green-after for points 3 and 4: test-issue-322-comment-guards.js against pure master source → 0 passed, 2 failed; on the branch → 2 passed (inside the 226).

Mutants [T]

Seven, all resolved. Five are mine (B, C, D, E, F, G below); A is the one the issue names, which I reproduced in both directions.

#	Mutation	Result
A	app.js (routeType === 2 || routeType === 3) → (routeType === 2)	Killed — test-packets.js 1 failed and test-frontend-helpers.js 1 failed from a single edit. On master the equivalent edit survives both (0 failed), reproducing #322 point 2.
B	app.js return size <= 3 ? size : null; → return size; (drops the 0b11 rule)	Killed in both suites
C	app.js TRACE test ((headerByte >> 2) & 0x0F) === 9 → === 8	Killed in both suites
D	packets.js call site drops the header argument: pathHashSizeFromByte(pathByte0, pkt.route_type)	Killed in test-packets.js (the TRACE case) — the call-site wiring is covered
E	Strip every filtersBuilt mention from the pktEsc header comment	Killed — the guard's second assertion is live, not vacuous. (A first attempt that removed only one of the two mentions survived; the header still documented the guard, so that was a bad mutant, not a test gap.)
F	channels.js reword renderSenderPathHashBadge does not → the sender badge is unrelated	Killed — point 4's second assertion is live
G	Remove "pathHashSizeFromByte": "readonly" from .eslintrc.json	Killed — eslint reports 'pathHashSizeFromByte' is not defined no-undef at packets.js:3939, so CI's lint gate protects the new global

Each of the three firmware rules in the helper (width formula, 0b11 invalid, zero-hop on routes 2/3) is therefore covered through both callers, and the TRACE rule through both as well.

CI [K]

Run 37478839183, attempt 1, no re-runs, checked per job:

• ✅ Go Build & Test — success (15m14s)
• 🎭 Playwright E2E Tests — success (25m28s)
• 🏗️ Build & Publish Docker Image — success (53s)
• Release Artifacts / Publish Badges / Deploy Staging — skipped (branch/fork-scoped, expected)

Neither known flake (#256 Hash Stats sort, #267 backfill write-hold) appeared. The author's CI claims match what I see.

What I did not verify

• The full Playwright E2E suite. I ran the PR's two own E2E files plus my own breakdown and path-display probes against both trees. I relied on CI's green E2E job for the rest.
• The N1 malformed-raw_hex path end to end. I measured it through buildFieldTable in a sandbox [T] and traced raw_hex to hex.EncodeToString [K], but I did not try to get a non-hex raw_hex into a real DB row, so "unreachable in practice" is inference [A].
• N3's list-column mismatch as a live side-by-side screenshot. The column is live and renders identically on both trees [T], and I computed the two divergent values from the shipped expression [K], but the fixture's default view did not surface a 0b11 row for me to photograph next to its detail panel.
• The seeded transport-route rows I added to my fixture copy never reached the UI — the server caps its in-memory load at 500 transmissions, so my negative-id rows were not loaded. The transport-route browser evidence above therefore comes from route-interception against the real page and the real buildFieldTable, the same technique the PR's own passing E2E uses, not from a DB row.
• Staging and production were not touched, no API key was used, and I made no writes to the PR, the branch or any issue.
• Head was 40f36f8aa2d1538e8ca39caea982a06ba9d5290d before and after the review; origin/master was bf3151a4 throughout.