Skip to content

fix(analytics): sort the Hash Stats adopters table by the clicked column, with direction (#226) - #230

Merged
dborup merged 2 commits into
masterfrom
codex/issue-226-hash-stats-sort
Oct 5, 2026
Merged

fix(analytics): sort the Hash Stats adopters table by the clicked column, with direction (#226)#230
dborup merged 2 commits into
masterfrom
codex/issue-226-hash-stats-sort

Conversation

@adminopenclaw8-sketch

@adminopenclaw8-sketch adminopenclaw8-sketch commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator

Relates to #226

The Hash Stats multi-byte adopters table sorted by the wrong column and had no sort state. This makes the sort state of the card, applies it to the row data before the table renders, and deep-links it next to mbf=.

What was wrong

  • The header click used colIdx = { name: 0, status: 1, hashSize: 2, packets: 3, lastSeen: 4 }, but the table has six columns (Role was added with the map in 45623672). Each header read the cell to its left: Role did nothing, Status read Role, Adverts read Hash Size, Last Seen read Adverts.
  • Last Seen would have compared the timeAgo text ("5m ago") as strings even with the right index.
  • The sort was ascending only, showed no indicator, and the next filter click re-rendered the table in the server's order.

Change (public/analytics.js)

  • The card keeps { col, dir }. A header click re-renders the table through the same buildTableContent the filter click uses, so a filter click keeps the sort and the header shows it.
  • Each column sorts by its own row value, not by a cell index: Node and Role as lower-case text, Status by weight (Confirmed, Suspected, Unknown), Hash Size and Adverts as numbers, Last Seen by timestamp. A missing Last Seen is last in both directions; ties keep the server's order.
  • Clicking the sorted header flips the direction, another header starts ascending. The sorted header gets aria-sort, sort-active and the arrow the channel table already uses (channelSortArrow); no new colours or CSS.
  • Deep link: mbsort=<name|role|status|hashSize|packets|lastSeen> and mbdir=<asc|desc>, URL only like mbf=, through the analytics: deep-link inner views (Scopes sub-tabs + window, audit other tabs' local view state) #205/follow-ups from #203, #204 and #206 reviews (P3: test gaps, inactive-card wording, history defaults, Hash Stats audit) #208 view-param helpers. Values are only compared with the fixed column keys, never put in a selector or markup; an unknown value is the default (the server's order), and a direction without a column is dropped. Both keys are dropped on a tab switch. Small enough to keep: two specs, one restoreViewParams call, one _writeViewParams on click.
  • The filter and mbf= are unchanged.

Performance

The adopters list is small (21 rows on the E2E fixture; the card lists nodes that advertised with multi-byte hashes). A header click sorts that list once, O(n log n), and re-renders only the table inside the card, as a filter click already does. No API call, no work on other tabs.

Tests

  • test-hash-stats-sort-226.js (new, in test-all.sh): per column and direction, the column's own cells come out in order; Last Seen follows the timestamps; no sort or an unknown column keeps the server order; aria-sort and the arrow on the sorted column only; header and filter clicks through the card's real click handler; mbsort=/mbdir= written next to mbf=; hostile values never reach the markup. 18 of 21 red on master.
  • test-analytics-subtab-deeplinks-205.js: mbsort=/mbdir= deep links on the whole page, hostile values, URL only, dropped on a tab switch. 16 of the new tests red on master.
  • test-issue-226-hash-stats-sort-e2e.js (new, added to the Playwright step): every header, both directions, against the fixture's own cells (Last Seen against the API timestamps), filter click, reload, deep link, hostile value, tab switch. 12 of 15 red against master's frontend.

Details, mutants and the screenshot are in the report comment.

🤖 Generated with Claude Code

dborup and others added 2 commits October 4, 2026 15:09
@dborup @claude
test(analytics): pin the Hash Stats adopters sort per column, directi…
8990b6d 
…on and deep link (#226)

Red on master: the header map misses Role, so each header sorts by the
cell to its left (Role does nothing, Last Seen compares "x ago" text);
the sort is ascending only, has no indicator, is lost at the next filter
click and is not in the URL.

- test-hash-stats-sort-226.js: each column's own cells in order, both
  directions; Last Seen by timestamp with a missing one last; aria-sort
  and arrow; header and filter clicks through the card's real handler;
  mbsort=/mbdir= written next to mbf=.
- test-analytics-subtab-deeplinks-205.js: mbsort=/mbdir= deep links on
  the whole page, hostile values give the server order and a canonical
  URL, URL only, dropped on a tab switch.
- test-issue-226-hash-stats-sort-e2e.js (added to the Playwright step):
  every header and both directions on the fixture, filter click, reload,
  deep link, hostile value, tab switch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dborup @claude
fix(analytics): sort the Hash Stats adopters table by the clicked col…
523b3fb 
…umn, with direction (#226)

The header click sorted the rendered rows by a column map that missed
Role, so every header read the cell to its left, and Last Seen compared
the "5m ago" text. It sorted ascending only, showed nothing, and the
next filter click re-rendered the table in the server's order.

The sort is now state of the card ({ col, dir }) and is applied to the
row data before the table is rendered, by the same buildTableContent the
filter click uses, so a filter click keeps it. Each column sorts by its
own value: Node and Role as lower-case text, Status by weight, Hash Size
and Adverts as numbers, Last Seen by timestamp (a missing one last in
both directions); ties keep the server's order. Clicking the sorted
header flips the direction, another header starts ascending; the sorted
header gets aria-sort, sort-active and the arrow the channel table uses.

The sort is deep-linked as mbsort= and mbdir= next to mbf=, URL only,
through the #205/#208 view-param helpers: values are only compared with
the fixed column keys, an unknown one is the default (the server's
order), and a direction without a column is dropped. Both keys are
dropped on a tab switch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@adminopenclaw8-sketch

adminopenclaw8-sketch commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator Author

Rapport — CS-Macmini PR#230 #226 — head 523b3fb

Status: Draft PR open. All local unit and E2E tests are green, the new tests are red on master, 8 of 8 mutants are killed, and the XSS diff gate is clean. CI was still running when I posted (run 37204718658, Go Build & Test in progress); see the CI section.

Evidence: [T] = ran it, [A] = read in source or diff, [K] = known from earlier work or another report, not re-verified here.

Base and commits:

  • Base: origin/master 7697a826, new worktree, branch codex/issue-226-hash-stats-sort.
  • 8990b6de tests (red on master), then 523b3fbd fix.
  • No rebase, amend or force-push. Both commits have author and committer dborup <kontakt@meshview.dk> [T].
  • git ls-remote shows head 523b3fbd [T].

Investigation first [A]

  • The filter re-renders only the table. It does not re-render the card. A filter click sets currentFilter, toggles the button classes and replaces #mbAdoptersTableWrap's innerHTML with buildTableContent(rows, currentFilter). The old header click instead sorted the rendered <tr>s in place, by colIdx, so the next filter click rebuilt the table from rows in the server's order.

  • The fix uses the filter's render path. The sort becomes card state { col, dir } beside currentFilter, and buildTableContent(rows, filter, sort) sorts the filtered rows before rendering. A header click re-renders through that same path. This removes the cell-index lookup and the text parsing entirely: each column sorts by its own row value.

  • The deep link can stay small:

    • two view-param specs (HASHSTATS_MB_SORT, HASHSTATS_MB_DIR, URL only, no storageKey, like mbf=);
    • one restoreViewParams call in renderHashSizes;
    • one _writeViewParams call on a header click;
    • both keys added to TAB_URL_PARAMS.hashsizes.

    Values are only compared with the fixed column keys (resolveViewParam, indexOf), and the header markup is built from the fixed key list, never from the URL. A direction without a column is reset to the default, so the URL stays canonical.

Acceptance criteria

Criterion Test Red on master Mutant (killed)
A unit test per column asserts the order of that column's own cells, both directions test-hash-stats-sort-226.js: 12 tests (6 columns × asc/desc), each reading the column's cells through its own header index and comparing them to the expected sequence, plus "every header click sorts its own column (Role included)" through the card's real click handler yes, 18 of 21 [T] X1 Role shift re-introduced (each column sorts by the one to its left): 13 unit, 6 deep-link, 6 E2E failures
Last Seen is ordered by timestamp, not by text; a missing one is last unit: Last Seen follows the timestamps… (the fixture's text order "10m < 2h < 3d < 45s" differs from its time order); E2E: Last Seen checked against the /api/analytics/hash-sizes timestamps, and the text order differs on the fixture yes [T] X2 Last Seen as timeAgo text: 4 unit, 1 deep-link, 4 E2E. X5 missing value not kept last: 3 unit
Asc/desc toggle with aria-sort unit: clicking a header sorts ascending, clicking it again descending, aria-sort and the arrow mark the sorted column only; E2E: every header clicked twice, with aria-sort and arrow checked yes [T] X3 direction ignored: 9 unit, 2 deep-link, 7 E2E. X7 no aria-sort: 2 unit
The sort survives a filter click (mbf=) unit: a filter click keeps the sort and still filters, a header click keeps the filter; E2E: sort, then Confirmed, then All, then reload yes [T] X4 filter click renders with the initial sort: 1 unit
Deep link mbsort=/mbdir=, #206/#214 pattern (no selector from the URL, unknown gives the default) test-analytics-subtab-deeplinks-205.js: 4 deep links, with mbf=, 8 hostile mbsort= and 5 hostile mbdir= values (server order, canonical URL, nothing in the markup), URL only, dropped on a tab switch; unit: clicks write the keys, default direction left out; E2E: cold load, hostile value, reload, tab switch yes, 16 new tests [T] X6 keys not dropped on a tab switch: 1 deep-link. X8 header click does not write the URL: 1 unit
Filter and mbf= unchanged the existing #208 mbf= tests (unit harness and test-issue-205-analytics-subtab-deeplinks-e2e.js) pass unchanged; the filter branch of the click handler only gained the sort argument n/a the X4 test also asserts mbf= and the active button
No hardcoded colours, no per-item API calls, XSS diff gate clean scripts/check-xss-sinks.sh --diff origin/master: exit 0, nothing flagged [T]; no new CSS (reuses .analytics-table th.sortable/.sort-active and channelSortArrow) [A]; no fetch added [A] n/a n/a

All mutants were applied one at a time to public/analytics.js and restored with git checkout from the fix commit; git status was clean afterwards [T].

Tests run [T]

Screenshot

A screenshot of #/analytics?tab=hashsizes&mbsort=lastSeen&mbdir=desc on the E2E fixture was taken locally [T]. Last Seen is highlighted with ↓ and runs from 4m to 2h ago; the other headers show ⇅. I have no way to upload an image to a GitHub comment from the CLI, so it is not attached here.

CI

Run 37204718658 on 523b3fbd: Go Build & Test in progress when I posted. Playwright E2E had not started. I did not poll; a follow-up comment will give the result per job.

Remaining

  • The headers are clickable but not keyboard-focusable, the same as the channel table's sortable headers. Making them buttons is a separate, page-wide a11y change.
  • On the E2E fixture all 21 adopters are Confirmed, 2-byte and repeaters except one room, so Status and Hash Size only sort trivially there. The unit tests cover them with distinct values.
  • /observers mobile contrast (above) is pre-existing and unrelated.
  • The key order in the URL follows the click order (…&mbsort=…&mbdir=…&mbf=… when the filter is clicked last), as URLState.updateHashParams appends new keys. Cold loads keep the URL as given.

@adminopenclaw8-sketch

adminopenclaw8-sketch commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator Author

Review — CS-Minimax PR#230 hash-stats-sort — head 523b3fb

Dom: APPROVE med nits

An independent, read-only review. I made no changes to the PR or the branch. git ls-remote showed head 523b3fbd both before and after the review [T].

Evidence: [T] = I ran it, [A] = I read it in the source or diff, [K] = known from the author's report or earlier work, not re-verified here.

Trees:

  • The PR's merge base is 7697a826; origin/master is 9eb13156, 41 commits ahead.
  • The merged tree is git merge-tree --write-tree origin/master 523b3fbd…, which gives f8bc186e with no conflicts [T].
  • Every test below ran on a git archive of that tree in scratch.

Findings

# Severity Finding Evidence
F1 Nit (UX) Every column starts ascending on its first click. The page's other sortable tables start numeric and time columns descending: the channel table at analytics.js ~1462 (name/hash asc, otherwise desc), and the Areas sections through ascByDefault, desc by default. Here a first click on Adverts shows the fewest first, and a first click on Last Seen the oldest first. A per-column default in nextMbSort would fix it, for example { packets: 'desc', lastSeen: 'desc' }. The unit tests and the E2E pin asc first, so they would need the same change. Optional. [A]
F2 Nit (a11y, pre-existing pattern) The headers can't be reached by keyboard. The <th> has no tabindex and no <button>, the same as the channel table's headers; the author noted this. Also, channelSortArrow's ⇅/↑/↓ span has no aria-hidden="true", so the glyph becomes part of each header's accessible name. aria-sort is now set (better than the channel table), so the arrow is redundant to screen readers. Both are shared with the channel table and fit a page-wide follow-up, not this PR. [A]
F3 Nit Node and Role compare as lower-cased strings by code point. This is the same as sortChannels, so it is consistent. However, names that start with a non-ASCII letter (Æ/Ø/Å, accented letters) or with an emoji sort after z. localeCompare would place them naturally. Not blocking. [A]
F4 Info (DRY) This is the page's third local table sort: sortChannels, the Areas makeAreasSection, and now sortMbAdopterRows. The Areas helper is closure-scoped and has no missing-value or URL handling, so reusing it here was not realistic. The new comparator (stable via the index tie-break, NaN last in both directions) is the most complete of the three and a good candidate to extract later. [A]
F5 Info The URL key order follows the click order, for example …&mbsort=…&mbdir=…&mbf=…, because URLState.updateHashParams appends new keys. This is cosmetic and the author noted it. [T]

There are no blocking findings.

1. Correct column — OK

  • The header markup is built from one fixed sortCols list, and the sort value comes from the row object via mbAdopterSortValue(r, col), not from a cell index. A column offset is therefore no longer possible [A].
  • Per column [A]:
    • Node and Role sort on lower-cased text. Role uses the same 'unknown' fallback as the cell, so the sort matches what the cell shows.
    • Status sorts by weight: confirmed 0, suspected 1, unknown 2. An unknown status gets the weight of unknown, and the hasOwnProperty check means __proto__ cannot leak in.
    • Hash Size and Adverts sort with Number().
    • Last Seen sorts with Date.parse(r.lastSeen), and a missing value gives NaN and is placed last.
  • The API sends lastSeen as ISO with a Z suffix, for example 2026-10-04T12:52:49Z from /api/analytics/hash-sizes on the fixture, so Date.parse is safe across browsers [T].
  • In the browser [T]:

2. Direction — OK

  • Clicking the sorted header flips asc/desc; another header starts at asc (see F1) [A][T].
  • Only the sorted <th> gets aria-sort="ascending|descending" and sort-active. The arrow is channelSortArrow [A][T].
  • No new CSS is added. It reuses .analytics-table th.sortable/.sort-active/.sort-arrow, which use var(--link-color). The diff adds no hardcoded colours [A].

3. State — OK

  • The sort is card state (currentSort) next to currentFilter. The header and filter clicks share one render path, buildTableContent(rows, filter, sort) [A].
  • In the browser [T]:
    • A sort followed by a Confirmed click keeps packets:descending.
    • A theme-refresh re-render keeps sort and filter (a new table element, same state from the URL).
    • Changing the time window to 24h, which reloads the data and re-renders, keeps the state too.
    • The new card's handler works after the re-render.

4. Deep link (mbsort= / mbdir=) — OK

  • feat(analytics): deep-link Scopes sub-tabs and window (#205) #206/fix: P3 follow-ups from the #203/#204/#206 reviews (#208) #214 pattern:
    • Values go through resolveViewParam and are only compared against the fixed allowed lists with indexOf/=== [A].
    • The data-sort attribute and the label come from sortCols, never from the URL [A].
    • An unknown value gives the default (the server's order) [A][T].
    • A direction without a column is reset, so the URL stays canonical: mbdir=desc alone is removed [A]. My mutant M5 confirms this behaviour is tested.
    • The unit test injects an "><img onerror> payload, and it does not reach the markup [T].
  • Collisions: the other #/analytics keys are tab, window, range, observer, from, to, section, bytes, sub, swin, wdwin, mbf and the region/area filters. There is no mbsort/mbdir anywhere else in public/ [A].
  • Tab switch: both keys are in TAB_URL_PARAMS.hashsizes. In the browser, Hash Stats with a sort, then Collisions, then back to Hash Stats gives #/analytics?tab=hashsizes&window=24h&bytes=1, with no sorted header [T]. bytes= stays on purpose (Allow to link to Mesh Analytics with a path hash byte size set via url Kpa-clawbot/CoreScope#1914).
  • Like mbf=, the keys are URL only (no storageKey) and are written with replaceState, so they add no history entries [A].

5. No behaviour change elsewhere — OK

  • The filter branch: the only change is the extra currentSort argument [A].
  • mbf= tests:
    • The existing mbf= tests in test-analytics-subtab-deeplinks-205.js pass, 121 of 121 [T].
    • test-issue-205-analytics-subtab-deeplinks-e2e.js passes, 15 of 15 [T].
  • Other analytics tabs: untouched except for the TAB_URL_PARAMS entry [A].
  • No per-item API calls: the diff adds no fetch [A].
  • XSS gate: scripts/check-xss-sinks.sh --diff origin/master, on a --shared scratch clone checked out at head, exits 0 with nothing flagged [T].

6. Perf — OK

  • The sort is O(n log n): it decorates once (map to {r, i, v}), runs one Array.prototype.sort and undecorates. No nested scans [A].
  • Re-render scope: a header click replaces only #mbAdoptersTableWrap's innerHTML, the same scope as the existing filter click. The rest of the card and page are left alone [A].
  • Scale: n is the number of multi-byte adopters (21 on the fixture). Nothing runs on ingest, WS or other tabs.
  • Rule 0's proof requirement is aimed at PRs that claim a perf improvement. This one claims none, so I don't think a benchmark is needed here.

7. Rules — OK

Tests [T]

On the merged tree (origin/master 9eb13156 + head):

Test Result
sh test-all.sh 216 passed, 0 failed (216 files), exit 0
node test-frontend-helpers.js 707 passed, 0 failed
node test-hash-stats-sort-226.js 21 passed
node test-analytics-subtab-deeplinks-205.js 121 passed

E2E setup:

E2E Result
test-issue-226-hash-stats-sort-e2e.js 15 passed
test-issue-205-analytics-subtab-deeplinks-e2e.js 15 passed
test-issue-1306-collisions-terminology-e2e.js 23 passed

Red on master: I swapped in origin/master's public/analytics.js on the merged tree and restored it afterwards (checked with cmp):

  • test-hash-stats-sort-226.js: 18 of 21 fail.
  • test-analytics-subtab-deeplinks-205.js: 16 of 121 fail.
  • test-issue-226-hash-stats-sort-e2e.js: 12 of 15 fail.

These match the author's numbers.

Mutants (my own, applied one at a time to public/analytics.js, then restored) [T]

Mutant unit 226 deeplink 205 E2E 226 E2E 205 Killed
M1 column offset +1 (each header's data-sort = the next column's key) 15 fail pass 9 fail pass yes
M2 Last Seen sorted as timeAgo text 4 fail 1 fail 4 fail pass yes
M3 direction ignored after a filter click (re-render with dir: 'asc') 1 fail pass 1 fail pass yes
M4 mbdir left out of TAB_URL_PARAMS (not dropped on a tab switch) pass 1 fail 1 fail pass yes
M5 no reset of a direction without a column pass 8 fail 1 fail pass yes
M6 Status weights rotated (unknown first) 3 fail pass pass pass yes
M7 aria-sort inverted 2 fail pass 9 fail pass yes
M8 missing Last Seen first when descending 2 fail pass pass pass yes
M9 a new column inherits the previous direction 1 fail pass pass pass yes
M10 ties in reverse server order (unstable) 4 fail pass pass pass yes

test-frontend-helpers.js stayed green under every mutant, as expected, since it does not cover this card. After the run the file was byte-identical to the merged tree. M6, M8, M9 and M10 are killed only by the unit test, because the fixture's Status, Hash Size and Adverts values are almost uniform. The author pointed this out, and the unit fixture covers those cases with distinct values.

Browser [T]

On the local server and fixture, in the built-in browser:

  • Deep link: #/analytics?tab=hashsizes&mbsort=lastSeen&mbdir=desc shows LAST SEEN highlighted in the link colour with ↓, the other headers with ⇅, and rows 7m, 10m, 26m, 27m … ago.
  • Filter: "All (21)" is active.
  • Re-renders: a header click, a filter click, theme-refresh, a time-window change and a tab switch behave as described in sections 3 and 4.
  • Screenshot: taken locally. I can't attach an image to a comment from the CLI, so it is not included here.

CI

Run 37204718658 on 523b3fbd:

Not verified

  • Not run locally: the full Playwright suite (test-e2e-playwright.js and the rest of the deploy.yml E2E list), test-a11y-axe-1668.js and instrumented-coverage collection. I ran only the PR's E2E and the analytics E2Es named above.
  • Browsers: Safari and Firefox were not tested. Date.parse on the ISO-Z format is standard.
  • Screen reader: not tested with a real one; F2 is based on reading the markup.
  • Large meshes: no measurement with a large adopter list. The complexity argument is in section 6.
  • No staging or prod access, per the review scope.

@dborup
dborup marked this pull request as ready for review October 5, 2026 06:37
@dborup
dborup merged commit 341a196 into master Oct 5, 2026
6 checks passed
This was referenced Oct 5, 2026
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@adminopenclaw8-sketch @dborup