Skip to content

feat: keep embeddings current while Discord capture continues - #217

Merged
hannesrudolph merged 2 commits into
mainfrom
feat/live-embedding-workers
Sep 11, 2026
Merged

feat: keep embeddings current while Discord capture continues#217
hannesrudolph merged 2 commits into
mainfrom
feat/live-embedding-workers

Conversation

@hannesrudolph

@hannesrudolph hannesrudolph commented Sep 11, 2026

Copy link
Copy Markdown
Member
Additional instructions

Keep Allow edits from maintainers enabled.

Related: openclaw/crawlkit#115

What Problem This Solves

Fixes an issue where users running continuous Discord capture could not also keep embeddings current without an external stop/embed/restart cycle. The standalone commands both need the archive's exclusive writer ownership, while queued content can remain unembedded until the next scheduled drain.

Why This Change Was Made

Add opt-in tail --embed-live, backed by the general worker runtime in published crawlkit v0.16.0. Capture and embedding share one process owner, with provider calls outside transactions, separate read-only preparation, and short result commits. Revision/lease fencing discards stale work after edits, deletes, or claim recovery. Fresh Gateway work takes priority over historical sync catch-up.

User Impact

  • With configured embeddings, tail --embed-live queues and continuously processes content. Existing --with-embeddings remains queue-only and the one-shot embed command remains available.
  • Provider outages, missing credentials and throttling pause processing while capture continues. Status adds local-only background-worker state, backlog age, pending/in-flight work and safe error codes.
  • Material queued edits invalidate old vectors while their replacements are pending. Late results cannot resurrect deleted/private content or overwrite newer input.
  • Every writable archive open migrates schema 5 to 6, including when live mode is disabled. This intentional versioned migration adds queue fields and an index and preserves messages and vectors. Older binaries reject schema 6. The supported feature rollback retains the new binary and restores queue-only capture plus one-shot embedding, which remain covered by the CLI tests; it never overwrites newly captured data with a pre-upgrade database.

Evidence

  • Full Go race suite passed, using init.defaultBranch=master to match the existing publishing fixture's Linux CI behavior. Focused migration, concurrency, lease recovery, edit/delete, credential failure and fairness tests pass.
  • Provider-specific batch sizes are honored below the 64-input ceiling; the worker deadline and lease include the configured provider timeout. Focused non-default batch/timeout regressions pass.
  • Lint and vet pass. Overall measured statement coverage exceeds the repository's 85% floor.
  • A 10-message/second test with 256 catch-up jobs and a simulated 200 ms provider measured embedding freshness p95 around 0.62 seconds, below the 10-second target.
  • A migration preflight on a copied 15 GB archive completed in 8.64 seconds and preserved message, vector and job-state counts. The live store was not used for tests.
  • crawlkit v0.16.0 is available through the public Go proxy; no local module replacement is committed.

@hannesrudolph
hannesrudolph requested a review from a team as a code owner September 11, 2026 20:24
@clawsweeper

clawsweeper Bot commented Sep 11, 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. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Sep 11, 2026
@clawsweeper

clawsweeper Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Codex review: blocked before merge. Reviewed September 11, 2026, 4:36 PM ET / 20:36 UTC (Revision 2).

ClawSweeper review

What this changes

Adds opt-in continuous embedding during Discord capture, with durable worker claims, background status, schema migration, and supporting tests and documentation.

Merge readiness

Blocked before merge - 1 item remains

Both earlier findings are resolved, and no remaining blocking code defect was found. Continuous draining remains absent from main and v0.14.1, so this PR still provides distinct value.

Priority: P2
Reviewed head: 22ba8681703000fb8b59261a7bc72892250d9a64

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A coherent implementation with focused regression coverage, resolved prior findings and an explicit upgrade contract.
Proof confidence 🌊 off-meta tidepool Not applicable: The external-contributor gate does not apply to this MEMBER-authored PR. Inspected tests exercise the production SQLite adapter with simulated providers; the body separately reports a successful copied-archive migration preflight.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The external-contributor gate does not apply to this MEMBER-authored PR. Inspected tests exercise the production SQLite adapter with simulated providers; the body separately reports a successful copied-archive migration preflight.
Evidence reviewed 9 items Earlier findings resolved: The current CLI passes configured BatchSize and RequestTimeout; the worker caps batches at 64 and derives its deadline from the provider timeout plus 30 seconds. The GitHub comparison against the earlier reviewed head confirms these repairs and adds focused regression coverage.
Main retains separate queue and drain operations: Main documents sync queueing and a separate bounded embed drain. Inspection of v0.14.1's tail implementation also found only queue-only --with-embeddings, without --embed-live.
Merged related work establishes the preserved default: #159 is merged and explicitly keeps tail embedding queueing opt-in. Its scope does not include continuous draining; this PR preserves that queue-only flag and adds a separate opt-in.
Findings None None.
Security None None.

How this fits together

Discrawl stores captured Discord messages in a local SQLite archive. Background workers turn queued message text into provider-generated vectors used by semantic search while capture continues.

flowchart LR
  A[Discord events and repair] --> B[Archive and embedding queue]
  B --> C[Fresh or catch-up claims]
  C --> D[Embedding provider]
  D --> E[Check revision and lease]
  E --> F[Vectors for semantic search]
  C --> G[Local worker status]
Loading

Before merge

  • Resolve merge risk (P1) - Writable archive opens upgrade schema 5 to 6 even without --embed-live; older binaries then cannot open the archive. The documented rollback retains a schema-compatible binary, and the reported copied-archive preflight supports this intentional compatibility tradeoff.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test LOC Production +542/-30; tests +603/-2 Production growth implements the durable queue adapter and lifecycle; substantial test coverage accompanies it.

Merge-risk options

Maintainer options:

  1. Accept the documented schema upgrade (recommended)
    Accept schema 6's older-binary incompatibility with the copied-archive preflight and documented rollback through a schema-compatible binary.

Technical review

Best possible solution:

Retain opt-in continuous draining and queue-only defaults, shipping the documented schema-compatible rollback path with the reported archive-upgrade evidence.

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

Not applicable as a new opt-in mode; source inspection confirms the existing queue/drain limitation and repairs to both earlier findings. Tests were inspected but not executed during this read-only review.

Is this the best way to solve the issue?

Yes: sharing the existing writer owner and using crawlkit's bounded runtime addresses the lock conflict while preserving established queue-only behavior.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 34bb67ea0969.

Labels

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool 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 gate does not apply to this MEMBER-authored PR. Inspected tests exercise the production SQLite adapter with simulated providers; the body separately reports a successful copied-archive migration preflight.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: This is an opt-in improvement to continuous archive indexing, without evidence of an urgent existing outage.
  • merge-risk: 🚨 compatibility: The automatic schema upgrade prevents older binaries from reopening upgraded archives, including users who leave live embedding disabled.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool 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 gate does not apply to this MEMBER-authored PR. Inspected tests exercise the production SQLite adapter with simulated providers; the body separately reports a successful copied-archive migration preflight.

Evidence

What I checked:

  • Earlier findings resolved: The current CLI passes configured BatchSize and RequestTimeout; the worker caps batches at 64 and derives its deadline from the provider timeout plus 30 seconds. The GitHub comparison against the earlier reviewed head confirms these repairs and adds focused regression coverage. (internal/store/embedding_worker.go:117, 22ba86817030)
  • Main retains separate queue and drain operations: Main documents sync queueing and a separate bounded embed drain. Inspection of v0.14.1's tail implementation also found only queue-only --with-embeddings, without --embed-live. (docs/commands/embed.md:36, 34bb67ea0969)
  • Merged related work establishes the preserved default: feat: queue embeddings for live Discord events with opt-in #159 is merged and explicitly keeps tail embedding queueing opt-in. Its scope does not include continuous draining; this PR preserves that queue-only flag and adds a separate opt-in. (internal/cli/admin_commands.go:333, 146e72773ab8)
  • Durable queue and completion boundary: Read the full worker implementation and tests. Claims use random tokens; completion, retry and release check revision, token, pending state and lease expiry. Completion checks current message text and eligibility within the result transaction. Provider calls occur outside writer transactions. (internal/store/embedding_worker.go:296, 22ba86817030)
  • Explicit dependency contract: The worker directly imports github.com/openclaw/crawlkit/worker, establishing the dependency signal. go.mod selects published v0.16.0 without a local replacement. (internal/store/embedding_worker.go:16, 22ba86817030)
  • Tagged worker runtime inspected: Read the dependency's full root AGENTS.md, worker implementation and background-worker contract at v0.16.0. The runtime derives lease duration from task and completion budgets, reserves catch-up opportunities, and bounds cancellation cleanup. The tag resolves to this dependency SHA; its introducing worker PR is merged. (worker/worker.go, f74d37a66c55)

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)
  • hannesrudolph: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; 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.

History

Review history (1 earlier review cycle)
  • reviewed 2026-09-11T20:27:39.840Z sha 19ce421 :: blocked before merge. :: [P2] [P2] Honor the configured embedding batch size | [P2] [P2] Derive the worker deadline from the provider timeout

@clawsweeper clawsweeper Bot added 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. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Sep 11, 2026
@hannesrudolph
hannesrudolph merged commit 8f0e93b into main Sep 11, 2026
18 of 19 checks passed
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: 🐚 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.

1 participant