improve(search): adopt Crawlkit v0.16.6 vector speedups - #262
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. Reviewed September 28, 2026, 1:43 AM ET / 05:43 UTC. ClawSweeper reviewWhat this changesThe branch updates Crawlkit, removes a redundant calculation from exact semantic search, and adds a store-query benchmark and zero-vector tests. Merge readiness✅ Ready for maintainer review Keep open: current main and the latest Discrawl release still use the older Crawlkit version. The branch has specific before-and-after benchmark evidence, and this review found no actionable patch defect. Priority: P2 Review scores
Verification
How this fits togetherDiscrawl stores Discord messages and their embeddings in SQLite. A semantic search supplies a query embedding, scores matching stored vectors, then returns ranked message details. flowchart LR
A[Semantic search request] --> B[Query embedding]
B --> C[SQLite message vectors]
C --> D[Decode and score]
D --> E[Rank matches]
E --> F[Message results]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Exact semantic search should use the published faster vector operations while preserving stored-vector bytes, ranking behavior, and zero-vector errors. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction: the contributor reports before-and-after measurements through the production SQLite search path. Is this the best way to solve the issue? Yes. Updating the published vector dependency and removing the extra exact-search norm pass is a narrow way to obtain the measured improvement while retaining the turbovec check. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against db4b140893ee. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
vincentkoc
left a comment
There was a problem hiding this comment.
Reviewed this exact head. The Crawlkit v0.16.6 release is published and available through the Go proxy. Focused semantic/hybrid search tests and trusted review passed; zero-vector error behavior is preserved for both backends. Native source checks pass. No schema/index activation or live-data canary is part of this PR.
Summary
Adopts the vector speedups from crawlkit v0.16.6 (openclaw/crawlkit#144) in exact semantic search.
DecodeFloat32no longer goes throughbinary.Readone coordinate at a time, andCosineSimilarityfuses the candidate norm with the dot product. This also picks up v0.16.5's sidecar path hardening.go.sumchanges only crawlkit's checksums; SQLite and libc versions are unchanged.SearchMessagesSemanticcomputedvector.Norm(stored)for every row before scoring, butCosineSimilarityalready rejects zero candidates. The exact backend now scores directly and computesNormonly on the error path, so the user-visible error stays exactlyscore embedding for message <id>: stored embedding vector is zero. The turbovec backend keeps its pre-check because it batches without callingCosineSimilarity.Benchmarks
New
BenchmarkSearchMessagesSemanticExactruns the real store query (SQLite scan, decode, scoring, ranking, hydration) over 10,000 stored 1,536-d normalized embeddings with limit 20. Apple M3 Ultra, Go 1.27.1, 10 interleaved rounds with rotating order. The host load average was 60–80, hence the wide intervals:GOEXPERIMENT=simdNo SIMD release opt-in.
GOEXPERIMENT=simdadds only 1.14× over the new default, under the 1.3× bar used for openclaw/gitcrawl#218. In the new default build, SQLite row iteration is ~40% of search CPU, with decoding and scoring ~23% each. Release builds, the Dockerfile and CI are unchanged.Correctness
EqualError). The existingstore_test.goassertion is unchanged.go vet,make fmt,make lint, the goreleaser snapshot build for all six targets,GOEXPERIMENT=simd go test ./internal/store, andGOEXPERIMENT=simd GODEBUG=simd=0 go test ./internal/storepass. The full default suite is below.internal/cli'sTestTailLiveEmbeddingsKeepsCaptureAndWriterOwnershipflakes under heavy host load on main too. Over 30 interleaved runs each, main and this branch both passed 29 of 30, and both failures were its wall-clock< 1supsert assertion. Its fixture vectors are 2-dimensional, so this change can't affect it. A separate deflake is tracked as a follow-up.