feat(server): persist multi-byte capability across restart + O(1) per-key lookup (#903)#1324
Open
efiten wants to merge 3 commits into
Open
feat(server): persist multi-byte capability across restart + O(1) per-key lookup (#903)#1324efiten wants to merge 3 commits into
efiten wants to merge 3 commits into
Conversation
5 tasks
Owner
|
Heads up — this needs a small architectural shift before merge. The server has been read-only since #1289 (DB opened with Same pattern as #1283/#1287/#1289: write paths should live in the ingestor (with the file-marker queue or a maintenance ticker). Also: schema migrations now belong in I'll push a relocated version to this branch shortly so you don't have to redo it. Thanks for the work — persistence is exactly what we need; just the wrong side of the fence right now. |
Kpa-clawbot
pushed a commit
to efiten/CoreScope
that referenced
this pull request
May 23, 2026
…pshot (Kpa-clawbot#1324) PR Kpa-clawbot#1324 as-shipped added persistMultibyteCapability() to cmd/server/ that executes UPDATE on nodes/inactive_nodes — impossible after Kpa-clawbot#1289 made the server's *sql.DB read-only. This test asserts the relocated contract: server publishes a snapshot file via internal/mbcapqueue, ingestor's maintenance loop applies it. Adds: - internal/mbcapqueue: snapshot file protocol (mirrors prunequeue pattern) - cmd/ingestor/multibyte_persist.go: STUB returning zero stats - cmd/ingestor/multibyte_persist_test.go: failing integration test Fails on assertions (ReadEntries=0, UpdatedActive=0, etc.) — proves the test gates real behavior. Green commit follows. Refs Kpa-clawbot#1289 Kpa-clawbot#1287 Kpa-clawbot#1322 Kpa-clawbot#1324
Kpa-clawbot
pushed a commit
to efiten/CoreScope
that referenced
this pull request
May 23, 2026
…a-clawbot#1324) PR Kpa-clawbot#903 as-merged-into-this-branch added persistMultibyteCapability() to cmd/server/store.go that executed UPDATE on nodes / inactive_nodes. This is impossible after Kpa-clawbot#1289 made the server's *sql.DB read-only (mode=ro) and violates the canonical-writer invariant established in Kpa-clawbot#1283 / Kpa-clawbot#1287 (ingestor owns ALL DB writes). Architecture (option b — file-marker handoff): server analytics cycle ──snapshot.json──▶ internal/mbcapqueue │ ingestor (5m ticker) ◀┘ │ ▼ UPDATE nodes/inactive_nodes Rationale for (b) over (a, ingestor recompute): the capability computation lives in ~500 LOC of server-side analytics (computeMultiByteCapability + hash-size adopter inference + observer gating). Duplicating it in the ingestor would couple two binaries to the same compute graph and double maintenance cost. File handoff mirrors the established prunequeue pattern from Kpa-clawbot#738 / Kpa-clawbot#1287. Changes: - internal/mbcapqueue: snapshot file protocol (atomic write/rename) - internal/dbschema: canonical schema migration (replaces the ad-hoc applySchema migration in cmd/ingestor/db.go); AssertReady extended to verify the four columns exist on nodes / inactive_nodes - cmd/ingestor/multibyte_persist.go: RunMultibyteCapPersist applies the snapshot — only runtime writer of multibyte_sup / multibyte_evidence - cmd/ingestor/main.go: 5-minute ticker (staggered 2m after boot) - cmd/server/store.go: replaces persist/maybePersist with publishMultibyteCapSnapshot; load path preserved - cmd/server/multibyte_capability_test.go: persistence tests removed (covered by ingestor side now); LoadFromDB + GetMultibyteCapFor read-path tests kept - cmd/server/readonly_invariant_test.go: grep + reflect bans on persistMultibyteCapability / maybePersistMultibyteCapability and on UPDATE … nodes/inactive_nodes SET multibyte_* Data-destruction guard preserved: entries with status=='unknown' (sup==0) are filtered in the ingestor before any UPDATE, so a stale analytics snapshot can never overwrite a confirmed/suspected DB row. Server-side write-call audit: $ grep -rn 'multibyte.*UPDATE\|persistMultibyte' cmd/server/*.go | grep -v _test (zero hits) Tests: cmd/ingestor go test ./... ok (26s), cmd/server go test ./... ok (44s). Refs Kpa-clawbot#903 Kpa-clawbot#1283 Kpa-clawbot#1287 Kpa-clawbot#1289 Kpa-clawbot#1322 Kpa-clawbot#1324
…kup (Kpa-clawbot#903) Adds the persistence layer and per-key O(1) getter that were net-new versus current master after Kpa-clawbot#1002 and related PRs shipped the overlay UI. **What's added:** - `multibyte_sup_v1` migration: adds `multibyte_sup INTEGER NOT NULL DEFAULT 0` and `multibyte_evidence TEXT` to `nodes`/`inactive_nodes` so capability survives ingestor restart - `hasMultibyteSupCols` detection in `detectSchema()` gates persist/load paths - `loadMultibyteCapFromDB()`: pre-populates `mbCapSnapshot`/`mbCapIndex` at server startup so cold starts serve last-known capability without waiting for the first ~15s analytics cycle - `maybePersistMultibyteCapability()` + `persistMultibyteCapability()`: called after each analytics cycle; TryLock-gated so concurrent cycles coalesce; skips `sup==0` entries to prevent overwriting confirmed/suspected DB values - `GetMultibyteCapFor(pk)`: O(1) map lookup via `mbCapIndex`; both route call sites in `handleNodes` and the node-detail handler now use this instead of the O(N)-alloc `GetMultiByteCapMap()` **Not changed:** API field names (`multi_byte_status`, `multi_byte_evidence`, `multi_byte_max_hash_size`), `EnrichNodeWithMultiByte`, `GetMultiByteCapMap`, `map.js`, `live.css`, `Dockerfile`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pshot (Kpa-clawbot#1324) PR Kpa-clawbot#1324 as-shipped added persistMultibyteCapability() to cmd/server/ that executes UPDATE on nodes/inactive_nodes — impossible after Kpa-clawbot#1289 made the server's *sql.DB read-only. This test asserts the relocated contract: server publishes a snapshot file via internal/mbcapqueue, ingestor's maintenance loop applies it. Adds: - internal/mbcapqueue: snapshot file protocol (mirrors prunequeue pattern) - cmd/ingestor/multibyte_persist.go: STUB returning zero stats - cmd/ingestor/multibyte_persist_test.go: failing integration test Fails on assertions (ReadEntries=0, UpdatedActive=0, etc.) — proves the test gates real behavior. Green commit follows. Refs Kpa-clawbot#1289 Kpa-clawbot#1287 Kpa-clawbot#1322 Kpa-clawbot#1324
…a-clawbot#1324) PR Kpa-clawbot#903 as-merged-into-this-branch added persistMultibyteCapability() to cmd/server/store.go that executed UPDATE on nodes / inactive_nodes. This is impossible after Kpa-clawbot#1289 made the server's *sql.DB read-only (mode=ro) and violates the canonical-writer invariant established in Architecture (option b — file-marker handoff): server analytics cycle ──snapshot.json──▶ internal/mbcapqueue │ ingestor (5m ticker) ◀┘ │ ▼ UPDATE nodes/inactive_nodes Rationale for (b) over (a, ingestor recompute): the capability computation lives in ~500 LOC of server-side analytics (computeMultiByteCapability + hash-size adopter inference + observer gating). Duplicating it in the ingestor would couple two binaries to the same compute graph and double maintenance cost. File handoff mirrors the established prunequeue pattern from Kpa-clawbot#738 / Kpa-clawbot#1287. Changes: - internal/mbcapqueue: snapshot file protocol (atomic write/rename) - internal/dbschema: canonical schema migration (replaces the ad-hoc applySchema migration in cmd/ingestor/db.go); AssertReady extended to verify the four columns exist on nodes / inactive_nodes - cmd/ingestor/multibyte_persist.go: RunMultibyteCapPersist applies the snapshot — only runtime writer of multibyte_sup / multibyte_evidence - cmd/ingestor/main.go: 5-minute ticker (staggered 2m after boot) - cmd/server/store.go: replaces persist/maybePersist with publishMultibyteCapSnapshot; load path preserved - cmd/server/multibyte_capability_test.go: persistence tests removed (covered by ingestor side now); LoadFromDB + GetMultibyteCapFor read-path tests kept - cmd/server/readonly_invariant_test.go: grep + reflect bans on persistMultibyteCapability / maybePersistMultibyteCapability and on UPDATE … nodes/inactive_nodes SET multibyte_* Data-destruction guard preserved: entries with status=='unknown' (sup==0) are filtered in the ingestor before any UPDATE, so a stale analytics snapshot can never overwrite a confirmed/suspected DB row. Server-side write-call audit: $ grep -rn 'multibyte.*UPDATE\|persistMultibyte' cmd/server/*.go | grep -v _test (zero hits) Tests: cmd/ingestor go test ./... ok (26s), cmd/server go test ./... ok (44s). Refs Kpa-clawbot#903 Kpa-clawbot#1283 Kpa-clawbot#1287 Kpa-clawbot#1289 Kpa-clawbot#1322 Kpa-clawbot#1324
Kpa-clawbot
force-pushed
the
feat/multibyte-persist-903
branch
from
1004696 to
fbb50e2
Compare
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
2 participants
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.
Summary
Follows the reconciliation recommendation in #916 — extracts only the NET-NEW persistence layer from that PR (which is now superseded by #1002 for the overlay UI) into a focused 6-file change against current master.
What this adds:
multibyte_sup_v1migration:multibyte_sup INTEGER NOT NULL DEFAULT 0+multibyte_evidence TEXTonnodes/inactive_nodesso capability survives restarthasMultibyteSupColsschema detection gates the persist/load pathsloadMultibyteCapFromDB(): pre-populatesmbCapSnapshot/mbCapIndexat startup — cold starts serve last-known capability without waiting for the first ~15s analytics cyclemaybePersistMultibyteCapability()+persistMultibyteCapability(): after each analytics cycle; TryLock-gated (concurrent cycles coalesce); skipssup==0entries (data-destruction guard)GetMultibyteCapFor(pk): O(1) map lookup; bothhandleNodesand node-detail call sites updated from the O(N)-allocGetMultiByteCapMap()What this explicitly does NOT change:
multi_byte_status,multi_byte_evidence,multi_byte_max_hash_size)EnrichNodeWithMultiByte— unchangedGetMultiByteCapMap— still present for any external callerspublic/map.js,public/live.css,Dockerfile,docs/— zero frontend churnTest plan
TestMultibyteCapPersistRoundTrip— confirmed values survive persist → fresh-store loadTestMultibyteCapPersistSkipsUnknown— data-destruction guard:sup==0entry does not overwrite DB-confirmed valueTestMultibyteCapMaybePersistCoalesces— TryLock coalesces 10 concurrent callers without deadlockTestMultibyteCapGetMultibyteCapForO1— O(1) index returns correct entry / false for unknown pubkeyTestMultibyteCapLoadFromDB— onlysup>0rows loaded;sup==0row excludedTestSchemaMultibyteSupColumns— migration adds columns to both tables; idempotent on secondOpenStoreTestMultiByteCapability_*tests pass unchangedokin 27sgo build ./cmd/server/ && go build ./cmd/ingestor/clean🤖 Generated with Claude Code