chore(selfhost): unify queue/adapter/vectorize backend contracts - #4188
Merged
Conversation
Defines one shared, project-owned interface per self-host swappable-backend pair in a new src/selfhost/backend-contracts.ts (#4010): - DurableQueue replaces the sqlite/postgres queue backends' two independently-declared interfaces (DurableQueue/PgDurableQueue), which had drifted (every sqlite method sync vs the postgres equivalent Promise- wrapped, plus an init() only postgres had) and were reconciled only by a loose T | Promise<T> union on server.ts's Backend.queue. Every method is now Promise-returning on both backends; the sqlite side's init() is a documented no-op since its setup already runs synchronously at construction. - SelfHostD1Database/SelfHostD1PreparedStatement give d1-adapter.ts and pg-adapter.ts a real typed checkpoint before their previously-blind `as unknown as D1Database` cast (D1Database is a `declare abstract class`, so that final cast is unavoidable, but nothing previously checked either adapter's own shape against it or its sibling first). - SelfHostVectorRecord/SelfHostVectorizeQueryOptions/SelfHostVectorizeMatch/ SelfHostVectorize do the same for vectorize.ts, qdrant-vectorize.ts, and pg-vectorize.ts, and fix the one already-manifested divergence: only vectorize.ts's QueryOptions carried returnMetadata. Traced the real call path (src/review/adapters.ts's reviewVectorAdapter, fed by src/review/ rag.ts, which always sets returnMetadata: "all") and confirmed every backend is invoked with this option identically regardless of which is bound to env.VECTORIZE, so it belongs on all three - typed as the real three-value union Cloudflare's own VectorizeQueryOptions.returnMetadata uses, not removed from vectorize.ts. Adds a contract test per pair under test/contract/ that runs the same assertion suite against every concrete implementation of each pair, and a new selfhost-pg-adapter.test.ts covering pg-adapter's own D1 surface with a mocked pg.Pool (previously only exercised by the real-Postgres integration test). Both alongside, not replacing, each backend's existing implementation-specific test file.
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4188 +/- ##
==========================================
- Coverage 93.74% 93.74% -0.01%
==========================================
Files 387 387
Lines 36398 36397 -1
Branches 13330 13330
==========================================
- Hits 34120 34119 -1
Misses 1621 1621
Partials 657 657
🚀 New features to boost your workflow:
|
Contributor
|
Important 🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪 🔍 Gittensory is reviewing…AI analysis is in progress. This comment will update when the review is complete. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟪 Reviewing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/selfhost/backend-contracts.ts: one shared, project-owned interface per self-host swappable-backend pair (queue, D1-shaped storage adapter, Vectorize-shaped RAG store), replacing the sqlite/postgres queue pair's two independently-declared interfaces (DurableQueue/PgDurableQueue) and theT | Promise<T>unionserver.tsused to paper over their drift, and inserting a real typed checkpoint before each adapter's unavoidableas unknown as <ambient type>cast (D1Database/Vectorizearedeclare abstract class, so that cast itself can't be removed — but nothing previously checked the object's own shape against it, or against its sibling, first).size,deadCount,processingCount,stats,snapshot,pressureSignals, the dead-letter admin methods,topBacklogRepos,releaseStaleForegroundDeferrals) fully async — a strict superset of its former sync surface, matching the Postgres backend exactly — and adds a documented no-opinit(). Fixed a latent bug this surfaced:releaseStaleForegroundDeferralsSafely'stry/catchneeded to becomeawait-based to keep catching errors from the now-async call (there's an existing regression test for exactly this failure mode).returnMetadatadivergence: onlyvectorize.ts(sqlite)'s privateQueryOptionsdeclared it. Traced the real call path (src/review/adapters.ts'sreviewVectorAdapter→vectorize.query(vector, opts), fed bysrc/review/rag.ts, which always setsreturnMetadata: "all") and confirmed every backend is invoked with this option identically regardless of which is bound toenv.VECTORIZE— so it belongs on all three, typed as the real three-value union Cloudflare's ownVectorizeQueryOptions.returnMetadatauses, not removed from vectorize.ts.test/contract/(describe.each/ shared-spec-function style) that runs the identical assertion suite against every concrete implementation of each pair, alongside — not replacing — each backend's own existing implementation-specific test file. Also addstest/unit/selfhost-pg-adapter.test.ts(mockedpg.Pool, no real Postgres needed), closing a gap wherepg-adapter.ts's own D1 surface (batch/first/run/raw) was previously only exercised by the real-Postgres integration test.Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlint(unaffected by this diff; no workflow files touched)npm run typechecknpm run test:coveragelocally — full unsharded run, 593 passed / 2 skipped files, 12073 passed / 12 skipped tests; targeted per-file coverage checks confirm every line this PR touches in a non-Codecov-ignored file (sqlite-queue.ts,d1-adapter.ts,vectorize.ts,qdrant-vectorize.ts,pg-vectorize.ts) is covered (pg-queue.ts/pg-adapter.ts/server.tsare Codecov-ignored percodecov.yml, validated by the real-Postgres integration test instead)npm run test:workersnpm run build:mcp(unaffected; not touched, but ran clean)npm run test:mcp-packnpm run ui:openapi:check(unaffected — no API/schema changes)npm run ui:lint(unaffected — noapps/gittensory-uichanges)npm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderate— 0 vulnerabilitiesselfhost-pg-adapter.test.tsaboveSafety
UI Evidencesection. (n/a — no UI change)UI Evidence
Not applicable — this is a backend-only change (self-host queue/adapter/vectorize contracts + tests); no visible UI surface changed.
Notes
src/selfhost/queue-common.ts's existingSelfHostQueueDeadLetterAdmin/DeadLetterJobpattern (a project-defined interface shared by both queue backends),src/selfhost/d1-adapter.ts's existingSqliteDriver(a minimal project-defined interface abstracting a concrete dependency, already imported by three other self-host files), andsrc/review/rag.ts'sVectorAdapter/VectorMatch(a structurally-similar but distinct higher-layer interface that wraps whicheverVectorize-shaped binding is active).src/queue/processors.ts, the visual-capture epic, or anything under the config-as-code epic (Epic: config-as-code should fully control every review feature, not just restrict it #4092).