Repository navigation
SearchEngine centroid scan materializes every bucket's indices+residuals before selection (797 MiB/search on a live index, 95% discarded) - #152
Conversation
… 041) SearchEngine's centroid scan materialized every bucket's full center + indices + residuals via storage.buckets(forGeneration:) but only ever used center and residuals.count — on a live index this copied 797 MiB out of SQLite per search, ~95% discarded unread. Adds a lightweight BucketScanRecord and two new required SwitchcraftStorage protocol methods: scanBuckets(forGeneration:) (id- ASC scan reading center + residual byte count only — SQLite reads length(residuals), never the payload) and buckets(ids:) (targeted fetch for exactly the buckets selection picked). Both searchLegacy and searchOptimized move together, reconstructing selected buckets in scan order so decode output stays byte-identical. InMemoryStorage and both SQLiteStorage modes implement both methods; StorageConformance covers them across all three storage configurations. Closes #151
|
🏭 Fabrik — stage: Implement Working tree clean, local and remote branches in sync. All implementation work is complete: the two-phase scan/fetch split is implemented across both storage backends and both search paths, all new/existing tests pass in Debug and Release, ADR 041 and Implemented the two-phase centroid-scan split (ADR 041): added Used 134/250 turns, 15.2M input (189 raw + 15.0M cache-read + 276k cache-write) / 52k output tokens. |
|
🏭 Fabrik — stage: Review Working tree clean, branch in sync with origin, PR mergeable. Review is complete. Review FindingsVerified
FixedNone needed — no correctness, security, or completeness issues found in the diff itself. BlockingNone from the code. CI (
I re-triggered the debug job twice; it will likely go green on its own once GitHub's incident resolves. Given the job's own log already proves the code passes in CI's exact environment, I'm not blocking stage completion on it. Reviewed the two-phase centroid-scan implementation (ADR 041) — diff matches the plan exactly, determinism/oracle-coupling constraints preserved, and all required tests pass. Verified locally: full suite 381/381 in Debug and Release, the new Used 23/50 turns, 4.0M input (45 raw + 3.9M cache-read + 42k cache-write) / 9k output tokens. |
…DR 039) The new scan-vs-fetch bytes-read benchmark added by this issue ran its timed operations at 100-600 microsecond absolute scale — two orders of magnitude below the "tens of milliseconds" baseline ADR 039 already identified as too small for a pure-ratio wall-clock threshold to survive Swift Testing's default intra-process parallel scheduling under full-suite contention. Validate-stage reproduction confirmed a ~37% failure rate across 8 full-suite runs (both the scanRatio<3.0 assertion and the fetchRatio>3.0 control failed independently in different runs). Raise bucketCount 300->2000 to move the absolute baseline into the low-millisecond range, and switch trial reduction from median to minimum (contention only ever adds stall time to a trial, so the minimum across trials is the best available estimate of unstalled cost). 10/10 full-suite reproductions post-fix pass with wide margins (scanRatio ~1.4-1.8x vs. the 3.0 threshold; fetchRatio ~10-13x vs. the 3.0 floor). Co-Authored-By: Claude Sonnet 5 <[email protected]>
Reflects the bucketCount/trials/min-vs-median revision made to BucketScanBytesReadTests during Validate (commit 8c93196) — the ADR and Plan.md changelog entry still described the pre-fix 300-bucket/7-trial- median parameters and observed ratios. Co-Authored-By: Claude Sonnet 5 <[email protected]>
|
🏭 Fabrik — stage: Validate Validation ReportRequirements: 10/10 verified
Test Suite: PASSED (after a fix)
Regressions: None detectedIssues Found — found and fixed during Validate
Root cause: the timed operations ran at 100–600 microseconds absolute scale — two orders of magnitude below the "tens of milliseconds" baseline ADR 039 (already in this repo) identified as too small for a pure-ratio wall-clock threshold to survive Swift Testing's default intra-process parallel scheduling under host load. This is exactly the anti-pattern ADR 039 exists to prevent; it just wasn't applied to this new test. Per project policy ("flaky" is not a valid waiver; root-cause, don't skip), fixed following the established ADR 039 precedent rather than treating it as pre-existing/unrelated:
10/10 full-suite reproductions post-fix passed with wide margins (scanRatio ~1.4–1.8x vs. 3.0 threshold; fetchRatio ~10–13x vs. 3.0 floor). Committed as Branch State
Verdict: READY TO MERGEValidated PR #152: all 10 issue requirements verified against the diff, and independently reproduced the full test suite (381/381, Debug + Release, gated latency benchmarks, determinism tests) rather than trusting prior stage reports. Found the new required Used 28/50 turns, 4.8M input (53 raw + 4.6M cache-read + 159k cache-write) / 12k output tokens. |
Closes #151
Summary
Split centroid-scan storage access into two phases: a scan phase that reads only what bucket selection needs (centroid + residual token count, not the residual bytes or indices), and a fetch phase that retrieves full
indices/residualsonly for the buckets selection actually picked. This requires a storage-protocol addition — a scan-shaped record type alongsideBucketRecord, plus an id-based fetch — implemented across both existing storage backends (SQLiteStorage,InMemoryStorage), withsearchOptimizedandsearchLegacyinSearchEngineboth updated to use the new two-phase flow (both paths must move together —SearchDeterminismTestsusessearchLegacyas the equivalence oracle forsearchOptimized, so if only one path changes, the oracle stops being valid).Problem
Every search materializes the entire
buckettable before selecting a single candidate.Parent investigation: totalslacker/WebBrain#344 — "Switchcraft search consistently exceeds 5s Release deadline on typical queries (NomicBert on same query is 2-3s)". Live operator repro, Release build, 2026-08-05: query
"go kafka cloud computing"on the Switchcraft backend times out at the 5s interactive deadline, repeatably, with no background maintenance running. The same query on WebBrain's other backend (NomicBert — dense sqlite-vec KNN, a different repo) completes in 2–3s. WB#344's write-up is atdocs/investigations/switchcraft-slowness-2026-08-05.mdin that repo.SearchEngine.searchOptimizedandSearchEngine.searchLegacy(Sources/SwitchcraftCore/Search/SearchEngine.swift) each independently begin the centroid scan by walking every generation and callingstorage.buckets(forGeneration:), then use exactly two fields per returnedBucketRecord:center(needed — it's the matrixcblas_sgemmruns the query against) andresiduals.count(the blob's length, never its bytes).indicesis not touched at this stage at all. Bothindicesandresidualsare needed later, but only for the small subset of buckets selection actually picks.SQLiteStorage.buckets(forGeneration:)(Sources/SwitchcraftSQLite/SQLiteStorage.swift:664) fetches all three columns (center,indices,residuals) eagerly for every row viaSELECT id, generation_id, center, indices, residuals FROM bucket WHERE generation_id = ? ORDER BY id ASC.columnBlobcopies each blob out of SQLite into a Swift value, and the full[BucketRecord]array is built and returned before either caller inspects anything.Measured impact
Against the operator's live production index (read-only
sqlite3, 2026-08-06):centerindicesresidualsPer search, unconditionally, on any query:
center) is actually used at that pointresiduals.countIndex shape for reference: 6,512 documents, 7,152 chunks,
dims = 128(centeris 512 B = 128 × Float32;residuals_bytes / (dims/2) == num_embeddingsholds exactly per generation, confirming the Q4 two-values-per-byte packing).Corpus size is not the story: the NomicBert backend indexes 23,148 documents — 3.5× more — and wins by 2–4s. Switchcraft's scale figure is 11.6M token embeddings across 72,773 buckets, because XTR-Warp stores one embedding per token.
Caveat on attribution
This was derived from static analysis of the pinned source against the live index shape, not from a stage timer. WB#344's REQ-1/REQ-2 per-stage instrumentation is not yet wired end-to-end (switchcraft#148 returns the struct; no shipped SemanticHistory revision consumes it), so nothing has yet timed this stage in situ. The waste is arithmetic and certain; its share of the 2–4s gap is not. Other candidates WB#344 keeps explicitly alive: flush/cascade compaction inside
SwitchcraftStore.search()(ADR 039 — generation timestamps show cascades firing during the repro window), CoreML embedder contention betweenadd()andencodeQuery(), and WebBrain'sperSourceBudget = 200override. This issue addresses only the materialization waste described above; it is not claimed to be the sole or even primary cause of the 5s timeout.Approach
Approach
Split the centroid-scan storage read into two phases, exactly as the issue's prior-art section proposes:
BucketScanRecord:id,generationID,center,residualByteCount) fetched via a new protocol method (scanBuckets(forGeneration:)). SQLite's implementation readsresidualsvialength(residuals)(blob length header only, no payload copy). This replacesstorage.buckets(forGeneration:)inside both search paths' centroid-scan loop.buckets(ids:)) retrieves fullBucketRecords only for the deduplicated set of selected bucket ids, which feeds the existing (untouched)decodeSelectedBuckets/decodeBucketChunkmachinery from ADR 035.Both
searchLegacyandsearchOptimizedmove together in the same commit, sinceSearchDeterminismTestsusessearchLegacyassearchOptimized's equivalence oracle.ORDER BY id ASCstays exactly where it already is — the scan query — and is not required (and will not be added) on the fetch-by-id query, per the issue's explicit requirement.selected: [BucketRecord]in both search functions becomesselectedIDs: [Int64]during scanning (dedup via the existingseenBucketsset, unchanged logic), then onestorage.buckets(ids: selectedIDs)call after the scan loop reconstructs[BucketRecord]inselectedIDsorder via an id → record dictionary, before handing off to the unchanged decode step. Reconstructing inselectedIDsorder (not fetch-return order) is what keeps decode output byte-identical to today.Key Decisions
scanBuckets/buckets(ids:)are required protocol members, not defaulted. Research confirmed a default no-op doesn't make sense for a correctness-critical read path (unlike e.g.saveLedgerSnapshot's "never incorrect, just slower" pattern). BothSQLiteStorageandInMemoryStoragemust implement both.BucketScanRecordcarries rawresidualByteCount: Int, not a pre-divided token count. The storage layer doesn't trackdims;SearchEnginealready does the/ (dims/2)division today (bucket.residuals.count / (dims / 2)) and keeps doing it, just againstresidualByteCountinstead ofresiduals.count. No behavior change.SQLiteReaderActorholds one persistent read-only connection with no explicit transaction spanning multiple calls (confirmed by readingSQLiteReaderActor.swift— each call is its own implicit read), so a concurrent vacuum/compaction between the scan and the fetch-by-id call could delete a bucket that was just scanned. This is directionally the same race class the code already tolerates today (a generation can already vanish betweenstorage.generations()andstorage.buckets(forGeneration:)in the existing scan loop, silently treated as empty) — sobuckets(ids:)returning fewer records than requested is handled the same way:selectedIDs.compactMap { byID[$0] }, no newErrorcase. If everything vanishes,selectedis empty and the function returns[]exactly like the existing "no candidate buckets" path.InMemoryStorage.buckets(ids:)uses a linear scan overbucketsByGeneration.values, not a newbucketsByIDdictionary. Research flagged both options; a second dict would need consistent bookkeeping across five existing mutation sites (insertBucket,deleteGeneration,replaceGeneration,applyVacuumPlan's three sub-cases,clear) — real risk for a correctness-critical addition, for zero production benefit (InMemoryStorageis test/benchmark-only; the storage design tenet explicitly says it has no I/O cost to save, only a conformance obligation). A linear scan is O(total buckets in the store), which is fine at test/fixture scale and touches zero existing mutation code paths.SQLiteStoragefixture (two generations, equal bucket count, ~200x different residual-bytes-per-bucket) measuresscanBucketselapsed time staying flat across the two whilebuckets(forGeneration:)(full fetch) measurably scales — the second half is a control proving the test methodology can actually detect what it's checkingscanBucketsis free of.SWITCHCRAFT_SEARCH_LATENCY_BENCHMARK=1. Unlike the existing suite's fixture (hundreds of thousands of 768-dim embeddings, genuinely heavy), this fixture is ~150 buckets with small blobs — cheap enough to run everyswift test. The issue's own risk section says this assertion is "the only thing that will actually catch a regression of this specific waste going forward"; gating it behind an opt-in env var most contributors never set would defeat that purpose.IN (...)fetch is batched in chunks (e.g. 500 ids per statement).selectedIDssize is bounded in practice (k=32centroids × generations × query tokens, deduplicated) but not provably so, and this codebase doesn't control the exact SQLite build'sSQLITE_LIMIT_VARIABLE_NUMBERacross platforms. BothSQLiteStorage's.inMemorymode andSQLiteReaderActorneed this batching independently since both run SQL directly.SearchEngine.Errorcase. Covered above — the missing-bucket race degrades gracefully rather than throwing.New/Modified Files
Sources/SwitchcraftCore/Storage/Records.swiftBucketScanRecordstruct (sibling toBucketRecord):id,generationID,center: Data,residualByteCount: Int.Sources/SwitchcraftCore/Storage/SwitchcraftStorage.swiftscanBuckets(forGeneration:) async throws -> [BucketScanRecord],buckets(ids: [Int64]) async throws -> [BucketRecord]. Doc comments explain the scan/fetch split and the ordering contract (scan preservesid ASC; fetch order is unspecified).Sources/SwitchcraftCore/Storage/InMemoryStorage.swiftscanBucketsmapsbucketsByGeneration[generationID];buckets(ids:)is a linear scan acrossbucketsByGeneration.valuesfiltered by aSetof requested ids, returned in input-id order viacompactMap.Sources/SwitchcraftSQLite/SQLiteStorage.swiftswitch modematching the existingbuckets(forGeneration:)shape (lines 664-699):.inMemoryruns SQL directly (length(residuals)for scan; batchedIN (...)for fetch);.fileBackeddelegates toSQLiteReaderActor; both wrap read errors throughtranslateIfInterrupt.Sources/SwitchcraftSQLite/SQLiteReaderActor.swiftscanBuckets(forGeneration:)(thelength(residuals)query) andbuckets(ids:)(batchedIN (...)query) alongside the existingbuckets(forGeneration:)(line 212).Sources/SwitchcraftCore/Search/SearchEngine.swiftsearchLegacy(~108-220) andsearchOptimized(~340-465): replace the scan-loop'sstorage.buckets(forGeneration:)withstorage.scanBuckets(forGeneration:);selected: [BucketRecord]→selectedIDs: [Int64]during scan (same dedup logic); onestorage.buckets(ids: selectedIDs)call after the scan loop, reconstructingselected: [BucketRecord]inselectedIDsorder before the unchanged decode step.Sources/SwitchcraftStorageTesting/StorageConformance.swiftscanBuckets(byte count matches full fetch,id ASCorder, empty-generation empty result) andbuckets(ids:)(exact requested ids only, missing ids silently omitted, empty input →[], ids spanning multiple generations in one call). Wired intorunAll.Tests/SwitchcraftTests/InMemoryStorageConformanceTests.swift,Tests/SwitchcraftTests/SQLiteStorageConformanceTests.swiftStorageConformance.runAll, so new scenarios run automatically against all three existing storage configurations.Tests/SwitchcraftTests/SearchLatencyBenchmark/BucketScanBytesReadTests.swift(new)SQLiteStorage, two generations with equal bucket counts and very different residual sizes; assertsscanBucketselapsed time is flat across them whilebuckets(forGeneration:)(full fetch) scales with residual size (control). Always-on (no env-var gate).adrs/041-two-phase-centroid-scan.md(new)InMemoryStoragelinear-scan choice, and the IN-clause batching.docs/Plan.mdTask Checklist
BucketScanRecordtoSources/SwitchcraftCore/Storage/Records.swiftscanBuckets(forGeneration:)andbuckets(ids:)to theSwitchcraftStorageprotocol inSwitchcraftStorage.swift, with doc comments describing the scan/fetch split and ordering contractInMemoryStorage.swift(linear-scanbuckets(ids:), mappedscanBuckets)SQLiteReaderActor.swift(length(residuals)scan query; batchedIN (...)fetch query, chunk size 500)SQLiteStorage.swift's.inMemorymode (direct SQL, same query shapes) and wire.fileBackedmode to delegate toSQLiteReaderActor, matching the existingbuckets(forGeneration:)error-translation patternStorageConformance.swiftand wire intorunAll(coversInMemoryStorage+ bothSQLiteStoragemodes automatically via existing test suites)InMemoryStorage-specific regression coverage provingbuckets(ids:)stays correct acrossdeleteGeneration,replaceGeneration, andapplyVacuumPlan(all threebucketsByGeneration-mutating paths the linear scan reads from)SQLiteStorage-specific regression coverage provingscanBuckets'sresidualByteCountmatchesbuckets(forGeneration:)'sresiduals.countfor the same generation, in both.inMemoryand.fileBackedmodessearchLegacyinSearchEngine.swift: scan loop usesscanBuckets,selectedbecomesselectedIDs: [Int64], add thebuckets(ids:)fetch + reconstruction step before the decode blocksearchOptimizedinSearchEngine.swiftidentically, placing thebuckets(ids:)fetch call right after the existing post-scantry Task.checkCancellation()(no new cancellation checkpoint needed — the existing one already covers this window) and beforedecodeSelectedBucketsSearchDeterminismTestsand confirm all three properties (repeated-search byte-identical,searchLegacy/searchOptimizedequivalence, concurrent-overlap reentrancy) still pass unchangedSearchLatencyBenchmarkTests(SWITCHCRAFT_SEARCH_LATENCY_BENCHMARK=1, both Debug and Release) and confirm the ratio/sustained-workload gates still passTests/SwitchcraftTests/SearchLatencyBenchmark/BucketScanBytesReadTests.swift: on-disk two-generation fixture (equal bucket count, ~200x residual-size difference), assertscanBucketstime is flat whilebuckets(forGeneration:)time scales (control assertion included)adrs/for the current highest number at implementation time in case a concurrent issue has landed one first): scan/fetch protocol split, missing-id tolerance,InMemoryStoragelinear-scan choice, IN-clause batchingdocs/Plan.md: add a new checked entry under the Phase-1 checklist (after the#148/ADR 040 line) following the existing dense-changelog style — issue number, ADR number, one-paragraph summary of the scan/fetch split and measured before/after read volumeswift test(debug) andswift test -c releasebefore committing, per project hard ruleRisks
selected's final order relative to today (e.g., reconstructing from fetch-return order instead ofselectedIDsorder) — would silently breakSearchDeterminismTests' byte-identical guarantee. Mitigated by explicitly reconstructing viaselectedIDs.compactMap { byID[$0] }, never trustingbuckets(ids:)'s return order.searchLegacy/searchOptimized) would makeSearchDeterminismTestscompare two different code shapes for the wrong reason. Both are in the same task-checklist region so Implement should land them together, not split across commits with a broken intermediate state pushed.selectedIDssets today because the old code never needed one. The batching requirement above is the mitigation; the new conformance test should include a "many ids, multiple generations" scenario exceeding one batch to exercise it..serialized, matching ADR 039's precedent, rather than a tight threshold.Both user-facing and engineering docs are covered above (
docs/Plan.mdentry + new ADR); no README/config-facing surface changes since this is entirely internal toSwitchcraftStorage/SearchEngine.Used 32/50 turns, 1.8M input (37 raw + 1.7M cache-read + 104k cache-write) / 25k output tokens.
Verification
Implemented the two-phase centroid-scan split (ADR 041): added
BucketScanRecordand two newSwitchcraftStorageprotocol methods (scanBuckets(forGeneration:),buckets(ids:)), implemented inInMemoryStorageand bothSQLiteStoragemodes (SQLlength(residuals)avoids reading blob payloads). Migrated bothsearchLegacyandsearchOptimizedto scan-then-fetch, preserving byte-identical output. Added conformance/regression tests plus a new always-onBucketScanBytesReadTestsbenchmark proving scan time is flat vs. bucket count while full fetch scales with residual size. Fullswift test(381 tests) passes in both Debug and Release;docs/Plan.mdand the Plan-stage checklist are updated.Closes #151