Repository navigation
feat(ui): warn on 1-byte path hash in channels, packet detail and node badge (#353) - #356
Conversation
Unit tests for the shared isPathHashSizeWarn/renderPathHashSize helper, the channel "Sent with" badge, the packet-detail Hash Size row, the node badge and the customizer mapping, plus a browser regression in light and dark theme and at 375x812. All fail until the warning is implemented. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e badge (#353) One shared helper in app.js decides (size 1 = warning) and renders the markup for all three places: the channel "Sent with" badge, the packet-detail Hash Size row and a new node-detail badge. The warning has a Phosphor warning icon (aria-hidden), sr-only text and a tooltip naming the firmware settings (companion app Experimental Settings, repeater path.hash.mode; firmware/docs/faq.md 3.9.x). Colour comes from the new --path-hash-warn (default var(--danger)), mapped in the customizer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
renderPathHashSize and isPathHashSizeWarn live in app.js and are used from channels.js, packets.js and nodes.js; the no-undef gate needs them listed like senderPathHashSize. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rapport — CS-pve-agent2 PR#356 #353 — head 7be5b5aStatus: Implemented and green. All three places warn on a 1-byte path hash through one shared helper. CI passes on head Evidence tags: [T] test run (local or CI), [A] analysis or code reading, [K] known or caveat. Requirements
Mutant totals: 8 unit mutants and 4 E2E mutants, all killed [T]. Invariants
Local runs (prepared like CI: freshened
|
| Job | Result |
|---|---|
| Go Build & Test | pass [T] |
| Playwright E2E Tests | pass. The new #353 E2E ran: 20 passed, 0 failed [T] |
| Build & Publish Docker Image | pass [T] |
| Release Artifacts / Publish Badges & Summary / Deploy Staging | skipping (fork-guarded) [T] |
The first head, 963a81c6, failed Go Build & Test at the eslint no-undef step. The new helpers were not yet declared as globals in .eslintrc.json. This was a real failure, not the known-flaky #271. Commit 7be5b5a1 fixed it and lint passes [T]. No reruns were needed.
Leftovers
- The customizer Advanced row "1-byte Path Hash" shows an empty (black) swatch until an override is set, because the server theme defaults do not carry this key. "Nav Active Pill" behaves the same way today. I left it this way so the default keeps following
--danger[K] - The "1 = warning" threshold is hardcoded. A customizer option for the recommended minimum is a later milestone (AGENTS.md rule 8) [K]
- The badge reflects the advert/frame width CoreScope observed (
hash_size/ header). It is not a guaranteed permanent sender setting [A]
Review — CS-pve-agent3 PR#356 — head 7be5b5aDom: REQUEST CHANGES This is an independent, read-only review. The design is sound: one shared helper drives all three sites, the token and the customizer are wired up, the tests are real and kill mutants, and the tooltip matches the firmware docs. There is one blocker. In the light theme, the new packet-detail warning fails WCAG AA colour contrast, and axe confirms it. CI did not catch it only because no axe route opens a 1-byte packet detail. The fix is small. Evidence tags: [T] I ran it, [A] code or doc reading, [K] known, caveat or not verified. Trees tested:
Findings
Suggested fix for #1. Either option stays within #353:
Whichever option is chosen, add an axe Answers to the review points
Invariants.
Tests and mutants (merged tree, local server prepared like CI)The fixture was prepared as in CI: freshen, inline seed SQL,
My mutants, each applied alone to the merged tree and reverted afterwards [T]:
CI per job (run 37663726415, head
No known flaky failures (#256, #267) occurred. Not verified
|
PR #356 review, round 2: - F1: --path-hash-warn defaults to --danger (#dc2626), which is 4.43:1 on the light packet page (--content-bg #f4f5f7), below WCAG AA 4.5:1. The unit test resolves the token from style.css in the light and both dark blocks and checks it against every surface the warning renders on. The E2E runs axe color-contrast on the three warning elements and requires axe to have measured them. - F2: hashSizeDetailHtml(4) emits <dt>Hash Size</dt><dd></dd>. A width outside 1-3 must render no row. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Scope note for the next round: per the operator decision on #353, the 1-byte flag becomes an amber recommendation (not a red error). Contrast must still meet WCAG AA in both themes. |
Mid-animation, axe samples a blended, lighter foreground (e.g. #e25b5c on the light node page at 375px) and reports a contrast the settled page never has. Wait until no running animation targets the warning element or an ancestor before running color-contrast. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…353) PR #356 review, round 2: - F1: --path-hash-warn defaults to --palette-red-700 (#b91c1c) in the light theme: 5.93:1 on the packet page (#f4f5f7) and 6.47:1 on the white badge surfaces, where --danger (#dc2626) was 4.43:1 on the page. Both dark blocks set it back to --danger (#f87171): 6.82:1 on surface-0 and 5.48:1 on surface-2. The variable and the shared helper are unchanged, so the operator can still retune the tone. - F2: hashSizeDetailHtml renders the Hash Size row only when renderPathHashSize returns markup, so a width outside 1-3 gives no row instead of an empty <dd>. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…353) The dark variants also set prefers-color-scheme: dark, so the media block masked a missing --path-hash-warn override in the [data-theme="dark"] block (#b91c1c on #0f0f23 is 2.92:1). A light-OS context with the dark toggle exercises that block alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… error (#353) Operator decision on #353 (round 3): a 1-byte path hash is a recommendation, not an error. It is the firmware default and the FAQ still recommends it for companions until most repeaters run 1.14+. - --path-hash-warn now defaults to var(--warning) (amber) instead of var(--danger) (red), following the warning palette in both themes; still mapped in the customizer (pathHashWarn). This also clears the round-2 blocker: red on the light packet-detail background was 4.43:1 (< AA); amber is 4.60:1. WCAG AA (>= 4.5:1) on every surface, both themes: channel/node 5.02:1 light / 10.51:1 dark, packet row 4.60:1 light / 13.09:1 dark (measured in-browser). - Wording reads as a recommendation, not a problem: the tooltip leads with "Recommended: use a 2- or 3-byte path hash", the node badge shows "Recommended: 2- or 3-byte path hash" as its visible label, and the inline channel/packet sites keep their factual value with an sr-only "— recommended: 2- or 3-byte path hash" clause (no "Warning:" prefix). - Icon stays the warning glyph (ph-warning), not an error glyph, so the cue survives without colour. - hashSizeDetailHtml now gates the row on the helper output, so an out-of-range width renders no row instead of an empty <dd></dd> (#356 review round 2, finding 2). Tests updated (red before, green after); E2E gains an in-page WCAG AA contrast assertion on all three sites in light and dark. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rapport — CS-pve-agent3 PR#356 runde-2 — head 1c46e95Review feedback addressed (commit Evidence tags: [T] I ran it (locally or in CI), [A] analysis or code reading, [K] known, caveat or not verified.
The PR body's Theming and Tests bullets said Commits (all by
|
| Commit | What |
|---|---|
628c5545 |
test: red tests for F1 (unit contrast plus E2E axe) and F2 |
f13eb735 |
merge origin/master 2dc04642, clean, no conflicts |
25c4dd34 |
test: the E2E axe check waits for fade-ins. Mid-animation, axe sampled a blended #e25b5c (3.48:1) on the light node page at 375 px, which the settled page never shows |
90c2e18e |
fix: F1 and F2 |
1c46e95e |
test: a new E2E variant, dark toggle on a light OS (see M2) |
Tests (red before, green after)
| Finding | Test | Before fix | After fix |
|---|---|---|---|
| F1 | test-issue-353-path-hash-warn.js: resolves --path-hash-warn from style.css (light :root and each dark block) and checks it is at least 4.5:1 on --content-bg, --detail-bg, --surface-2 and --card-bg. Also checks that both dark blocks set the token, to the same value |
2 ❌ ("#dc2626 on --content-bg #f4f5f7 is 4.43:1", "a dark block lacks --path-hash-warn") [T] | 17/17 [T] |
| F1 | test-issue-353-path-hash-warn-e2e.js: axe color-contrast on each of the three warning elements. It requires 0 violations and that axe actually measured the element (a pass, not "incomplete"). The colour check is now color = --path-hash-warn; the old "= --danger" check is dropped |
2 ✗: light desktop and light 375 packet detail, "insufficient color contrast of 4.42 (#dc2626 on #f4f5f7)" [T] | 25/25, run twice locally [T] |
| F2 | test-packets.js: "an out-of-range width renders no Hash Size row, never an empty one" |
1 ❌ ("for 4") [T] | 166/166 [T] |
Mutants (each applied alone on top of the fix, then reverted)
| Mutant | Killed by |
|---|---|
M1 (F1): light --path-hash-warn back to var(--danger) |
unit: light contrast ❌ 4.43:1. E2E: 2 ✗, axe on light packet detail [T] |
M2 (F1): [data-theme="dark"] block loses its --path-hash-warn override |
unit: 2 ❌ (out of sync; #b91c1c on #0f0f23 = 2.92:1) [T]. The first E2E run did not catch it (20/20), because its dark variants also set the OS to dark, so the media block masked the missing override. The new variant (1c46e95e) runs the dark toggle on a light OS, and the E2E now kills M2 with 3 ✗ (axe on channel, packet and node) [T] |
M3 (F2): row no longer gated on the helper output (old if (!hashSize) shape) |
test-packets.js ❌ "for 4" [T] |
Local runs (merged tree; fixture prepared like CI: copied e2e-fixture.db, freshen, inline seed SQL, corescope-migrate, seeds 2073/199/245; local Go server -public public)
sh test-all.sh: 234 files passed, 0 failed [T]node test-frontend-helpers.js: 712 passed, 0 failed [T]test-issue-353-path-hash-warn.js17/17,test-packets.js166/166,test-channels-observed-path-hash-size.js13/13,test-customizer-v2.js63/63 [T]cmd/servergo test ./...: ok (899 s) [T]cmd/ingestorgo test ./...: ok (1018 s) [T]- E2E:
test-issue-353-path-hash-warn-e2e.js25/25 [T] - E2E:
test-channels-observed-path-hash-size-e2e.js8/8,test-packet-detail-sender-hash-size-obs-e2e.js3/3,test-customize-theme-e2e.js8/8 [T] - E2E:
test-a11y-axe-1668.js0 violations in 120 cells [T] - eslint@8
public/*.js: 0 errors (89 existingno-unused-varswarnings) [T] scripts/check-xss-sinks.sh --diff origin/master: exit 0 [T]- Screenshots checked by eye: light packet detail at 375 px and light node detail on desktop. The warning reads as dark red with its icon, with no overflow [T]
Invariants
- No Go diff against master (
git diff origin/master...HEAD -- cmd internalis empty) [T]. - 0 added
map[string]interface{}[T]. - No hardcoded colours outside
:root. The#b91c1cfallback sits in the light:rootnext to the existing palette fallbacks [A]. - Fork-guard counts (
github.repository) are the same as master:deploy.yml12/12,release-fast-path.yml2/2 [T].
CI per job (run 37687784276, head 1c46e95e)
| Job | Result |
|---|---|
| Go Build & Test | success [T] |
| Playwright E2E Tests | success. The log shows the #353 E2E ran with "25 passed, 0 failed", and the axe gate reported "zero violations across 120 cells" [T] |
| Build & Publish Docker Image | success [T] |
| Release Artifacts / Deploy Staging / Publish Badges & Summary | skipped (fork guards) [T] |
The known flaky tests #271 and #301 did not occur, and no reruns were needed [T].
Not verified
- The node side pane is not driven by the E2E. It uses the same
.node-path-hash-badgeon--surface-2, so the values in the table apply [A][K]. - Chromium only. No real screen reader [K].
…tion decision (#353) # Conflicts: # public/packets.js # public/style.css # test-issue-353-path-hash-warn-e2e.js # test-issue-353-path-hash-warn.js # test-packets.js
Rapport — CS-MacBook PR#356 runde-3 — head 1249b94Review feedback addressed (commit Evidence tags: [T] executed, [A] analysis/reading, [K] known/caveat.
Tests (red before, green after)
Mutants (one per finding, applied alone, each killed, then reverted)
Branch stateThis branch had received an independent round-3 push from another agent ( CI per job (run 37729327591, head
|
| Job | Result |
|---|---|
| Go Build & Test | success [T] |
| Playwright E2E Tests | success — the #353 E2E ran 25/25 [T] |
| Build & Publish Docker Image | success [T] |
| Release Artifacts / Deploy Staging / Publish Badges & Summary | skipped (fork-guarded) [T] |
No known-flaky (#271/#301) failures occurred; no reruns were needed. PR stays draft.
Review — CS-Macmini PR#356 — head 1249b94Dom: APPROVE med nits Independent, read-only re-review of round 3. The operator decision is implemented as asked: a 1-byte path hash now reads as an amber recommendation in all three places, the wording carries the recommendation in text (no error phrasing, in the visible label or the screen-reader clause), the colour comes only from Evidence tags: [T] I ran it, [A] code or doc reading, [K] known, caveat or not verified. Trees tested:
Findings
Answers to the review points1. Operator decision (c): amber recommendation, not a red error. Yes, in all three places [T][A].
2. WCAG AA — measured, not only calculated. Confirmed, and my numbers reproduce the report's table exactly [T]. Measured in Chromium against a local Go server with the CI-prepared fixture: computed
3. Integration of the parallel red round-3 push. Clean [T][A].
4. Nit F2 — no empty
5. Invariants.
Acceptance criteria → tests, red before and green afterI ran head's five test files against master's
Tests I ran (merged tree, fixture prepared like CI)Fixture: fresh copy,
Screenshots checked by eye: channels light and dark, packet detail light, node detail at 375×812, nodes side pane light and dark. The recommendation reads amber and advisory, clearly distinct from the red REPEATER role badge beside it, with the icon visible and nothing clipped [T]. MutantsMy own, each applied alone to the merged tree and reverted afterwards [T]:
Author's mutant re-verified [T]: CI per job (run 37729327591, head
|
| Job | Result |
|---|---|
| Go Build & Test | success [T] |
| Playwright E2E Tests | success — the log shows the #353 E2E ran with "25 passed, 0 failed" [T] |
| Build & Publish Docker Image | success [T] |
| Release Artifacts / Deploy Staging / Publish Badges & Summary | skipped (fork guards) [T] |
I read the E2E job log myself; there are no failing assertions in it, and neither known flaky test (#256 Hash Stats sort, #267 backfill write-hold) appears — the only matches for those names are branch names printed during checkout [T].
Not verified
- No real screen reader. I checked the DOM only:
aria-hiddenon the icon, the sr-only clause, the visible label [K]. - Chromium only; Firefox and WebKit not checked [K].
- The customizer was exercised through its stored overrides and the rendered panel, not by hand through drag, reset and preset flows.
test-customize-theme-e2e.jspasses [K]. - The node side pane does not open at 375×812 in the nodes list, so I measured it at desktop widths only; it uses the same classes and surface as the node card, which I did measure at 375 [T][K].
- Go suites were run on the merged tree only, not compared against a master baseline; there is no Go diff [K].
Relates to #353
Summary
A 1-byte path hash reads as an amber recommendation (not a red error) in all three places it appears. 2- and 3-byte hashes stay neutral, and missing or invalid sizes still render nothing. This reflects the operator decision on #353 (2026-10-07): 1-byte is the firmware default and the MeshCore FAQ still recommends it for companions, so it is surfaced as "use a 2- or 3-byte path hash", not an error.
public/channels.js).ch-path-hash-badge--warn, warning icon, factualSent with: 1-byte+ sr-only— recommended: 2- or 3-byte path hash, recommendation tooltippublic/packets.js).detail-hash-size--warn, warning icon, factual1 byte+ the same sr-only recommendationrenderNodeBadges,public/nodes.js).node-path-hash-badge--warn, warning icon, visible labelRecommended: 2- or 3-byte path hashDesign
public/app.jshasisPathHashSizeWarn(size)for the decision (1= recommend),pathHashSizeValue(size)to cast at the boundary (only a number or numeric string 1–3 counts), andrenderPathHashSize(size, text, { block, cls, title, recInLabel })for the markup. All three call sites use it.--path-hash-warndefaults tovar(--warning)(amber) instead ofvar(--danger)(red), in the light semantic:root. The dark blocks redefine--warning, so the variable follows both themes. Mapped in the customizer aspathHashWarn(Advanced row, label, hint). No hardcoded colours.ph-warningglyph (a warning/advisory glyph, not an error glyph),aria-hidden="true", so the cue never relies on colour alone.firmware/docs/faq.md§3.9.3); repeater advertsset path.hash.mode 1|2(§3.9.4,cli_commands.md); repeaters older than 1.14 drop 2/3-byte packets, so switch once most of the mesh runs 1.14+ (§3.9.1 / §3.9.6).hashSizeDetailHtmlis gated on the shared helper's output, so a size the helper rejects renders no row instead of an empty<dd></dd>(round-2 review, finding 2).WCAG AA contrast (round-2 review, finding 1)
--dangeron the light packet-detail background was 4.43:1 (< AA). The amber--warningpasses AA (≥ 4.5:1) on every surface it renders on, in both themes — measured in-browser:#b45309)#fcd34d)#ffffff/#232340)#ffffff/#232340)#f4f5f7/#0f0f23)Tests
test-issue-353-path-hash-warn.js: helper decision; warn class, icon, recommendation sr clause vsrecInLabellabel;--path-hash-warndefaults tovar(--warning)(not--danger); neutral 2/3; empty output for invalid sizes; escaping.test-channels-observed-path-hash-size.js,test-packets.js,test-frontend-helpers.js,test-customizer-v2.js: the three call sites and the customizer mapping/hint, incl. the no-empty-row guard.test-issue-353-path-hash-warn-e2e.js(indeploy.yml): channels, packet detail (stubbed 1- and 2-byte) and node detail (fixture nodes withhash_size1 and 2), in light desktop, dark desktop, light 375×812, dark 375×812, and manual dark toggle on a light OS. Each checks the warn class, a visiblearia-hiddenicon, the recommendation sr/label, the recommendation tooltip, that the computed colour equals--path-hash-warn=--warning(and ≠--danger), no horizontal overflow, and a WCAG AAcolor-contrastcheck (real@axe-core/playwright, with a direct-luminance fallback when axe's overlap heuristic leaves a narrow-viewport badge "incomplete").Results and CI per job follow in a report comment.
🤖 Generated with Claude Code