Skip to content

improve(search): adopt Crawlkit v0.16.6 vector speedups - #262

Merged
vincentkoc merged 1 commit into
mainfrom
perf/crawlkit-0.16.6-vector
Sep 28, 2026
Merged

vincentkoc merged 1 commit into
mainfrom
perf/crawlkit-0.16.6-vector

Conversation

@steipete

Copy link
Copy Markdown
Contributor

Summary

Adopts the vector speedups from crawlkit v0.16.6 (openclaw/crawlkit#144) in exact semantic search.

  • Bump crawlkit v0.16.4 → v0.16.6. DecodeFloat32 no longer goes through binary.Read one coordinate at a time, and CosineSimilarity fuses the candidate norm with the dot product. This also picks up v0.16.5's sidecar path hardening. go.sum changes only crawlkit's checksums; SQLite and libc versions are unchanged.
  • Drop a redundant pass per candidate. SearchMessagesSemantic computed vector.Norm(stored) for every row before scoring, but CosineSimilarity already rejects zero candidates. The exact backend now scores directly and computes Norm only on the error path, so the user-visible error stays exactly score embedding for message <id>: stored embedding vector is zero. The turbovec backend keeps its pre-check because it batches without calling CosineSimilarity.

Benchmarks

New BenchmarkSearchMessagesSemanticExact runs 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:

origin/main (crawlkit v0.16.4) This PR This PR + GOEXPERIMENT=simd
sec/op 505.7 ms ± 73% 136.7 ms ± 32% 119.9 ms ± 33%
allocs/op 15.5M 140.5k 140.5k
B/op 236.7 MiB 177.6 MiB 177.6 MiB

No SIMD release opt-in. GOEXPERIMENT=simd adds 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

  • A new test covers both backends and asserts the zero-stored-vector error text exactly (EqualError). The existing store_test.go assertion is unchanged.
  • go vet, make fmt, make lint, the goreleaser snapshot build for all six targets, GOEXPERIMENT=simd go test ./internal/store, and GOEXPERIMENT=simd GODEBUG=simd=0 go test ./internal/store pass. The full default suite is below.
  • internal/cli's TestTailLiveEmbeddingsKeepsCaptureAndWriterOwnership flakes 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 < 1s upsert assertion. Its fixture vectors are 2-dimensional, so this change can't affect it. A separate deflake is tracked as a follow-up.

@steipete
steipete requested a review from a team as a code owner September 28, 2026 05:40
@clawsweeper

clawsweeper Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 28, 2026
@clawsweeper

clawsweeper Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 28, 2026, 1:43 AM ET / 05:43 UTC.

ClawSweeper review

What this changes

The 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
Reviewed head: b25ea01ad52f0bbbb146c7636735eca358d3dc84

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A narrow, well-covered optimization has relevant before-and-after store benchmarks, with timing precision limited by reported host load.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (live_output): The contributor reports ten before-and-after runs of the changed production store query against a temporary SQLite archive on an Apple M3 Ultra, observing 505.7 ms to 136.7 ms per search and substantially fewer allocations; the benchmark also checks the top result. The dependency's byte-equivalence test verifies compatibility with existing stored vectors without migration.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The contributor reports ten before-and-after runs of the changed production store query against a temporary SQLite archive on an Apple M3 Ultra, observing 505.7 ms to 136.7 ms per search and substantially fewer allocations; the benchmark also checks the top result. The dependency's byte-equivalence test verifies compatibility with existing stored vectors without migration.
Evidence reviewed 9 items Introduced search change: The pinned PR diff moves the zero-vector check to the exact scorer's error path while retaining the pre-check for batched turbovec scoring.
Production boundary: The store reads matching embedding blobs from SQLite, decodes and scores each candidate, ranks matches, and hydrates the selected messages; the CLI calls this store path for semantic search.
Dependency contract signal: Discrawl directly calls Crawlkit vector scoring and decoding, so the Crawlkit vector contract applies to this review.
Findings None None.
Security None None.

How this fits together

Discrawl 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]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Code and test delta production +3 net lines; tests +143 lines The production change is narrow, with a store benchmark and backend error coverage.
Reported exact-search benchmark 505.7 ms → 136.7 ms per search The contributor measured the full store query path, although host load made timing intervals wide.

Technical review

Best 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.

Labels

Label changes:

  • add P2: This is a bounded performance improvement to optional semantic search, with no reported urgent regression.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The contributor reports ten before-and-after runs of the changed production store query against a temporary SQLite archive on an Apple M3 Ultra, observing 505.7 ms to 136.7 ms per search and substantially fewer allocations; the benchmark also checks the top result. The dependency's byte-equivalence test verifies compatibility with existing stored vectors without migration.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The contributor reports ten before-and-after runs of the changed production store query against a temporary SQLite archive on an Apple M3 Ultra, observing 505.7 ms to 136.7 ms per search and substantially fewer allocations; the benchmark also checks the top result. The dependency's byte-equivalence test verifies compatibility with existing stored vectors without migration.

Label justifications:

  • P2: This is a bounded performance improvement to optional semantic search, with no reported urgent regression.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The contributor reports ten before-and-after runs of the changed production store query against a temporary SQLite archive on an Apple M3 Ultra, observing 505.7 ms to 136.7 ms per search and substantially fewer allocations; the benchmark also checks the top result. The dependency's byte-equivalence test verifies compatibility with existing stored vectors without migration.
  • proof: sufficient: Contributor real behavior proof is sufficient. The contributor reports ten before-and-after runs of the changed production store query against a temporary SQLite archive on an Apple M3 Ultra, observing 505.7 ms to 136.7 ms per search and substantially fewer allocations; the benchmark also checks the top result. The dependency's byte-equivalence test verifies compatibility with existing stored vectors without migration.

Evidence

What I checked:

  • Introduced search change: The pinned PR diff moves the zero-vector check to the exact scorer's error path while retaining the pre-check for batched turbovec scoring. (internal/store/query.go:503, b25ea01ad52f)
  • Production boundary: The store reads matching embedding blobs from SQLite, decodes and scores each candidate, ranks matches, and hydrates the selected messages; the CLI calls this store path for semantic search. (internal/store/query.go:440, b25ea01ad52f)
  • Dependency contract signal: Discrawl directly calls Crawlkit vector scoring and decoding, so the Crawlkit vector contract applies to this review. (internal/store/embeddings.go:500, b25ea01ad52f)
  • Scoring contract: The published v0.16.6 scorer fuses dot product and candidate norm and returns an error for a zero candidate. (vector/vector.go:63, 610c54c15ab1)
  • Stored-vector compatibility: Crawlkit's encoding test compares the new byte sequence and decoded float bits with the previous little-endian implementation, including edge-case values. Existing stored vectors need no migration. (vector/encoding_test.go:12, 610c54c15ab1)
  • Behavior coverage: The added test asserts the exact zero-stored-vector error for both supported backends; the benchmark invokes the production store query against a temporary SQLite archive. (internal/store/store_test.go:1045, b25ea01ad52f)

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)
  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@vincentkoc vincentkoc changed the title perf(search): adopt crawlkit 0.16.6 vector speedups improve(search): adopt Crawlkit v0.16.6 vector speedups Sep 28, 2026

@vincentkoc vincentkoc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@vincentkoc
vincentkoc merged commit 4792f2b into main Sep 28, 2026
25 checks passed
@vincentkoc
vincentkoc deleted the perf/crawlkit-0.16.6-vector branch September 28, 2026 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants