fix(map): render an unobserved hash size as unknown, not as 1 byte

[TeTeHacko]

The bug

map.js turns a missing hash_size into 1:

var hs = node.hash_size || 1;

That field is evidence, not a default — computeNodeHashSizeInfo populates it only from adverts it could read a size out of, so a node with no countable advert in the retention window has no value at all. Rendering that absence as 1 states a 1-byte configuration nobody observed, and it does so in the one place where a reader is most likely to act on it.

It is also inconsistent with the rest of the UI for the same field on the same node:

view	code	renders
node detail	nodes.js:683	Hash Prefix: **Unknown**
analytics prefix table	analytics.js:1553	(**?**B)
map popup + label + filter	map.js:140, :1588, :1775	C8 **(1B)**

On analyzer.meshcore.cz right now, 701 of 1007 nodes have hash_size: null, so the map's 1-byte bucket is mostly nodes that were never measured. The Byte Size filter has the same problem from the other end: picking "1-byte" returns measured 1-byte nodes and every unheard node, which makes it hard to use for the thing it exists for.

The fix

• roles.js: shared hashPrefixInfo(node) → {known, bytes, prefix}, so the map stops re-deriving the prefix in three places and the "unknown" rule lives in one.
• map.js:
  • label still draws a 1-byte prefix (it has to draw something) but carries .hash-unconfirmed and its aria-label says …, hash size unknown;
  • popup says Unknown, matching nodes.js wording;
  • filter gets its own Unknown bucket instead of folding unmeasured nodes into 1-byte.
• style.css: dotted underline for the unconfirmed prefix — a shape cue rather than a colour one, so it survives forced-colors and colour-vision differences, consistent with the a11y(map): cluster bubbles + role pills + multi-byte hash labels encode signal by color only (WCAG 1.4.1) #1356 approach for these labels.

nodes.js and analytics.js are left alone: they already behave correctly, and switching them to the helper would widen the diff without changing behaviour. Happy to do it in a follow-up if you'd rather have the call site count at zero.

Tests

node test-frontend-helpers.js → 635 passed, 2 failed; the two failures are favStar, pre-existing on master (baseline run before this change: 625 passed, 2 failed — same two).

10 new cases: hashPrefixInfo across missing / null / 0 / 1 / 2 / 3-byte inputs plus missing pubkey and a null node, and a guard asserting map.js contains no bare hash_size || 1 so this cannot quietly come back.

Browser validation

Headless Chromium against a live instance carrying real mesh data, same viewport (#/map?lat=50.038502&lon=14.570556&zoom=17), unpatched vs patched:

	before	after
aria-label	repeater hash C8	repeater hash C8, hash size unknown
label class	mc-mb-label	mc-mb-label hash-unconfirmed
filter buttons	all,1,2,3	all,1,2,3,unknown

The four nodes in that viewport that do have evidence (157E, 381E, FA74, C029) render exactly as before.

[efiten]

Review from the queue triage. Written 2026-08-30 against the tree at that time; posting now that the maintenance window in #1922 has opened.

Verdict: approve. Best-argued PR in the queue. Shared hashPrefixInfo helper, a new
"Unknown" filter bucket, a dotted-underline shape cue that survives forced-colors, an
aria-label carrying "hash size unknown", nine unit tests, and a regression guard that greps
map.js for a bare hash_size || 1.

Load-order dependency checked and safe: the helper is defined on window in roles.js
(index.html:173) and consumed by map.js (index.html:205), and map.js is loaded from
index.html only. Worth one comment that a hash-prefix helper living in roles.js is a
surprising home for it.

[efiten]

Recycling this again, and the reason is my mistake rather than anything about your PR.

Earlier today I approved the pending workflow run on this PR. That was the wrong order: approving an action_required run makes it execute against the merge commit from when the run was created, not against current master. This one was created weeks ago, so it tested a base that predates the #1923 fix and five merges that have landed since. The result it produced says nothing useful.

Closing and reopening now gets a fresh merge commit against current master, which is what the run should have been all along. No action needed from you, and apologies for the second round of noise.

[efiten]

Recycling for a fresh run. The result you have is from a stale base and says nothing about your PR.

That run was created on 2026-08-25 and I approved it earlier today, which was my mistake: approving an action_required run executes it against the merge commit from when the run was created, not against current master. It therefore tested a tree from before #1923 was fixed and before several merges landed today. It reported ✗ makeRepeaterLabelIcon prepends MB_GLYPHS glyph to the hash text, which is a test your PR touches, so a verdict from the wrong base is exactly the misleading kind.

Closing and reopening to get a real one. My review above is unchanged: this is the best-argued PR in the queue.

[efiten]

The fresh run failed too, so this one is real and not the stale base I described earlier. It is a one-line fix, and the behaviour of your PR is not wrong.

What fails: test-issue-1356-map-a11y.js:133

✗ makeRepeaterLabelIcon prepends MB_GLYPHS glyph to the hash text

That assertion is a source grep with a distance limit, not a behavioural test:

assert(/MB_GLYPHS\[[^\]]+\][\s\S]{0,200}shortHash|shortHash[\s\S]{0,200}MB_GLYPHS\[/.test(mapSrc),
  'makeRepeaterLabelIcon prepends MB_GLYPHS glyph to the hash text');

It requires MB_GLYPHS[...] and shortHash to sit within 200 characters of each other in public/map.js. On your branch they are 312 characters apart, because the unknownWidth line lands between them:

var shortHash = hashInfo.prefix;
var unknownWidth = hashInfo.known ? '' : ' hash-unconfirmed';   // <- inserted here
// #1356 V3: glyph is the primary non-color status carrier, hash is the data,
// status color is a thin left-border (CSS class drives the hue).
var status = mbStatus || null;
var glyph = status ? (MB_GLYPHS[status] || MB_GLYPHS.unknown) : '';

The glyph is still prepended — visible = (glyph ? glyph + '\u2009' : '') + shortHash is untouched — so what broke is the grep, not the feature.

Simplest fix: move the unknownWidth assignment below the glyph line. It is not needed until the html string is built, so the move is free and puts the gap back under 200.

The deeper problem is that #1356's assertion tests where two identifiers sit in the source rather than what the function produces, so it fails on any reordering. Worth fixing separately; I would rather not rewrite someone else's a11y test inside your PR.

My review above otherwise stands: this is still the best-argued PR in the queue, and the regression guard you added (grepping map.js for a bare hash_size || 1) is exactly the right instinct. Ironic that a grep-based test is what caught you out.

[TeTeHacko]

Pushed ffda450. Your diagnosis is right — that assertion is a source grep, not a behavioural test — but the suggested fix does not clear it. I measured both before pushing, by running the test rather than reading the regex.

The assertion needs MB_GLYPHS[...] and shortHash within 200 characters of each other in public/map.js:

variant	gap	test-issue-1356-map-a11y.js
this PR at 66bd57e	299 chars	39 ✓ / 1 ✗
unknownWidth moved below glyph (your suggestion)	233 chars	39 ✓ / 1 ✗
shortHash moved below glyph (ffda450)	—	40 ✓ / 0 ✗

Moving unknownWidth saves 66 characters, and the comment block plus the status line still leave it 33 over. (Small correction to your figure: the gap on the branch head measures 299, not 312. Doesn't change the conclusion.)

So ffda450 moves shortHash instead. That is safe — nothing between the two statements uses it, its first use is ariaStatus — and it reads in the order the assertion actually describes: glyph, then hash. Rendering is untouched either way; visible = (glyph ? glyph + ' ' : '') + shortHash was never edited by this PR.

I also left a comment at the site recording the 200-character constraint, so the next reorder does not rediscover this the hard way.

Verified: the whole JS unit list from .github/workflows/deploy.yml passes, 65/65, with test-issue-1356-map-a11y.js at 40/40.

On the underlying problem — agreed, and thanks for not rewriting #1356's test inside this PR. An assertion that pins where two identifiers sit in the source will fail on any refactor that touches the neighbourhood, and it gives no signal about what the function produces. Happy to open a separate PR turning it into a check on the emitted label string if that is wanted; it seemed out of scope here.

No apology needed for the recycling, by the way. Getting a CI result against current master is worth a couple of notifications.

[TeTeHacko]

Correction to my previous comment, and a self-inflicted one — pushed 19c0a46.

The explanatory comment I added in ffda450 spelled out shortHash and MB_GLYPHS[ within 200 characters of each other. That is exactly the pattern the assertion greps for, so the assertion was being satisfied from inside my comment, independent of the code. I confirmed it the ugly way: with ffda450 applied I moved shortHash back to its original position, reintroducing the very defect this PR was fixing, and test-issue-1356-map-a11y.js still reported 40/40 green.

So for one push this PR did not fix the check — it disabled it. A grep-based assertion that a comment can satisfy is worse than one that fails on refactors, because green stops meaning anything.

19c0a46 rewords the comment to avoid naming either identifier while keeping the substance. Mutation-tested this time, which is the step I skipped before:

tree	test-issue-1356-map-a11y.js
19c0a46 as pushed	40 ✓ / 0 ✗
same, shortHash moved back to its original position	39 ✓ / 1 ✗
restored	40 ✓ / 0 ✗

Full JS unit list from .github/workflows/deploy.yml: 65/65.

The numbers in my earlier comment stand — the gap measurements and the point that moving unknownWidth only reaches 233 were made against the code, not the comment, and I have re-checked them. What was wrong was the claim that ffda450 was verified; it passed the test for the wrong reason.

This is a fairly direct argument for replacing that assertion with a check on the emitted label string rather than on source layout. A source grep cannot distinguish code from a comment about the code. Happy to open that as a separate PR against #1356 if it is wanted.

[TeTeHacko]

One more correction, and this one is against my own earlier comment.

I told you the gap was "299, not 312". Both numbers are the distance between the two declarations — and that is not the distance the regex uses. It is satisfied through the other alternative: MB_GLYPHS[...] reaching the later shortHash inside ariaStatus. On master that is 212 characters, which is why the assertion passes there at all; the declaration order it is named after never enters into it.

Measured on the pair that actually matters:

tree	MB_GLYPHS[ → next shortHash	assertion
master	212	pass
this PR before the fix	277	fail
unknownWidth moved below the glyph	343	fail
shortHash moved below the glyph (19c0a46)	54	pass

So your suggestion does not fall 33 characters short — it moves the gap the wrong way, because unknownWidth lands between the glyph and ariaStatus. The conclusion in my first comment held, the reasoning under it did not. Your 312 was a correct measurement of the declaration distance; I was wrong to "correct" it, and wrong again to treat that pair as the relevant one.

I have opened #1933 against master to replace these three assertions with checks on what the function renders, with the mutation table showing what each version catches. It is independent of this PR and touches nothing this one changes.

[efiten]

You are right on both counts and my advice was worse than wrong, it was actively harmful. Taking them in order.

The pair I measured is not the pair the regex uses. The assertion has two alternatives, and on master it is satisfied through the second one, MB_GLYPHS[...] reaching the later shortHash inside ariaStatus, at 212 characters. My 312 was the declaration-to-declaration distance, which the regex never evaluates on a passing tree. So I measured something real, reported it confidently, and it was not the number that decides the outcome. Thank you for not just taking it.

My suggested fix moves the gap the wrong way. 277 → 343 by your table, because unknownWidth lands between the glyph and ariaStatus. Had you applied what I suggested, you would have re-run the test, seen it still fail, and lost time on my say-so. Your 19c0a46 gets it to 54.

And the ffda450 detour is the most useful thing in this thread. An assertion that a comment can satisfy is not a weak test, it is a test that silently stops existing. That you caught it by reintroducing the original defect and watching the suite stay green is exactly the check most people skip. I would not have found it from the outside.

I have approved the workflow run on 19c0a46.

#1933 is the right follow-up and I will review it on its own merits. Replacing a source grep with a check on what the function renders is worth more than this PR is, in the long run: right now the a11y guarantees in that file are pinned by where identifiers sit in the source, which is a promise about formatting rather than about behaviour.

My review verdict on this PR is unchanged: approve. The regression guard you added, grepping map.js for a bare hash_size || 1, has aged into a small irony given the thread, but it is still the right instinct: it pins an invariant nobody can restate by accident.

[efiten]

Recycling again, and again the failure is not yours.

19c0a46 failed on TestNeighborEdgesBuilderPathTrustExcludesOneByte, which is a breakage I put on master this morning: #1929 moved the pathTrust default from 2 to 1 and #1930 carried a test that assumed 2. Each was green alone, neither run saw the other. Your run started before the fix (#1932) landed, so it tested a broken base.

Master is fixed now. Closing and reopening to get you a run against it.

[efiten]

Recycling both of your PRs, and it is worth saying why, because it is a trap that has cost this queue a lot of time today.

Both of your runs are attempt=2, so they were re-run rather than recycled. A re-run does not help here. GitHub re-runs against the same commit the original run used, which for a pull_request event is the merge commit as it stood when that run was first created. It never sees a newer master.

That fully explains #1912. Its re-run at 11:44 still tests a base from before #1932, which is the fix for the master breakage I caused this morning by merging #1929 and #1930 together. So it keeps failing on TestNeighborEdgesBuilderPathTrustExcludesOneByte, which has nothing to do with your change and is already fixed on master.

For #1933 it is less clear-cut. Its run started at 09:31, which is the same minute #1932 merged, so its base may or may not have included the fix. The failure there was setting persists across reload: multibyte toggle should restore checked=true from localStorage, in test-live-multibyte-only-e2e.js.

I said I would look properly if it failed twice rather than send you chasing it, so: I read your public/map.js change and it cannot cause that failure. It adds makeRepeaterLabelIcon to the existing window.__meshcoreMapInternals object and reformats the literal across lines. Nothing in it touches the live view, the multibyte toggle or localStorage. Since it has only genuinely run once, a real fresh run is the right next step before anyone goes digging.

Closing and reopening both, which forces a new merge ref against current master. Nothing needed from you.