Repository navigation
feat(reach): Reach leaderboard with a shared visible-population rank - #69
Merged
Merged
Conversation
Answers "my node is Rank #X on its Reach page, who is #1?". Backend (cmd/server/reach_rank.go): - One neighbour-degree snapshot (60s TTL) now backs both the Reach Rank card and the new GET /api/reach-rank. Rebuilds are singleflight-shared, run on a detached bounded context, and never publish a failed or partial read (rows.Err and scan errors fail the load); a failed refresh keeps serving the last complete snapshot with its own timestamp. - Rank contract: neighbours = distinct neighbours in neighbor_edges; ranked population = endpoints with a node/observer record that are not node-/observer-blacklisted or hidden by name prefix; competition ranking (1, 1, 3), ties in pubkey order. Blacklist / hidden-prefix generations re-rank from the cached snapshot without a DB read. - Previously the rank counted hidden and blacklisted nodes and pubkeys without a Reach page (incl. the empty pubkey), so a hidden node could push every visible node down and was counted in the total. - /api/nodes/{pk}/reach applies rank fields at serve time from the same view (cached bodies are re-marshalled once per view change) and adds rank_status / rank_snapshot_at; an unreadable snapshot marks the rank "unavailable" instead of caching a zero rank. - /api/reach-rank: q (<=64 chars, name/pubkey substring), offset (>=0), limit (1-100, default 50); search and paging keep global placements. Frontend: new #/reach-rank page (search, 50-row pages, snapshot age, "Historical neighbour count — not a measure of radio quality or range"), and a "View leaderboard" link plus "Not ranked"/unavailable states on the Reach Rank card. No nav changes. Tests: Go unit/handler tests (ranking, visibility after warm cache, consistency with /reach, cache hit/expiry, singleflight, canceled waiter, cold/partial/refresh DB errors), benchmarks, a vm unit test, a Playwright E2E, and /reach-rank in the axe gate; both new JS tests are registered in deploy.yml. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…background Independent review follow-ups for the Reach leaderboard: - Neighbour count now uses valid edges only: both endpoints must be 64-hex MeshCore pubkeys (case-insensitive) and differ; case and orientation variants of one pair count once. Legacy rows (e.g. the fixture's 55 empty-endpoint edges) are filtered at computation time, nothing is deleted. Applies to the shared snapshot, so the Reach page and the leaderboard stay identical (fixture #1: 9, not 10). - Pubkeys are "known" exactly as buildNodeInfoMap knows them (a node row, or an observer row with id and name), so every placement has a Reach page; every node/observer name of a pubkey is checked against the hidden prefixes. - Stale-while-revalidate: an expired snapshot is served while one background rebuild refreshes it, so Reach cache hits never wait on the aggregate; only a cold start waits. A failed rebuild backs off 15s instead of re-querying on every request, and a panic in the load becomes an error instead of crashing the process (DoChan). - View rebuilds after a visibility change are serialised so a burst builds one view. - Reach page shows the Neighbours / Rank cards also for nodes without a reliable path token; the leaderboard clips queries by code point (no split emoji / URIError) and clamps absurd ?page values. - Tests for all of the above, including the committed fixture; docs and OpenAPI describe the valid-edge rule and exact limit semantics. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nctions The valid-edge rule as SQL (lower/GLOB + DISTINCT) made the snapshot rebuild ~4x slower than master's plain GROUP BY (885 ms vs 223 ms on 20k nodes / 100k edges). Scan neighbor_edges once and do the checks in Go: a row already in the form the ingestor writes (lower-case, node_a < node_b) is unique by the primary key and counted directly; other valid rows (upper-case or reversed) are canonicalised and deduplicated against each other and against an existing canonical row with one bulk row-value lookup. Same results (tests unchanged), and the rebuild is now faster than master's degree-only query: 5.0 / 27 / 135 ms for 1k / 5k / 20k nodes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…pshot - Background-refresh test: wait until the held load has started, check results on the test goroutine, and wait for the snapshot to publish so no goroutine outlives the test (was flaky at -cpu 1 and could race the next test's hook swap). - A queued refresh re-checks the backoff instead of starting a second load; only real load attempts record failures. - Stale requests don't each join the background refresh (in-flight flag). - The edge scan, the odd-row lookup and the name queries run in one read transaction, so they see one database state. - NULL endpoints are skipped instead of failing the load. - Hidden-name check also uses a node's inactive_nodes name, but only while it has no nodes row (that table keeps a returning node's old row, so for an active node the name is stale) — same rule as the Reach visibility fix. - Docs: which requests wait for a snapshot read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Match the Reach visibility rule: a hidden node that returns with a nameless advert (stored as an empty name) keeps its hidden inactive_nodes name instead of being ranked again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review found that the stale-path in-flight flag could be set by a request whose DoChan joined an already-finishing refresh after that refresh had cleared the flag; nothing cleared it again, and the rank snapshot stopped refreshing until restart. The flag only avoided a bounded pile-up of joined channels during a slow refresh, so remove it: stale requests start or join the singleflight refresh directly, and the in-closure backoff re-check still prevents repeat loads after a failure. Adds a liveness property test (expired snapshot always refreshes under concurrent stale requests) and captures the stale pointer before releasing the held load in the background-refresh test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…napshot Probe shape from review: each round, callers keep calling until the refresh publishes; the round fails if the expired snapshot is never replaced. Cheap (≈0.4s under -race); the removed flag's race window is narrow, so this is a property check rather than a reliable reproduction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Integrates the merged Reach privacy/visibility work (#68, identity_visibility.go) with the leaderboard's rank-view cache (reach_rank.go). Real conflicts in cmd/server/node_reach.go and docs/api-spec.md — both files touched the response cache and the handler — resolved by combining both freshness dimensions on the cache entry: - hiddenKey (#68): which identities the served body's links / direct_observers omit, from the live visibility check. - viewID (#69): which reachRankView the served body's rank fields were applied from. reachBody now re-marshals only when either one moved on since the entry was computed; both are re-checked on every serve, cached or not, so neither a blacklist/prefix/rename change nor a rank-view change waits for the 5-minute cache TTL. computeNodeReach no longer sets the rank fields itself (unchanged from #69's design) — the handler applies them from the shared rank view (applyReachRank) after visibility filtering, so a node's own NeighborDegree still counts a hidden neighbour's edge (a number, not an identity) while DegreeRank/NodesWithEdges reflect only the visible population. Also fixes a parallel visibility rule found during review: reachRankVisible in reach_rank.go reimplemented the same IsBlacklisted/IsObserverBlacklisted/IsNameHidden combination as identityHidden instead of calling it, so the two endpoints could have drifted apart on a future change to the shared rule. It now delegates directly. Updated two of #69's own tests that asserted the pre-merge cache API/contract (reachCacheSetBody's new viewID param; NothingHiddenBodyUnchanged and FilteringLeavesRankFieldsUnchanged now apply the rank view before comparing, and assert the agreed 'visible population' contract — NodesWithEdges/DegreeRank reflect only visible nodes, NeighborDegree does not). Added TestNodeReach_HiddenNeighbourCountedNotListedOrRanked (cross-endpoint: a hidden neighbour is counted but never listed or ranked) and TestReachRankVisible_MatchesIdentityHidden (equivalence probe against identityHidden across blacklist/prefix/name-slot combinations), and strengthened TestReachRank_OnlyValidEdgesCount with a self-edge that exists only in non-canonical case form, closing a gap where the self-edge skip could be removed without any existing test catching it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
BenchmarkReachAndRankParallel measures /api/reach-rank and
/api/nodes/{pk}/reach served concurrently (3:1) on a warm
snapshot/view/cache, at GOMAXPROCS parallelism. Confirms the shared
degree-snapshot/rank-view mutex is not a bottleneck under concurrent
readers — part of the performance verification for the PR #68 sync.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Synced with master (#68 merged)
dborup/CoreScope#68 (Reach privacy fix) is now merged into master. This branch has been updated onto the new master with an ordinary merge commit (no rebase, no force-push), and the leaderboard's visibility check now calls #68's shared
identityHiddendirectly (see below) instead of its own copy of the same rule.Old head:
13f4e61c. New head:fc4ba5ab, one commit on top of the merge commit130f9cc8(parents13f4e61c, this branch, andd9ed1e16, master — fix(reach): hide blacklisted and hidden identities on the Reach page #68 + PRs port(upstream#1860): Phosphor icon instead of emoji in the RX coverage header #28/port(upstream#2007): 23 more names in the channel rainbow list #50/fix(packets): keep the Details summary on one line so rows stay bounded #67). The extra commit only adds a benchmark (see Performance after the merge); it changes no production code.Real conflicts:
cmd/server/node_reach.goanddocs/api-spec.md— both PRs touched the Reach response cache and handler. Resolved by combining both freshness dimensions on the cache entry:hiddenKey(fix(reach): hide blacklisted and hidden identities on the Reach page #68): which identities the served body'slinks/direct_observersomit, from the live visibility check.viewID(feat(reach): Reach leaderboard with a shared visible-population rank #69): whichreachRankViewthe served body's rank fields were applied from.Both are re-checked on every serve, cached or not — neither a blacklist/prefix/rename change nor a rank-view change waits for the 5-minute cache TTL.
computeNodeReachstill doesn't set the rank fields itself (unchanged from this PR's original design) — the handler applies them from the shared rank view (applyReachRank) after visibility filtering, so a node's ownneighbor_degreestill counts a hidden neighbour's edge (a number, not an identity) whiledegree_rank/nodes_with_edgesreflect only the visible population.Fixed during the merge:
reachRankVisibleinreach_rank.goreimplemented the sameIsBlacklisted/IsObserverBlacklisted/IsNameHiddencombination asidentityHiddeninstead of calling it — a parallel rule that could have drifted from fix(reach): hide blacklisted and hidden identities on the Reach page #68's shared one on a future change. It now delegates directly:return !identityHidden(cfg, pubkey, id.names...).Two of this PR's own tests asserted the pre-merge cache API/contract and were updated:
reachCacheSetBodygained theviewIDparameter;NothingHiddenBodyUnchangedandFilteringLeavesRankFieldsUnchangednow apply the rank view before comparing, and the latter now asserts the agreed contract precisely —nodes_with_edges/degree_rankreflect only the visible population (2 vs 4 in its fixture, with/without hiding), whileneighbor_degreedoes not change (3 either way).Summary
First version of a Reach leaderboard. It answers: "My node is Rank #X on its Reach page. Who is #1?"
New page
#/reach-rank. For each ranked node it shows:The page also offers:
New endpoint
GET /api/reach-rank?q=&offset=&limit=.Reach page Rank card changes:
#0 / N;Not changed: the main menu and Priority+ are untouched. No schema changes, migrations, data deletion or new dependencies.
Rank definition (one definition, one snapshot)
Rank is an all-time neighbour count. It is not range in km, not traffic volume, and not links in the selected time window.
neighbor_edgesrow whose endpoints are both MeshCore pubkeys (exactly 64 hex characters, case-insensitive) and differ from each other.neighborEdgesMaxAgeDaysretention. Case and orientation variants of one pair count once.nodesrow, or anobserversrow with a name. A pubkey is excluded when it is node-blacklisted or observer-blacklisted, or when any of its names starts with a hidden-name prefix: its node name, its observer name, or itsinactive_nodesname while it has no namednodesrow./api/nodes/{pk}/reachand/api/reach-rankread rank, total and neighbour count from the same snapshot and expose its time (rank_snapshot_at/snapshot_at). The E2E test asserts equality for real fixture nodes; the reviewer checked all 75.Changes to the existing Reach numbers
Both points were agreed before implementation.
Visible population. The old snapshot grouped every endpoint in
neighbor_edgeswith no filter, so three kinds of pubkey sat in both the ranking andnodes_with_edges:A hidden node above a visible one would have pushed it down, revealing that the hidden node exists.
Legacy-data correction. The CI fixture holds 55 legacy edges with an empty endpoint (55 of 110 rows). The current ingestor no longer writes these rows; they expire with edge retention. Under the old count:
""was ranked Foreign Traffic analytics + Scopes tab redesign #1 with 55 "neighbours";Those rows are now ignored at computation time; nothing is deleted. The effect on the fixture:
1000💥Repeater G2ESP1 Gilroy Repeaternodes_with_edges)The 31 nodes whose only edges were legacy rows now show "Not ranked" with 0 neighbours. The Reach page's "Neighbours" card uses the same rule, so it changes in the same way.
Performance after the merge
Fixture contract, confirmed live on the migrated, freshened CI e2e fixture:
GET /api/reach-rankreturnstotal=75,#1(1000💥Repeater G2) with 9 neighbours,#2(ESP1 Gilroy Repeater) with 5. The same node's ownGET /api/nodes/{pk}/reachreportsneighbor_degree=9,degree_rank=1,nodes_with_edges=75, andrank_snapshot_atbyte-identical to the leaderboard'ssnapshot_at— same snapshot, confirmed live, not just asserted in a test.Cache-hit cost of the merge integration.
BenchmarkNodeReachCacheHitVisibility(Scale)(from #68, unchanged, run before/after on the same file) isolates the added cost of looking up the shared rank view on every Reach cache hit, on top of #68's own visibility check:d9ed1e16, #68 alone)130f9cc8)Medians of
-count=3 -benchmem, Apple M2 Pro. Measured under heavy contention — this machine was running many other agent sessions concurrently (load average 60–90 throughout, roughly 5–8× normal), so these numbers are directional, not precise. The consistent part across runs: the "no prefix" cases (the cleanest isolation of the new per-request cost, since the visibility name lookup itself is skipped without a configured prefix) show +5–10 µs, matching one added mutex-guarded map lookup (currentReachRankView→v.lookup) per request, independent of report size. The "prefix configured" deltas are noisier and don't move consistently with size, which is what you'd expect if the added constant cost (single digit µs) is being swallowed by run-to-run scheduling noise against a background of hundreds of µs to ms from the name lookup itself.Concurrent mixed traffic, added for this merge (
BenchmarkReachAndRankParallel, 3:1/api/reach-rankto/api/nodes/{pk}/reach,b.RunParallelatGOMAXPROCS=12), on a warm snapshot/view/cache:No sign of lock contention on the shared snapshot/view mutex even under 12-way concurrency on this heavily loaded machine — throughput holds (actually improves slightly at larger N, likely just more requests hitting the O(1) unfiltered-page path).
Unchanged from the original benchmarks (snapshot rebuild, view rebuild, warm page/search — see below), because none of those paths were touched by the merge conflict resolution; only the Reach page's cache-hit path (above) and the new parallel benchmark are new measurements for this sync.
Implementation
Backend (
cmd/server/reach_rank.go; the snapshot moved out ofnode_reach.go)One read transaction per load. No per-row lookups. The steps:
neighbor_edges, validated and counted in Go;nodes,observersandinactive_nodesnames.Any query, scan or
rows.Err()error fails the whole load, so a partial read is never published. Names are read here rather than throughgetCachedNodesAndPM, because that cache swallows DB errors, which would silently drop every node.Refresh (stale-while-revalidate).
snapshot_at, while one background rebuild (singleflight) refreshes it./api/reach-rankreturns 500, never an empty leaderboard. It deliberately does not return 503: the SPA'sapi()treats 503 as warm-up and retries for about a minute.Ranked view. It is built from the snapshot and keyed by the blacklist and hidden-prefix generations. A visibility change re-ranks at once from the cached snapshot, with no DB read. View builds are serialised, so a burst of requests builds one view.
Reach response cache (5 min). It stores the report plus its body. Rank fields are applied at serve time from the current view, so each cached body is re-marshalled once per view change. An unreadable snapshot is reported as
rank_status: "unavailable"instead of caching0.Input validation.
qis at most 64 characters and must be UTF-8 (else 400).offsetmust be ≥ 0 (else 400).limitdefaults to 50; values above 100 become 100; zero, negative or non-numeric values give 50 (the existingclampLimit).Frontend.
public/reach-rank.jsreuses the.nq-*Reach styles, the.nodes-searchinput and.btn.replaceState), so Back from a Reach page restores them.?page=is clamped.--link-color, the A11y: contrast + typography pass to fix readability complaints Kpa-clawbot/CoreScope#1668 AA link token.Registration
/api/reach-rankis added toopenapi.goanddocs/api-spec.md./reach-rankis added to the axe gate'sROUTES/REGISTERED_PAGES.deploy.yml: only two test lines are added (node test-reach-rank.js, andtest-reach-rank-e2e.jsin the E2E list). Triggers, permissions,needsand the fork guards are unchanged.Screenshots (local, CI e2e fixture DB)
All screenshots come from a local Go server built from this branch, running on the committed e2e fixture (freshened and migrated as in CI), in headless Chrome. They were inspected visually; the files are not attached.
Cache and performance
Complexity.
neighbor_edgesplus linear reads of the name tables, at most once per 60 s, in the background and shared by all requests.Memory. One snapshot and one view per server, both bounded by the node count and replaced rather than grown.
Environment: Apple M2 Pro (12 cores), 32 GB, macOS 26.6.2, Go 1.26.0 (CI uses 1.27.1),
modernc.org/sqlitev1.34.5, file-backed DB in a temp dir.Datasets: synthetic and seeded (
reachRankBenchSetsinreach_rank_bench_test.go). Each node gets 4–5 random neighbours, and 2 % of nodes also have an observer row:Values are medians of
-count=5 -benchmemat the original feature head13f4e61c(unaffected by the merge — see Performance after the merge for what changed).GROUP BYonly; baseline)GET /api/reach-rankpage 1offset=N/2)q=node-01)What changed in the rebuild.
lower/GLOB+DISTINCT) made the rebuild about 4× slower than master: 885 ms at 20k nodes.sql.RawByteswas measured and gave no gain.Existing Reach hot path.
GET /api/nodes/{pk}/reachon a cache hit (1k dataset), measured with the same benchmark on both trees: master 21.7 µs, this PR 21.5 µs, both at 200 allocs/op. The rank-view lookup adds no measurable cost.Only local synthetic data was measured. There is no production benchmark, and no live Reach endpoints were scanned.
Tests (original, before the master sync)
All results below are from feature head
13f4e61c(before the master sync), except where noted, on the environment above.Go
cd cmd/server && go test -race ./...: ok (241.8 s).cd cmd/ingestor && go test ./...: ok (88.9 s).go vetclean; gofmt clean on every changed.gofile;git diff --checkclean.cmd/server/reach_rank_test.go:buildNodeInfoMapknows (including NULL-name observers); name fallback.inactive_nodes-hidden identities, in both directions. Checked after the cache is warm, viaSetHiddenNamePrefixes/SetNodeBlacklist, including a cached Reach body being re-ranked./reachfor every row.rows.Err) is never published; no DB configured.-cpu 1,2,4under-racewith no failures. The reviewer independently ran 50–300× per test.cmd/server/reach_rank_bench_test.go(see above).Frontend and CI gates (registered in
deploy.yml)test-reach-rank.js. That test runs in a vm sandbox and covers:hrefbreakout;index.htmlscript tag.--diffagainst master: clean.node --checkpasses on the changed JS files.Browser (local server on the fixture, headless Chrome through the Playwright library)
test-reach-rank-e2e.js: 14/14. It covers:/reach-rank(desktop and mobile × dark and light, 13 rules): 0 violations.test-issue-1630-reach-mobile-e2e.js: 7/7.test-node-reach-coverage-e2e.js: SKIP, because the fixture has client RX coverage disabled, the same as in CI.test-node-reach-e2e.jstimes out locally on the fixture, on master's frontend too.Tests (fresh, on the merged head
130f9cc8)Go
cd cmd/server && go test -race ./...: ok (481 s). No races.go vetclean; gofmt clean on every changed.gofile;git diff --checkclean.TestReachRank_OnlyValidEdgesCount,TestReachRank_NullEndpointSkipped,TestReachRank_FixtureLegacyEdgesNotCounted,TestReachRank_ConsistentWithNodeReach.TestNodeReach_HiddenNeighbourCountedNotListedOrRanked— cross-endpoint: a hidden neighbour of a visible node is (a) absent from that node'slinks, by pubkey or name, (b) absent from the leaderboard's rows and unfindable by any search term (pubkey, pubkey prefix, name, name substring), yet (c) still counted in the visible node's ownneighbor_degree; the leaderboard'stotalreflects only the visible population.TestReachRankVisible_MatchesIdentityHidden—reachRankVisiblecompared directly againstidentityHiddenacross every blacklist/observer-blacklist/hidden-prefix combination and 7 name-slot patterns (missing, empty, plain, hidden, mixed, case variants) — an equivalence probe against the parallel-rule bug fixed during the merge, so it can't silently reappear.TestReachRank_OnlyValidEdgesCountstrengthened: added a self-edge that exists only in non-canonical (mixed-case) form, with no literal canonical row for the same pair. The original fixture's two self-edge cases both happened to also exist as an exact-match canonical row, which meantcanonicalEdgesExisting's existing-row check accidentally masked a missing self-edge guard (found via mutation testing below) — the new case closes that gap.Mutation / negative controls (each applied to a scratch copy of the merged code, confirmed to make at least one test fail, then reverted):
pubkeyFormaccepts invalid input as validTestReachRank_OnlyValidEdgesCountla == lb) removedTestReachRank_OnlyValidEdgesCount(only after strengthening — see above; the original fixture did not catch this)canonicalEdgesExisting) disabledTestReachRank_OnlyValidEdgesCountbuildReachRankViewTestReachRankView_HiddenAndBlacklistedNeverPlaced,TestReachRank_VisibilityChangesAfterWarmCache,TestNodeReach_HiddenNeighbourCountedNotListedOrRanked(3 independent tests)reachBody'sviewIDfreshness check bypassed (cached rank never refreshes)TestReachRank_VisibilityChangesAfterWarmCacheFrontend / E2E, run locally against a fresh copy of the CI fixture (freshened + seeded + migrated,
corescope-serveron port 13594):node test-reach-rank.js: pass (unit test, vm sandbox).node test-xss-escape-sinks.js: 34/34 pass.npx eslint public/*.js(eslint@8, matching CI): 0 errors (89 pre-existing warnings, none in the changed files).test-reach-rank-e2e.js: 14/14, including the fixture's 9/not-10 assertion, mobile 375×812, keyboard (search → Tab → Enter → Back), hostile node names, and the API-failure error state.test-issue-1630-reach-mobile-e2e.js(existing Reach mobile coverage): 7/7.test-a11y-axe-1668.js, full route list: 0 violations across 120 cells (2 viewports × 2 themes × 30 routes, including/reach-rank).Independent review
A separate reviewer agent, which did not write the code, reviewed the branch in four rounds. It read the commits through git and ran its own probes in isolated copies. The focus was the rank contract, visibility, cache and concurrency, failure modes, performance, and regressions in the existing Reach page.
Round 1 (
c9b2022f): no blockers. SHOULD-FIX and NIT findings, all addressed:?pageclamping;Two pre-existing Reach privacy gaps were moved to the prerequisite PR by product-owner decision. The valid-edge contract was also decided then.
Round 2 (
20809fdd,15026d21): no blockers. Addressed:Round 3 (
95e2cc3f,f3919f13): one BLOCKER. The new "refresh in flight" flag could stick at true and stop all refreshes until a restart; the reviewer reproduced it in stress runs. Fixed by removing the flag (3b1a91a4).Round 4 (
3b1a91a4): no BLOCKER remains. The reviewer's 3000-round stress probe got stuck in 2 of 6 runs before the fix and in 0 of 8 after.The reviewer also checked on the running fixture server that all 75 leaderboard rows match
/api/nodes/<pk>/reachin rank, total, neighbours and snapshot time.13f4e61cadded only a test; the merge to130f9cc8/fc4ba5abis reviewed fresh below.130f9cc8/fc4ba5ab, after fix(reach): hide blacklisted and hidden identities on the Reach page #68 landed): a fresh reviewer, focused on the six areas above (privacy, SQL/edge dedup, ranking, cache/concurrency, abuse potential, frontend) plus explicitly diffing against both merge parents to isolate what the merge itself changed. No blocker. Traced every path that serves a Reach body (cache hit, singleflight miss, the shared computation closure) throughwriteReachEntry→reachBody→visibleReach, confirmedhiddenKey/viewIDare collision-free freshness keys that force a re-marshal on any mismatch, and confirmedreachRankVisiblenow delegates toidentityHidden(the actual conflict-resolution fix). Re-derived the edge validation/dedup logic againstTestReachRank_OnlyValidEdgesCount's strengthened fixture and the concurrency machinery inreach_rank.go, and ran the suite including-race. Three low-severity nits, none privacy- or correctness-affecting, none requiring a fix:inactive_nodeshidden name can over-hide an observer-only identity that now has a distinct current observer name (already covered byTestReachRank_InactiveNamesOnlyForInactiveNodes; fails safe — over-hides, never leaks).CI, fresh after the merge (run 35442541721, head
fc4ba5ab)Tested as GitHub's own merge ref
40c0ceb8(masterd9ed1e16+fc4ba5ab) — the same tree verified locally above. Result: ✅ success, all the way through.Confirmed explicitly, not inferred from the green checkmark alone: nothing was deployed to staging, nothing was pushed to GHCR, no release was created, and no badges were committed. Coverage badges were only uploaded as CI run artifacts (an ordinary
actions/upload-artifactstep inside the Go Build & Test / E2E jobs), which is not the same as the📝 Publish Badges & Summaryjob that commits them to the repo — that job showsskipped.Tests of particular interest, all green on this run:
test-channel-color-picker-e2e.js: 9/9, including "ArrowRight cycles focus across swatches" — the test that was flaky on fix(reach): hide blacklisted and hidden identities on the Reach page #68's earlier run is stable here.test-reach-rank-e2e.js: 14/14, including the fixture's 9-not-10 assertion (total=75) and mobile/keyboard coverage.test-issue-1122-packets-filter-ux-e2e.js: 6/6, including "Path column row height stays bounded < 60px" — the test that was time-dependent-flaky on this branch's pre-merge CI run (see the old CI section above) passes cleanly now, consistent with it being a fixture-timing artifact unrelated to this branch's code, as already diagnosed there.test-issue-1122-details-row-clamp-e2e.js: 18/18 (from fix(packets): keep the Details summary on one line so rows stay bounded #67, carried in by the master sync)./reach-rank.This fresh run replaces the old CI section below for merge-readiness purposes; the old section is kept for history (it covers the original, pre-sync implementation and its own independent-review rounds).
CI (run 35364106106, head
13f4e61c)test-reach-rank.js.--diffgate pass.test-issue-1122-packets-filter-ux-e2e.js("Path column row height stays bounded < 60px"). This PR does not cause it:/reach-rankto the axe gate (four cells, ~1 m 50 s) makes the Packets test run later./reach-rankon desktop and mobile in dark and light.test-reach-rank-e2e.jsdid not run in CI. It passes 14/14 locally, see Tests.Known limitations (updated for the merge)
inactive_nodesname.Until the prerequisite PR merges, master's Reach page itself still shows observer-hidden identities— resolved: fix(reach): hide blacklisted and hidden identities on the Reach page #68 is merged, and the leaderboard now shares itsidentityHiddenrule directly (this merge also removed a parallel copy of that rule found during review — see Synced with master).reading '_leaflet_pos'(the E2E ignores exactly that message);.nq-linkon the Reach page is 2.52:1 contrast in the light theme;test-node-reach-e2e.js(not registered in CI) times out locally on the fixture.Not tested
Staging and production were not tested. All validation used a local server on the committed CI e2e fixture DB and synthetic benchmark data. Nothing was deployed.
🤖 Generated with Claude Code