Skip to content

feat(search): add turbovec vector backend - #83

Closed
vincentkoc wants to merge 4 commits into
mainfrom
feat/turbovec-support
Closed

feat(search): add turbovec vector backend#83
vincentkoc wants to merge 4 commits into
mainfrom
feat/turbovec-support

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

Summary

Adds optional turbovec-backed semantic scoring for discrawl.

  • adds [search.embeddings].vector_backend with exact as the default
  • wires the configured backend into semantic and hybrid search
  • documents the Python turbovec dependency and dimensions-divisible-by-8 contract
  • updates discrawl to the crawlkit turbovec branch pseudo-version

Release order

Depends on openclaw/crawlkit#18. Before discrawl release, replace the crawlkit pseudo-version with the final crawlkit release tag.

Verification

  • GOWORK=off go test -count=1 ./...
  • temp-home CLI smokes using a built binary:
    • discrawl --help
    • discrawl --version
    • discrawl metadata --json
    • discrawl status --json
    • discrawl tui --json

@clawsweeper

clawsweeper Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed June 7, 2026, 10:57 PM ET / 02:57 UTC.

Summary
The PR adds an optional [search.embeddings].vector_backend setting for exact versus turbovec semantic scoring, wires it into semantic and hybrid search, updates docs/tests, and points crawlkit at a turbovec pseudo-version.

Reproducibility: not applicable. This PR adds a new optional backend rather than reporting broken existing behavior. Source inspection shows current main has no vector backend selector.

Review metrics: 2 noteworthy metrics.

  • Config surface: 1 new TOML key. [search.embeddings].vector_backend changes user configuration and needs upgrade-safe defaults and docs.
  • Dependency release state: 1 crawlkit pseudo-version. The merge path depends on replacing the temporary crawlkit version before release.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Replace the crawlkit pseudo-version with a released tag after the upstream PR lands.
  • Remove the direct CHANGELOG.md entry.
  • [P2] Add or post a discrawl-level turbovec semantic-search smoke if maintainers want release confidence for the opt-in path.

Risk before merge

  • [P1] Merging or releasing before feat(vector): add turbovec search backend crawlkit#18 is merged and tagged would leave discrawl on an unreleased crawlkit pseudo-version.
  • [P1] The PR body lists Go tests and CLI smokes, but there is no discrawl-level runtime artifact showing semantic search with vector_backend = "turbovec" against a real Python turbovec install.

Maintainer options:

  1. Wait for the crawlkit tag (recommended)
    Merge and tag the upstream crawlkit vector backend first, then update go.mod and go.sum from the pseudo-version to the release tag before this PR lands.
  2. Accept draft-only dependency risk
    Maintainers can keep the pseudo-version while iterating on the draft branch, but should not ship a discrawl release from that state.
  3. Pause if the upstream API changes
    If the crawlkit vector API changes during review, pause this PR and refresh the integration against the final API instead of preserving the stale branch shape.

Next step before merge

  • [P2] Needs maintainer and release review because the PR depends on an open upstream crawlkit PR and currently uses an unreleased pseudo-version; the changelog cleanup is mechanical but not the only blocker.

Security
Cleared: No concrete security or supply-chain regression was found; the Python turbovec bridge is opt-in and exact scoring remains the default, but the pseudo-version should be replaced before release.

Review findings

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:7-9
Review details

Best possible solution:

Land the crawlkit backend first, tag it, update discrawl to that tag, remove the direct CHANGELOG edit, and keep exact scoring as the default with a focused turbovec smoke before release.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR adds a new optional backend rather than reporting broken existing behavior. Source inspection shows current main has no vector backend selector.

Is this the best way to solve the issue?

Unclear until the release dependency is resolved: reusing crawlkit vector.Search is the right ownership boundary, but the branch should not land with the pseudo-version and direct changelog edit intact.

Full review comments:

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:7-9
    OpenClaw keeps CHANGELOG.md release-owned, so this feature PR should not add the release note directly. Move the note to PR/release context and leave the changelog for the release pass.
    Confidence: 0.9

Overall correctness: patch is correct
Overall confidence: 0.82

AGENTS.md: not found in the target repository.

Codex review notes: model gpt-5.5, reasoning high; reviewed against f1ca1ceabac1.

Label changes

Label changes:

  • add P2: This is a normal-priority feature PR with limited blast radius because exact scoring remains the default.
  • add merge-risk: 🚨 compatibility: The branch currently points at an unreleased crawlkit pseudo-version and changes the user-facing search configuration surface.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply to this MEMBER-authored PR; the body lists tests and CLI smokes but no separate discrawl turbovec runtime artifact.

Label justifications:

  • P2: This is a normal-priority feature PR with limited blast radius because exact scoring remains the default.
  • merge-risk: 🚨 compatibility: The branch currently points at an unreleased crawlkit pseudo-version and changes the user-facing search configuration surface.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply to this MEMBER-authored PR; the body lists tests and CLI smokes but no separate discrawl turbovec runtime artifact.
Evidence reviewed

What I checked:

  • Protected author association: The provided GitHub context marks the author association as MEMBER, so conservative cleanup should keep this PR open for maintainer handling.
  • No repository policy file found: No target AGENTS.md or VISION.md was present in the checkout; generic OpenClaw review guidance was used.
  • Current main has no backend selector: Current main's semantic search options have no VectorBackend field and SearchMessagesSemantic scores candidates directly with vector.CosineSimilarity. (internal/store/query.go:246, f1ca1ceabac1)
  • PR adds backend config: The PR branch adds EmbeddingsConfig.VectorBackend, defaults it to exact, and normalizes accepted values to exact or turbovec. (internal/config/config.go:93, b76bbe7cf6cc)
  • PR wires semantic search to crawlkit backend: The PR branch passes opts.VectorBackend into crawlkit vector.Search with the existing created-at/message-id tie breaker. (internal/store/query.go:259, b76bbe7cf6cc)
  • Upstream dependency is still release-ordered: The branch depends on a crawlkit pseudo-version, while the upstream crawlkit PR is still open and its body says downstream apps should move from the pseudo-version to a released crawlkit tag before app releases. (go.mod:47, b76bbe7cf6cc)

Likely related people:

  • vincentkoc: Current main blame for the semantic/config files points to the recent Cloudflare archive refactor, and this PR also updates the same search/config surface. (role: recent area contributor; confidence: high; commits: dcf0c9c8dbb3, edb8559ea29d; files: internal/store/query.go, internal/config/config.go, internal/cli/query_commands.go)
  • MrBrain: Git history shows the original semantic and hybrid message search work came through commits authored by MrBrain. (role: feature introducer; confidence: medium; commits: 3fc8defc3596, 3ea1d4aa7ff6; files: internal/store/query.go, internal/store/store_test.go)
  • Peter Steinberger: Git history shows follow-up work on semantic ranking and test coverage for the search path touched by this PR. (role: adjacent search contributor; confidence: medium; commits: ffc622cd5ebb, 8282f4dc5acc; files: internal/store/query.go, internal/store/store_test.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jun 8, 2026
@steipete

Copy link
Copy Markdown
Collaborator

Thanks @vincentkoc. I landed this via maintainer PR #88 as commit 1916f8e because this PR branch has maintainerCanModify=false, so I could not update the Crawlkit pseudo-version in place.

What landed:

  • Updated github.com/openclaw/crawlkit to the released v0.12.0.
  • Added optional [search.embeddings].vector_backend = "turbovec", with exact cosine still the default.
  • Kept exact search streaming/top-K and batched turbovec scoring to avoid whole-archive payload and memory regressions.
  • Added maintainer changelog credit for @vincentkoc.

Proof on #88 / 1916f8e:

  • Real turbovec command and store path with an isolated /tmp Python venv containing the turbovec package.
  • Default/exact semantic behavior regression coverage.
  • Full Go test and race suites, coverage gate, lint/analyzers, vet/build, go mod verify, govulncheck, gosec, gitleaks, GoReleaser snapshot.
  • Autoreview rerun clean with no accepted/actionable findings.

For future PRs, enabling “Allow edits by maintainers” lets maintainers update dependency pins or fix CI directly on the contributor branch.

@steipete steipete closed this Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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