Skip to content

regions: trim back to the first Regions PR (#501) - #524

Merged
dborup merged 10 commits into
masterfrom
fix/523-regions-trim
Oct 23, 2026
Merged

dborup merged 10 commits into
masterfrom
fix/523-regions-trim

Conversation

@dborup-agent

@dborup-agent dborup-agent commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Relates to #523

Summary

Trims Regions back to the first Regions PR (#501): the period view (1h / 24h / 7d) with retained-history coverage and /api/regions/observed. Everything added after it is reverted for its memory and recompute cost.

Plan / commits

# Commit What
1 fded234f git revert -m 1 6fe9603a: #510 budget redesign and capture diagnostics
2 a7ef1bc8 git revert -m 1 45c96070: #507 per-bucket/per-scope evidence, membership capture
3 e4274365 git revert -m 1 c6574b46: #505 Connections view, /api/regions/routes, StoreObs transport fields
4 67e7fc7e git revert -m 1 9933d05d: #503 Health & trends
5 f4a20a2e git revert -m 1 5ff889db: #502 Timeline and comparison, /api/regions/history
6 80d4fb2f Re-applies only the generic CI part of a3f3716: scripts/report-e2e-shards.js imports coverage from every successful shard, plus its two tests in test-ci-shards.js
7 63bf299c Regression guards for the trim, and docs/regions-observed.md no longer lists the removed views as upcoming milestones

All five reverts applied cleanly on d18ffd2c, newest first.

What stays

Leftover sweep

I grepped cmd/, public/, docs/, scripts/, .github/ and the tests for regions/history, regions/routes, regions-history.js, regions-routes.js, captureAt, cacheObservationWireEvidence, wireTransport, transportKnown, regionMemberships, RegionsSnapshotMemory, regionsSnapshot, stepEndpointLimit and the membership budget code. Nothing is left.

The remaining "evidence"/"membership" hits are generic: relay-evidence rules from #501, node region cache, configured_scope, and Areas. The only leftover that belonged to the reverted features was the "Following milestones" section in docs/regions-observed.md. It listed timeline, health and connections as planned work. It is replaced by a short note on why they were removed.

Tests (each red on d18ffd2c, green here)

  • cmd/server/regions_trim_523_test.go:
    • GET /api/regions/history and /api/regions/routes return 404 through the production router, and /api/regions/observed stays registered.
    • Neither removed endpoint is in OpenAPI, and observed still is.
  • test-regions-map.js:
    • No file in public/ (excluding vendor) or docs/ references the removed endpoints or scripts.
    • regions-history.js and regions-routes.js do not exist.
    • The doc no longer has the milestones section.
  • test-regions-map-e2e.js: at 1440px and 390px the controls are exactly Period, Scope and Refresh, with no view/timeline/compare/health elements. The page never requests /api/regions/history|routes.
  • test-ci-shards.js: the two re-applied tests fail without the script change.

Perf justification

The reverts remove work from the 5-minute relay snapshot recompute (computeRepeaterRelaySnapshot). Before, it captured route, history, membership and evidence cells for every transmission and observation and kept them in a published snapshot. They also remove two fields from StoreObs, which fit in existing padding so the struct size stays at 200 B.

Local benchmark: a temporary uncommitted benchmark ran the same 30K-transmission / 2K-node / 8-scope fixture as master's BenchmarkRegionsRoutesComputeMixedEvidence30K2K through computeRepeaterRelaySnapshot(24), 6 runs each:

ns/op B/op allocs/op
master d18ffd2c ~136–143 ms ~132.5 MB ~488.5K
this branch ~32.9–34.4 ms ~28.4 MB ~127.5K

The unscoped BenchmarkConfirmedRelayBulk30KPackets2KNodes (present in both trees) goes from ~130.9K to ~100.5K allocs/op, with time unchanged (~31 ms). On master, BenchmarkRegions168HBudgets reported 3.6–7.3 MB of retained Regions snapshot, which this PR removes.

The coordinator measures RSS and relay recompute time on staging before and after; this PR does not touch staging or prod.

Round 2 additions

  • Cherry-picked 29cd1a05 (test(e2e): await initial theme before node table style checks) as 43b9c391. It is an E2E deflake, not Regions code, and stays. After the origin/master merge test-e2e-playwright.js is byte-identical to origin/master.
  • test-regions-map.js: the residual scan skips docs/release-notes/ and docs/RELEASE-NOTES-*, so a release note about the removal can name the removed endpoints. A fixture test proves it and proves stale docs and public/ are still flagged.
  • test-regions-map-e2e.js: a comment at the exact control set says to update it when a legitimate control is added.

Local validation

  • cmd/server: go vet . and go test ./... pass. go test -race passes on the regions/store/load/coverage/lock tests.
  • cmd/ingestor: go test ./... passes. go test -race passes on the region/store/load/retention tests.
  • sh test-all.sh passes: 259 files, 0 failed.
  • node test-frontend-helpers.js: 719 passed.
  • E2E against a local Go server on e2e-fixture.db, prepared as in CI (freshen, inline seed, migrate, seed files):
    • test-regions-map-e2e.js: 13/13
    • test-home-coverage-e2e.js: passed
    • test-issue-304-my-mesh-e2e.js: passed
    • test-issue-2027-my-mesh-node-page-e2e.js: 25/25
  • scripts/check-xss-sinks.sh --diff origin/master is clean. The non-test diff adds no map[string]interface{} and adds no hex colours. TestNoSQLUnderStoreLock (perf(store): move runtime SQL outside packet-store locks #495) passes.

Config / customizer

The default period should still be exposed in the customizer in a later milestone, as #501 noted. The thresholds and playback settings of the removed views no longer apply.

🤖 Generated with Claude Code

dborup and others added 7 commits October 23, 2026 06:36
This reverts commit 6fe9603 (PR #510), reversing changes made to
2dc2c29.

Regions is trimmed back to the first Regions PR (#501). Relates to #523.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-history"

This reverts commit 45c9607 (PR #507), reversing changes made to
6e58e02.

Regions is trimmed back to the first Regions PR (#501). Relates to #523.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…utes"

This reverts commit c6574b4 (PR #505), reversing changes made to
9933d05.

Regions is trimmed back to the first Regions PR (#501). Relates to #523.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This reverts commit 9933d05 (PR #503), reversing changes made to
5ff889d.

Regions is trimmed back to the first Regions PR (#501). Relates to #523.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mparison"

This reverts commit 5ff889d (PR #502), reversing changes made to
68f243d.

Regions is trimmed back to the first Regions PR (#501). Relates to #523.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Re-applies only the generic CI part of a3f3716 (from #502), which the
revert of #502 removed: aggregateShards imports coverage from every
verified shard, namespaces non-core shard files and rejects duplicate
targets. Its two tests in test-ci-shards.js come back with it.

Relates to #523.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pins the #523 trim: /api/regions/history and /api/regions/routes stay
unregistered and out of OpenAPI while /api/regions/observed stays;
public/ and docs/ carry no history/routes references or scripts; the
page shows only period, scope and refresh controls on desktop and
mobile and never fetches the removed endpoints.

docs/regions-observed.md no longer lists the removed views as upcoming
milestones and records why they were removed.

Relates to #523.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dborup-agent

Copy link
Copy Markdown
Collaborator Author

Rapport — CS-pve-agent3 PR#524 #523 — head 63bf299

Status: Draft PR ready for review: all five reverts plus the re-applied CI-shard change are in, every requirement has a test that is red on d18ffd2c and green here, every mutant was killed, and all CI jobs pass with no reruns.

Evidence tags: [T] test or command run locally, [A] analysis or reading of code/diff, [K] CI result.

Requirements

# Requirement Test (red on d18ffd2c → green on head) Mutants (all killed) Evidence
1 Revert #510, #507, #505, #503 and #502 with git revert -m 1, newest first regions_trim_523_test.go: removed endpoints return 404 through the production router (master gave 503), are absent from OpenAPI, and observed stays. test-regions-map.js: no references in public/ or docs/ and no removed scripts. test-regions-map-e2e.js: only Period, Scope and Refresh at 1440px and 390px, and no history/routes requests. On master's public/ the E2E failed with 1440px selects. M1 re-register /api/regions/history; M2 re-add GET /api/regions/routes to OpenAPI; M3 re-add the regions-history.js script tag; M4 restore a "Region view" select; M5 fetch /api/regions/history on init [T] [K]
2 One commit re-applying only the CI-shard coverage import from a3f3716 (80d4fb2f) test-ci-shards.js: tests 8 and 9 fail without the script change (9 pass / 2 fail) and pass with it (11/11). Both files are identical to origin/master. Drop shard namespacing; core shard only; drop duplicate-target guard; skip JSON parse [T] [A]
3 Leftover sweep, removing only what belongs to the reverted features Grep found no leftovers in cmd/, public/, docs/, scripts/, .github/ or the tests. The one leftover, the "Following milestones" section in docs/regions-observed.md, is replaced by a removal note and guarded by test-regions-map.js. Restore the ## Following milestones heading [T] [A]
4 Keep #501 intact The Regions files equal post-#501 68f243d8 except for #511 hunks and the doc section. regions_observed_test.go and the regions E2E (13/13) pass. Covered by M1/M4: the guards fail if the period view's controls change [T] [A]
5 Keep #511 intact history_coverage_509_test.go passes, including -race. #511 files are untouched. Skip loading re-heard transmissions → TestCoverage509StagingShapeBackgroundLoad fails [T] [A]
6 #495 lock rule (no SQL under PacketStore.mu) TestNoSQLUnderStoreLock passes Hold s.mu across the re-heard SQL query → fails [T]
7 cmd/server read-only, no new map[string]interface{} Non-test diff adds 0 occurrences and no write paths. readonly_invariant tests pass in go test ./.... n/a (nothing added) [T] [A]
8 No hardcoded colours, XSS gate clean 0 hex colours added. scripts/check-xss-sinks.sh --diff origin/master exits 0. n/a [T]
9 Fork guards unchanged deploy.yml has 9 and release-fast-path.yml has 1. No diff under .github/. n/a [T] [A]

Local runs

Suite Result Evidence
cmd/server go vet . and go test ./... ok [T]
cmd/server go test -race on Region/Store/Load/Chunk/Coverage/Lock/SQL/Relay/Activity tests ok (420s) [T]
cmd/ingestor go test ./... ok [T]
cmd/ingestor go test -race on Region/Store/Load/Retention/Prune/Scope/IATA tests ok [T]
sh test-all.sh 259 files passed, 0 failed [T]
node test-frontend-helpers.js 719 passed [T]
node scripts/run-go-server-shard.js --validate ok [T]
E2E against a local Go server on e2e-fixture.db, prepared as in CI: regions, home coverage, My Mesh #304, My Mesh Kpa-clawbot#2027 13/13, pass, pass, 25/25. The server was stopped by pid afterwards. [T]

Perf (local benchmark; staging is measured by the coordinator)

A temporary, uncommitted benchmark ran computeRepeaterRelaySnapshot(24) on the same shape as master's mixed-evidence fixture (30K tx / 2K nodes / 8 scopes), 6 runs each:

ns/op B/op allocs/op
master d18ffd2c ~136–143 ms ~132.5 MB ~488.5K
head ~32.9–34.4 ms ~28.4 MB ~127.5K

The unscoped BenchmarkConfirmedRelayBulk30KPackets2KNodes goes from ~130.9K to ~100.5K allocs/op, with time unchanged. [T]

CI per job (run 39914615813, first attempt, no reruns)

Job Result
Go Server Tests (1/3, 2/3, 3/3) pass [K]
Go Ingestor Tests pass [K]
Go Libraries & CLI pass [K]
Go Build & Test pass [K]
JavaScript Unit Tests & Lint pass [K]
Script & Container Unit Checks pass [K]
Verify Go Test Assignments pass [K]
Playwright E2E Tests (1/3, 2/3, 3/3) pass [K]
Verify E2E Shards & Coverage pass [K]
Validate Docker Image pass [K]
Build & Publish Docker Image pass [K]
Release Artifacts, Publish Badges & Summary skipping (not run for PRs) [K]

The known flaky test #271 did not fail.

Remaining

@dborup

dborup commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Review — CS-Minimax PR#524 — head 63bf299

Dom: REQUEST CHANGES. One small fix is needed: the #502 revert also dropped a generic E2E deflake (29cd1a05) that should be re-applied, just as the CI-shard change was.

Evidence tags: [T] test or command I ran myself, [A] analysis or reading of code/diff, [K] CI result.

Reviewed head 63bf299caa10fbda0383e3cfca817eace5102e79, checked with git ls-remote before and after the review. Merged tree: git merge-tree --write-tree origin/master 63bf299c → 0066120f, against origin/master a7780350. It merges cleanly.

Findings

# Severity Finding Evidence
1 Must fix Revert 5 (f4a20a2e, #502) also removes 29cd1a05 "test(e2e): await initial theme before node table style checks" from test-e2e-playwright.js (−7 lines). That commit is not Regions code. It fixed a detached-table race in the #382 node stats table rows follow the rounded corners E2E, which #502's first CI run exposed and which was reproduced locally there (isConnected=false, empty computed styles). #523 keeps every non-Regions change from the period, and this PR's own scope is "only Regions-specific code disappears". The PR description does not mention the file. Fix: git cherry-pick 29cd1a05 on top. It applies cleanly on head, and afterwards test-e2e-playwright.js is byte-identical to origin/master. [A] [T]
2 Nit The test-regions-map.js residual scan covers all of docs/, including docs/release-notes/. A legitimate release-note line such as "Removed /api/regions/history and /api/regions/routes (#523)" makes the unit test fail (verified, see below). Consider excluding docs/release-notes/ (and docs/RELEASE-NOTES-*), or scanning only public/, docs/api-spec.md and docs/regions-observed.md. [T]
3 Nit test-regions-map-e2e.js pins the exact control set (['Region period', 'Region scope'] and ['Refresh']). That is what #523 asks for, but any future, legitimate control on the page will need this assertion edited. It would help to say so in the test comment ("update when adding a control; the guard is about the removed view/timeline/compare/health controls"). The class-based check (.regions-health, .regions-timeline, .regions-compare-label) and the removedRequests check are the durable parts. [A]
4 Info The ## Following milestones guard only pins one heading string, so it is a weak guard. That is fine as a doc tripwire. [A]

Answers to the review points

1. Are the five reverts faithful? Yes. [A]

2. Was anything outside Regions lost? Only finding 1. [A] [T]

3. Does the CI-shard re-apply equal a3f3716? Yes. [A] [T]

  • git diff 80d4fb2f^ 80d4fb2f and git diff a3f37164^ a3f37164 -- scripts/report-e2e-shards.js test-ci-shards.js have the same patch-id (a9ee5984).
  • Both files are identical to origin/master on head.
  • Head's test-ci-shards.js against the post-revert script (f4a20a2e): 9 pass and 2 fail, namely the two re-applied tests. With head's script: 11/11. [T]

4. Are the guard tests meaningful? Yes. Two are slightly brittle (nits 2 and 3).

  • On base d18ffd2c, regions_trim_523_test.go fails both tests: 503 instead of 404, and both endpoints are present in OpenAPI. test-regions-map.js fails with "removed Regions history/routes references remain". [T]
  • They go through the production-style router, so a stray SPA fallback cannot mask a 404. observed is positively asserted, so deleting the whole Regions route would not pass as "trimmed". [A]

Acceptance criteria (#523)

Criterion Status Evidence
Five reverts + CI-shard re-apply in one PR Met (but see finding 1) [A]
No references to history/routes endpoints or scripts in server, frontend, index.html, OpenAPI or docs Met. git grep over the head tree finds them only in the guard tests and in the explanatory note in docs/regions-observed.md, which mentions neither path. Live server: /api/regions/history and /routes → 404, /observed?window=24h → 200. [T]
Regions page desktop/mobile: period view with windows and coverage, no dead controls Met. Screenshots at 1440px and 390px show only Period, Scope and Refresh, plus partial-coverage notice and map. [T]
go test ./... server and ingestor, -race on regions/store/load Met [T]
Frontend unit + regions and home/My Mesh E2E Met [T]
#495 lock rule TestNoSQLUnderStoreLock passes in the full suite and under -race [T]
Perf justification in PR Present. I did not re-run the benchmark. [A]

Invariants

  • cmd/server is still read-only. The non-test diff adds no INSERT/UPDATE/DELETE/Exec and no map[string]interface{}. [A]
  • No hex colours added under public/. [A]
  • scripts/check-xss-sinks.sh --diff a7780350 on head exits 0. [T]
  • Fork guards: deploy.yml 9 if: guards and release-fast-path.yml 1. There is no diff under .github/. [T] [A]
  • No closing keywords in commits or the PR body ("Relates to regions: trim back to the first Regions PR (#501) — revert #502, #503, #505, #507, #510 #523"). [A]
  • All 7 commits have author and committer dborup <kontakt@meshview.dk>, with a Co-Authored-By trailer. [A]

Tests run (merged tree 0066120f, unless noted)

Suite Result
cmd/server go vet . + go test ./... -count=1 ok (69.8s) [T]
cmd/ingestor go test ./... -count=1 ok (113s) [T]
cmd/server go test -race (Region, Coverage509, NoSQLUnderStoreLock, ChunkedLoad, BackgroundLoad, RelaySnapshot, ReadOnly) ok (214s) [T]
sh test-all.sh 259 passed, 0 failed (259 files); some flock-only shell cases are SKIPPED on macOS [T]
node test-frontend-helpers.js 719 passed, 0 failed [T]
node test-regions-map.js, node test-ci-shards.js pass, 11/11 [T]
E2E against a local Go server on e2e-fixture.db, prepared as in CI (freshen, inline seed SQL, corescope-migrate, seeds 2073/199/245/122) test-regions-map-e2e.js 12/12 (13/13 with REGIONS_SCREENSHOT_DIR); test-home-coverage-e2e.js 12/12; test-issue-304-my-mesh-e2e.js pass; test-issue-2027-my-mesh-node-page-e2e.js 25/25; test-e2e-playwright.js 154/157, 3 skipped, including both #382 node stats table rows ... (one run, so finding 1's race did not show here) [T]
CI run 39914615813, attempt 1 All jobs pass: Go Server 1/2/3, Go Ingestor, Go Libraries & CLI, Go Build & Test, JS Unit & Lint, Script & Container, Verify Go Test Assignments, Playwright 1/2/3, Verify E2E Shards & Coverage, Validate Docker Image, Build & Publish Docker Image. Release/Badges/Deploy were skipped, as they are for PRs. Known flakes #256 and #267 did not fire. [K]

Own mutants (all on the merged tree)

# Mutant Result
M1 Re-register /api/regions/history in routes.go Killed: TestRegionsTrimRemovedEndpointsReturn404 (503 ≠ 404) [T]
M2 Re-add "GET /api/regions/routes" to routeDescriptions() Killed: TestRegionsTrimRemovedEndpointsNotInOpenAPI [T]
M3 Re-add a regions-routes.js script tag in index.html Killed: test-regions-map.js [T]
M4 Add a "Region view" <select> to the Regions controls (served by a second local server) Killed: E2E 1440px selects [T]
M5 fetch('/api/regions/' + 'history') on init, concatenated to evade the static scan Static scan passes, as expected. Killed by the E2E removedRequests assertion [T]
B1 Brittleness probe: release-note line naming the removed endpoints test-regions-map.js fails → nit 2 [T]

Not verified

  • Staging RSS and relay recompute time. Left to the coordinator, as the PR says; I did not touch staging or prod.
  • I did not re-run the author's temporary benchmark (master ~136–143 ms / 132.5 MB vs head ~33 ms / 28.4 MB).
  • I did not repeat the #382 node stats table E2E enough times to reproduce the original race. Finding 1 rests on feat(regions): timeline playback and period comparison #502's recorded CI failure and its local reproduction, not on a new failure here.
  • The full ingestor -race run was not done. The PR does not touch cmd/ingestor/.
  • I did not inspect the Verify E2E Shards job's log to confirm that coverage was actually imported from every shard. I checked only the job's status.

@dborup-agent

Copy link
Copy Markdown
Collaborator Author

Rapport — CS-pve-agent3 PR#524 runde-2 — head a1a3e91

Review feedback addressed (commit a1a3e910)

  1. Finding 1 (must fix): E2E deflake. 29cd1a05 is cherry-picked as 43b9c391. I merged origin/master (a7780350) as 344b050f. git diff origin/master -- test-e2e-playwright.js is now empty, so the file is byte-identical to master. [T]
  2. Nit 2: residual scan. test-regions-map.js now skips docs/release-notes/ and docs/RELEASE-NOTES-*. A fixture test checks that release notes naming the removed endpoints pass. It also checks that a stale non-release doc and a public/ file are still flagged. I wrote the test first and it failed with "release notes may mention the removal". It passed after the fix. [T]
  3. Nit 3: control-set comment. test-regions-map-e2e.js has a comment at the exact control set. It says the guard exists to catch the removed view, timeline, compare and health controls, and that the lists must be updated when a legitimate control is added. [A]
  4. The PR description now mentions the cherry-pick and the two test changes. The PR is still draft.

Tests

  • sh test-all.sh: 259 passed, 0 failed. node test-frontend-helpers.js: 719 passed. [T]
  • go test ./... passes in cmd/server and cmd/ingestor. [T]
  • Local Go server on a scratch copy of e2e-fixture.db, prepared as in CI (freshen, inline seed, migrate, seed files). [T]
    • test-regions-map-e2e.js: 13/13.
    • test-e2e-playwright.js: 154/157 passed, 3 skipped. [T]

Mutants

  • Finding 1: the pre-cherry-pick test-e2e-playwright.js (from 63bf299c) differs from master, so the byte-identity check fails on it. [T]
  • Nit 2: with the release-note exclusion replaced by false, test-regions-map.js fails. [T]
  • Nit 3 is a comment only, so I made no mutant. [K]

CI (run 39923893642), per job

All jobs pass. No re-run was needed, and #271 and #301 did not appear.

  • Go Build & Test, Go Ingestor Tests, Go Libraries & CLI, Go Server Tests 1/3, 2/3 and 3/3
  • JavaScript Unit Tests & Lint, Script & Container Unit Checks, Verify Go Test Assignments
  • Playwright E2E 1/3, 2/3 and 3/3
  • Verify E2E Shards & Coverage, Validate Docker Image, Build & Publish Docker Image
  • Deploy Staging, Release Artifacts and Publish Badges are skipped, as expected for a PR.

I did not touch stg or prod.

@dborup
dborup marked this pull request as ready for review October 23, 2026 08:55
@dborup
dborup merged commit 0350322 into master Oct 23, 2026
18 checks passed
@dborup

dborup commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Coordinator: round-2 verified at a1a3e91. Finding 1 is fixed: test-e2e-playwright.js, scripts/report-e2e-shards.js and test-ci-shards.js are byte-identical to master. Nits 2 and 3 are done and come with a fixture test. CI is green. Merged on the strength of Minimax' review plus this verification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants