feat(packets): optional Hide CONTROL packets filter (#96)

[dborup]

Relates to #96, #242

Adds an opt-in Hide CONTROL packets checkbox to the packets page. It is unchecked by default, so nothing changes for users without a saved choice.

Plan

1. Pure helpers in public/packets.js:
   • readHideControlPref(urlValue, stored): the URL wins, then the saved choice, default off;
   • filterHiddenControl(list, hide): drops payload type 11 only.
2. Filter once, in renderTableRows' existing filter chain, right after the type filter. That one place covers the historical list and live updates: the WebSocket handler already merges live packets into the loaded list and re-renders through renderTableRows.
3. Keep the pinned-hash bypass (hashOnly), so a direct link to a CONTROL packet still opens it.
4. Save the choice as meshcore-hide-control = '1' or '0'. Put hideControl=1 in the URL hash via buildPacketsQuery()/updatePacketsUrl().
5. Place a checkbox in the filter bar's toggles group, styled with CSS variables only.
6. Tests first: a unit test (in test-all.sh) and a Playwright E2E (in the deploy.yml E2E step), both red on master. Then the fix.

Behaviour

• Default: unchecked; CONTROL packets are listed as before.
• Checked: CONTROL packets disappear from the list at once and from later live updates. There is no reload and no new /api/packets request.
• Unchecked again: they come back, still subject to the other filters. CONTROL packets are never dropped from the loaded data, live ones included, so they can come back.
• Saved choice: both checked and explicitly unchecked survive a reload.
• Storage failures: if localStorage throws (quota, blocked), the checkbox still filters and updates the URL for this view; only the save is lost. The saved choice is read and written through readStoredHideControl()/saveHideControlPref(), both in try/catch.
• URL:
  • hideControl=1 is in the URL while the box is checked.
  • Unchecked (the default) is omitted, like the other filters' defaults, unless the saved choice is on. Then hideControl=0 stays in the URL, so the page's own URL rewrites (load, time window, detail open/close) do not drop it, and a reload of that URL shows what was on screen.
  • A hideControl=1 or hideControl=0 in the URL wins over the saved choice for that view. Only the checkbox changes the saved choice.
• Direct links: #/packets/<hash> (and ?hash=) bypass the filter, like every other client-side filter, so a CONTROL packet stays reachable. The standalone #/packet/<id> page is not filtered at all.
• Other filters: type, observer, region, node, channel, My Nodes and the filter expression are unchanged. If a type filter selects CONTROL while the box is checked, the box wins and CONTROL stays hidden. The type selection itself is not modified.
• Empty result: the list says "No packets found (CONTROL packets are hidden)" only when hiding CONTROL is what emptied it, i.e. a hidden CONTROL packet would get through the observer and expression filters. If another filter empties the list anyway, it says "No packets found".
• Clear filters does not reset it. It is a display preference, like Group by Hash and Hex Paths.
• Scope: display only. Ingest, retention, statistics and anomaly detection are untouched. cmd/server and cmd/ingestor are unchanged.

Known limitation: the fetch cap

The filter runs on the packets already fetched. The page fetches at most PACKET_LIMIT packets for the time window: 1,000 on mobile, 50,000 on desktop.

• Effect: if the newest 1,000 (or 50,000) packets in the window are all CONTROL, hiding them leaves an empty list. There is no load-more path to older matching packets.
• Not new: the existing type filter has exactly the same limit, since it also filters client-side after the capped fetch.
• Why not fixed here: /api/packets can only include one type, not exclude types. A real fix needs a server-side exclusion and a new request when the box changes, which is a separate change covering both filters. The known limit is tracked in feat(packets): server-side exclusion of packet types in /api/packets for the type filter and Hide CONTROL #242.

Placement

The checkbox sits in the filter bar's toggles group, next to My Nodes.

• Desktop: it is in the first filter row.
• Mobile: it is inside the collapsible filter panel (navbar Filters button).
• Size: the box is 16 px. On coarse pointers the label is the 44 px touch target, matching the filter bar's other controls.
• Colours: var(--text) and accent-color: var(--accent) only.

Performance (rule 0)

• Hot path: this touches renderTableRows, the packets table render.
• Off (default): filterHiddenControl returns the same array, so there is no copy and no extra work.
• On: one O(n) pass with a constant check per row, the same shape as the existing type filter.
• Empty-list note (round 2): controlHidingEmptiedList runs only when the list is already empty. It is one O(n) pass to pick out the CONTROL packets, then the observer and expression filters on those only. Nothing changes on the non-empty path.
• No change otherwise: no extra API call, no change to the WebSocket handler, and the existing virtual-scroll render path.
• Measured (filterHiddenControl alone, Node, linux/amd64 container, 15 runs, median [min–max]):

Rows	Off	On
30,000	0.000 ms [0.000–0.086]	0.82 ms [0.64–3.56]
50,000	0.000 ms [0.000–0.004]	1.28 ms [1.16–4.44]

Tests

• test-issue-96-hide-control.js (unit, test-all.sh), 28 tests:
  • default off, saved '1'/'0' and unknown values;
  • the URL winning both ways;
  • CONTROL-only filtering, with order kept and no copy when off;
  • hideControl=1 in buildPacketsQuery();
  • round 2: a throwing getItem/setItem, the checkbox change still filtering and updating the URL when the save fails, hideControl=0 kept while the saved choice is on, and the empty-list note's condition.
• test-issue-96-hide-control-e2e.js (Playwright, deploy.yml), 10 steps against the e2e fixture's 4 CONTROL packets:
  • default unchecked with all packets listed;
  • checking hides exactly the CONTROL packets, with no new /api/packets request, and puts hideControl=1 in the URL;
  • the checked choice survives a reload;
  • a direct link opens a CONTROL packet while the filter is on;
  • unchecking shows them again, and the unchecked choice survives a reload;
  • the URL wins over the saved choice;
  • round 2: hideControl=0 over a saved 1 through the load-time URL rewrite and a reload;
  • round 2: the empty-list note with an expression matching only CONTROL vs. one matching nothing;
  • round 2: a throwing setItem on the checkbox;
  • at 390 px the checkbox is reachable through the Filters toggle, fits the screen and keeps its size.
• Sandboxes: the #121 and #147 unit-test sandboxes extract buildPacketsQuery. They declare the hideControl and savedHideControl closure state it reads, and the code says so.

Mutants

Mutant	Caught by
filterHiddenControl does nothing	unit + E2E
renderTableRows skips the filter	E2E
Direct links not exempt (hashOnly bypass removed)	E2E (direct-link step)
Default on (stored !== '0')	unit + E2E
hideControl=1 missing from the URL	unit + E2E
Round 2: the save not in try/catch	unit + E2E
Round 2: the change handler stops when the save fails	unit + E2E
Round 2: the read not in try/catch	unit
Round 2: hideControl=0 not kept while saved is on	unit + E2E
Round 2: savedHideControl follows a failed save	unit
Round 2: the empty-list note back on controlHidden	E2E
Round 2: the note ignores the later filters	unit + E2E
Round 2: the note skips the expression filter	E2E

🤖 Generated with Claude Code

[dborup]

Rapport — CS-cloud PR#211 #96 — head 23b89f5

Status: Draft PR with all acceptance criteria met: the tests were red on master and are green on the branch, and CI is green on the head. Ready for review; it was not set to ready, merged or closed.

Evidence tags: [T] test or CI run, [A] analysis, [K] known, not run again.

Branch

• codex/issue-96-hide-control-packets from origin/master 6bdaa8f7, head 23b89f5bf4bb5686cc317391cce0a6acb0f76ebd.
• Commits, all as dborup <kontakt@meshview.dk> (checked with git log -1 --format='%an <%ae>' after each one; no rebase, amend or force-push):
  1. dbc6cd58 test: unit test, E2E and their registration (red on master).
  2. a9174c3b feat: the filter, URL param, checkbox and CSS, plus the #121/#147 test sandboxes.
  3. 24380db3 merge of origin/master 8bafcdf2, which arrived during the work; clean.
  4. 23b89f5b test: the first E2E assertion tolerates packets that earlier CI E2E steps add (see CI).
• Changed: public/packets.js (+38), public/style.css (+6), the two new tests, test-all.sh (+1) and deploy.yml (+1 E2E line), and one line in each of the #121 and #147 unit tests.
• No change in cmd/server or cmd/ingestor. No new map[string]interface{}. deploy.yml still has 9 × github.repository == 'Kpa-clawbot/CoreScope'. [T]

Acceptance criteria

Criterion	Result	Evidence
No saved choice: checkbox unchecked, CONTROL packets visible	Met	E2E step 1, unit readHideControlPref(null, null) === false [T]
Toggling updates the list without a reload, and live updates follow it	Met. The filter runs in renderTableRows, which also renders live packets; checking makes no new /api/packets request.	E2E step 2 [T]; live path [A]
Unchecked shows CONTROL again, with the other filters still applied	Met. CONTROL packets are never dropped from the loaded data, live ones included, so unchecking brings them back. Type/observer/expression filters run unchanged around it.	E2E step 5 [T]; [A]
The choice survives a reload, including an explicit unchecked choice	Met. Saved as meshcore-hide-control = '1' or '0'.	E2E steps 3 and 5, unit [T]
Existing filters behave as before	Met. Type, observer, region, node, channel, My Nodes and the expression filter are untouched. If the type filter selects CONTROL while the box is checked, the box wins, and the type selection is not modified.	test-all.sh 215/215, test-frontend-helpers.js 707/0, #147 E2E 12/12 [T]
Historical loading and pagination stay usable; no stranding on an empty page	Met within the page's model. There is no paging: the page loads up to its row limit and virtual-scrolls it. Hiding is a client-side pass over that load, like the existing type filter. If hiding leaves nothing, the list says "No packets found (CONTROL packets are hidden)".	[A]
Direct links still open a CONTROL packet	Met. #/packets/<hash> and ?hash= bypass the filter like every client filter; #/packet/<id> is not filtered.	E2E step 4 [T]
No collected data discarded; display only	Met. Ingest, retention, statistics and anomaly detection are unchanged.	diff [T]
Deep link: state in the URL hash, URL wins	Met. hideControl=1 comes from buildPacketsQuery() and is written by updatePacketsUrl(). A hideControl=1/0 in the URL wins over the saved choice for that view; only the checkbox changes the saved choice.	unit, E2E steps 2, 3 and 6 [T]
Performance: no extra API call, no full DOM rebuild, inside the existing filter chain	Met. Off returns the same array (no copy). On is one O(n) pass: 0.82 ms at 30,000 rows and 1.28 ms at 50,000 rows (median of 15, Node, linux/amd64 container). It uses the existing virtual-scroll render.	micro-benchmark [T]
No hardcoded colours; works at mobile and desktop widths	Met. var(--text) and accent-color: var(--accent) only. The box is 16 px and the label is the 44 px touch target on coarse pointers, like the rest of the filter bar. At 390 px it sits in the collapsible filter panel.	E2E step 7 (fits 390 px, box ≤ 24 px, label ≥ 44 px); E2E step 1 (box ≤ 24 px on desktop); screenshots checked [T]
XSS gate	Clean: scripts/check-xss-sinks.sh --diff on the branch diff, rc 0. The new markup interpolates no node data.	[T]

Tests

Test	Master	Branch
test-issue-96-hide-control.js (unit, in test-all.sh)	1 passed, 13 failed	14 passed, 0 failed
test-issue-96-hide-control-e2e.js (Playwright, local Go server on a migrated copy of e2e-fixture.db, stopped by port)	0 passed, 7 failed	7 passed, 0 failed, three runs in a row
sh test-all.sh	214/214 files	215/215 files, after merging the new master
node test-frontend-helpers.js	–	707 passed, 0 failed

[T]

The two existing unit tests that extract buildPacketsQuery into a sandbox (#121, #147) first failed with hideControl is not defined. They now declare that closure state, and both pass. [T]

test-packets.js (not registered in test-all.sh or deploy.yml) has 1 failure, buildGroupRowHtml … right caret, identical on master. [K]

Mutants

Each mutant was applied to a copy of the branch, then the unit test and the E2E were run against a server serving that copy:

Mutant	Unit	E2E
filterHiddenControl does nothing	red	red (3 steps)
renderTableRows skips the filter	green	red (3 steps)
Direct links not exempt (hashOnly bypass removed)	green	red (direct-link step)
Default on (stored !== '0')	red	red (7 steps)
hideControl=1 missing from the URL	red	red (2 steps)

All five are red with the mutant and green on the code. [T]

A sixth mutant, dropping a width: auto CSS rule, survived because that rule had no effect, so the rule was removed. The checkbox's real sizing fix is pinned by the box-size assertions in E2E steps 1 and 7. [T]

CI (head 23b89f5b, run 37192193058, pull_request)

Job	Result
Go Build & Test	success
Playwright E2E Tests	success; the log shows the #96 E2E "7 passed, 0 failed"
Build & Publish Docker Image	success
Release Artifacts / Deploy Staging / Publish Badges & Summary	skipped (PR)

[T]

The first CI run, on 24380db3, failed in this PR's E2E step 1 only. The list showed 509 packets against the 500 in the test's API snapshot, because earlier E2E suites in the same job add packets to the server. Commit 23b89f5b makes that assertion "at least the snapshot". The steps that hide exactly the 4 CONTROL packets compare against the list's own count, and they passed in both runs. [T]

Remaining

• [A] Row limit: with the mobile row limit of 1,000, if most of the newest 1,000 packets are CONTROL, hiding them leaves fewer rows. Older non-CONTROL packets outside that load are not fetched to fill the gap. The existing type filter behaves the same way, and avoiding it would need a server-side filter, which is out of scope (no extra API calls). The empty-list message says why the list is empty.
• [A] Clear Filters does not reset the checkbox: it is a display preference, like Group by Hash and Hex Paths.
• [A] Live updates: no test injects a live WebSocket packet. That they follow the checkbox rests on code analysis: they merge into the loaded list and render through the same renderTableRows path.
• Not done here: staging verification, and the customizer (no new configurable value).

---

Generated by Claude Code

[dborup]

Review — CS-MacBook PR#211 hide-control — head 23b89f5

Dom: APPROVE med nits

The filter does what the issue asks: opt-in, type 11 only, one pass in the existing chain, live packets follow the checkbox with no reload, and the URL/saved/default order is right. The nits are an unguarded localStorage.setItem in the change handler, an explicit hideControl=0 that does not survive the next URL rewrite, and an empty-list message that can blame the wrong filter.

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

This review was read-only. git ls-remote showed head 23b89f5b before and after. I reviewed git archive copies in scratch: the head, and the merged tree against origin/master 0f88865b (git merge-tree --write-tree, clean, tree bc7b56ca). The merged tree differs from the head only in cmd/server files that come from master; public/ is identical [T]. Nothing was pushed or changed.

Findings

#	Severity	Finding
1	Nit	localStorage.setItem(HIDE_CONTROL_KEY, …) in the change handler is not in a try/catch (public/packets.js, the fHideControl listener). If it throws (quota, storage blocked after load), the handler stops before updatePacketsUrl() and renderTableRows(): the box ticks, nothing is filtered, the URL is not updated. Probe with setItem throwing for that key: 60 rows before and after, no hideControl in the URL [T]. The unguarded getItem at module load (line 720, and the pre-existing line 718) would already break the page where storage is blocked from the start, so the exposure is the post-load failure only [A]. Fix: wrap the write in try/catch and keep the filter and URL update outside it.
2	Nit	An explicit hideControl=0 is dropped at the next URL rewrite. With a saved 1, …&hideControl=0 shows everything, but after changing the time window the URL is {timeWindow:1440}, and a reload hides CONTROL again because the saved 1 wins [T]. It follows the "defaults are omitted" convention, but the URL no longer reproduces what the user is looking at. Acceptable; worth a line in the PR text, or keep the param while it differs from the saved choice.
3	Nit	"No packets found (CONTROL packets are hidden)" is chosen from controlHidden, computed right after the CONTROL pass and before the observer and expression filters (renderTableRows). If some CONTROL packets were hidden and a later filter empties the list anyway, the message blames CONTROL [A]. Not probed.
4	Info	#180 E2E "Clear Filters … reload shows N packets, Clear showed N+1" failed once on the PR and once on master's frontend, and passed on the other runs of each (time-window drift on the freshened fixture). Not caused by this PR [T].
5	Info	The hideControl state is a closure variable read by buildPacketsQuery, so the #121 and #147 unit sandboxes had to declare it. Done in the PR; any further sandbox that extracts buildPacketsQuery will need the same [A].

No blocking defects found.

1. Behaviour

• Opt-in, default off [T]: no saved choice gives an unchecked box and all packets (E2E step 1; my URL probe with no param and no saved value: count 517 = all, including 8 CONTROL groups).
• Only type 11 [T, A]: PAYLOAD_TYPE_CONTROL = 11. Firmware: PAYLOAD_TYPE_CONTROL 0x0B in src/Packet.h (shallow clone of the firmware repo; the payload constants are in Packet.h, not Mesh.h; I did not read docs/payloads.md beyond confirming it has a "Control data" section). Mutant MU2 (type 10) is killed by unit and E2E.
• Grouped rows [T, A]: grouped rows carry payload_type (the server's grouped and flat builders both emit it; the live new-group object copies it). The E2E ground truth is the grouped API and the list hides exactly its CONTROL groups. The header count, Showing X of Y and the empty-state all read the filtered list, which is also what drives virtual scroll and sorting. With the box on, the header showed 509 of 517 after hiding 8 CONTROL groups [T]. A hash group has one payload type, so a group is hidden or kept whole.

2. Live updates [T]

I inserted real rows into the fixture DB (CONTROL and GRP_TXT, as an ingestor would) so the server's poller broadcast them over the WebSocket; the page was on ?timeWindow=525600. Grouped and flat mode, each:

• off: both arrive;
• check: the CONTROL row that had arrived disappears at once, the chat row stays;
• on: a later chat row arrives, a later CONTROL row does not appear;
• uncheck: both CONTROL rows return with no reload;
• no /api/packets request during any toggle (apiReqs unchanged).

14 of 14 [T]. [A] why: the WebSocket handler only filters by window, type, observer, region and node, and always merges into packets, then scheduleWSRender() → renderTableRows(); CONTROL packets are never dropped from the loaded data. MU7 (dropping CONTROL from packets when checked, so they could not return) is killed by the E2E. The toggle re-renders through renderTableRows like every other filter: no new rebuild path.

3. Deep link and persistence [T]

• Order URL > saved > default: hideControl=1 beats saved 0, hideControl=0 beats saved 1; unknown values (yes, 2, empty, true, ON, 1 , %23fHideControl) give the default with no saved choice and fall through to the saved choice when there is one.
• No selector read from the URL: the value is compared with '1'/'0' and nothing else; no DOM lookup uses it [A].
• Clearing: unchecking removes hideControl from the URL and saves '0'; checking writes hideControl=1 next to timeWindow.
• With fix(packets): keep ?obs=/?viewPath= on packet detail URLs (#147) #167/fix(packets): URL and modal leftovers from #167 (#180) #204: opening a detail gives #/packets/<hash>?timeWindow=525600&hideControl=1, closing it keeps hideControl=1 in the list URL; #147 E2E 12/12 and #180 E2E (see finding 4) pass. A pinned hash bypasses the filter [A, E2E step 4].
• Clear Filters leaves the box and the param alone (stated in the PR) [T].
• localStorage: getItem at load is not guarded (same as the line above it); setItem is not guarded (finding 1).

4. Performance [T]

In-page on the fixture's packets page, 30,000 and 50,000 synthetic packets (1 in 97 CONTROL), renderTableRows through the checkbox, median of 11: 8.3 / 8.4 / 8.4 ms (off / on / off again) at 30k and 8.5 / 8.4 / 9.0 ms at 50k, so the checkbox adds nothing measurable to the render. filterHiddenControl alone: 0.3 ms at 30k and 0.4 ms at 50k, one filter pass; off returns the same array (checked with ===). No extra pass, no O(n²), no API call [A, T]. The only extra work in the chain is the length comparison for the empty-state message.

5. UI [T, A]

• Colours: var(--text) and accent-color: var(--accent) only; no hex in the added CSS.
• Accessibility: the input is inside its <label>, so the accessible name is "Hide CONTROL packets". The label is 165 × 44 px with pointer: coarse (measured at 390 px), the box 16 × 16.
• Mobile: at 390 px the checkbox is in the opened Filters panel, no horizontal scroll (scrollWidth 390). Screenshots at 1280 and 390 px looked right (the box sits beside My Nodes; it wraps with the other toggles on the phone).
• scripts/check-xss-sinks.sh --diff on the head: exit 0. The new markup interpolates no node data [A].

6. Tests [T]

• Merged tree: sh test-all.sh 215/215 files; node test-frontend-helpers.js 707 passed, 0 failed; node test-issue-96-hide-control.js 14/0.
• E2E against a local Go server built from the merged tree on a copy of e2e-fixture.db, prepared as deploy.yml does it (freshen, the grouped-row seed, corescope-migrate, the two seed files); stopped by port. test-issue-96-hide-control-e2e.js 7/0, test-issue-147-packets-url-obs-e2e.js 12/0, test-issue-180-packets-url-modal-e2e.js 10/0 on the second run (finding 4).
• My own probes (not in the PR): live updates 14/14; URL and persistence probe 15 of 18, the 3 non-passes being findings 1 and 2.
• Mutants, each applied to a copy of public/ served by its own server, unit then E2E:

Mutant	Unit	E2E
MU1 filter always on (ignores the checkbox)	1 failed	4 failed
MU2 wrong type (10)	2 failed	4 failed
MU3 URL parameter ignored on load	green	2 failed
MU4 hideControl=0 no longer beats the saved choice	1 failed	1 failed
MU5 any non-empty URL value counts as on	2 failed	1 failed
MU6 checkbox change does not re-render	green	1 failed
MU7 CONTROL dropped from the loaded data when checked	green	2 failed

All seven killed. [T]

• Browser validation: done, on headless Chromium at 1280 and 390 px with screenshots; live behaviour through real DB rows and the server's own WebSocket.

7. Rules

• Fork guards: grep -c 'github.repository ==' gives 9 in deploy.yml and 1 in release-fast-path.yml on the head [T].
• No closing keyword in the title, the body or the four commit messages (grep for fix/close/resolve followed by an issue reference: none; the body starts with "Relates to Feature request: Add an optional “Hide CONTROL packets” checkbox to the packet monitor #96") [T].
• All four commits are by dborup <kontakt@meshview.dk> [T].
• Rule 8: the PR says no new configurable value and leaves the customizer alone; the saved key meshcore-hide-control is a per-browser display preference like the existing type and observer filter keys, which the customizer does not manage either [A]. A customizer entry could be a later follow-up, not a requirement.

Not verified

• Browser Back/Forward across a hideControl change (whether the checkbox follows the URL). Not probed.
• Behaviour with a real ingestor and production-size data; the fixture has 500 groups and the 30k/50k figures are synthetic rows in the page.
• Firefox and Safari; only Chromium was used. In particular the storage-failure behaviour in finding 1 was simulated, not seen.
• The expression-filter plus CONTROL empty message (finding 3) from reading the code only.
• The PR's own CI and its benchmark numbers were not re-run; I did not look at CI beyond the author's report [K].

[dborup]

Review: changes recommended before merge

Reviewed head 23b89f5bf4bb5686cc317391cce0a6acb0f76ebd against master 8bafcdf2a6bb0014127bce4822c8ad0399bd2f12. The head was rechecked before posting and is unchanged.

Two P2 findings are attached inline, both reproduced in a local Chromium browser:

1. Filtering only after the capped fetch can leave an empty list with no way to load older matching packets. This misses the historical-loading acceptance criterion in #96.
2. URL serialization drops an explicit hideControl=0, so a saved enabled preference takes over after reloading the same page.

The basic behavior otherwise worked in the checks performed: default visibility, checkbox persistence, direct CONTROL links, and incoming live packets being hidden and restored without being discarded. No backend changes are included in this PR.

Verification

• Local test-all.sh: 215/215 files passed.
• The PR's test-issue-96-hide-control-e2e.js: 7/7 passed against a disposable local fixture.
• Independent browser reproduction of the URL override problem used the real fixture: 499 rows before reload, 495 after reload, with the saved preference unchanged at 1.
• Independent historical-loading reproduction used controlled API responses: 1,001 transmissions, newest 1,000 CONTROL, then one ADVERT, all within the selected time window. The mobile page requested limit=1000, displayed zero rows and issued no continuation request.
• Controlled live WebSocket messages: the visible count increased from 1 to 2 after one CONTROL and one ADVERT arrived while hiding was enabled; unchecking restored the CONTROL and the count became 3.
• Current CI Go, Playwright and Docker checks are green. Full Go suites were not rerun locally for this frontend-only change.

The existing green tests do not cover the two failing end-to-end scenarios. No staging or production validation was performed, and no implementation changes were made.

Posted as a comment review because the signed-in account is the PR author; the recommendation remains to address both findings before merging.

[dborup]

[P2] Do not let hidden CONTROL packets exhaust the fetched history window

This removes CONTROL only after loadPackets() has fetched the capped list (PACKET_LIMIT is 1,000 on mobile and 50,000 on desktop). If the newest 1,000 transmissions are CONTROL and the next older transmission is an ADVERT within the selected time window, enabling this filter leaves the mobile list empty. There is no continuation request or load-more path to reach the matching ADVERT. I reproduced this with a browser and a controlled 1,001-transmission response: the only list request had limit=1000, the visible count was zero, and the remaining matching transmission was never requested. This violates #96's explicit requirement that hiding CONTROL must not strand historical browsing on an empty page when matching packets exist. Apply the exclusion before the retrieval limit or provide a bounded continuation mechanism while keeping direct links and unchecking behavior intact. Add a browser regression with more than one fetch window and the only non-CONTROL packet beyond the first window.

[dborup]

[P2] Preserve an explicit hideControl=0 URL override

Omitting the parameter whenever hideControl is false loses the distinction between the default and an explicit URL override. With meshcore-hide-control=1 saved, opening #/packets?timeWindow=525600&hideControl=0 initially shows CONTROL correctly, but renderLeft() calls updatePacketsUrl() and this serializer removes hideControl=0. Reloading that same page then restores the saved enabled preference and hides CONTROL. In Chromium on the local fixture the count changed from 499 to 495 after reload, while localStorage remained 1. Preserve the explicit off override in the canonical/shareable URL without changing the saved preference. Add a browser regression covering saved 1 plus URL 0, the initialization URL rewrite and a reload; the pure preference-reader test alone does not exercise this failure.

[adminopenclaw8-sketch]

Rapport — CS-Minimax PR#211 runde 2 — head 39d1651

Status: Round 2 pushed to the same branch, still draft. Nits 1–3 and info 5 are fixed with tests that were red first; 8 of 8 new mutants are killed; local unit and E2E tests are green. CI is red only on the Kpa-clawbot#1122 Details-clamp E2E, which fails the same way on master aff158c7; the PR's own E2E passed in CI, 10 of 10.

Review feedback addressed (commit 39d16513)

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

Branch and commits:

• The round 1 head 23b89f5b was the remote head before the push. The push was a fast-forward to 39d16513 [T].
• New commits, all by dborup <kontakt@meshview.dk> as author and committer [T]:
  • 4f6b37c0: merge of origin/master aff158c7, a merge commit with no conflicts;
  • 243064db: tests, red;
  • 39d16513: fix.
• No rebase, amend or force-push.
• The original author (the cloud agent) is no longer used, so I took over the branch for this round.

Nit 1 — a throwing localStorage.setItem stopped the checkbox

• Fix [A]:
  • The change handler is now setHideControl(this.checked).
  • setHideControl saves through saveHideControlPref(hide), which wraps setItem in try/catch and returns false on failure. It then always calls updatePacketsUrl() and renderTableRows(), outside the try.
  • The saved choice is read through readStoredHideControl() (try/catch, null on failure), both at module load and on page init.
  • The pre-existing observer/type filter reads at module load (the getItem pair before the Feature request: Add an optional “Hide CONTROL packets” checkbox to the packet monitor #96 block) are wrapped in one try/catch too. It was cheap.
  • The remaining module-level reads (time window, hex hashes, obs sort) and app.js's own reads are still unguarded. So storage that is blocked from the start still breaks the page, as it did before this PR. That needs a page-wide fix and is out of scope here.
• Tests [T]:
  • Unit: a throwing getItem reads as no saved choice; a throwing setItem is caught and reported; setHideControl(true) with a throwing setItem still sets the state and calls URL update and re-render, in that order.
  • E2E: a context whose Storage.prototype.setItem throws QuotaExceededError for the key. Checking the box still hides exactly the CONTROL packets, writes hideControl=1 to the URL, and lets no storage error escape.
  • Red before the fix: 3 unit tests (helpers missing) and the E2E step (hideControl=1 not in the URL).
• Mutants [T]:
  • N1a, the save without try/catch: unit 3 fail, E2E 1 fail.
  • N1b, the handler returns when the save fails: unit 1, E2E 1.
  • N1c, the read without try/catch: unit 1.

Nit 2 — an explicit hideControl=0 was dropped by the next URL rewrite

• Chosen fix: keep the param while it differs from the saved choice. It is the smallest change that makes the URL describe the screen again: one else if in buildPacketsQuery and one tracked variable.
• Why not only document it: the issue requires the unchecked choice to survive a reload. Both reviews reproduced a reload of the page's own URL hiding CONTROL again, because the load-time renderLeft() rewrite drops the param.
• Implementation [A]:
  • savedHideControl holds the saved choice as far as the page knows. It is set from storage on load and init, and updated only when a save succeeds.
  • buildPacketsQuery writes hideControl=1 while on. Otherwise it writes hideControl=0 while savedHideControl is on, and omits the param when both are off.
  • Effects:
    • The URL override survives the load-time rewrite, time window changes and detail open/close.
    • Unchecking the box saves '0', so the param is dropped as before.
    • If that save fails, the saved '1' still applies, so hideControl=0 correctly stays.
    • The default URL (nothing saved) is unchanged.
• Tests [T]:
  • Unit: off with saved on gives ?hideControl=0; off with saved off gives ''; on gives ?hideControl=1; unchecking with a saved 1 drops the param; unchecking with a failing save keeps hideControl=0.
  • E2E: saved 1, then …&hideControl=0. The box is unchecked, all packets are listed and hideControl=0 is still in the URL after the load-time rewrite. After page.reload() of that URL the box is still unchecked, all packets are listed, the param is kept and the saved choice is still 1.
  • Red before the fix: unit 1 (plus 2 that need setHideControl); E2E hideControl=0 dropped by the URL rewrite: {"timeWindow":"525600"}.
• Mutants [T]:
  • N2a, the else if removed: unit 2 fail, E2E 1 fail.
  • N2b, savedHideControl follows a failed save: unit 1.

Nit 3 — the empty-list note could blame the wrong filter

• Fix [A]:
  • renderTableRows keeps a reference to the list from before the CONTROL pass (no copy).
  • On the empty-list path only, it calls controlHidingEmptiedList(beforeHide, laterFilters). The helper picks out the CONTROL packets and runs only those through the filters below the CONTROL pass (applyObserverFilter, then the filter expression).
  • The note says "(CONTROL packets are hidden)" only if one of them gets through, i.e. without the CONTROL filter the list would not be empty.
  • Cost: nothing on the non-empty path. On the empty path it is one O(n) pass plus the later filters on the CONTROL packets.
• Tests [T]:
  • Unit: CONTROL that passes the later filters means CONTROL is blamed; CONTROL that fails them means it is not; only CONTROL packets reach the later filters; with no CONTROL packet the later filters are not run.
  • E2E: hideControl=1&filter=type == CONTROL gives "No packets found (CONTROL packets are hidden)"; hideControl=1&filter=snr > 999 gives "No packets found".
  • Red before the fix: unit 4 (helper missing); E2E expression matching nothing blamed CONTROL.
• Mutants [T]:
  • N3a, the note back on controlHidden: E2E 1 fail.
  • N3b, the helper ignores the later filters: unit 2, E2E 1.
  • N3c, the later filters skip the expression: E2E 1.

Info 5 — sandboxes that extract buildPacketsQuery

• The comment above the hideControl/savedHideControl declarations says that buildPacketsQuery reads both, so a test sandbox that extracts it must declare both, naming the #121, #147 and #96 unit tests [A].
• test-issue-121-clear-filters-selection.js and test-issue-147-packets-url-detail-params.js now declare savedHideControl too. Both failed without it, 10 and 31 tests [T].

Codex review (two P2 comments on round 1)

• P2 "explicit hideControl=0": the same as nit 2, fixed above.

• P2 "hidden CONTROL packets can exhaust the fetched history window": not fixed in this PR, by decision. It is documented in the PR description under "Known limitation: the fetch cap".

  • The filter runs on the fetched packets, and PACKET_LIMIT is 1,000 on mobile and 50,000 on desktop. If the newest packets in the window are all CONTROL, hiding them leaves an empty list with no load-more path.
  • The existing type filter has exactly the same limit [A].
  • /api/packets can only include one type, not exclude types (routes.go, q.Type) [A].
  • A real fix needs a server-side exclusion plus a request when the filter changes. That is a separate change covering both filters.

• Proposed follow-up issue (not filed):

  Title: feat(packets): server-side exclusion of packet types in /api/packets for the type filter and Hide CONTROL

  Body: The packets page fetches at most PACKET_LIMIT packets for the time window (1,000 on mobile, 50,000 on desktop) and then filters by type client-side: both the type filter and the Feature request: Add an optional “Hide CONTROL packets” checkbox to the packet monitor #96 Hide CONTROL checkbox. If the newest packets in the window are all of excluded types, the list is empty and there is no load-more path, although older matching packets exist.

  /api/packets only accepts a single type to include. Proposal:

  • Add an exclusion (for example excludeTypes=11) and a multi-type include to /api/packets, both grouped and flat, applied before limit.
  • Have the packets page pass the type filter and Hide CONTROL to the request, re-fetching when they change.
  • Keep direct-link (hash=) requests unfiltered, and keep live WebSocket packets merged so that unchecking still restores them.

  Acceptance: with 1,001 packets in the window, the newest 1,000 CONTROL and one ADVERT, hiding CONTROL on mobile shows the ADVERT; the same for a type filter selecting ADVERT. The server stays read-only (bug(db): vacuumOnStartup fails with SQLITE_BUSY when ingestor + server share DB (auto_vacuum migration #919 broken in single-container topology) Kpa-clawbot/CoreScope#1283).

Other observations

• Pre-existing, not from this PR: at narrower widths (about 800 px, where the packets table hides columns behind a "+N hidden" pill), every empty-list message is invisible.
  • Cause: TableResponsive's colIndexCells maps a hidden column to the full-width colspan cell and gives it col-hidden. I saw the cell's text with computed display: none [T].
  • At 1280 px the message shows [T].
  • This code is not touched by this PR [A]. It is suggested as a separate task.
• Screenshot: taken locally at 1280 px of hideControl=1&filter=type == CONTROL, showing "No packets found (CONTROL packets are hidden)" with the box checked. It is not attached, because I can't upload images to a comment from the CLI.
• test: 13 red orphan unit tests + 4 orphan E2E files left after #187; collapsed packet groups show an up-caret #189's caret fix in public/packets.js is not touched; it waits for this PR.

Tests run [T]

On the branch head 39d16513, which includes origin/master aff158c7:

Test	Result
sh test-all.sh	218 passed, 0 failed (218 files), exit 0
node test-frontend-helpers.js	707 passed, 0 failed
node test-issue-96-hide-control.js	28 passed (14 before this round); 12 of 28 red before the fix
node test-issue-121-clear-filters-selection.js	10 passed
node test-issue-147-packets-url-detail-params.js	all passed
node test-clear-filters.js	all passed

E2E setup:

• A local Go server built from the branch, on a scratch copy of e2e-fixture.db.
• The fixture was prepared as in deploy.yml: freshen-fixture.sh, the Packets page collapse button in the left column of the table opens the dialog. Kpa-clawbot/CoreScope#1486/"Group Data" message type missing from packet view window's "message type" filter Kpa-clawbot/CoreScope#1791 seed, corescope-migrate, then the [feat] node detail, separate flood-adv from zero-hop adv Kpa-clawbot/CoreScope#2073 and Active observers vanish as nodes after retention.nodeDays without an advert; observer link dead-ends on 'Node not found' #199 seeds.
• The server served the branch's public/. It was started with nohup and stopped by the pid of the port listener, not $!.

E2E	Result
test-issue-96-hide-control-e2e.js	10 passed; the 3 new steps were red before the fix, the 7 old ones green
test-issue-147-packets-url-obs-e2e.js	12 passed
test-issue-180-packets-url-modal-e2e.js	12 passed
test-filter-ux-e2e.js	12 passed

Mutants: 8 of 8 killed. Each was applied alone to public/packets.js and the file was restored afterwards, byte-identical to the fix (cmp).

Rules:

• scripts/check-xss-sinks.sh --diff origin/master: exit 0, nothing flagged.
• Fork guards: == 'Kpa-clawbot/CoreScope' occurs 9 times in deploy.yml and once in release-fast-path.yml, unchanged. This round changes no workflow.
• No closing keywords in the commits or the PR body, which still starts with Relates to #96.

Browser [T]: in the built-in browser against the same local server:

• Saved 1 plus …&hideControl=0, then a reload: unchecked, all packets listed, the URL keeps hideControl=0, and the saved choice stays 1.
• The two empty-list notes are as in the E2E.

CI

Run 37276455116 on 39d16513 [T]:

Job	Result
Go Build & Test	success
Playwright E2E Tests	failure, in test-issue-1122-details-row-clamp-e2e.js only
Release Artifacts	skipped
Build & Publish Docker Image, Deploy Staging, Publish Badges & Summary	skipped (after the failure)

Not caused by this PR:

• The failure: 2 of 18 steps failed, [desktop-1200] and [tablet-900] "advert links in Details stay visible and clickable": advert link in Details is not hit-testable … "R5-D4 300D Rak". These are the only two failed steps in the whole job log.
• Master fails the same way: master's own push run 37274272380 on aff158c7 fails with the same two steps in the same file [T].
• Locally the same 2 steps fail against this branch's public/, master aff158c7's public/ and 341a1961's public/. 341a1961's CI run was green earlier today, so the failure depends on data or time, not on this code [T].
• Mechanism, probed [T]:
  • At 1200 px the packets table's expand column gets an inline width: 43.45% (512 px), which leaves Details about 65 px.
  • The advert link wraps inside the one-line clip, so its bounding box is 70 px tall.
  • The test's hit point (the box's vertical centre) then lands on a lock icon in the next row.
  • Neither the expand column's width nor the test is touched by this PR [A]. This deserves its own issue.

This PR's E2E in CI [T]:

• test-issue-96-hide-control-e2e.js ran before the bug(packets): filter UX disaster — help panel overlaps table, toolbar chaotic, path chips spill rows Kpa-clawbot/CoreScope#1122 step and passed 10 of 10, including the three new round 2 steps.
• test-filter-ux-e2e.js, test-issue-147-packets-url-obs-e2e.js and test-issue-180-packets-url-modal-e2e.js also ran before it and passed.
• Because the step is fail-fast, about 40 later E2E files did not run in CI. These include the touch-target, live, logo and channels groups; none of them is a packets-filter test.

I did not rerun the job, because the same failure reproduces on master.

Remaining

• Codex P2, the fetch cap: documented, with the follow-up issue proposed above (not filed).
• Blocked storage from the start: still breaks the page through the other unguarded reads, as before this PR. This round only guards the Feature request: Add an optional “Hide CONTROL packets” checkbox to the packet monitor #96 reads and the filter pair next to them.
• CI: red on the bug(packets): filter UX disaster — help panel overlaps table, toolbar chaotic, path chips spill rows Kpa-clawbot/CoreScope#1122 Details-clamp E2E, which master aff158c7 shares. This PR's checks will only go green once master's is fixed, or with a rerun if it turns out to be time-dependent.
• Narrow-width empty-list message: invisible. This is pre-existing (TableResponsive) and suggested as a separate task.
• Not verified: Firefox and Safari, a real quota error instead of a simulated one, Back/Forward across a hideControl change, and the full test-e2e-playwright.js suite locally (CI runs it).

[dborup]

Review — CS-MacBook PR#211 runde 2 — head 39d1651

Dom: APPROVE med nits

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

Setup: git fetch --no-prune origin; the head was 39d16513 before and after (git ls-remote). Master is 3878d7ea (it contains #234 and, for public/, is identical to aff158c7: git diff aff158c7 3878d7ea -- public is empty). The merged tree is git merge-tree --write-tree 3878d7ea 39d16513; it merges cleanly and changes eight files against master: public/packets.js, public/style.css, test-all.sh, deploy.yml (one E2E line), the two unit-test sandboxes, and the two new tests. Everything ran from git archive copies in scratch; no checkout was modified. The PR is still a draft and mergeable. [K]

Round-1 findings and Codex P2

#	Round 1	Status	Evidence
Nit 1	localStorage.setItem unguarded in the change handler	Fixed. The handler is setHideControl(this.checked): the save is saveHideControlPref (try/catch, returns false), then updatePacketsUrl() and renderTableRows() always run. My own probe, with setItem throwing QuotaExceededError for the key: 509 rows → 505 on check, hideControl=1 in the URL, no page error, and unchecking restores 509 and drops the param. A getItem that throws for the key at load: the page loads, the box is unchecked, 509 rows. The two other getItem reads next to the #96 block are also in a try/catch now.	[T] probe 5/5
Nit 2	An explicit hideControl=0 did not survive the next URL rewrite	Fixed by keeping hideControl=0 while the saved choice is on (savedHideControl), not by documenting it. Probe with a saved 1: …&hideControl=0 gives an unchecked box, 509 rows against 505 hidden, and the param is still in the URL after the load-time rewrite, after a time-window change (?timeWindow=1440&hideControl=0), and after a reload of that rewritten URL. The saved value stays 1. Check then uncheck saves 0 and drops the param. With nothing saved the URL is unchanged (no hideControl).	[T] probe 6/6
Nit 3	The empty-list note could blame CONTROL for an emptiness another filter caused	Fixed. controlHidingEmptiedList runs only for an empty list, on the CONTROL packets only, through the observer filter and the expression filter, so the note names CONTROL only if one of them would get through. Probe: type == CONTROL + box on → "No packets found (CONTROL packets are hidden)"; snr > 9999 + box on → "No packets found"; type == CONTROL && snr > 9999 + box on → "No packets found"; the same with the box off → "No packets found"; an unknown observer + box on → "No packets found". applyObserverFilter is pure, so the extra call has no side effects.	[T] probe 5/5; [K]
Info 5	Comment about sandboxes that extract buildPacketsQuery	Fixed. The comment above hideControl / savedHideControl names the #121, #147 and #96 sandboxes; the #121 and #147 tests now declare savedHideControl.	[K][T]
Codex P2 (fetch cap)	Filtering after a capped fetch can empty the list	Documented as a known limit; the documentation is correct. See point 5.	[K]
Codex P2 (hideControl=0)	Same as nit 2	Fixed (nit 2).	[T]

New findings

#	Sev	Finding
N1	nit	The PR body still says "A follow-up issue is proposed in the round 2 report", but the issue now exists: #242. Adding "Relates to #242" to the body (or a link in the "Known limitation" section) would make the limit trackable from the PR. [K]
N2	info	The red CI check is the Kpa-clawbot#1122 Details-clamp E2E, and it is a timing issue that this PR can trigger without causing. See point 4. It is tracked as #244; until that is fixed or the job is rerun, this PR's Playwright job can stay red. [T][K]
N3	info, pre-existing	At about 800 px, the empty-list cell is display: none (col-hidden), so the empty-list note, either variant, is not visible there. Seen on the merged tree; the PR does not touch that code, but I did not repeat the check on master. [T]

No blocking finding.

1. Behaviour is unchanged otherwise

• Opt-in and type 11 only: with nothing saved the box is unchecked and all 509 rows show; filterHiddenControl removes only payload_type === 11 and returns the same array when off. [T][K]
• Direct links: the CONTROL pass is skipped when hashOnly, as before; E2E step 4 passes. [K][T]
• Live updates (my own probe; real rows inserted into a scratch copy of the fixture DB so that the server's poller broadcasts them over the WebSocket), in grouped and in flat mode: with the box off, a live CONTROL row arrives; checking hides it at once; with the box on a later chat row arrives and a later CONTROL row does not; unchecking brings both CONTROL rows back (512 rows in grouped mode, 508 in flat mode); no /api/packets request during the toggles. 10 of 10. [T]
• Diff scope: git diff --stat shows the eight files above only, with no cmd/server or cmd/ingestor change. [K]
• scripts/check-xss-sinks.sh --diff: exit 0 on the diff of public/ between master and the merged tree. As a sanity check, the same gate exits non-zero on a deliberately unsafe innerHTML line. The new markup interpolates no node data. [T]

2. Rules

• Fork guards: 9 in deploy.yml and 1 in release-fast-path.yml (github.repository == 'Kpa-clawbot/CoreScope'); the only workflow change is the one added E2E line. [K]
• No closing keyword in the PR body (it starts with "Relates to Feature request: Add an optional “Hide CONTROL packets” checkbox to the packet monitor #96") or in any of the seven commit messages. [K]
• All seven commits: author and committer dborup <kontakt@meshview.dk>. [K]
• No new map[string]interface{}: the diff is JS, CSS, shell and YAML plus two JS tests. [K]

3. Tests

On the merged tree against master 3878d7ea:

• sh test-all.sh: 218 passed, 0 failed (218 files). [T]
• node test-frontend-helpers.js: 707 passed, 0 failed. node test-issue-96-hide-control.js: 28 passed, 0 failed. [T]
• E2E against a local Go server built from the merged tree, serving the merged public/, on a copy of test-fixtures/e2e-fixture.db prepared as deploy.yml does it (freshen, the grouped-row and GRP_DATA seed, corescope-migrate, the two seed files). Servers were stopped by port, not by $!. The files ran in deploy.yml order: [T]

E2E	Result
test-issue-96-hide-control-e2e.js	10 passed, 0 failed
test-filter-ux-e2e.js	12 passed
test-issue-1758-ng-filter-rerenders-e2e.js	exit 0
test-issue-1692-packets-init-parallel-e2e.js	1 passed
test-issue-147-packets-url-obs-e2e.js	12 passed
test-issue-180-packets-url-modal-e2e.js	12 passed
test-issue-1122-packets-filter-ux-e2e.js	6 passed
test-issue-1122-details-row-clamp-e2e.js	18 passed at a fixture age of about 2 minutes (see point 4 for older fixtures)
test-issue-1128-packets-layout-e2e.js	5 passed
test-packet-trace-alignment-e2e.js	17 checks passed

• Mutants, each applied to a copy of the merged public/packets.js and run against its own server (unit test, then the PR's E2E):

Mutant	Unit	E2E
MU1: the save is not in a try/catch	3 failed	1 failed (the setItem step)
MU2: hideControl=0 is not kept while the saved choice is on	2 failed	1 failed (the hideControl=0 step)
MU3: the note blames CONTROL whenever any packet was hidden	green	1 failed (the expression-matching-nothing step)

All three are killed. [T]

4. The red CI check: the Kpa-clawbot#1122 Details-clamp E2E

• What CI shows. [K]
  • The PR's Playwright job (run 37276455116, head 39d16513) has exactly two failed steps, both in test-issue-1122-details-row-clamp-e2e.js: [desktop-1200] and [tablet-900] "advert links in Details stay visible and clickable" (not hit-testable … "R5-D4 300D Rak"). 16 of 18 steps pass.
  • The PR's own E2E passes in that job (10 passed, 0 failed).
• Master fails the same way, on its first attempt. [K]
  • Master aff158c7, run 37274272380, attempt 1: the Playwright job failed with the same two steps. The run's conclusion now shows success only because attempt 2 passed.
  • git diff aff158c7 3878d7ea -- public is empty, so the front end is the same on current master.
• Cause: the fixture age at which the step runs. [T]
  • In CI the clamp step ran 827 s after the "Freshen fixture timestamps" step in the PR run. In master attempt 1 it ran 825 s after, and failed. In master attempt 2 it ran 724 s after, and passed. [K]
  • I reproduced it locally on two servers, one serving master's public/ and one the merged public/, on identically prepared fixtures, running the clamp E2E every ~50 s. Both were 18/18 up to a fixture age of 794 s and 16/2 from 843 s on (also at 892 s and 941 s), with the same two steps. So the threshold sits between 794 s and 843 s, and master and the PR behave identically at every age. [T]
• Consequence. The PR does not change the Details column or the test. But it adds an E2E step before the clamp step, which moves the clamp step later in the job. Together with runner speed, that can put it past the threshold, as in the PR run. Every E2E added to the job makes this more likely. The root cause is tracked as test: Details-clamp E2E 'advert links in Details stay visible and clickable' is time-dependent and flaky #244. [A]
• So the claim "the failure is the same on master" is confirmed, with the refinement that master's CI passes only when the step happens to start earlier. A rerun may turn the PR green; a fix for test: Details-clamp E2E 'advert links in Details stay visible and clickable' is time-dependent and flaky #244 is the real remedy.

5. Codex P2 (fetch cap): is the documentation correct?

The "Known limitation: the fetch cap" section of the PR body says the following; each claim holds. [K]

• PACKET_LIMIT is 1,000 on mobile and 50,000 on desktop: packets.js:808 (isMobile ? 1000 : 50000), used as the request limit (:1588) and as the live-list cap (:1475–1482).
• The type filter is client-side after the capped fetch: buildPacketsParams sends no type, and renderTableRows filters on filters.type. So hiding CONTROL has the same limit as the type filter.
• /api/packets accepts one type to include (routes.go:1305) and has no exclusion parameter, so a real fix needs server-side work.
• Issue feat(packets): server-side exclusion of packet types in /api/packets for the type filter and Hide CONTROL #242 matches the PR text: a server-side exclusion in QueryPackets and the grouped path, the page sending it and refetching, the client filter kept for live packets, and a benchmark. Its acceptance criteria name the all-CONTROL window case for both Hide CONTROL and the type filter. The only gap is N1 (the PR body does not link it yet).
• The empty-list note says why the list is empty (nit 3), which is as much as the client can do within the cap.

Not verified

• Firefox and Safari. Only headless Chromium was used.
• A real quota error. The storage failure was simulated with an init script.
• Back and Forward across a hideControl change.
• The full test-e2e-playwright.js suite and the many later E2E files that CI would run after the clamp step: CI stops at the first failure, and I ran the packets-related files, not all of them.
• Production-size data. The fixture has about 500 packets. I did not repeat the 30,000 and 50,000 row timings from round 1; the round-2 changes add work only on the empty-list path.
• The ≤800 px empty-list cell on master (N3): checked only on the merged tree.
• CI on the merged tree. There is none; only my local runs on it.

[dborup]

Follow-up review

Reviewed head 39d16513fae473b18bf56a943a9b68e5c34b46b8 against base aff158c7ee9f7cdea1e6c00abd63d2f73557d4c7. The head is unchanged at posting. The PR has already been merged, so this review records the verified fix and outstanding follow-up rather than a pre-merge decision.

Fixed: explicit URL override

The previous hideControl=0 finding is resolved. With the saved preference set to 1, the explicit URL override now survives both the page's URL rewrite and a reload. My independent browser check showed 499 packets before and after reload, the checkbox remained unchecked, the URL retained hideControl=0, and the saved preference remained 1.

Remaining P2: filtering after the fetch cap can still strand the list

At public/packets.js:3048, CONTROL packets are removed only after the capped fetch (1,000 on mobile; 50,000 on desktop). I reproduced this again with controlled browser API responses containing 1,001 transmissions: the newest 1,000 were CONTROL, followed by one ADVERT, all within the selected time window. The mobile page made one request with limit=1000, displayed zero packets, and issued no continuation request for the available ADVERT.

This is now explicitly documented and tracked in #242, but it is deferred, not fixed. The historical-loading acceptance criterion in #96 therefore remains unmet. The follow-up should exclude hidden types before applying the cap, or provide bounded continuation that reaches matching packets, with a regression test for this scenario. See the original finding.

Verification

• Local test-all.sh: 218/218 frontend test files passed, including 28/28 Hide CONTROL unit checks.
• PR feature browser suite: 10/10 passed against an isolated local fixture.
• Independent Chromium checks confirmed the URL fix and reproduced the remaining historical-loading limitation.
• ESLint for public/packets.js: passed.
• Current GitHub Go, Playwright, and Docker checks: successful. Full Go suites were not rerun locally for this frontend-only change.

No additional defects were identified in this re-review. No implementation changes or staging/production validation were performed. The merged change fixes the URL regression, but #242 is still needed to satisfy the original historical-loading requirement.