Repository navigation
Conversation
A13xB0
force-pushed
the
perf/resolved-path-keys-without-pointers
branch
from
October 13, 2026 03:26
a66f426 to
058d8e5
Compare
Owner
Author
|
Rebased onto upstream master and submitted as Kpa-clawbot#2197. |
…atterns (Kpa-clawbot#2184) (Kpa-clawbot#2189) Fixes Kpa-clawbot#2184. ## Problem `nodeBlacklist` and `hiddenNamePrefixes` did not apply to Route Patterns: - **`subpaths` and `subpaths-bulk`:** `filterBlacklistedFromSubpaths` asserted `data["subpaths"].([]interface{})`, while the store builds `[]map[string]interface{}`, so it returned its input unchanged. It then read `hops` as a list, which is an `int` (the run length). It also ran only when `nodeBlacklist` was set, so `hiddenNamePrefixes` alone never filtered anything. - **`subpath-detail`:** the check compared each raw hop (a prefix) with full keys, so it matched only a client that sent a full key. - **Parent paths** in the detail response were not filtered. ## Change All in `cmd/server/routes.go`; `store.go` is untouched, so this doesn't overlap Kpa-clawbot#2182. - **Lists:** an entry is dropped when a name in its resolved `path` is the name of a blacklisted node (names looked up in the store's node cache) or has a hidden prefix. A subpath carries names and prefixes, not keys, and the store picks a node per prefix from context, so re-resolving a prefix in the handler would not be guaranteed to land on the same node. Matching the names the reader would see avoids that. Trade-off: another node with exactly the same name as a blacklisted one is hidden too. - **Detail:** 404 when a hop resolved to a blacklisted node (by the resolved key in `nodes[].pubkey`, so a node without a name is caught and one that only shares its name is not) or to a hidden-prefix name; parent paths that show one are dropped (by name, like the lists). The existing raw-hop check stays. - The filter runs when either `nodeBlacklist` or `hiddenNamePrefixes` is set. - `dropHiddenPaths` replaces `filterBlacklistedFromSubpaths` and returns a copy: the input can be the store's cached response, which the old function mutated in place. - `totalPaths` and `pct` are unchanged, so percentages stay relative to every stored path, the filtered ones included. **Perf:** per request, one pass over the node cache (about 2k nodes on the deployment below) to collect blacklisted names, then at most 8 lookups per entry on at most `limit` entries. It runs outside the store lock and only when a filter is configured. `map[string]interface{}` count in `routes.go`: 3 removed, 3 added. ## Tests - `subpaths_blacklist_2184_test.go`: on the seeded path `["aa","bb"]` (first hop resolves to TestRepeater), for `subpaths`, `subpaths-bulk` and `subpath-detail`: - unfiltered, the repeater is listed (so the test is not vacuous); - a blacklisted key, also in capitals, removes it and the detail returns 404; - a hidden name prefix removes it; - a different key with the same 1-byte prefix leaves it listed; - clearing the blacklist afterwards shows it again (the cached response was not changed). - subpath-detail: a blacklisted node without a name returns 404, a different node that only shares its name does not hide the detail. - Red commits first (`0ebee485`, `4acfb70a`, failing on assertions), then green. - `cd cmd/server && go test ./...` passes (`TestSaveGeoFilterPreservesFileMode` skipped, a Windows-only failure on master too); `go test -race -run 2184` passes; `go vet` clean. ## Validation on a live-fed staging instance `nodeBlacklist` temporarily set to the key of the node in the top Route Patterns entry (5848 occurrences), then restored: | | blacklist set | restored | |---|---|---| | `subpaths?limit=200` entries showing the node | 0 (185 entries left) | 15 (200 entries) | | `subpaths-bulk` groups 2-2 / 3-3 / 5-8 showing it | 0 / 0 / 0 | | | `subpath-detail` on a path through it | 404 | | | `subpath-detail` on a path without it | 200 | | ## Not done - Other analytics responses that carry node names (for example topology) were not audited for the same gap. - The UI still shows `pct` relative to all paths; whether a filtered list should renormalise is left open. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…he key set grows (Kpa-clawbot#2133) Part 1 of Kpa-clawbot#2107, scoped as discussed there: the backfill on its own, no catalogue promotion. ## Problem Ingest decides a GRP_TXT's channel and a transport packet's region scope once, at insert time. A row stored before its key was known keeps `channel_hash = 'enc_XX'` / `scope_name = ''` forever, even after the key is added — via `hashChannels` plus a restart, or at runtime when the `autoRegionKeys` tier picks up a newly declared region. On live.saarmesh.de we have been closing that gap with an out-of-tree script that rewrites the DB and restarts the container; this moves it into the ingestor and removes the restart. ## What this does **Ingestor** — `cmd/ingestor/key_backfill.go` - Runs after the write path is ready, and again whenever the key set grows (kicked after every `autoRegionKeys` refresh; a no-op when nothing is new). - Touches **only unresolved rows**: `enc_*` GRP_TXT and transport rows with `scope_name = ''`. Every `UPDATE` carries the unresolved value in its `WHERE`, so a row resolved meanwhile by live ingest keeps its value. A decrypted channel or named scope is never rewritten. - **Same decision as ingest**: `DecodePacket` with the full channel key map → `PayloadJSON`, and `regionKeySnapshot.match()` with the full snapshot — including abstaining on an ambiguous collision (Kpa-clawbot#1609). The scope-match counters are not touched; they describe live traffic. - **Bounded**: each batch covers a fixed window of 5000 transmission ids. Unresolved rows are sparse (hundreds in a million), and the ingestor has a single SQLite connection, so a "next N matching rows" query would scan most of the table in one statement while ingest waits. A fixed id window keeps every read a short primary-key range scan. The writer is yielded between windows. - **Resumable and transactional**: one transaction per window holds the rewrites, their change-feed rows and the advanced cursor (`key_backfill_state`). A cancelled or crashed run resumes at its last commit; nothing is applied twice. - **Key-set growth**: a run is tied to the fingerprint of the key set it started with. If the set changes mid-run, the run restarts from the beginning, so a key added halfway still sees all of history. Each key gets one complete pass (`key_backfill_done`); a restart with an unchanged key set does no work. A key that leaves the set forgets its pass and is replayed if it returns. - **Derived field**: a named ADVERT fills `nodes.default_scope` / `inactive_nodes.default_scope` only where it is still empty. A known `default_scope` is newer evidence than a backfilled advert and is never overwritten. **Change feed** — `internal/dbschema/key_backfill.go`, `cmd/server/tx_rewrite_feed.go` - Each rewrite appends a row to `tx_rewrite_feed` in the same transaction as the rewrite. - The server tails it exactly like `advert_route_evidence`: cursor captured before `Load`, absent table re-probed (not required by `AssertReady`, so an older ingestor simply means no feed), polled from `IngestNewObservations`. It refreshes `DecodedJSON` / `ScopeName` of loaded transmissions, resets the `ParsedDecoded` cache, recharges tracked bytes, and invalidates the channel caches. **No restart needed.** - The feed carries only the transmission id; current values are read from `transmissions`, so replaying a row is harmless. Rows older than 24h are pruned. ## Tests Ingestor (`key_backfill_test.go`) — fixtures are real wire frames: GRP_TXT encrypted AES-128-ECB with the firmware MAC, transport frames with the real region HMAC. - decrypts only unresolved rows; already-decrypted and unknown-key rows untouched - backfilled `decoded_json` / `channel_hash` are byte-identical to what `BuildPacketData` stores at ingest - unique match named; a constructed two-name collision on the same payload stays unmatched; named and non-transport rows untouched - ingest scope-match counters unchanged - ADVERT fills only an empty `default_scope` - idempotent: second pass writes nothing, done keys recorded, state cleared - cancel between commits → resume from the committed cursor, every row exactly once, in order - stale run fingerprint (key added mid-run) → restart reaches rows below the old cursor - a later key only adds its own rows; a removed key is replayed over the gap when it returns - a row resolved between decide and commit keeps its value and gets no feed entry - channel hash byte derivation matches the builtin Public key (0x11) - old feed rows pruned Server (`tx_rewrite_feed_test.go`) - rewrite reaches the loaded store through the normal `IngestNewObservations` poll, including a previously parsed `ParsedDecoded` (verified to fail without the `sync.Once` reset) - cursor starts at the load watermark - absent feed is a no-op and is picked up once created, without restart - failed read keeps the cursor and retries - feed row for an unloaded transmission advances the cursor and is ignored `gofmt` clean, `go vet` clean, no `go.mod` / `go.sum` changes. ## Validation against production data Ran the backfill on a snapshot of live.saarmesh.de taken **before** last night's run of our out-of-tree script (13,100 unresolved rows, 164 channel keys, 24 region keys incl. 2 derived), with the current config. It rewrote **the same 31 transmissions with the same channel names** as the script did on production (compared by id), named 0 scopes (script: 0), and a second pass wrote nothing. Browser validation (AGENTS rule 2): this changes no UI. The visible effect — a decrypted message or named scope appearing without a restart — is covered by the server feed tests; I have not yet run it against a deployed instance. ## Notes for review - Window size (5000 ids), inter-batch yield (10 ms) and feed retention (24h) are constants for now; per rule 8 they can move to config/customizer later if anyone needs to tune them. - `client_rx_observations.scope_name` is not backfilled here; happy to add it in a follow-up if wanted. - The orphaned "scope-repair" mention in `region_keys.go`'s `scopeMatch.Candidates` comment predates this PR; I left it alone. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01BKaogNXgy8dEQruqzR5foP --------- Co-authored-by: SaarMesh-Bot <300107934+SaarMesh-Bot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
…ns (Kpa-clawbot#2183) Part of Kpa-clawbot#1870: this interns the three fields the issue names (`ObserverID`, `ObserverName`, `ObserverIATA`), plus `Direction`. ## Problem Each `StoreObs` kept the row scan's own copy of its observer ID (64 hex characters for most observers), observer name, IATA code and direction. A busy regional network's 7-day window holds about 400k observations from a few hundred observers, so a few hundred distinct values were stored hundreds of thousands of times. ## Change - `internStr` returns the canonical copy of each value from a map guarded by an `RWMutex`, so every observation shares one copy. - The copies are held for the life of the process. That is what makes the sharing survive garbage collections. An earlier version of this PR used `unique.Make`, which holds its copies weakly, so a value interned again after a GC came back as a second copy (thanks to the review for catching that). - Observer strings number in the hundreds. The map is capped at 65,536 entries as a guard (AGENTS.md: no unbounded structures); past the cap a string is returned as it came, so nothing breaks. - A hit takes only the read lock and allocates nothing. A new value is cloned, so it doesn't pin a larger buffer it may be a slice of. - All five places that build a `StoreObs` use it: `Load`, `loadChunk`, `scanAndMergeChunk`, and both ingest paths. - The ingest paths intern while scanning rows, **before** taking `s.mu`, so interning adds nothing to the time API readers wait on the store. - `cmd/server/go.mod` stays at `go 1.22`. The earlier 1.27 bump was only there for `unique`, and is reverted here; the module upgrade belongs in its own PR. ## Scaling `BenchmarkInternStr` (a hit, the steady state): | | alone | 12 cores (per call) | allocs | |---|---|---|---| | plain `Mutex` map (the first version of this PR) | 20 ns | 57 ns | 0 | | `unique` (the second version, withdrawn) | 48 ns | 8 ns | 0 | | **`RWMutex` map (this PR)** | 17 ns | 26 ns | 0 | ## Measured on a copy of that window (`GOMEMLIMIT=750MiB`) The same database and harness, one run each, the `unique` version against this one: | | `unique` version | this PR | |---|---|---| | live heap at the last GC after load | 581 MB | **535 MB** | | string bytes allocated by row scans still live (`bytealg.MakeNoZero`) | 106 MB | **80 MB** | | peak RSS | 777 MB | 781 MB | | load to ready | 15 s | 15 s | Peak RSS is set by the load itself, not by these strings. Against master, the first measurement of this PR found the strings retained from row scans went from 84 MB to 58 MB. An earlier version of this PR (the `unique` one) also ran on a staging instance (live MQTT ingest, `GOMEMLIMIT=1GiB`) through five back-to-back restarts and a two-hour soak, with no OOM kills and every request succeeding. This version has not had a staging soak yet. ## Tests - `observer_strings_intern_test.go`: - one copy of each value across 50 observations, through `Load`, `LoadChunked` and `loadChunk`; - **a value interned again after `runtime.GC()` is the same copy** (new: red against the `unique` version, green now); - 16 goroutines interning concurrently get the same copy; - a hit allocates nothing; - `BenchmarkInternStr` (serial and parallel). - Red commits first, then green. - `cd cmd/server && go test ./...` passes at `go 1.22`, and `-race` passes on the intern tests. Backend only: no UI, API shape or config change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…a-clawbot#2137) Found while reworking Kpa-clawbot#2125, unrelated to that PR's change. In the Docker image the entrypoint symlinks `/app/config.json` to the bind-mounted `/app/data/config.json`. `SaveGeoFilter` takes the first path `os.Stat` resolves, so `configPath` stays the symlink, and the tmp+rename then **replaces the link with a regular file inside the container**: - the mounted `config.json` keeps the old `geo_filter`, so the filter the operator just saved appears nowhere on the host; - server and ingestor read the detached file from then on, so the UI keeps showing the new filter and nothing looks wrong; - the next container start reverts it: the entrypoint's `ln -sf` puts the link back, and re-creating the container drops the file anyway. **Fix:** resolve a symlink at `configPath` before the write, so the save lands in the file it points at and the link stays a link. Only a regular file is followed — a link to a device, fifo or directory is left alone and handled as before. This is deliberately the opposite of `writeStatsAtomic` (refs Kpa-clawbot#1170), where replacing a destination symlink is the point because the default stats path can be world-writable `/tmp`. `config.json` is different: the link is one the image itself creates, and the directory is not world-writable. Happy to narrow it further (e.g. only follow a link that stays inside `configDir`) if you'd rather keep the two paths consistent. **Verification:** `TestSaveGeoFilterSymlinkedConfig` fails on master — ``` --- FAIL: TestSaveGeoFilterSymlinkedConfig config_symlink_test.go:47: .../config.json is no longer a symlink (mode -rw-------) — the rename replaced it config_symlink_test.go:60: geo_filter missing from .../data/config.json: {"port":3000,"_comment":"keep me"} ``` — and passes here. `TestSaveGeoFilterKeepsOwner` covers keeping the file's owner, and `TestSaveGeoFilterFifoSymlinkTarget` checks that only regular files are followed. `go test ./...` in `cmd/server` passes. **Single-file mounts:** when `config.json` is mounted as a single file, the save now fails with 500 (`EBUSY`) instead of writing a copy that the next start discards. Our own instance has no geo filter configured, which is why this went unnoticed there; its `/app/config.json` is still a symlink. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ot#2196) Refs Kpa-clawbot#2148. ## Problem `StoreObs` is the store's most numerous struct: 400k of them on a week of a busy regional network. Its lazy timestamp cache (Kpa-clawbot#1481) is a `time.Time`, a `sync.Once` and a `bool`, together 40 bytes. On current master that makes the struct 200 bytes, which puts every observation in the 208-byte size class. ## Change The cache is now a single `atomic.Int64`: - `0` before the first call; - the parsed time as UTC nanoseconds after it; - or one of two sentinels: one for empty or unparseable strings, and one for times it can't hold exactly (a non-UTC offset, or outside `UnixNano`'s range). Those are re-parsed on each call so their location is preserved. Concurrent first calls may both parse; they store the same value. `StoreObs` is now 160 bytes, in the 160-byte size class: 48 bytes × 400k observations ≈ **19 MB** less heap. The first parse costs the same; cached reads now build the `time.Time` from an int. ## Tests - `TestStoreObsFitsIn160Bytes`: red commit first. - `TestStoreObsParsedTimeMatchesParse`: Z, nanosecond, offset, SQL, empty, garbage and epoch inputs give the same time and location as `time.Parse`, on the first and the cached call. - `TestStoreObsParsedTimeConcurrentFill`. - The existing `storeobs_parsedtime_test.go` still passes. - `go test ./cmd/server` passes, and `-race` passes on the ParsedTime, retransmission and observer analytics tests. ## This PR or Kpa-clawbot#2149 Kpa-clawbot#2149 fixes Kpa-clawbot#2148 by reordering these same three fields. The two conflict, so here is how they compare. Sizes are `unsafe.Sizeof(StoreObs{})` on each branch, with Go's heap size class beside them: | | struct | heap per observation | saved per observation | saved at 400k observations | |---|---|---|---|---| | master | 200 B | 208 B | – | – | | Kpa-clawbot#2149 (field reorder) | 192 B | 192 B | 16 B | ~6.4 MB | | this PR (one 8-byte field) | 160 B | 160 B | 48 B | ~19 MB | - **Kpa-clawbot#2149:** a pure field reorder with no change in behaviour. It is the smallest, safest change and the easiest to review. It saves a third of what this PR does. - **This PR:** three times the saving, but more code. It adds two sentinel values, and re-parses on each call any timestamp with a non-UTC offset. The store writes UTC (`Z`), so that path is rare. It is tested against `time.Parse` on edge cases, concurrently, and under `-race`. Options: 1. **Take this PR.** Kpa-clawbot#2149 then has nothing left to reorder. 2. **Take Kpa-clawbot#2149 now, and this PR after it** as a follow-up for the further 32 bytes per observation. I'll rebase onto Kpa-clawbot#2149 if it lands first. 3. **Take only Kpa-clawbot#2149** if the smaller, simpler change is preferred. No config or UI changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ot#2195) ## Problem `relayCandidates` runs once per hop of every observation the cold loader resolves, and again from the direct-heard and neighbor analytics. Once any listener-only observer was marked (Kpa-clawbot#1290), every call built a filtered copy of the candidate `nodeInfo` structs, even when none of the candidates was a listener. On a week of a busy regional network that accounted for 1.1 GB of the 5.4 GB allocated over one startup, the biggest single source of startup churn. ## Change `markNonRelay` now builds a map once, holding the filtered list for each prefix whose candidates include a listener. A listener can only be in the lists of its own prefixes and its full key, so only those are filtered: at most 9 lists per listener, not every list in the prefix map. `relayCandidates` returns either that list or the prefix map's own list, without copying. Every caller only reads the slice (checked: direct-heard, neighbor persist, node hop analytics and `resolveWithContext`). ## Measured on a copy of that week | | before | after | |---|---|---| | allocated over startup | 5.44 GB | 4.32 GB | | server CPU over the run | 175.6 s | 169.1 s | | peak RSS | 1074 MB | 1077 MB (unchanged; the GC limit sets it) | ## Cost per rebuild against cost per lookup `markNonRelay` now does once per node cache rebuild (as often as every 30 s, `nodeCacheTTL`) the filtering that master did on every `relayCandidates` call. 5,000 repeaters and 3 listeners, median of 6 runs (`BenchmarkMarkNonRelay5000`, `BenchmarkRelayCandidates256`): | | master | first version of this PR | this PR now | |---|---|---|---| | `markNonRelay`, once per rebuild | 0.59 µs, 320 B, 3 allocs | 14.8 ms, 6.57 MB, 36,285 allocs | 20 µs, 20.6 KB, 33 allocs | | one `relayCandidates` lookup | ~7.8 µs, 6.7 KB | ~30 ns, 0 B | ~30 ns, 0 B | | 256 lookups (one per 2-character hop) | 2.0 ms, 1.71 MB, 1,453 allocs | 7.7 µs, 0 B | 7.7 µs, 0 B | The extra ~20 µs per rebuild is paid back after about 3 lookups. Every ingested observation looks up each hop of its path, so a running server makes far more than that between rebuilds, and a cold load makes millions (the 1.1 GB above). ## Tests - `TestRelayCandidatesDoesNotAllocate`: zero allocations per call, and listeners still filtered. - `TestRelayCandidatesAfterSecondMark`. - `TestMarkNonRelayVisitsOnlyListenerPrefixes`: at most 80 allocations for 3 listeners among 5,000 repeaters (36,285 before the fix). It has its own red commit. - `BenchmarkMarkNonRelay5000` and `BenchmarkRelayCandidates256` give the table above. - `TestMarkNonRelayMatchesFilteringEachList`: every prefix of the 5,000-repeater map matches filtering its list on the spot. - Red commit first, then green. - `go test ./cmd/server` passes. No config or UI changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…pa-clawbot#2202) Fixes Kpa-clawbot#2136. ## Problem The README badges are stale: `e2e-tests` shows 821 passed (1161 when Kpa-clawbot#2136 was filed), both Go coverage badges have not changed since 2026-03-30 and frontend coverage since June. There are two causes. 1. **The PAT.** The `publish` job wrote the badge JSON to `master` through the Contents API with `BADGE_PUSH_TOKEN`, a PAT that could push past the branch protection on `master`. That token returns 401: run 38360160932 on 2026-10-13 logs `gh: Bad credentials (HTTP 401)` for every badge. The step is `continue-on-error` and only echoed `Failed to update`, so every run stayed green. 2. **Old badges inside `e2e-badges`.** `.badges/` is tracked on `master`, so every e2e shard checkout and the aggregate checkout already hold the old files. The shard upload and the aggregate `cp` shipped them in `e2e-badges`, and `publish` downloaded `e2e-badges` after `go-badges`, so the old values overwrote the fresh ones. In run 38407143126, `go-badges` holds 85.8% and 77.1%, while `e2e-badges` holds 85.4%, 70.2% and frontend 36.64%, the same as `master`'s `.badges/`. Fixing only the token would have left three of the four README badges frozen. ## Change 1. `5f430966` `ci:` the `publish` job pushes the badges to an unprotected `badges` branch with its own `GITHUB_TOKEN`. - Job-level `permissions: contents: write`. No other job gains a permission. The checkout uses `persist-credentials: false`, since the push passes its own token. - The branch is a single orphan commit, force-pushed, so it does not grow. Master push runs are already serialised by the workflow `concurrency` group. - Each run starts from the branch as it is and overwrites only the badges it downloaded, so a missing artifact keeps those badges at their previous value. The first time, when `git ls-remote --exit-code` reports the branch absent (exit 2), it starts from `master`'s `.badges/`. - No downloaded artifacts, or any other `ls-remote`, fetch, extract, commit or push failure, ends in a `::warning::` in the run summary and leaves the branch alone. The step stays `continue-on-error`, so a badge problem never turns the pipeline red. - `go-badges` is downloaded after `e2e-badges`, so its files win. 2. `2134c1e5` `docs:` the four README badge URLs read `.../CoreScope/badges/<name>.json` instead of `.../master/.badges/<name>.json`. 3. `47e57482` `ci:` the frontend-coverage step in the e2e shards and the aggregate badge step start from an empty `.badges/`, so `e2e-badges` carries only what the run produced. This also stops shard 3's old `frontend-coverage.json` from replacing the one shard 2 generates. No PAT is involved any more, so nothing expires. ## Testing - The publish step script, taken from the YAML, run against a local bare repository standing in for `origin`: | Run | Downloaded | Result on `badges` | |---|---|---| | 1, branch absent | a, b | a, b new; c from `.badges/`; one commit | | 2 | c only | a, b kept from run 1; c new; still one commit | | 3 | none | `::warning::`, branch unchanged, step exit 0 | | 4, `origin` unreachable for the read | a | `::warning::`, branch unchanged, step exit 0 | | 5, push to a missing remote | a | `::warning::`, branch unchanged, step exit 0 | - The artifacts of run 38407143126 land flat in the download path, so `badges-new/*.json` matches them. - `shellcheck` at style level on the step script: no findings. - `actionlint` 1.7.7 (shellcheck off): one finding, the self-hosted runner label `meshcore-runner-2`, which `master` reports as well. **Not tested:** the e2e change and the push with a real `GITHUB_TOKEN`. Both only run on a push to `master`, so the first master run after merge is the real test. The `badges` branch already exists (`80fda6c0`, a copy of `master`'s `.badges/`), so the README badges keep working from the merge on, with the old values until that run. After it, I will check the `badges` branch contents against that run's go and frontend numbers, the step log and the rendered README. ## Not in this PR - Removing the now unused `.badges/` from `master`: the first run seeds the branch from it, so that goes in a follow-up once the branch exists. - Deleting the `BADGE_PUSH_TOKEN` secret: that needs a repository admin. - `frontend-tests.json`: no CI step produces it and no README badge uses it. It is carried over to the branch unchanged. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…pa-clawbot#2204) Fixes Kpa-clawbot#2203. ## Problem The frontend coverage badge (35.16% on run 38413268001) came from two sources on shard 2: one dump part-way through `test-e2e-playwright.js` and the `collect-frontend-coverage.js` tour. The other E2E suites run against the instrumented build but never save `window.__coverage__`, including the suites written for single pages (`path-inspector.js` 4.72%, `touch-gestures.js` 14% on that run). ## Change 1. `9d3bfad5` `test(e2e): red:` `tests/unit/test-e2e-coverage.js`, with fake pages, contexts and browsers (no Chromium needed). On the placeholder module the first assertion fails. 2. `c00bbb12` `feat(e2e):` the collector. - `scripts/e2e-coverage-hook.js` is a preload (`node --require`) that wraps `chromium.launch` from `playwright`. `playwright` re-exports `playwright-core`, so a suite's own `const { chromium } = require('playwright')` gets the same, wrapped object. - `scripts/e2e-coverage.js` reads `window.__coverage__` before every call that replaces or ends a page's document: `goto`, `reload`, `setContent`, `goBack`, `goForward`, `close`, and context and browser `close`. Pages from `newPage`, `newContext` and the context `page` event are all tracked. - Each read zeroes the page's counters, so a document read twice is not counted twice. Maps travel once per file; later reads carry counts only. - The page returns a JSON string. Returning the object cost about 10 s per suite, because Playwright serialises it node by node. - A read is given up after 5 s, so a busy page cannot hold up a suite's cleanup. - On exit the hook writes one file per suite to `.nyc_output/`, or nothing without coverage. A failed write is logged and never changes the exit code. 3. `64487c4c` `ci:` the workflow. - On master pushes the E2E run step sets `NODE_OPTIONS=--require ./scripts/e2e-coverage-hook.js`. On PRs it is empty, so PR runs are unchanged. - Each shard runs `nyc merge` (one file per suite is about 6 MB, mostly the same maps) and uploads `coverage-shard.json` with its results instead of `.badges/`. - The `e2e-test` job merges the three shards and makes the frontend badge (it used to come from shard 2 alone). - The merge, setup-node and badge steps are `continue-on-error`. When `nyc report` gives no statement percentage the step warns and writes no badge, so a registry hiccup cannot publish 0%. - `test-e2e-playwright.js` no longer writes its own `e2e-coverage.json`: the hook covers that suite completely, including the tests after the old dump. `collect-frontend-coverage.js` stays as an extra source. ## Performance Measured locally against an instrumented server (Windows, Chrome): | Suite | Without hook | With hook | |---|---|---| | `test-path-inspector-coverage-e2e.js` | 1.9 s | 2.2 s | | `test-touch-gestures-coverage-e2e.js` | 17 s | 16 s | | `test-e2e-playwright.js` (stops at the same local-fixture failure in both runs) | 52 s | 52 s | | `browser.close()` on a page in a 30 s busy loop | 153 ms | 5.2 s | The hook only loads on master pushes. ## Testing - `tests/unit/test-e2e-coverage.js` covers: - reads before each document-replacing call and before close; - pages opened by the page itself; - context and browser close; - summing without double counts; - a failed read, and a read that never answers; - the file writer, including a failing write. - Nine deliberate breaks of the library each turn it red: - dropping the counter reset; - dropping the `page` event, the `goto` read, the `goForward` read, the `close` read or the browser `close` read; - removing the timeout or the write guard, or writing an empty store. - The test also fails if it stops before its end. Without that check, a read that never returns would let node exit 0 half-way. - Real suites with the hook, measured on their own (the badge run had all suites): | File | Badge run 38413268001 | Own suite with the hook | |---|---|---| | `path-inspector.js` | 4.72% | 52.75% | | `touch-gestures.js` | 14% | 72.96% | - Exit codes and pass counts were unchanged with the hook for these suites (the first three were run before the 5 s read timeout was added): - `test-slideover-1056-e2e.js` (6 contexts); - `test-a11y-axe-1668.js` (`@axe-core/playwright`); - `test-issue-1150-404-state-e2e.js`; - the three above. - The badge step, run locally against two merged shard files: - It gives the same per-file numbers as `nyc report` on the inputs. - With no shard coverage, or with `nyc report` failing, it warns and writes no badge. - `sh test-all.sh` passes locally, apart from `test-issue-1956-release-routing.js`, which needs the workflow's `GITHUB_REF_NAME` and was skipped. The test inventory classifies 321 suites. `actionlint` 1.7.7 reports only the self-hosted runner label, as on master. **Not tested:** the shard and badge steps in CI. They only run on a master push, so the first master run after merge is the real test. I do not know what the merged number will be. I will compare the badge with the merged report in that run's `e2e-test` log. **Not counted:** a full page load the suite does not start through one of the wrapped calls, such as a link that leaves the SPA or `location.reload()` from page code. ## Not in this PR - The `tests/unit/` suites, which load frontend files through `vm.createContext` without instrumentation (noted in Kpa-clawbot#2203). - Coverage on PR runs. The overhead above is small, so this could follow if wanted. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…-clawbot#2205) ## Summary The README is the repo's front page. I checked each claim against `master` and fixed what no longer holds. | README said | Now | Source | |---|---|---| | `POST /api/packets` for manual injection | route is GET only; points at `POST /api/decode` | `cmd/server/routes.go:317`, `:320` | | `PORT` env var overrides the port | not read anywhere; documents `port` and `-port` | `cmd/server/main.go:96` | | 5 store indexes | 7 (`byPathHop`, `byPayloadType` added) | `cmd/server/store.go:222-231` | | 11 analytics tabs | 17 `data-tab` values | `public/analytics.js` | | VCR up to 4× | up to 8× | `public/live.js:681` | | 14 Go modules | 16 `go.mod` files (also fixed in the `Makefile` header comment) | | | 327 / 53 / 380 Go tests, 150+ Node, 49 Playwright | 1827 / 710 / 2742 `func Test`, 199 unit files, 131 `test(` calls; counts removed so they stop going stale | | | live instance + Swagger on analyzer.00id.net | times out (from two networks, 2026-10-13); links analyzer.on8ar.eu, `/api/docs` and `/api/spec` return 200 there | curl | Added: Scope Audit in the feature list, and an "Optional Features" section for user accounts and mobile RX coverage, each linking its existing doc. ## Not checked - the performance figures (~300 MB for 56K packets, < 1 ms queries) - the "AES-128-ECB with rainbow table" line - whether the screenshots still match the current UI - the "-or-later" in the license line (`LICENSE` is GPL v3) Docs only, no tests run. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…-clawbot#661) (Kpa-clawbot#2208) Fixes Kpa-clawbot#661 (the narrowed scope from the 2026-10-14 comment). ## What changes - `GET /api/observers/metrics/summary` adds `tx_airtime_pct_1h` and `rx_airtime_pct_1h` per observer: the airtime share over the hour before now, whatever `window` is, from the cumulative `tx_air_secs` / `rx_air_secs` counters in `observer_metrics`. - A counter that goes down marks a reboot; only the stretch after the last reset counts. - Under 15 minutes of samples (one sample, or a reboot late in the hour) returns `null`. - Clamped to 0..100, rounded to 2 decimals. - The response also carries `duty_cycle_warn_pct`, from the new config key `dutyCycleWarnPct` (default 10, the EU 868 MHz g3 sub-band limit). Added to `config.example.json`. - RF Health tiles show `TX 1h: x%`; the observer list gets a sortable numeric `TX 1h` column. Both use the shared `public/observer-airtime.js` helper and flag values at or over the threshold with `.airtime-over` (`var(--status-red-text)`). - The handler now returns a named `MetricsSummaryResponse` struct instead of `map[string]interface{}` (one occurrence fewer). ## Why the warning matters Repeater and room-server firmware default to `airtime_factor = 1.0` (`examples/simple_repeater/MyMesh.cpp:890`), which `Dispatcher.cpp:26` turns into a 50% TX budget per 1 h window. The device therefore does not keep itself under the EU 10% limit; this view is where an operator can see it. ## Cost - Server: one extra query per summary request, `SELECT observer_id, timestamp, tx_air_secs, rx_air_secs FROM observer_metrics WHERE timestamp >= now-1h`, on the existing `idx_observer_metrics_timestamp`, then one O(rows) pass. On our instance that is 35 observers x about 12 samples (5 min interval). - Observer list: one extra bulk call `summary?window=1h` per load (client TTL 30 s, failure ignored), not a call per observer. ## Tests - Go: `TestRollingAirtimePct` (steady, reboot mid-hour, single sample, span under 15 min, reboot leaving under 15 min, nil counters, clamp, rounding), `TestGetMetricsSummaryAirtime1h` (samples older than 1 h ignored inside a 24 h window, single-sample observer gets null), `TestMetricsSummaryDutyCycleWarnPct` (default 10, config override, `observers` is `[]` not `null`). Full `cmd/server` suite passes locally (skipping `TestSaveGeoFilterPreservesFileMode`, which fails on Windows on clean master too). - Frontend unit: `tests/unit/test-issue-661-observer-airtime.js` (8 assertions, real file in `vm`), added to `test-all.sh`. `test-observers-headings.js` updated for the new column. Full `test-all.sh` passes locally. - E2E: `tests/e2e/test-issue-661-airtime-e2e.js`, registered in `deploy.yml` and `scripts/non-unit-tests.json`. The fixture DB has no `observer_metrics` table, so the summary is served through `page.route` with known values for the fixture's own observer ids. It checks the column header, over/under/unknown cells, sort order and the RF Health tile. 4/4 locally. It caught a real bug during development: `loadRFHealthData` declares a local `const window`, so the tile calls the helper as the global `ObserverAirtime`. ## Validation Deployed to a staging instance with live MQTT data (35 observers with metrics) and checked in the browser: - The observer list shows the column for 42 rows (33 with a value, 9 without enough samples). Sorting puts the highest share (5.3%) first. - RF Health shows `TX 1h` on all 35 tiles. With a fourth stat the line wrapped inside items on narrow tiles; the stats row now wraps between items only. - No observer was over 10% at the time, so the red state was checked by rendering one cell client-side at 12.5% (computed colour `rgb(185, 28, 28)` in light mode), and by the E2E test. ## Not done - The threshold is config-only; exposing it in the customizer is a follow-up. - No airtime comparison across observers (that is Kpa-clawbot#671) and no per-packet airtime (Relay Airtime Share covers it). - Dark mode was not checked in the browser; `--status-red-text` is defined for both themes. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…#2125) Runs `corescope-server` and `corescope-ingestor` unprivileged, opt-in with `RUN_AS_UID`. A code-execution bug in either one then no longer lands as root next to the mounted data and `config.json`. `supervisord` stays root, for mosquitto and caddy. | `RUN_AS_UID` | Go services run as | |---|---| | unset (default) | root, as before. Upgrades change nothing | | set | `RUN_AS_UID` / `RUN_AS_GID` | With `RUN_AS_UID` set: - Each start hands the data directory to that uid (`chown -h`), except `.env` and anything mounted into it on its own. Failures are logged. - `.env` is read only if it is root-owned and only root can write it. `RUN_AS_UID` and `RUN_AS_GID` are never taken from it. - `/app` stays root's. **Depends on Kpa-clawbot#2137.** - A uid with no account in the image gets one at start; `caddy` and `mosquitto` are reused. A uid that can't be provided fails startup instead of falling back to root. The logic is in `docker/select-service-user.sh`, tested by `docker/test-select-service-user.sh`, which needs no Docker or root and runs in CI. See also "Running the Go services unprivileged" in `docs/deployment.md`. Verified in the built image (master + this + Kpa-clawbot#2137) across 14 cases. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Mythos 5.1 <noreply@anthropic.com> Co-authored-by: nullrouten <201262769+nullrouten0@users.noreply.github.com>
) ## Problem `Load`, `loadChunk` and `scanAndMergeChunk` declared their ~20 `sql.Null*` scan targets, and built the `Scan` argument slice, inside the row loop. Each target's address goes into the `[]interface{}`, so each one escaped and was heap-allocated again on every row, along with the slice and its appends. On a week of a busy network that was the biggest flat cost inside `loadChunk`, about 330 MB. ## Change - The targets and the argument slice are declared once, before the loop. - `Scan` overwrites every target on each row. - Nothing else takes their addresses; the `null*` helpers copy their argument. ## Measured | | before | after | |---|---|---| | allocations per row, fixture (`Load` / `LoadChunked` / `loadChunk`), on current master | 85.9 / 87.1 / 86.0 | 63.9 / 65.1 / 64.0 | | allocated over startup (copy of a busy week) | 5.41 GB | 5.05 GB | | server CPU over the run | 175.6 s | 172.4 s | The per-row figures are from the test on current master. The startup figures were measured before Kpa-clawbot#2183 and Kpa-clawbot#2185 landed; this change saves the same ~22 allocations per row on top of them. **Scale:** this is the cold-load hot path, once per observation row (about 400k rows on that week). The change removes per-row work and adds none; complexity is unchanged. ## Tests - `TestBulkLoadersReuseScanTargets` sets an allocations-per-row bound for each loader, between the before and after values. Red commit first, then green. - `go vet ./...` is clean and `go test ./...` passes in `cmd/server`; `-race -count=2` passes on the loader and observer-string tests. - Local SonarQube: quality gate passed, no issues on added lines. - `map[string]interface{}` in `cmd/server`: 934 on master and here. No config or UI changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…a-clawbot#2198) ## Problem `ParsedDecoded` caches each packet's decoded payload for as long as the packet is stored, and the analytics passes fill it for every packet. It kept fields that no server code reads: - the ciphertext and MAC of encrypted payloads; - advert signatures (and their validity flag); - ISO copies of timestamps; - raw path data; - control-packet details. Together those are about a third of the cached JSON. ## Change - After decoding, the cache deletes those keys (`decodedCacheDroppedKeys`). - The live feed was the only place that sent the whole map. `broadcastPayload` now passes the stored `DecodedJSON` through as a `json.RawMessage` (after `json.Valid`), so clients still get every field, byte for byte. Nothing in the server reads the broadcast payload as a map; it is only serialised. This replaces the shallow copy of the cache, so no JSON is decoded or map copied under the store's write lock in either ingest path, and no nested map (such as `flags`) is shared with the cache. - `TestDecodedCacheDroppedKeysAreUnread` is a guard: it fails if any server code starts naming a dropped key, so a future reader can't silently get nothing. ## Measured | | before | after | |---|---|---| | decoded cache's live heap (copy of a busy week) | 46 MB | 39.5 MB | The saving is smaller than the JSON share suggests, because the per-packet map overhead dominates. A typed struct would be the next step, but that touches 27 call sites. ## Tests - `TestParsedDecodedDropsUnreadKeys` and `TestBroadcastPayloadKeepsEveryKey` (every key present in the broadcast, and invalid JSON left out as before): red commit first. - The guard test above. - `go test ./cmd/server` passes. `map[string]interface{}` in `cmd/server`: 933 on master, 932 here. No config or UI changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Red: resolvedPubkeysFromJSON (a stub over unmarshalResolvedPath + extractResolvedPubkeys) makes 13 allocations for three pubkeys. The equivalence test pins what decoding returns. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H
Load, loadChunk and scanAndMergeChunk decode each observation's resolved_path (a JSON array of pubkeys and nulls, ~420 chars, on most observations of live data) only to collect its non-empty pubkeys. The decode into []*string allocated a string and a pointer per hop and two slices. resolvedPubkeysFromJSON reads the usual array of escape-free strings and nulls in place and anything else falls back to decoding, so it returns exactly extractResolvedPubkeys(unmarshalResolvedPath(s)) (pinned on 20k random inputs plus edge cases). Relay pubkeys repeat across nearly every observation, so a bounded table holds one copy of each: a pubkey seen before is found by substring without copying, and the indexes it feeds share that copy. On a copy of a busy week: allocations here over startup 236MB -> 61MB, server CPU over the run 175.6s -> 170.6s. Three known pubkeys: 13 -> 1 allocations. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H
Red: resolvedPubkeysForEvictionBatch makes about 51 allocations per evicted packet (3 observations, 4 hops) by decoding each resolved_path into pointers and building a de-dup map per packet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H
resolvedPubkeysForEvictionBatch runs for every eviction batch, once a minute in steady state. It scanned each resolved_path into a string, decoded it into a pointer per hop and built a de-dup map per packet. It now scans the column as raw bytes, reads it with resolvedPubkeysFromJSON without copying (nothing keeps a slice of the buffer: new pubkeys are cloned into the shared table, the decoder fallback makes its own strings), and de-duplicates each packet's short list by scanning it. Same keys, same first-seen order. Fixture: 51 -> 15 allocations per evicted packet. A five-minute steady window on a copy of a busy week spent 130MB here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H
Review feedback on Kpa-clawbot#2197: - Eviction scans resolved_path into sql.NullString again. mattn/go-sqlite3 already allocates a string per TEXT column, so RawBytes + unsafe.String only added a copy into database/sql's buffer and an aliasing risk. - pubkeyInterner is folded into internStr, now on master. With the NullString scan, a key returned uninterned past internMax is a slice of that row's own string, not of a reused buffer. - Benchmarks for resolvedPubkeysFromJSON and the eviction batch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H
A13xB0
force-pushed
the
perf/resolved-path-keys-without-pointers
branch
from
October 15, 2026 09:17
058d8e5 to
a7f00ad
Compare
Owner
Author
|
Merged upstream as Kpa-clawbot#2197; closing. |
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.
Refs Kpa-clawbot#1870.
Problem
Load,loadChunkandscanAndMergeChunkdecode each observation'sresolved_pathonly to collect its non-empty pubkeys. On live data that's a JSON array of pubkeys and nulls, about 420 characters, present on most observations. Decoding into[]*stringallocated a string and a pointer for every hop, plus two slices.Change
resolvedPubkeysFromJSONreads the usual array of escape-free strings and nulls in place. Anything else falls back to decoding, so it returns exactlyextractResolvedPubkeys(unmarshalResolvedPath(s)).byNodeand friends) share that copy.resolvedPubkeysForEvictionBatchruns on every eviction batch, about once a minute in steady state, and spent about 130 MB per five minutes on a busy week. It now scans the column as raw bytes, reads it in place without copying (nothing keeps a slice of the driver buffer), and de-duplicates each packet's short list by scanning it instead of building a map per packet.unmarshalResolvedPathare unchanged.Measured
Tests
TestResolvedPubkeysFromJSONAllocations: red commit first, with the bound tightened in green.TestResolvedPubkeysForEvictionBatch: keys, order, and an allocation bound. It has its own red commit.TestResolvedPubkeysFromJSONMatchesDecodingcompares against decode-then-extract on 26 edge cases and 20,000 random inputs.go test ./cmd/serverpasses.With Kpa-clawbot#2183: Kpa-clawbot#2183 adds a general capped interner,
internStr, with the same contract as this PR'spubkeyInterner(look up by substring without copying, clone only on insert, bounded). Whichever lands second, I'll fold this one intointernStrso there is a single interner.No config or UI changes.
🤖 Generated with Claude Code
https://claude.ai/code/session_01KaR4xu8gVykQBkH8yG8m6H