fix(packets): filter excluded types before pagination (#242)

[dborup]

Summary

Relates to #242.

• Add bounded excludeTypes filtering before pagination and totals for raw/grouped packet queries, including SQLite retention fallback, memory index paths and grouped cache identity.
• Accept numeric wire types 0–15, preserve NULL/unknown stored types, and intersect with existing inclusion filters. Invalid input returns 400. Nonempty exclusions with the separate nodes multi-node path are explicitly rejected; single node is supported.
• Have the Packets type menu and opt-in Hide CONTROL checkbox refetch matching history. Preserve pinned-hash bypass, storage and URL contracts, and client-side live filtering. Hidden live CONTROL traffic cannot evict matching cached history.
• Fence stale requests across filter changes/navigation, and preserve custom saved time windows absent from the dropdown.
• Document the API contract and register the browser regression in CI.

No ingestor, schema, config/customizer, dependency or deployment changes. No packet deletion.

Performance

Exclusion membership is a bounded 16-bit mask checked in the existing scan; no extra store traversal or per-packet API request. Filtering is O(n), followed by the existing O(m log m) sort of matching rows. SQL uses at most 16 bound values. UI actions refetch one list request; existing expanded-row requests are unchanged.

Independent local benchmark, Apple M5, 30,000 synthetic transmissions, 50-row page, cold grouped cache:

Query	No exclusions	Exclude 20%	Exclude 80%
Raw	288 µs	280 µs	188 µs
Grouped	457 µs	365 µs	144 µs

These are current-path comparisons, not before/after speed claims or production latency estimates.

Validation

• Test-first: capped CONTROL history and malformed-input regressions failed before implementation, then passed.
• Full Go server and ingestor test suites passed; server build and vet passed.
• Focused exclusion tests passed under the race detector; memory/database/fallback, raw/grouped, pagination, NULL, overlapping filters, cache and indexed filters covered.
• Full frontend suite passed (227 test scripts).
• New Chromium regression: 8 scenarios covering a real 1,000-row UI cap with a deterministic API fixture, raw/grouped, pause/live flood, uncheck/type/Clear, stale requests, direct hashes and navigation.
• Existing Hide CONTROL Chromium regression: 10 cases passed against the local updated server.
• Broad Chromium regression: 132 passed, 3 fixture-dependent skips, zero failures. An initial missing-time-window failure led to the regression fix above; a separate detached-element failure was caused by a duplicate synthetic observation added during local fixture setup. After restoring the fixture, the unchanged broad suite passed.
• Screenshot captured and inspected; syntax, whitespace and XSS-diff checks passed. All 8 new Chromium scenarios also passed after push on commit 3f12bea4.

Tests use disposable localhost fixtures; no staging or production systems were contacted. An independent reviewer will inspect the published commit and post an English review. CI remains a separate gate.

User-visible changes

• The feat(packets): optional Hide CONTROL packets filter (#96) #211 empty-list note "No packets found (CONTROL packets are hidden)" now appears only between ticking Hide CONTROL and the refetch completing. CONTROL is excluded server-side and dropped from the live feed, so a server-filtered empty list cannot tell whether CONTROL packets were hidden and just says "No packets found". The browser test pins that window.
• A URL time window with no dropdown option (for example ?timeWindow=240) is now used for refetches and for the cold load. Before, a blank select made the cold load ask for all time.
• Changing the type menu or Hide CONTROL now sends one list request, as the observer menu already does.

[dborup]

Independent review of commit 3f12bea4f3d808bae7c2a7291f83d39cfa0b124e against 6444294d.

No actionable findings identified in the reviewed changes.

Checked exclusion parsing and bounded canonical masks, parameterized SQL and NULL handling, raw/grouped filtering before pagination and total counting, SQL fallback, bypass prevention in indexed memory paths, grouped-cache identity, frontend complement construction, pinned-hash exceptions, filter refetches, and asynchronous response guards.

Independently executed from an archive of the fetched PR head:

• go test ./... -run 'TestPacketExclusion' -count=1 in cmd/server: passed.
• node test-packets.js: 161 passed.
• node test-issue-96-hide-control.js: 28 passed.
• New packet-exclusion Chromium suite against the local test server: all 8 scenarios passed, including capped pages, live updates, type selection, stale responses, and view remounts.
• Existing Hide CONTROL Chromium suite: 10 passed.

Limitations: this review did not independently rerun the full Go/frontend suites, race tests, or performance benchmarks. The new browser suite uses deterministic intercepted packet responses; backend exclusion semantics were independently exercised by the Go regressions. No production or staging deployment was tested. This is a comment review, not a merge approval or CI-completion claim.

[dborup]

Taking this PR over from here. Next: an independent review (and, where CI is red, a fix round first). The issue link was changed from a closing keyword to "Relates to" to match the fork's convention; the issue is closed manually after merge.

[dborup]

Rapport — CS-Minimax PR#348 ci-fix — head d7f264e

Review feedback addressed (commit cdd14df6)

Evidence tags: [T] = executed test/command, [A] = code analysis/argument, [K] = known/pre-existing behaviour outside this PR.

1. Playwright job red: Kpa-clawbot#1122 Details row clamp E2E failed at 900px and 375px — BLOCKING, fixed

The independent review on 3f12bea4 reported no actionable findings, and the Go job
passed, so this was the one blocker. It is a real fault introduced by this PR, not a
flake, and not the known-unstable #271/#301 class.

Symptom [T] — run 37605035332, job "Playwright E2E Tests": === Results: passed 11 failed 7 === in test-issue-1122-details-row-clamp-e2e.js. desktop-1200 passed all
six steps; tablet-900 rendered a single row and mobile-375 rendered none
(navigate to /packets: page.waitForFunction: Timeout 8000ms exceeded, then
Sample: []).

Root cause [A] — that test widens the list to "All time" by assigning
#fTimeWindow.value = '0'. Below 1025px packets.js deliberately omits the
<option value="0"> ("All time" is disabled on mobile), so the assignment selects
nothing and leaves the select blank. Its change handler then clamps
savedTimeWindowMin to 15 (<= 0 → 15), and the blank-value fallback this PR added
to loadPackets() resolves the refetch to those 15 minutes instead of the all-time
window the test assumed. The e2e fixture's newest observation is ~now at
freshen-fixture.sh time, so a 15-minute window is empty once the fixture has aged
past it — in CI the test ran 13.7 minutes after freshen (freshen 10:26:22, test
10:40:04), which is why it was invisible locally on a fresh fixture. desktop-1200
kept passing because the option does exist above 1024px.

Deterministic A/B [T] — fixture prepared exactly as deploy.yml does, then aged
20 minutes, same Chromium, two servers:

Tree	Result
origin/master (5cdeffd)	18 passed, 0 failed
PR head 3f12bea4	10 passed, 8 failed (tablet-900 + mobile-375)
PR head + this fix	18 passed, 0 failed

The repaired test also still passes on origin/master (18/18), so it is not a
branch-specific crutch.

Fix — the production fallback stays; it is the behaviour we want. A shared
?timeWindow=240 URL also leaves the select blank (no option carries 240), and
reading that as Number('') === 0 would silently widen every refetch this PR adds to
All time. Reading the DOM cannot tell the two blanks apart [A], so the test stops
forcing an option the page does not offer: it selects "All time" only where that
option exists, and otherwise keeps the URL window it already pins, which spans the
whole fixture (499 of 533 packets at timeWindow=180 on the aged fixture) [T].

New regression test, red before / green after [T] — added a 9th scenario to this
PR's own test-issue-242-type-exclusions-e2e.js pinning the contract the PR
introduces: a ?timeWindow=240 URL whose window has no dropdown option must survive
the Hide CONTROL refetch. Mutant — revert the guard to
const selectedWindow = Number(selectedWindowValue);:

AssertionError [ERR_ASSERTION]: the refetch dropped the time window and asked for all time

Restored, 9/9 green.

Observation (no change made)

[A] Moving the CONTROL filter server-side means the client no longer sees the excluded
packets, so the empty-list note can no longer say "(CONTROL packets are hidden)" for a
server-filtered list. The PR adjusts that assertion in
test-issue-96-hide-control-e2e.js accordingly, which is correct — the client has no
evidence left to blame. The hint still works for live CONTROL packets filtered at
render. This is an inherent trade-off of the fix rather than a defect, but it is a
user-visible change that the PR description does not mention; flagging it for the
owner's call.

Merge

[T] origin/master (5cdeffd) merged into the branch with a merge commit
(d7f264ee), no conflicts. cmd/server/db.go and .github/workflows/deploy.yml
auto-merged; the PR's own diff against master is unchanged in substance afterwards
(packet_exclusions.go, the two buildPacketWhere/buildTransmissionWhere hooks,
the grouped-cache key, the three index-bypass guards, routes.go validation,
openapi.go, docs/api-spec.md, public/packets.js).

Tests

All of the following executed locally [T] on the merged tree (d7f264ee), macOS
arm64, Go 1.26.0, against a local server on port 13701 with e2e-fixture.db
prepared exactly as deploy.yml prepares it (freshen → Kpa-clawbot#1486/Kpa-clawbot#1791 seeds → migrate →
2073/199/245 seeds):

• cmd/server: go build ./..., go vet ./..., go test ./... -count=1 → ok 50.739s
• cmd/ingestor: go build ./..., go vet ./..., go test ./... -count=1 → ok 115.397s
• go test -race -count=1 -run 'TestPacketExclusion|TestExcludeTypes|TestPacketType' →
  3 tests pass, including TestPacketExclusionsBeforePagination subtests
  memory/database/fallback
• sh test-all.sh → 228 passed, 0 failed (228 files)
• node test-frontend-helpers.js → 709 passed, 0 failed
• node test-packets.js → 161 passed, 0 failed
• node test-issue-96-hide-control.js → 28 passed, 0 failed

E2E (Chromium, BASE_URL=http://localhost:13701), fresh fixture:

Suite	Result
test-e2e-playwright.js (broad)	132/135 passed, 3 skipped, 0 failed
test-issue-242-type-exclusions-e2e.js	9/9 scenarios
test-issue-96-hide-control-e2e.js	10 passed, 0 failed
test-issue-1122-details-row-clamp-e2e.js	18 passed, 0 failed
test-issue-1122-packets-filter-ux-e2e.js	6 passed, 0 failed
test-issue-258-column-widths-e2e.js	22 passed, 0 failed
test-issue-147-packets-url-obs-e2e.js	12 passed, 0 failed
test-issue-180-packets-url-modal-e2e.js	12 passed, 0 failed
test-issue-1486-collapse-reopens-detail-e2e.js	4 passed, 0 failed
test-issue-220-live-filter-readiness-e2e.js	11 passed, 0 failed
test-issue-1692-packets-init-parallel-e2e.js	1 passed, 0 failed
test-filter-ux-e2e.js	12 passed, 0 failed

The broad suite's 3 skips are the pre-existing fixture/flaky skips (two "Packets
clicking row shows detail pane" entries and the GO_BASE_URL perf test) [K], not
introduced here. This independently reproduces the PR description's
"132 passed, 3 fixture-dependent skips, zero failures" claim [T].

The 242/96/1122/1122-ux/258 suites were additionally run against a deliberately
20-minute-aged fixture — all green [T]. test-issue-1692-packets-init-parallel-e2e.js
fails on an aged fixture because its default 15-minute window is empty; that is an
artifact of the aging and is not caused by this PR — origin/master fails it
identically under the same aged fixture [T], and it passes on a fresh one in both
trees and in CI.

Mutants

Three mutants, each reverted afterwards and re-verified green [T]:

1. loadPackets(): selectedWindowValue ? Number(selectedWindowValue) : NaN →
   Number(selectedWindowValue) — the new feat(packets): server-side exclusion of packet types in /api/packets for the type filter and Hide CONTROL #242 E2E scenario fails with
   the refetch dropped the time window and asked for all time. (Finding 1.)
2. packetTypeExclusions.excludes() → return false —
   TestPacketExclusionsBeforePagination fails grouped=false total=5 want 2,
   TestPacketExclusionMaskAndIndexes fails for all 16 wire types plus the
   ExcludeTypes:2048 index query (yielded 1 rows want 0). Confirms the in-memory
   predicate and the index-bypass guards are genuinely covered.
3. Grouped cache key: drop |x%d for the exclusion mask —
   TestPacketExclusionsBeforePagination fails grouped=true total=5 want 2.
   Confirms the grouped-cache identity claim is genuinely covered.

CI per job — run 37617607635, head d7f264ee

Job	Result
✅ Go Build & Test	pass (24m8s)
🎭 Playwright E2E Tests	pass (22m0s)
🏗️ Build & Publish Docker Image	pass (46s)
📦 Release Artifacts	skipping
🚀 Deploy Staging	skipping
📝 Publish Badges & Summary	skipping

No job was re-run; neither known-unstable test (#271, #301) appeared [K], so the
"re-run once" path was not needed. #1122 Details row clamp E2E reports === Results: passed 18 failed 0 ===
and the new scenario reports PASS: a URL time window absent from the dropdown survives an exclusion refetch.

One caveat, stated plainly [A]: in this run #1122 executed 11.3 minutes after
freshen-fixture.sh (12:22:27 → 12:33:47), i.e. still inside the 15-minute window,
whereas the failing run hit it at 13.7 minutes. So this green run is not by itself
a discriminating test of the fix — the proof is the deterministic 20-minute-aged A/B
above (master 18/18, PR head 8 failures, PR head + fix 18/18). The fix removes the
dependency on fixture age entirely at those viewports.

Scope

No production behaviour was changed in this round; the blank-select fallback this PR
introduced is kept as-is because it is correct. The round touches only the two E2E
files. No ingestor, schema, config, dependency or deployment changes. Not merged, not
marked ready, no issue closed; no staging or production system was contacted.

[dborup-agent]

Review — CS-pve-agent3 PR#348 — head d7f264e

Dom: APPROVE med nits

Independent, read-only review of d7f264ee (merge commit on top of cdd14df6/3f12bea4). All runs used git archive of the head and of the merged tree git merge-tree --write-tree origin/master d7f264ee… = f66b559d against origin/master e8e99c4a. The diff against master is unchanged by the merge: 14 files, +543/−27. git ls-remote showed head d7f264ee both before and after the review.

Evidence tags: [T] = I ran it, [A] = code reading/argument, [K] = known or pre-existing behaviour outside this PR.

Findings

#	Severity	Finding	Evidence
1	Low (behaviour/docs)	The #211 empty-list hint "No packets found (CONTROL packets are hidden)" is now effectively unreachable in steady state, not only for server-filtered history. With Hide CONTROL on, history arrives without CONTROL (excludeTypes=11), and the WS handler now drops live CONTROL before it enters packets (public/packets.js new line in the live filter). So controlHidingEmptiedList() only sees CONTROL in the short window between the checkbox change and the refetch. The report says "the hint still works for live CONTROL packets filtered at render", but that does not hold. Owner's call: either drop the hint machinery (controlHidingEmptiedList, beforeHideControl) as dead code, or keep it and record the user-visible change in the PR description. Not blocking.	[A] code reading; [T] test-issue-96-hide-control-e2e.js step "#242: server-filtered empty lists do not infer unseen CONTROL packets" passes and asserts the hint is gone
2	Low (perf)	With no exclusion, the in-memory default path is about 9–10% slower at 300K tx (raw 12.4–12.7 → 13.6–14.6 ms, grouped 14.5–15.8 → 15.9–17.5 ms). packetTypeExclusions.excludes() dereferences tx.PayloadType before it tests mask. Checking mask != 0 first restores parity. A one-line fix, worth taking before merge given AGENTS.md rule 0.	[T] interleaved A/B, see section 2
3	Nit (test strength)	The E2E scenario "late earlier filter response cannot replace newest results" cannot catch a missing stale-response guard on its own. The held Hide-CONTROL response, after the client-side type filter, renders the same [1001, 1003] as the fresh response. My mutant M3 (removing if (generation !== packetLoadGeneration) return; right after api()) passes that scenario. The later "departed view" scenario kills it. Asserting the header count (N), or making the held payload visibly different after client filters, would make the scenario stand alone.	[T] mutant M3 below
4	Nit	When the type menu is active, excludeTypes is the 4-bit complement, but NULL payload_type rows are kept on the server (documented) and dropped by the client type filter. A page can therefore come up short by the number of NULL-type rows, and total counts them. The fixture has no NULL payload types (0/533), so in practice the impact is likely about zero. I did not check production data.	[A], [T] fixture query
5	Nit (report only)	The CI-fix report names #271/#301 as the known-unstable tests. The known flakes for this repo are #256 (Hash Stats sort) and #267 (backfill write-hold). This has no effect: run 37617607635 is attempt 1, all jobs passed, and nothing was re-run.	[T] gh run list, gh issue view

No blocking findings.

1. Correctness: full pages, total/count, offset stability

• API, in-memory path [T]: local Go server, CI-prepared e2e-fixture.db, paging through every page with limit=37. excludeTypes=11, 4,5 and the complement of 5, each raw and groupByHash=true: every non-last page was full, the sum of rows equalled total, there were no duplicates, and no excluded type leaked. Example: excludeTypes=11 gave total 496 = 500 − 4 CONTROL, 14 pages, 496 unique rows.
• API, SQL fallback (since older than oldestLoaded) [T]: the same walk with limit=50 gave total 533 / 529 / 232 for none / 11 / 4,5, with full pages, no duplicates and no leaks. This matches SELECT COUNT(*) on the DB.
• Validation [T]: excludeTypes=abc → 400; excludeTypes=11&nodes=aa → 400 with a clear message; excludeTypes=11&node=aa → 200. The packets page never sends nodes (My Nodes filters client-side, and only compare.js/traces.js/channels.js call /packets elsewhere), so the 400 is not reachable from the UI [A].
• Cursor/offset [A]: /api/packets has no cursor, only offset. Exclusion is applied inside the existing filter before sort/slice, so offset paging on a fixed data set is stable, as shown above. Under live ingest, offsets drift exactly as they did before [K].
• Index bypasses and grouped cache [A]+[T]: all three fast paths (byHash, observer, byNode) now fall through when ExcludeTypes != 0. The grouped cache key carries |x<mask>. Both are covered by TestPacketExclusionMaskAndIndexes and TestPacketExclusionsBeforePagination (author mutants 2 and 3, and my M1).
• Real end-to-end, no API mock [T]: I added 1,100 newest CONTROL packets to a copy of the fixture and ran the packets page at 1024 px (mobile cap 1000) with ?timeWindow=180. Same DB on both sides:

Step	PR head	master
initial	(1000), all Control	(1000), all Control
Hide CONTROL on	(495) non-CONTROL, request excludeTypes=11	(19)
reload	(495), hideControl=1 in URL, meshcore-hide-control=1	(19)
Hide CONTROL off	(1000), no excludeTypes	(1000)
type = Channel Msg	(166), request excludeTypes=0,1,2,3,4,6,…,15	(4)

Both issue acceptance criteria 1 and 2 hold end to end. There were no page errors. I inspected the screenshot: the list is filled with RESPONSE/ADVERT/REQUEST/CHANNEL MSG and the checkbox is checked.

2. Performance

Query plan [T] (synthetic 1,000,533-row DB, EXPLAIN QUERY PLAN): the exclusion COUNT(*) uses the same SCAN … USING COVERING INDEX idx_transmissions_payload_type as the unfiltered count (SQLite counts by scanning in both cases), so there is no extra scan. The page query stays SCAN t in rowid order and stops after LIMIT matches. With since, the plan is unchanged: rowid lookups from the existing observations subquery.

SQL path timings [T] (db.QueryPackets/QueryGroupedPackets on the 1M DB, limit=1000, median of 5; 40% CONTROL, 30% ADVERT, 20% GRP_TXT, 10% TXT_MSG):

Query	raw, no excl	raw, excl 11	raw, complement of 5	grouped, no excl	grouped, excl 11	grouped, complement of 5
since = 24h (the real fallback shape)	112 ms	108 ms	112 ms	511 ms	360 ms	208 ms
no since	11 ms	45 ms	145 ms	9.35 s [K]	5.98 s	2.14 s

The SQL fallback is only entered when since/until predates oldestLoaded, so the 24h row is the realistic case: equal or faster with exclusions. The no-since raw page is slower with a selective exclusion because the rowid scan has to skip excluded rows. That row is only reachable without an in-memory store, or through an until-only fallback, which also bounds the scan. The grouped variant was already 9 s before this PR [K].

In-memory path [T], synthetic 300K-tx store, limit=1000, cold grouped cache, 3 interleaved runs × 20 iterations:

Tree	raw, mask 0	grouped, mask 0	raw, excl 11	raw, complement of 5	grouped, excl 11	grouped, complement of 5
master e8e99c4a	12.4–12.7 ms	14.5–15.8 ms	—	—	—	—
PR head	13.6–14.6 ms	15.9–17.5 ms	10.0 ms	6.1 ms	13.1 ms	8.1 ms
PR + mask-first (see finding 2)	12.0–12.4 ms	14.0–14.3 ms

Exclusion is a predicate in the existing single pass (filterTxSlice), with no extra traversal; with exclusions active, requests get cheaper. One measurable cost remains: with no exclusion (the default page), the PR is consistently about 9–10% slower at 300K. The cause is that excludes() checks typ != nil && *typ … before mask, so every tx dereferences its separately allocated PayloadType even when mask == 0. A one-line reorder, return mask != 0 && typ != nil && … (or hoisting hasExclude like hasType), restores master parity. I measured this in a scratch copy, and the PR's exclusion tests still pass with it. The PR's own BenchmarkPacketExclusions30K reproduces locally: 715/669/315 µs raw and 905/851/387 µs grouped for none/excl 5/excl 11.

3. Frontend: type filter, Hide CONTROL (#211), deep links, localStorage

• [T] test-issue-96-hide-control-e2e.js 10/10, test-issue-242-type-exclusions-e2e.js 9/9 and test-issue-1122-details-row-clamp-e2e.js 18/18 against the local merged server. The browser table above covers the URL hideControl=1, reload persistence, explicit 0 in localStorage after unchecking, and the type filter in localStorage (meshcore-type-filter=5; type stays out of the URL, as before).
• [A] Type-menu and Hide CONTROL changes now cost one list request each. On desktop that request is up to 50,000 rows, where previously the change was a zero-request re-render. The issue asks for exactly this, and the observer menu already refetches per click without a debounce, so this follows existing precedent. Stale responses are dropped through packetLoadGeneration but not aborted.
• [A] Behaviour change, disclosed in the PR description: a URL window with no dropdown option (e.g. ?timeWindow=240) leaves the select blank. On master the cold load then used Number('') = 0, i.e. all time, while the live filter used 240. The PR uses 240 for both. I consider this within scope: without it every new refetch would widen to all time. Scenario 9 of the feat(packets): server-side exclusion of packet types in /api/packets for the type filter and Hide CONTROL #242 E2E asserts it.
• [A] Live path: the type filter already applied to WS packets. The new CONTROL drop sits after the pinned-hash bypass, so a direct CONTROL link still works ([T] scenario 7).

4. The CI fix: real or flaky?

Real and deterministic, and the report's explanation is correct. [T] I aged a CI-prepared fixture copy by 20 minutes and ran both servers:

Test version	PR server	master server
master's test-issue-1122… (pre-fix)	10 passed, 8 failed (tablet-900 + mobile-375: "no packet rows rendered" / timeout)	18/0
PR's test-issue-1122… (fixed)	18/0	18/0

This reproduces the author's A/B exactly. The old test assigned '0' to a select that has no "All time" option below 1025 px, which left it blank. The PR's blank-select guard then resolves to the 15-minute saved window, which is empty once the fixture is older than 15 minutes. The failure looked intermittent only because it depends on fixture age. The test fix stops forcing an option the page does not offer; it is not a crutch and still passes on master. The production guard is the intended behaviour (see 3).

CI per job, run 37617607635 on d7f264ee, attempt 1 [T]: Go Build & Test pass (24m8s), Playwright E2E pass (22m0s; all 9 #242 scenarios PASS; broad suite 132/135 with 3 pre-existing skips [K]), Docker pass; Release/Deploy/Badges skipped (fork guards).

5. Tests, red before / green after, mutants

Merged tree f66b559d [T], linux/amd64, Go 1.27.1, Node 22:

• cmd/server: go build, go vet, go test ./... -count=1 → ok 436.163s
• cmd/ingestor: go test ./... -count=1 → ok 595.304s. The first attempt hit Go's 10-minute default timeout with 0 failing tests while my browser runs shared the 4-core box. I reran it with -timeout 30m on a quiet box. The PR has no diff in cmd/ingestor or internal/.
• sh test-all.sh → 229 passed, 1 failed (230 files). The failure is test-channels-client-state-152.js case "R4-3 S2" (pane stuck on "Decrypting…"). channels.js is untouched by this PR. The same file passed 15/15 in isolation on both master and the merged tree, and a full test-all.sh rerun gave 230 passed, 0 failed. This is a load-dependent timing flake unrelated to fix(packets): filter excluded types before pagination (#242) #348 [T]/[A]; I found no existing issue for it.
• node test-frontend-helpers.js → 709 passed, 0 failed
• E2E against a local merged server on a CI-prepared e2e-fixture.db (freshen → 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 SQL → corescope-migrate → seeds 2073/199/245); servers stopped by port: 242 9/9, 96 10/0, 1122 18/0 (fresh and 20-min aged).

Red before / green after, per acceptance criterion [T]:

Acceptance	Test	master	PR
AC1 Hide CONTROL fills capped page	TestPacketExclusionsBeforePagination (memory/database/fallback)	FAIL total=5 want 2 in all three modes	pass
AC1 (UI)	test-issue-242-type-exclusions-e2e.js	FAIL scenario 1 (excludeTypes null !== '11')	9/9
AC2 type filter	test-packets.js #242 block (complement + union)	4 failed	pass
AC2 (UI)	242 E2E scenario 4, plus my real-DB browser run (166 vs 4)	—	pass
AC3 OpenAPI + api-spec, invalid → 400	TestPacketExclusionsValidation	11 malformed inputs return 200	pass
AC4 store unit + capped E2E	the above; test-issue-96-hide-control.js (setHideControl refetch)	2 failed	28/28

Note: the #242 E2E mocks /api/packets, which is what the issue allows ("delayed or capped fixture"). My unmocked browser run above covers the real server path.

My own mutants [T]; each one was run in a scratch copy, so nothing touched the PR:

• M1 appendSQL: drop column IS NULL OR → TestPacketExclusionsBeforePagination/database and /fallback FAIL (total=1 want 2). Killed.
• M2 packets.js live filter: remove the hideControl && payload_type === CONTROL drop → 242 E2E scenario 2 FAILS (the matching live packet is evicted by 1,001 hidden CONTROL updates; times out). Killed.
• M3 loadPackets(): remove the generation check right after api() → scenario 5 still PASSES; scenario 8 FAILS. Killed by the suite, but see finding 3.

Invariants [T]: cmd/server stays read-only (no write SQL added; the exclusion is a WHERE on reads and the server still opens mode=ro). 0 new map[string]interface{} outside tests. No new hard-coded colours. scripts/check-xss-sinks.sh --diff origin/master is clean (exit 0, run in a scratch clone at head). Fork guards github.repository == 'Kpa-clawbot/CoreScope': 9 in deploy.yml, 1 in release-fast-path.yml, identical to master. No closing keywords in the title, body or commits ("Relates to #242"). All 3 commits are authored and committed by dborup <kontakt@meshview.dk>. Outside cmd/, public/, the tests, the docs and one deploy.yml E2E line, nothing changed: no ingestor, schema, config or dependency changes.

Not verified

• Production-scale behaviour with a real 1M+ in-memory store. I measured the SQL path on a synthetic 1M-row DB and the memory path on a synthetic 300K store, not live traffic.
• WebKit/Firefox. Only Chromium was used.
• The broad test-e2e-playwright.js suite locally (CI ran it: 132/135, 3 skipped).
• The transient feat(packets): optional Hide CONTROL packets filter (#96) #211 hint during the refetch window (finding 1), which I established by code reading only.
• Staging/production: none was contacted.

[dborup-agent]

Rapport — CS-pve-agent1 PR#348 master-sync — head 5899d46

Review feedback addressed (commit 5899d461)

Commits in this round, on top of d7f264ee (no rebase, amend or force-push):

Commit	What
8221eb5b	Merge origin/master (987c220d) with a merge commit
5a17ee36	Finding 3: stale-response E2E scenario now fails on its own
23ea5efd	Finding 2: test the exclusion mask before the payload type
5899d461	Finding 1: pin where the #211 hint still fires; documented in the PR description

Evidence tags: [T] = executed, [A] = code reading/argument, [K] = known or pre-existing behaviour outside this PR.

0. Master sync

[T] git merge --no-ff origin/master (987c220d) gave one conflicted file: public/packets.js, in the background hop job in loadPackets(). cmd/server/db.go, cmd/server/store.go and .github/workflows/deploy.yml auto-merged.

• Master (Port upstream #2099 hop ambiguity UI with observer/cache correctness fixes #165) replaced the flat allHops collection with resolveHopsForPackets(packets), which resolves hops per observer.
• This PR snapshots the loaded list (loadedPackets) and abandons the job when a newer load has started (generation !== packetLoadGeneration).
• Resolution keeps both: cacheResolvedPaths(loadedPackets), then the generation check, then resolveHopsForPackets(loadedPackets). Nothing else was changed in the merge commit.

[T] After the merge, git diff origin/master --stat is still the PR's own 14 files, +543/−27, as before the merge. test-issue-165-grouped-hop-warn-e2e.js, which covers master's side of the conflict, passes 3/3 locally.

1. #211 "CONTROL packets are hidden" hint is near-unreachable (Low): addressed by documenting it and pinning it in a test

I kept the machinery and did not delete it. [T] It is not dead: setHideControl() re-renders the already-loaded page before the refetch, so when that page was all CONTROL, the hint is rendered until the refetch lands. A new step in scenario 1 of test-issue-242-type-exclusions-e2e.js holds that refetch and asserts the hint, then releases it and asserts the refilled page. The call-site comment in renderTableRows() now says that the hint fires only in this window, and that a server-filtered empty list just says "No packets found". The PR description has a new User-visible changes section that records this, together with the custom time-window and per-change refetch behaviour.

Mutant [T]: controlHidingEmptiedList → return false. The new E2E step fails (The input did not match the regular expression /CONTROL packets are hidden/), and test-issue-96-hide-control.js drops to 26 passed / 2 failed. Restored: both green.

Side observation, no change made [K]/[A]: the hint row is in the DOM but invisible at 1024 px. The packets column-hiding pass (apply(), wired through a tbody MutationObserver) gives the single colspan <td> the class col-hidden whenever column 0 is hidden at that width. The same applies to the plain "No packets found" row, and that code is identical on master. This is why the step asserts textContent, not innerText. It is a pre-existing issue that is worth its own ticket; this PR does not change it.

2. Mask-0 path ~10% slower in memory (Low, perf): changed

excludes() now returns mask != 0 && typ != nil && …, so the default request no longer dereferences every tx's separately allocated PayloadType.

Proof [T], interleaved A/B, 300K synthetic tx, limit=1000, mask 0, cold grouped cache, 6 rounds × 3 runs, -benchtime 2s, n=18 per cell, medians:

	before (d7f264ee logic)	after (23ea5efd)	master 987c220d
raw	15.20 ms	13.08 ms	13.16 ms
grouped	17.89 ms	15.73 ms	15.94 ms

A second interleaved run, master vs after, gave raw 13.16 / 13.06 ms and grouped 15.94 / 15.88 ms. That is parity. The 300K benchmark was a scratch file and is not committed; the committed BenchmarkPacketExclusions30K is unchanged.

Red/green [A]: a pure evaluation-order change has no functional effect that a deterministic unit test can observe. The only way would be dereferencing a forged invalid pointer through unsafe, which go vet flags. So the benchmark is the detector, and the mutant is the old ordering (the "before" binary above): +16% raw and +14% grouped. TestPacketExclusion* passes under -race with the change.

3. Stale-response E2E scenario could not catch a missing guard on its own (Nit, test strength): changed

The held Hide-CONTROL response and the newer type response rendered the same rows after the client filters. Now the test server ages packet 1003 out between answering the held request and the newer request. The late response still carries 1003, so if it were applied it would be visible. Expected rows are now [1001] before and after the release, and the fixture is restored afterwards.

Mutant M3 [T], removing if (generation !== packetLoadGeneration) return; right after api():

• old test: scenario 5 passes, and only scenario 8 fails. This reproduces the review.
• new test: scenario 5 fails (a late earlier response replaced the newest results).

Restored: 9/9 scenarios green.

4. NULL payload_type rows can short a type-filtered page (Nit): no change, disagree

[A] The ingestor never writes NULL. PacketData.PayloadType is a plain int (cmd/ingestor/db.go:2032), filled from the decoded 4-bit header field (db.go:2638) and inserted as-is (db.go:1174). NULL is reachable only for legacy rows or hand-written SQL. [T] The CI-prepared fixture has 0/533 NULL rows. Keeping NULL/unknown rows on the server is the documented API contract, and it matches the in-memory predicate. Dropping them server-side would change what excludeTypes means for every other client in order to remove a short page that the ingestor cannot produce. If legacy data ever matters, the right fix is a migration, not the filter.

5. Known-flake issue numbers (report only): acknowledged

[T] #256 and #267 are flaky-test issues, but so are #271 (packets URL E2E fixture age) and #301 (data race under -race), which the brief names. All four are closed. No job needed a re-run in this round, so the choice had no effect.

Tests (local, linux/amd64, 4 cores, Go 1.27.1, Node 22), on 5899d461

Server on port 13711 with e2e-fixture.db prepared as deploy.yml does: freshen → Kpa-clawbot#1486/Kpa-clawbot#1791 seed SQL → corescope-migrate → seeds 2073/199/245. A copy was used, so the committed fixture was untouched.

• cmd/server: go build, go vet clean. go test ./... -count=1 → ok 864.430s [T].
  • The first full run, made while my E2E runs were loading the box, failed once: TestHandleAnalyticsSubpathsWithStore returned 503 instead of 200. [A] The test hits /api/analytics/subpaths right after store.Load() without waiting for the async subpath-index build, and this PR does not touch that path. [T] The test passes 50/50 in isolation on both the PR head and master, and the full suite passed on a quiet rerun. The rebuild panic: intentional test panic line in that log is an expected, recovered panic from TestNeighborGraphCacheRebuildPanicIncrementsCounter.
• go test -race -run TestPacketExclusion → ok [T]
• sh test-all.sh → 232 passed, 0 failed (232 files) [T]
• node test-frontend-helpers.js → 709 passed, 0 failed; test-packets.js 161/0; test-issue-96-hide-control.js 28/0 [T]
• bash scripts/check-xss-sinks.sh --diff origin/master → exit 0; git diff --check clean; 0 new map[string]interface{} [T]
• cmd/ingestor was not rerun: there is no diff against master in cmd/ingestor/ or internal/ [A].

E2E (Chromium) against the local server [T]:

Suite	Result
test-issue-242-type-exclusions-e2e.js	9/9 scenarios (with the new steps)
test-issue-96-hide-control-e2e.js (Hide CONTROL)	10 passed, 0 failed
test-issue-1122-packets-filter-ux-e2e.js	6 passed, 0 failed
test-filter-ux-e2e.js	12 passed, 0 failed
test-issue-1122-details-row-clamp-e2e.js	18 passed, 0 failed
test-issue-165-grouped-hop-warn-e2e.js (conflict area)	3 passed, 0 failed
test-issue-1692-packets-init-parallel-e2e.js	1 passed, 0 failed
test-issue-147-packets-url-obs-e2e.js	12 passed, 0 failed
test-issue-180-packets-url-modal-e2e.js	12 passed, 0 failed
test-e2e-playwright.js (broad)	132/135, 3 skipped, 0 failed

The 3 broad-suite skips are the pre-existing ones: two "clicking row shows detail pane" flaky skips and the GO_BASE_URL perf test [K].

Mutants (one per changed finding), all restored and re-verified green [T]

1. controlHidingEmptiedList → return false: the new feat(packets): server-side exclusion of packet types in /api/packets for the type filter and Hide CONTROL #242 E2E step fails, and test-issue-96-hide-control.js gives 26/2.
2. excludes() with the old order (typ before mask): +16% raw / +14% grouped at 300K (benchmark, see 2).
3. M3, drop the post-api() generation check: the strengthened scenario 5 fails on its own.

CI per job — run 37660906198, head 5899d461, attempt 1 [T]

Job	Result
✅ Go Build & Test	pass (24m15s)
🎭 Playwright E2E Tests	pass (25m4s): broad 132/135 with 3 skipped; all 9 #242 scenarios PASS; Hide CONTROL 10/0; Kpa-clawbot#1122 details 18/0
🏗️ Build & Publish Docker Image	pass (56s)
📦 Release Artifacts	skipped
🚀 Deploy Staging	skipped
📝 Publish Badges & Summary	skipped

No re-run was needed, and neither #271 nor #301 appeared [K].

Scope

The merge commit only resolves the one conflict. The review-fix commits touch cmd/server/packet_exclusions.go (one line + comment), a comment in public/packets.js and test-issue-242-type-exclusions-e2e.js; the PR description gained a "User-visible changes" section. No ingestor, schema, config, dependency or workflow changes. The PR was not merged, no issue was closed, and no staging, production or upstream system was contacted. Note: the PR was already non-draft when I took it over, and I left its draft state unchanged.

[dborup-agent]

Review — CS-pve-agent3 PR#348 — head 5899d46

Dom: APPROVE

This is a short, read-only delta re-review. It covers only what changed after my previous review on d7f264ee:

• the master-sync merge 8221eb5b, with its one conflict in public/packets.js;
• three reviewfix commits: 5a17ee36, 23ea5efd and 5899d461.

I did not re-review master's own changes. All runs used git archive of the head and of the merged tree. The merged tree was git merge-tree --write-tree origin/master 5899d461… = e54b272d, against current origin/master 650218d4. That merge is clean. The diff against master is 14 files, +561/−28. git ls-remote showed head 5899d461 both before and after this review.

Evidence tags: [T] = I ran it, [A] = code reading/argument, [K] = known or pre-existing behaviour outside this PR.

Findings

#	Severity	Finding	Evidence
—	—	No new findings in the delta. All three nits from my previous review are addressed, and finding 4 (NULL rows) is answered convincingly; see below.	[T]/[A]
obs	Info, pre-existing	The author noticed that at 1024 px the column-hiding pass gives the single colspan empty-state <td> the class col-hidden. That makes both "No packets found" messages invisible, which is why the new step reads textContent. The code is identical on master, so it is outside this PR. It is worth a ticket of its own, which the owner should create.	[A] (author's report)

1. Conflict resolution in public/packets.js

[A] git show --remerge-diff 8221eb5b -- public/packets.js shows that only the background hop job was touched. The resolution is:

await cacheResolvedPaths(loadedPackets);
if (generation !== packetLoadGeneration) return;
await resolveHopsForPackets(loadedPackets);
if (generation === packetLoadGeneration && filtersBuilt) renderTableRows();

This keeps both sides:

• feat(packets): show hop ambiguity, resolved per observer (port of upstream 2099) #185's per-observer resolution: master's resolveHopsForPackets() and the flat allHops loop are gone.
• fix(packets): filter excluded types before pagination (#242) #348's guards: the snapshot (loadedPackets, taken after the raw-mode flattening, so per-observation rows are included) and the generation guard. A job from a superseded load stops before resolving, and it never re-renders.

resolveHopsForPackets()/cacheResolvedPaths() keep their own hopCacheGen stop for destroy(). The PR's ++packetLoadGeneration in destroy() is compatible with it. On the live path, the WS handler drops CONTROL before resolveIncomingHops(filtered), so hidden CONTROL causes no hop work. Nothing else changed in the merge commit.

Verification:

• [T] Mutant MF1: I reverted the hop job to the old flat, observer-less loop (resolveHops([...allHops])). test-issue-165-grouped-hop-warn-e2e.js fails with "no .hop-path-warn rendered even without server answers" (2/1). So master's feat(packets): show hop ambiguity, resolved per observer (port of upstream 2099) #185 behaviour in the conflict area is covered, and the merged code passes it (3/0).

• [T] Hop rendering parity with master: the same CI-prepared fixture ran on two servers, master 650218d4 and the merged tree. I loaded #/packets?timeWindow=180&hideControl=1 at 1440 px and compared the rendered PATH cell text and .hop-path-warn row by row:

  • grouped: 112/112 rows identical;
  • raw: 112/112 rows identical;
  • no page errors.

  The path to the rows differs: master filters CONTROL client-side, while the PR fetches with excludeTypes=11. The rendered hops are the same.

Browser check, real data [T]: I took a copy of the CI-prepared fixture and added the 1,100 newest CONTROL packets to it, giving 1,633 tx, of which 1,104 are CONTROL. On the merged server, at 1024 px (cap 1000), with #/packets?timeWindow=180:

Step	Header count	Rows rendered (virtual)	Request	Hop links
initial	(1000)	all CONTROL	no excludeTypes	0
Hide CONTROL on	(495)	RESPONSE/ADVERT/REQUEST/TXT/CHANNEL MSG, 0 CONTROL	excludeTypes=11	358 named hops
reload	(495), hideControl=1 in URL	same	excludeTypes=11	358
Hide CONTROL off	(1000)	all CONTROL	none	0
type = Channel Msg	(166)	only type 5	excludeTypes=0,1,2,3,4,6,…,15	491

There were no page errors. I inspected the screenshot: the list is filled with non-CONTROL rows, the checkbox is ticked, and the paths show resolved node names. So filtering before pagination and hop resolution work together after the refetch. The numbers match my run on d7f264ee (495/166, against master's 19/4).

2. Server: mask != 0 && early exit

[A] The change is correct. parsePacketTypeExclusions returns 0 for an absent or empty excludeTypes. The previous expression could only return true when mask&(1<<typ) != 0, so with mask 0 it was already always false. The reorder changes no result for any input. It only skips the *typ dereference. The SQL side already had if mask == 0 { return }, so the two paths are now symmetric.

Is there a test? Yes, semantically. "Mask 0 = no exclusion" is asserted by check("", 5, …), before and after the exclusion queries in TestPacketExclusionsBeforePagination. It runs in memory, database and fallback mode, raw and grouped, and it counts the NULL row. My Go mutants [T]:

• mask == 0 || … → grouped=false total=0 want 5. Killed.
• mask != 0 || … → &excludeTypes=11 … total=0 want 2 and TestPacketExclusionMaskAndIndexes fails. Killed.
• Dropping mask != 0 && (the old ordering) → passes. This is an equivalent mutant by construction, and no functional test can tell it apart. I agree with the author that the benchmark is the only detector. [T] An interleaved A/B of the committed BenchmarkPacketExclusions30K, mask 0, -benchtime 2s, n=8 per cell, 4 rounds alternating between two test binaries that differ only in this line:

	old ordering	mask != 0 && first
raw	773 µs (732–818)	626 µs (611–649)
grouped	927 µs (905–962)	766 µs (750–882)

That is about −19%/−17% on the default no-exclusion request. It agrees with my 300K measurement on d7f264ee and with the author's n=18 run.

3. Test and comment changes: do they fix my nits?

My previous nit	Change	Verdict
1. The #211 hint is near-unreachable	Kept the machinery. The call-site comment explains that the hint fires only between the checkbox change and the refetch. Scenario 1 now holds that refetch and asserts the hint. The PR description has a new "User-visible changes" section.	Fixed. [T] My mutant: dropping the immediate renderTableRows() in setHideControl() (refetch only) → the new step fails (did not match /CONTROL packets are hidden/). [T] Against master's frontend the new step fails (Expected held packet request, no refetch). So the step pins real #242 behaviour.
2. Mask-0 path ~10% slower	mask != 0 && first	Fixed, see 2.
3. Stale-response scenario cannot stand alone	Packet 1003 is aged out of the mocked server after the held request was answered, so the late response is visibly different. Expected rows are [1001] before and after the release, and the fixture is restored afterwards.	Fixed. [A] selected is computed before the await held, so the held payload really does carry 1003. [T] My M3 again (remove if (generation !== packetLoadGeneration) return; after api()): scenario 5 now fails on its own ("a late earlier response replaced the newest results"). On d7f264ee the same mutant passed scenario 5.
4. NULL payload_type can short a typed page	No change; author disagrees	Accepted. [A] cmd/ingestor/db.go:2032 PayloadType int is non-nullable, so NULL only appears in legacy or hand-written rows. Keeping NULL rows matches the documented contract and the in-memory predicate.
5. Flake issue numbers	Acknowledged	Fine.

Tests

Merged tree e54b272d (linux/amd64, Go 1.27.1, Node 22) [T]:

• cmd/server: go build ./... and go vet ./... clean; go test ./... -count=1 → ok 392.6s, 0 --- FAIL
• cmd/ingestor: not rerun. Neither the PR nor the delta touches cmd/ingestor/ or internal/ [A]. CI's Go job ran it green on head [T].
• sh test-all.sh → 233 passed, 0 failed (233 files)
• node test-frontend-helpers.js → 709 passed, 0 failed. test-packets.js 161/0, test-issue-96-hide-control.js 28/0.

E2E (Chromium) [T]: local merged server, e2e-fixture.db prepared as in deploy.yml (freshen → Kpa-clawbot#1486/Kpa-clawbot#1791 seed SQL → corescope-migrate → seeds 2073/199/245). All servers were stopped by port.

Suite	Result
test-issue-242-type-exclusions-e2e.js	9/9 scenarios, including the new hint step and the strengthened scenario 5
test-issue-96-hide-control-e2e.js	10/0
test-issue-165-grouped-hop-warn-e2e.js (conflict area)	3/0
test-issue-1122-details-row-clamp-e2e.js	18/0
test-issue-1122-packets-filter-ux-e2e.js	6/0
test-issue-1692-packets-init-parallel-e2e.js	1/0
test-issue-147-packets-url-obs-e2e.js	12/0
test-filter-ux-e2e.js	12/0

My mutants, each in a scratch copy of public/ or cmd/server, with nothing touching the PR [T]:

#	Mutant	Result
MF1	hop job → flat observer-less resolveHops (pre-#185)	killed by #165 E2E
MF2	drop the post-api() generation check	killed by #242 scenario 5 alone
MF3	setHideControl() without the immediate re-render	killed by #242 scenario 1 (hint step)
G1	mask == 0 || …	killed
G2	mask != 0 || …	killed
G3	drop mask != 0 &&	equivalent (perf only), survives as expected

Red before / green after: unchanged from my previous review for AC1–AC4 (Go TestPacketExclusionsBeforePagination / …Validation, test-packets.js, test-issue-96-hide-control.js, #242 E2E). In this round [T]: the #242 E2E against master's frontend fails at the first step, and passes 9/9 on the merged tree.

CI per job, run 37660906198 on 5899d461, attempt 1 [T]:

Job	Result
Go Build & Test	pass (24m15s; cmd/server ok 1072.9 s, cmd/ingestor ok)
Playwright E2E Tests	pass (25m4s): broad 132/135 with 3 pre-existing skips [K]; #242 9/9 PASS; #96 10/0; #165 3/0; Kpa-clawbot#1122 details 18/0
Build & Publish Docker Image	pass
Release / Deploy Staging / Badges	skipped (fork guards)

The CI log has no failures. The known flakes #256 (Hash Stats sort) and #267 (backfill write-hold) did not occur, and nothing was re-run.

Invariants [T]:

• cmd/server stays read-only: the delta adds no write SQL, and the exclusion is still a read-side WHERE.
• 0 new map[string]interface{} outside tests.
• No new hard-coded colours.
• scripts/check-xss-sinks.sh --diff origin/master exits 0 in a scratch clone at head, and git diff --check is clean.
• Fork guards: 9 in deploy.yml and 1 in release-fast-path.yml, identical to master.
• No closing keywords in the title, body or commits ("Relates to").
• All 7 PR commits are authored and committed by dborup <kontakt@meshview.dk>.
• The delta outside the merge touches only packet_exclusions.go (+3/−1), one comment in packets.js and the feat(packets): server-side exclusion of packet types in /api/packets for the type filter and Hide CONTROL #242 E2E.

Not verified

• This round's benchmark used only the committed 30K store; my 300K measurement is from the previous round.
• I did not run cmd/ingestor locally (no diff), and I did not run the broad test-e2e-playwright.js locally (CI: 132/135, 3 skipped).
• Only Chromium was used; I did not test WebKit or Firefox.
• I did not use production-scale data. The browser check used a 1,633-tx fixture copy.
• I did not contact any staging or production system.