Skip to content

feat: queue embeddings for live Discord events with opt-in - #159

Merged
steipete merged 4 commits into
mainfrom
codex/pr154-tail-embedding-opt-in
Aug 2, 2026
Merged

feat: queue embeddings for live Discord events with opt-in#159
steipete merged 4 commits into
mainfrom
codex/pr154-tail-embedding-opt-in

Conversation

@steipete

@steipete steipete commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • preserve Hannes's live-event, replay, repair, OpenAI-dimensions, and pending-index work from feat: queue embeddings for live Discord events #154
  • keep tail embedding queueing behind a new explicit tail --with-embeddings opt-in
  • cover both the default-off and enabled paths at the CLI and syncer boundaries
  • prove the pending-job index is added when an existing schema-v5 archive is reopened, without losing its message or queued job
  • document the command contract; the credited changelog entry remains landing-owned, matching repository history

This replacement keeps Hannes's original commit intact and adds focused maintainer repair and regression-test commits. It supersedes #154 because that branch derives tail queueing directly from global search.embeddings.enabled, which would silently expand work for existing installations after upgrade.

Validation

make check

Passed module verification/tidiness, gofumpt, golangci-lint, go vet, staticcheck, gosec, govulncheck, dead-code analysis, 85.1% coverage, the race suite, CLI smoke checks, and a six-platform GoReleaser snapshot.

The focused existing-archive regression creates a populated schema-v5 database, removes idx_embedding_jobs_pending_order, reopens it through the production migration path, and verifies both index recreation and preservation of the original message and pending job.

Live built-binary proof against an isolated fresh archive:

discrawl tail --help
Usage: discrawl tail [--repair-every DURATION] [--with-embeddings] ...
Use --with-embeddings to queue live, replayed, and repair messages for embedding.

discrawl --plain tail --replay-failures-only
candidates=0
recovered=0
deferred=0
policy_deferred=0

discrawl --plain tail --with-embeddings --replay-failures-only
candidates=0
recovered=0
deferred=0
policy_deferred=0

The CLI proof used an isolated config, SQLite database, cache, and log root with a placeholder token; replay had no candidates and made no Discord request.

To prove the actual queue transition, a temporary uncommitted integration executable injected one synthetic Discord message through the production Syncer.RunTail event handler and real SQLite store twice, then queried embedding_jobs directly:

GOWORK=off go run ./cmd/discrawl-live-proof
default_jobs=0
enabled_jobs=1

The integration executable exited nonzero unless the default path produced zero jobs and the opted-in path produced exactly one. Its temporary source was removed after the proof run; no fixture or generated artifact is part of this PR.

AutoReview found no code or test defect in the final patch. Its only remaining P3 proposed restoring the contributor changelog entry; that was rejected because repository history removes contributor notes during review and adds the credited entry from the maintainer landing/release commit.

hannesrudolph and others added 2 commits July 30, 2026 13:01
Co-authored-by: Hannes Rudolph <49103247+hannesrudolph@users.noreply.github.com>
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. 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 Aug 2, 2026
@clawsweeper

clawsweeper Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 2, 2026, 12:27 AM ET / 04:27 UTC.

ClawSweeper review

What this changes

Adds discrawl tail --with-embeddings to queue live Discord events, replayed failures, and repair results for background embeddings only when explicitly requested, plus optional embedding dimensions and an archive queue-order index.

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

This PR is still necessary: current main supports embedding queueing through sync --with-embeddings, but does not provide opt-in queueing for live tail events, failure replay, or tail repair. The branch keeps that extra work explicitly default-off, includes a reopened-existing-archive index regression, and supplies concrete live SQLite proof; no actionable patch defect was found. Final landing should wait for the currently running lint and test checks on the current head. Likely related people: steipete (high confidence) and hannesrudolph (medium confidence).

Priority: P2
Reviewed head: 5992d67760255bba1865da7f2780b854206d3e51

Review scores

Measure Result What it means
Overall readiness 🦞 diamond lobster (5/6) The patch is narrowly upgrade-safe, covers the migration regression, and is backed by directly relevant live queue-transition proof.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (live_output): The PR body contains after-fix live output from an isolated production Syncer.RunTail and SQLite path (default_jobs=0, enabled_jobs=1), plus built-CLI flag proof; no private credentials are exposed in the reported transcript.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The PR body contains after-fix live output from an isolated production Syncer.RunTail and SQLite path (default_jobs=0, enabled_jobs=1), plus built-CLI flag proof; no private credentials are exposed in the reported transcript.
Evidence reviewed 7 items Current-main gap: Current main documents and implements sync --with-embeddings, while its tail help and embeddings documentation contain no tail embedding option; the central tail behavior remains absent from main.
Default-off implementation: The branch parses tail --with-embeddings, configures the syncer before either replay or live tail execution, and passes the setting into Gateway handlers and periodic repairs.
Existing-archive migration coverage: The branch adds the pending-job ordering index to the repeatable schema/index path and tests reopening a populated schema-v5 archive after the index is removed, verifying both index recreation and retained message/job data.
Findings None None.
Security None None.

How this fits together

Discrawl's tail subsystem receives live Discord Gateway events and writes them to the local SQLite archive; the optional embedding queue records archive changes for a later discrawl embed drain. This PR adds an explicit CLI decision between tail ingestion and queue creation, then carries that decision through replay and repair paths.

flowchart LR
  A[Discord Gateway events] --> B[tail command]
  C[Replay and repair inputs] --> B
  B --> D{with-embeddings enabled?}
  D -->|No| E[Archive messages]
  D -->|Yes| F[Archive messages and queue jobs]
  E --> G[SQLite archive]
  F --> G
  G --> H[embed command drains jobs]
Loading

Before merge

  • Resolve merge risk (P1) - The current GitHub lint and test checks are still in progress on the reviewed head; merge should wait for their completion even though the patch review and real-behavior proof are positive.
  • Complete next step (P2) - No repair lane is needed: there are no actionable findings, and the remaining action is ordinary maintainer landing after the active checks complete.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Change surface 17 files affected; 146 additions, 9 deletions The change crosses CLI parsing, live ingestion, replay, repair, SQLite migration, tests, and user documentation, so those boundary tests matter more than the modest line count.
Behavioral paths 3 queueing paths The opt-in is propagated through live Gateway events, failure replay, and periodic repair rather than only the primary tail handler.

Root-cause cluster

Relationship: canonical
Canonical: #159
Summary: This PR is the viable safer successor for the same live-embedding work because it preserves the earlier implementation while changing the upgrade behavior to explicit opt-in.

Members:

  • superseded: feat: queue embeddings for live Discord events #154 - The PR body and provided timeline explicitly identify this branch as the replacement for the earlier live-embedding branch, whose queueing behavior followed the global embedding setting rather than an explicit tail opt-in.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Land the explicit tail --with-embeddings contract after the current head’s remaining checks pass, preserving the default-off upgrade path and the existing-archive index regression coverage.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Land the explicit tail --with-embeddings contract after the current head’s remaining checks pass, preserving the default-off upgrade path and the existing-archive index regression coverage.

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

Not applicable as a feature request; the PR nonetheless provides a high-confidence after-fix production-path proof using Syncer.RunTail and SQLite, showing zero jobs by default and one job when opted in.

Is this the best way to solve the issue?

Yes. An explicit tail-only opt-in is the narrowest upgrade-safe solution because it adds live queueing without silently changing existing installations that enabled embeddings for batch sync.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 48b28c60866c.

Labels

Label justifications:

  • P2: This is a bounded opt-in enhancement to Discord archival and embedding workflows with no demonstrated urgent user regression.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster 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 PR body contains after-fix live output from an isolated production Syncer.RunTail and SQLite path (default_jobs=0, enabled_jobs=1), plus built-CLI flag proof; no private credentials are exposed in the reported transcript.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body contains after-fix live output from an isolated production Syncer.RunTail and SQLite path (default_jobs=0, enabled_jobs=1), plus built-CLI flag proof; no private credentials are exposed in the reported transcript.

Evidence

What I checked:

  • Current-main gap: Current main documents and implements sync --with-embeddings, while its tail help and embeddings documentation contain no tail embedding option; the central tail behavior remains absent from main. (internal/cli/output.go:125, 48b28c60866c)
  • Default-off implementation: The branch parses tail --with-embeddings, configures the syncer before either replay or live tail execution, and passes the setting into Gateway handlers and periodic repairs. (internal/cli/admin_commands.go:329, 5992d6776025)
  • Existing-archive migration coverage: The branch adds the pending-job ordering index to the repeatable schema/index path and tests reopening a populated schema-v5 archive after the index is removed, verifying both index recreation and retained message/job data. (internal/store/store_test.go:25, 5992d6776025)
  • Migration-path provenance: Current main's store migration deliberately reapplies query-index migrations after schema-version checks, so adding the new index there reaches existing schema-v5 archives without a version bump. (internal/store/store.go:217, 4f3017c8188d)
  • Feature-history routing: The current tail/store architecture is rooted in the current released main snapshot attributed to Peter Steinberger, while the initial live-embedding implementation commit on this branch is attributed to Hannes Rudolph and the opt-in repair commits to Peter Steinberger. (internal/syncer/tail.go:22, 7fbd1ed6f1a1)
  • Real behavior proof: The PR body reports an isolated production Syncer.RunTail plus SQLite run with default_jobs=0 and enabled_jobs=1; it also shows the built CLI accepts the opt-in flag. The provided context marks the proof sufficient. (internal/syncer/syncer_tail_test.go:155, 5992d6776025)

Likely related people:

  • steipete: Peter Steinberger owns the current released store/tail architecture in the available history and authored the opt-in and migration-repair commits on this branch. (role: recent area contributor; confidence: high; commits: 4f3017c8188d, 73ce2284c9ae, 8469767d03c3; files: internal/cli/admin_commands.go, internal/store/store.go, internal/syncer/tail.go)
  • hannesrudolph: Hannes Rudolph authored the preserved live-embedding implementation commit that this branch refines into an explicit opt-in flow. (role: original feature contributor; confidence: medium; commits: 7fbd1ed6f1a1; files: internal/syncer/tail.go, internal/store/store.go)

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 (3 earlier review cycles)
  • reviewed 2026-08-02T03:51:52.254Z sha 73ce228 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T04:03:42.815Z sha 73ce228 :: found issues before merge. :: [P2] Test the existing-database index upgrade | [P3] Remove the release-owned changelog entry
  • reviewed 2026-08-02T04:16:41.066Z sha 8469767 :: needs maintainer review before merge. :: none

@steipete

steipete commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

The PR body now includes the requested enabled-versus-default integration proof through production Syncer.RunTail and SQLite: default_jobs=0, enabled_jobs=1.

@clawsweeper

clawsweeper Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. 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. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 2, 2026
@steipete

steipete commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

The P2 migration finding is fixed in 8469767: the regression now creates a populated schema-v5 archive, removes the pending-order index, reopens through production migration, and verifies the index plus preserved message/job data. Focused store/CLI/syncer tests and the full make check suite pass.

The contributor changelog entry is removed as requested. The PR body now identifies the credited release note as maintainer landing-owned, consistent with the repository's prior chore: remove contributor changelog entry and later credited release-note commits.

@clawsweeper

clawsweeper Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed 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. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Aug 2, 2026
@steipete
steipete merged commit 146e727 into main Aug 2, 2026
12 checks passed
@steipete

steipete commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator Author

Landed as 146e72773ab887d1e7a156f1f62a384fa93a5293. Hannes's original work and co-author trailer are preserved in the squash commit. The maintainer-owned credited release note followed on main as bd17ef14156b385b39c2726506e3a9aa9e928bc4.

Verification on refreshed exact head 5992d67760255bba1865da7f2780b854206d3e51:

  • make check passed module verification/tidiness, gofumpt, golangci-lint, vet, staticcheck, gosec, govulncheck, coverage (85.1%), race tests, CLI smoke tests, and the six-platform GoReleaser snapshot.
  • Source-blind behavior validation against a prewritten contract passed all clauses: help documented the explicit opt-in; default and enabled empty replay paths both exited 0 with zero candidates/recoveries; an unknown flag exited 2 with a useful diagnostic; dead outbound proxies proved empty replay made no network request.
  • The production Syncer.RunTail plus real SQLite integration proof remained default_jobs=0 and enabled_jobs=1.
  • The existing-archive regression recreated the pending-job index while preserving its message and queued job.
  • All refreshed exact-head CI, Docker, CodeQL, and secret-scanning checks were successful.
  • Final AutoReview against updated main was clean with no accepted/actionable finding.

No release or tag was created.

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: 🦞 diamond lobster Very strong PR readiness with only minor 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