fix(packets): collapsed groups show a right caret, expanded a down caret (#189)

[dborup]

Relates to #189

A collapsed packet group in the grouped packets view showed an up-pointing caret (#ph-caret-up). Before the emoji-to-Phosphor migration (30627454, Kpa-clawbot#1648 M2) it showed ▶; the migration mapped ▶ to caret-up. This is the last open part of #189: the knownBug('#189') case in test-packets.js is now a plain assertion.

Convention

Collapsed is caret-right, expanded is caret-down. Disclosures elsewhere in the front end already use this:

Where	Collapsed	Expanded
channels.js section header	▸	▾
network-digest.js	▸	▾
analytics.js #ptOverviewChevron	ph-caret-right	rotated 90° (down)
route-view.js paths chevron	ph-caret-down, rotated −90° (right)	down
packet group before Kpa-clawbot#1648 M2	▶	▼

nodes.js uses up and down differently: "Show all N neighbors" has a down caret and "Show fewer" an up caret. Those are labelled buttons that name an action, not a fold state, so they stay as they are. Right/down needs the smallest change here (two glyph names, in one place) and matches the glyphs the packet group had before.

Change

• public/packets.js (buildGroupRowHtml): the expand cell renders ph-caret-right when collapsed and ph-caret-down when expanded. The group row, which is the toggle (tabindex="0", data-action="toggle-select"), gets aria-expanded="false|true" to match. A single-observation row cannot expand, so it has neither a caret nor aria-expanded.
• public/style.css: removes .group-header td:first-child::before (▶ / ▼ ). It dates from the initial commit and still applied, because that first cell is now the expand cell, so a collapsed group showed the CSS ▶ next to the sprite (▸ and ^ in the screenshot taken before the fix). Left in, the fix would have shown two right carets.
• public/packets.js also gains a test hook, _setExpanded, next to _setPackets and _setFilter, so the unit sandbox can expand a group.
• No colours added; nothing interpolates node data.

Tests

• test-packets.js: the collapsed-group case is now a plain test() and fails on master. New cases: an expanded group shows caret-down (not right, not up); the group toggle row reports aria-expanded false then true; a single-observation row has no caret and no aria-expanded. 132 passed, 0 failed (128 passed and 1 known bug before).
• test-issue-189-group-caret-e2e.js (Playwright, registered in deploy.yml) on the seeded 3-observation group, in the grouped view: collapsed (right caret, aria-expanded="false"), expand (down caret, "true"), collapse (right caret, "false"). In each state the cell holds exactly one caret and no CSS ::before triangle. Red on master (caret-up, no aria-expanded, ::before ▶/▼), 3 of 3 green on the branch. It is registered after the bug(packets): filter UX disaster — help panel overlaps table, toolbar chaotic, path chips spill rows Kpa-clawbot/CoreScope#1122 Details-clamp step so it does not move that time-dependent step (test: Details-clamp E2E 'advert links in Details stay visible and clickable' is time-dependent and flaky #244).
• Mutants, each against the unit test and the E2E: collapsed caret back to up (killed by both); aria-expanded renamed (both); the CSS triangle restored (E2E only); right and down swapped (both).
• sh test-all.sh 218/218 files; node test-frontend-helpers.js 707 passed, 0 failed; scripts/check-xss-sinks.sh --diff origin/master exit 0.
• The packets E2Es (#96, #147, #180, #966 filter UX, #1122 filter UX and Details clamp, #1128 layout and multi-viewport, #1486, #1657, #1692, #1758, packet-trace alignment, #1056 slide-over) pass against a local Go server on a copy of e2e-fixture.db, migrated as in CI.

Not changed

• Other inconsistent carets are left as a remainder, listed in the report comment.
• deploy.yml otherwise unchanged; the fork guards stay at 9 and 1.

🤖 Generated with Claude Code

[dborup]

Rapport — CS-MacBook PR#248 #189 caret — head 9959d7a

Status: The collapsed-group caret is fixed on a draft PR, tests and a mutant check are done locally, and CI is pending (not polled by me); the PR stays a draft.

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

Branch

• codex/issue-189-group-caret from origin/master 9989b97f, three commits, all by dborup <kontakt@meshview.dk> as author and committer; no rebase, amend or force-push; explicit git add only. [K]
  1. 356d1e10 tests: the knownBug('#189') case becomes a plain test, plus new cases (red on master).
  2. b720b21b the E2E, registered in deploy.yml (red on master).
  3. 9959d7a8 the fix.
• Fork guards unchanged: 9 in deploy.yml, 1 in release-fast-path.yml. [K]

Convention chosen: caret-right when collapsed, caret-down when expanded

I listed every fold-like glyph in public/*.js. [K]

Where	Collapsed	Expanded
channels.js section header (ch-section-caret)	▸	▾
network-digest.js	▸	▾
analytics.js #ptOverviewChevron	ph-caret-right	rotated 90° (down)
route-view.js paths chevron	ph-caret-down rotated −90° (right)	down
packet group before Kpa-clawbot#1648 M2	▶	▼
packet group on master	ph-caret-up	ph-caret-down

Four disclosures already use right/down, and it is what the packet group had before the migration, so it needs the least change: two glyph names in one place. nodes.js (the #229 note) uses down for "Show all N neighbors" and up for "Show fewer", but those are labelled action buttons and not fold states, so I left them.

Change

• public/packets.js, buildGroupRowHtml: the expand cell renders ph-caret-right collapsed and ph-caret-down expanded. The group row is the toggle (tabindex="0", data-action="toggle-select"), and it had no aria-expanded; it now gets aria-expanded="false|true" from the same isExpanded. A single-observation row cannot expand, so it has neither a caret nor aria-expanded. The toggle re-renders the rows (renderTableRows), so the attribute follows the state. [K][T]
• public/style.css, found while testing in a browser: .group-header td:first-child::before drew a second ▶/▼. The rule dates from the initial commit. The first cell is now the expand cell, so on master a collapsed group showed the CSS ▶ and the up caret next to each other (the screenshot before the fix shows ▸ and ^). Fixing only the sprite would have shown two right carets, so the two rules are removed. Computed ::before content was "▶ " / "▼ " before and none after. [T]
• public/packets.js test hook: _setExpanded(hash, on) in _packetsTestAPI, next to _setPackets and _setFilter, so the unit sandbox can expand a group. [K]
• No colours added, and nothing interpolates node data. scripts/check-xss-sinks.sh --diff origin/master exits 0. [T]
• ARIA caveat: aria-expanded is on the row, which is the element that toggles. ARIA formally allows it on row inside a treegrid; in a plain table, screen readers may ignore it. Moving it would need a button inside the cell, which is a bigger change. [A]

Tests and mutants

• node test-packets.js: red first with 2 failures (collapsed caret not right; no aria-expanded), then 132 passed, 0 failed (master: 128 passed plus 1 known bug). New and changed cases: the collapsed case as a plain test (right, not up, not down); expanded shows down (not right, not up); aria-expanded is false then true; a single-observation row has no caret and no aria-expanded. The knownBug helper stays in the file; nothing uses it now. [T]
• test-issue-189-group-caret-e2e.js (Playwright, in deploy.yml after the #1128 packets-layout step, i.e. after the bug(packets): filter UX disaster — help panel overlaps table, toolbar chaotic, path chips spill rows Kpa-clawbot/CoreScope#1122 Details-clamp step so it does not move that time-dependent step, test: Details-clamp E2E 'advert links in Details stay visible and clickable' is time-dependent and flaky #244): on the fixture's seeded 3-observation group in the grouped view: collapsed → right caret, aria-expanded="false"; click → down caret, "true"; click again → right, "false"; one caret and no ::before triangle in each state. Red on master (3 of 3), green on the branch (3 of 3). [T]
• Mutants, each run against the unit test and the E2E, then restored (cmp against the fixed copies):

Mutant	Unit	E2E
M1: collapsed caret back to up	1 failed	2 failed
M2: aria-expanded renamed	1 failed	3 failed
M3: CSS triangle rules restored	green	3 failed
M4: right and down swapped	3 failed	3 failed

All four are killed. [T]

• Suites on the branch: sh test-all.sh 218/218 files; node test-frontend-helpers.js 707 passed, 0 failed. [T]
• E2E, against a local Go server built from the branch on a copy of test-fixtures/e2e-fixture.db prepared as deploy.yml does it (freshen, grouped-row and GRP_DATA seed, corescope-migrate, the two seed files), stopped by port: #189 3/3, #1486 collapse 4/4, #96 10/10, filter UX 12/12, #1758 exit 0, #1692 1/1, #147 12/12, #180 12/12, #1122 filter UX 6/6, #1122 Details clamp 18/18 (fixture age under 15 minutes), #1128 layout 5/5 and multi-viewport 15/15, #1657 5/5, packet-trace alignment 17 checks, slide-over #1056 27/27. [T]
• Screenshots, taken locally with Playwright of the seeded group, before and after the fix, collapsed and expanded: after the fix there is one right caret collapsed and one down caret expanded; before, the collapsed row showed the CSS ▶ plus the up caret. They are not attached, because I cannot upload images to a comment from the CLI. [T]

CI

Pending: the run for head 9959d7a8 has not finished and I did not poll it. The one known flaky check is the Kpa-clawbot#1122 Details-clamp E2E (#244); it depends on the fixture age at which the step runs, and this PR adds its E2E after that step. [K]

Remainder: other carets that are inconsistent with right/down

• public/nodes.js affinity-debug toggle (about line 920): inverted carets, and broken. The heading shows caret-down while collapsed and caret-up when open, the opposite of the convention. Worse, its inline onclick="…" contains class="ph-icon" with unescaped double quotes inside a double-quoted attribute. I parsed the line in a browser: the attribute value is cut at the first " and new Function rejects it ("Invalid or unexpected token"), so the toggle's handler is a syntax error. It needs quote escaping or an event listener, so it is not a trivial change and I left it. Suggest a separate issue. [T]
• nodes.js "Show all N neighbors" (down) / "Show fewer" (up): labelled action buttons, left as they are (see the convention section). [K]
• Text glyphs instead of sprites: channels.js and network-digest.js use ▸/▾ characters. Their orientation matches the convention; they are not Phosphor sprites. [K]
• route-view.js:509 uses a text ⌃ for the mobile sheet chevron. Not a fold state of the same kind; left. [K]
• map.js and route-view.js side-panel toggles use left/right carets for panel collapse, a separate pattern. [K]

Not verified

• CI on the PR (pending).
• Firefox and Safari, and screen-reader behaviour of aria-expanded on a role="row" element.
• Narrow widths and the mobile layout: below the breakpoint the expand column is hidden by style.css, so the caret is not visible there; I only checked 1400 px.
• No staging or prod, and no upstream write.

[adminopenclaw8-sketch]

Review — CS-Minimax PR#248 group-caret — head 9959d7a

Dom: APPROVE med nits

Independent read-only review of the head above, rebased in scratch onto origin/master 9989b97f (merge-tree 7b11c35e, clean). Evidence legend: [T] = test or run by me, [A] = analysis of code/logs, [K] = known from the issue, earlier PRs or the author's report.

Findings

#	Severity	Finding	Evidence
F1	nit / follow-up (pre-existing, out of scope)	The nodes.js affinity-debug heading (line ~920) is the one real fold toggle that contradicts the chosen convention: caret-down while collapsed and caret-up while open. Its inline onclick is also broken: the sprite markup puts class="ph-icon" inside a double-quoted attribute. I parsed the line in Chromium: the attribute is cut at 234 chars and new Function(onclick) throws "Invalid or unexpected token", so the toggle cannot open. git log -L puts the break at 30627454 (Kpa-clawbot#1648 M2), the same migration that caused #189. The author lists it correctly as a remainder. I found no open issue for it, so I suggest filing one that covers both the parse bug and the caret direction.	[T] parse check; [A] git log -L920,920:public/nodes.js
F2	nit (a11y)	On mobile, mobile-page-actions.js (Kpa-clawbot#1461 #7) reroutes a group row's click to select-hash, and the expand column is hidden. The row still announces aria-expanded="false", so a screen-reader user hears "collapsed" on a row that selects when activated. That is minor and arguably better than no state on desktop. A follow-up could drop the attribute (or the toggle-select action) under the mobile breakpoint. Not a blocker.	[A] public/mobile-page-actions.js:94-105; not tested at mobile width
F3	nit (test)	The new E2E covers click → expand → collapse only. Caret and aria-expanded after a live WS update, a sort and a filter re-render are not pinned. I checked them with my own Playwright scenario (below, 11/11). The risk is low, because every re-render derives both from expandedHashes inside buildGroupRowHtml. Optional.	[T] own scenario; [A]
F4	nit (cleanup)	knownBug() in test-packets.js is now unused; only the knownBugs counter in the summary line remains. It is fine to keep as a ready-made hook; removing it would also be fine.	[A] test-packets.js:22-31

No blocking findings.

1. Convention

Collapsed = caret-right, expanded = caret-down is coherent, and the rationale holds [A]. From my own grep -rn "ph-caret" public/ plus the text-glyph and CSS disclosures:

• Same convention (right/down):
  • analytics.js #ptOverviewChevron: right, rotated 90° when open;
  • route-view.css .mc-rt-paths:not([open]): down, rotated −90°;
  • channels.js section caret ▸/▾;
  • network-digest.js ▸/▾;
  • two more the PR table does not list: home.css .checklist-q::after ▸, rotated 90° when .open, and style.css mobile .map-controls … legend.mc-label::after ▾ / .mc-collapsed ▸.
• Not fold states, so correctly left:
  • table-sort.js and nodes.js sort arrows (up/down = sort direction);
  • customize*.js move up/down;
  • map.js and route-view.js left/right side-panel collapse;
  • filter-ux.js saved-filters dropdown trigger (static caret-down = menu);
  • nodes.js "Show all N neighbors ▾" / "Show fewer ▴", which are labelled action buttons. The test: fix or retire the orphan tests left after #187 (#189) #229 note about nodes.js refers to these, and the author's reasoning for leaving them is sound.
• Directly inconsistent: only the nodes.js affinity-debug toggle (F1), which is noted correctly as a remainder.

2. Behaviour

• Rendered state: the caret and aria-expanded are both derived from isExpanded = expandedHashes.has(p.hash) in buildGroupRowHtml, which every re-render path goes through (toggle, WS update, sort, filter, virtual scroll) [A].
• Own scenario, grouped view, 1400 px, seeded 3-observation group, with live updates injected through wsListeners [T]. All of these hold:
  • collapsed on load: ph-caret-right, "false";
  • a live update while collapsed: still right/"false", badge 3 → 4;
  • click expands: ph-caret-down, "true";
  • a live update while expanded: still down/"true", child rows 3 → 4;
  • sorting by Time, Type, Time: still down/"true";
  • a filter expression that matches the row: still down/"true";
  • Enter on the focused row toggles, and aria-expanded follows;
  • collapsed again plus a live update: right/"false";
  • in every state: exactly one caret, .expanded matches aria-expanded, and computed ::before is none.
  • Result: 11/11 on head. The same script on master fails on the caret (ph-caret-up) and on aria-expanded (null).
• Caveat: with ?hash= pinned, the hash filter overrides a non-matching filter expression. I could not make the row disappear and come back that way, so the filter case covers only a re-render with a matching filter [T].
• aria-expanded on role="row": ARIA 1.2 lists aria-expanded among the row role's supported states, so it is valid markup. How screen readers treat it in a plain (non-treegrid) table is untested, as the author also says [A].
• Screenshots (local, Chromium, 1400 px; not attached because the CLI cannot upload images) [T]:
  • master, collapsed: two glyphs, the CSS ▸ in link colour plus the ^ sprite;
  • head, collapsed: a single right caret;
  • head, expanded: a single down caret, with the three child rows plus the injected one.

3. Test

• The knownBug('#189') case is now a plain test() with three assertions (right; not up; not down).
• The new test-packets.js against master's public/ gives 129 passed, 3 failed: the collapsed caret; expanded (_setExpanded missing); aria-expanded. Against head it gives 132 passed, 0 failed, 0 known bugs [T].
• The required mutant (collapsed → caret-up) is killed by both the unit test (1 failed) and the E2E (2 of 3 failed) [T].
• test-issue-189-group-caret-e2e.js against a server serving master's public/: 0 passed, 3 failed. Against head: 3/3 [T].

4. No other behaviour change

• The diff is limited to:

  • the caret glyph name;
  • aria-expanded on multi-observation rows only;
  • the removed .group-header td:first-child::before rules;
  • the _setExpanded test hook;
  • one deploy.yml line registering the E2E.

  The toggle handler, pktToggleGroup and expandedHashes are untouched [A].

• The removed CSS rule was the second, duplicate glyph (visible in the master screenshot). Its only visible side effect is that the caret is now drawn in the sprite's currentColor instead of var(--link-color). No colours are added or hardcoded [A][T].

• .group-header has no other users (grep), so dropping the rule cannot affect other tables [A].

• Gates: bash scripts/check-xss-sinks.sh --diff origin/master exits 0, and git diff --check is clean. The changed template interpolates only a boolean and a fixed glyph name [T][A].

5. Rules

• Fork guards: deploy.yml 9 and release-fast-path.yml 1 on head. The deploy.yml change is one added E2E line after the #1128 layout step [T].
• Closing keywords: none in the PR body or the three commit messages ("Relates to test: 13 red orphan unit tests + 4 orphan E2E files left after #187; collapsed packet groups show an up-caret #189"), and closingIssuesReferences is empty [T].
• Commit identity: all three commits (356d1e10, b720b21b, 9959d7a8) have author and committer dborup <kontakt@meshview.dk>, with a Co-Authored-By trailer. The order is test-first [T].

CI

Run 37291274005 on this head, attempt 1, conclusion success [T]:

• ✅ Go Build & Test: success. The log shows test-all.sh 218 passed, 0 failed, and the check-xss-sinks.sh --diff origin/master preflight ran.
• 🎭 Playwright E2E Tests: success. The log shows the #189 packet group caret E2E at 3 passed, 0 failed, and no ✗/❌ lines in the whole job.
• 🏗️ Build & Publish Docker Image: success.
• 📦 Release Artifacts / 🚀 Deploy Staging / 📝 Publish Badges: skipped (fork-guarded).

Neither known flake (#244 Details clamp, #250 TestStatsFileHasNoCredentials_118) fired, and no job needed a rerun.

Tests run (merged tree; Go server built from it; fixture freshened, CI seed SQL, corescope-migrate, seeds 2073 and 199) [T]

Suite	Result
sh test-all.sh	218 files, 0 failed
node test-frontend-helpers.js	707 passed
node test-packets.js	132 passed, 0 failed, 0 known bugs
test-issue-189-group-caret-e2e.js	3/3 (master frontend: 0/3)
Packets E2Es from deploy.yml: #96, #147, #180, #1122 filter UX + Details clamp, #1128 layout + multi-viewport, #1147, #1486, #1522, #1657, #1692, #1758, #1180 MQL, filter UX, packet-trace alignment, #1056 slide-over	all green
Own scenario (live update / sort / filter / keyboard)	11/11
test-e2e-playwright.js	fails at "Version info lives on Perf dashboard" (#navStats wait). It fails identically against master's frontend on the same local server, so it is environmental here and not this PR

The local servers were stopped by their pid files, and the ports were verified free.

Mutants (own) [T]

Mutant	test-packets.js	#189 E2E
A collapsed caret back to caret-up	killed (1)	killed (2/3)
B aria-expanded always "false"	killed (1)	killed (1/3)
C aria-expanded also on single-observation rows	killed (1)	survives
D aria-expanded from !!p._children instead of the expanded state (stale after collapse)	killed (1)	killed (1/3)
E CSS ::before ▼ restored for .expanded only	survives	killed (1/3)

All five are killed by at least one layer.

Not verified

• Mobile and narrow widths, where the expand column is hidden (F2).
• Firefox and Safari.
• Screen-reader output for aria-expanded on a table row.
• A filter that removes and restores the group (blocked by the hash pin, see 2).
• No staging or production checks, by design.