feat: ClickHouse historical search tier - #14682
Draft
danny-avila wants to merge 3 commits into
Draft
Conversation
Versioned ReplacingMergeTree DDL, xmin-fenced outbox consumer with a contiguous-prefix watermark, additive candidate adapter, fail-closed anti-join contract, and a CH-vs-PG audit job.
… tier Splits scope handling into the safety-critical half (resolve, normalize, reject, brand) and the ClickHouse rendering half, so the shared core in data-schemas is a drop-in. Adds the ClickHouse leak matrix and env-only, no-default credential loading.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new, self-contained ClickHouse-backed “historical search tier” under packages/api/src/history/**. The tier introduces a ClickHouse projection (fed by a PostgreSQL outbox consumer) and a scoped candidate-fetch API (text + vector) designed to be additive to the existing PostgreSQL search path, with a fail-closed anti-join contract for safety.
Changes:
- Adds ClickHouse DDL + query builders for scoped candidate retrieval (text and vector arms) using latest-version aggregation semantics.
- Implements a PostgreSQL outbox consumer with xmin-visibility reads, contiguous-prefix watermark advancement, lease-epoch fencing, and a permanent-gap barrier.
- Adds scope fencing (branding + per-store predicate rendering), fail-closed anti-join helpers, an audit job, and extensive unit/integration tests (ClickHouse-local + optional PostgreSQL).
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/api/src/history/types.ts | History-tier types and narrow ports for PostgreSQL/ClickHouse clients. |
| packages/api/src/history/sql/outbox.sql | Reference PostgreSQL DDL contract for outbox + watermark tables. |
| packages/api/src/history/sql/clickhouse.sql | ClickHouse schema for versioned documents + ingest log, with tombstone/TTL semantics. |
| packages/api/src/history/sql.spec.ts | ClickHouse-local tests validating DDL constraints and serving-query semantics. |
| packages/api/src/history/source.ts | PostgreSQL SQL + adapter for visible outbox reads, watermark I/O, and document+embedding fetches. |
| packages/api/src/history/scope.ts | Provisional scope resolution/branding used to fail closed on unscoped/unsafe input. |
| packages/api/src/history/predicate.ts | ClickHouse predicate rendering from a branded scope + validated kind. |
| packages/api/src/history/leakmatrix.spec.ts | ClickHouse-local leak matrix validating tenant/user isolation across arms/kinds. |
| packages/api/src/history/index.ts | History tier public surface (internal-only for now; not exported from package root). |
| packages/api/src/history/guard.ts | Fail-closed anti-join contract and batch lookup SQL for caller-side safety. |
| packages/api/src/history/guard.spec.ts | Unit tests for anti-join admit/reject behavior. |
| packages/api/src/history/frontier.ts | Pure contiguous-prefix frontier logic + permanent-gap barrier helpers. |
| packages/api/src/history/frontier.spec.ts | Unit tests for frontier and gap-barrier logic. |
| packages/api/src/history/consumer.ts | Outbox consumer: batching, ClickHouse insert-first, watermark advance, gap handling, fencing. |
| packages/api/src/history/consumer.spec.ts | Scripted PG + ClickHouse tests for ordering, batching, fencing, tombstones, embeddings, TTL derivation. |
| packages/api/src/history/consumer.integration.spec.ts | Optional real-PostgreSQL integration tests for xmin visibility and gap behavior. |
| packages/api/src/history/config.ts | Env-only config loading with required credentials and safe loggable target formatting. |
| packages/api/src/history/config.spec.ts | Tests for config fail-closed behavior and credential non-leakage. |
| packages/api/src/history/candidates.ts | Candidate query builders + adapter combining arms with degradations and readiness probing. |
| packages/api/src/history/candidates.spec.ts | Unit tests for arm selection, scoping injection, limits, degradations, and readiness. |
| packages/api/src/history/audit.ts | CH-vs-PG consistency audit (summary + sampled key gap detection). |
| packages/api/src/history/audit.spec.ts | Unit tests for audit cleanliness, gap reporting, sampling, and caps. |
Suppressed comments (3)
packages/api/src/history/consumer.ts:267
- When recording the permanent-gap barrier, the
commitWatermark()result is not checked. If this update is fenced, the consumer should stop rather than continuing with a stale view of the watermark/barrier state.
await commitWatermark(pg, {
appliedSeq: ctx.appliedSeq,
appliedVersion: ctx.appliedVersion,
leaseEpoch: ctx.leaseEpoch,
gapBarrierSeq: gapAt,
packages/api/src/history/consumer.ts:288
- When skipping a provably-permanent gap, the watermark update is also fenced but the
commitWatermark()result is ignored. If fenced here, the consumer may continue from an incorrect frontier and should fail fast instead.
await commitWatermark(pg, {
appliedSeq: resumeAt - BigInt(1),
appliedVersion: ctx.appliedVersion,
leaseEpoch: ctx.leaseEpoch,
gapBarrierSeq: null,
packages/api/src/history/audit.spec.ts:91
- Avoid
as unknown as Tdouble assertions in tests per the repo Type Safety guidance in CLAUDE.md. A single assertion is sufficient here.
return { json: async <TRow>() => rows as unknown as TRow[] };
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+242
to
+250
| if (ctx.watermark.gapBarrierSeq !== null) { | ||
| await commitWatermark(pg, { | ||
| appliedSeq: ctx.appliedSeq, | ||
| appliedVersion: ctx.appliedVersion, | ||
| leaseEpoch: ctx.leaseEpoch, | ||
| gapBarrierSeq: null, | ||
| gapBarrierXmax: null, | ||
| }); | ||
| } |
Comment on lines
+295
to
+299
| async function buildBatch( | ||
| deps: OutboxConsumerDeps, | ||
| chunk: readonly OutboxRow[], | ||
| _embeddingSpace: string, | ||
| graceMs: number, |
Comment on lines
+177
to
+189
| const rows: OutboxRow[] = []; | ||
| for (let i = 0; i < result.rows.length; i++) { | ||
| const row = result.rows[i]; | ||
| rows.push({ | ||
| outboxSeq: BigInt(row.outbox_seq), | ||
| tenantId: row.tenant_id, | ||
| userId: row.user_id, | ||
| kind: row.kind as HistoryKind, | ||
| recordId: row.record_id, | ||
| projectionVersion: BigInt(row.projection_version), | ||
| op: row.op === 'tombstone' ? 'tombstone' : 'upsert', | ||
| }); | ||
| } |
| export type ClickHouseDocumentRow = Readonly<{ | ||
| tenant_id: string; | ||
| user_id: string; | ||
| kind: string; |
Comment on lines
+29
to
+31
| const rows = this.rowsByQuery.get(arm) ?? []; | ||
| return { json: async <TRow>() => rows as unknown as TRow[] }; | ||
| } |
| }): Promise<{ json<TRow>(): Promise<TRow[]> }> { | ||
| if (params.query === auditClickHouseSummarySql) { | ||
| this.appliedVersionSeen = String(params.query_params?.applied_version); | ||
| return { json: async <TRow>() => this.summary as unknown as TRow[] }; |
Comment on lines
+104
to
+109
| const rows = query<Record<string, unknown>>( | ||
| params.query, | ||
| (params.query_params ?? {}) as Record<string, ClickHouseParam>, | ||
| ); | ||
| return { json: async <TRow>() => rows as unknown as TRow[] }; | ||
| }, |
…ration Deletes the provisional local scope validation half in favour of @librechat/data-schemas, so resolve/normalize/reject/brand exists once for both stores. The candidate adapter now gates a branded Scope instead of re-deriving one from a plain object. Also drops this module's duplicate chat_search DDL: the integration spec runs against search/migrations/001_schema.sql, which caught two real divergences (source_created_at/source_updated_at, and vector(1024) with NOT NULL model metadata).
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.
Adds a ClickHouse-backed historical-search tier: a versioned projection of chat records that serves candidate IDs for older content, so a PostgreSQL search store does not have to hold the entire corpus hot.
It is additive — PostgreSQL still searches the full corpus, ClickHouse candidates only augment the result set, and PostgreSQL wins deduplication ties. Nothing is wired into a route yet; all code lives under
packages/api/src/history/**and is not exported from the package index (a one-line change when a consumer exists). Import as~/history.What's here
sql/clickhouse.sqlReplacingMergeTree,ORDER BY (tenant_id, user_id, kind, record_id),version = projection_versionconsumer.ts/frontier.ts/source.tspredicate.tsScopecandidates.tsguard.tsaudit.tsconfig.tsThere is deliberately no
chat_searchDDL here. That schema is owned by one migration; a second copy is the same drift hazard the shared scope core removes.Watermark correctness — the bug this exists to prevent
outbox_seqis abigserialdrawn at INSERT time, so sequence order is not commit order. ReadingWHERE outbox_seq > W ORDER BY outbox_seqand advancingWto the highest row skips a lower sequence that commits later — that record then becomes permanently invisible to both search arms.Demonstrated live against PostgreSQL 17, with transaction A holding seq 3 in flight while seq 4 commits:
The naive read advances
Wto 4 and losesm3forever. After A commits, the guarded read returns both, in order.Two rules, not interchangeable:
xminis a 32-bitxidwhilepg_snapshot_xmin()returns a 64-bitxid8, so comparing decimal renderings degrades to always-true after the first transaction-id wraparound.Wadvances only acrossW+1, W+2, …with no missing value, and rows above a gap are withheld from the ClickHouse insert as well, preserving the invariant that no served candidate exceedsW.Permanent-gap barrier. Aborted transactions burn sequence values permanently, so waiting unconditionally for a gap to fill would stall ingestion forever. On first sight of a gap the consumer persists
pg_snapshot_xmax()captured in the same statement that observed it; the gap may be skipped only oncepg_snapshot_xmin()reaches that bound — i.e. once every transaction alive at observation time has ended. Verified end to end against a real aborted transaction.Ordering is ClickHouse-insert-first, watermark-second: a crash between them replays the batch, and
ReplacingMergeTreecollapses the duplicate.Tombstones — never a bare TTL
TTL deleted_at + INTERVAL 7 DAY DELETEis a resurrection path and is explicitly forbidden in the DDL comments: in aReplacingMergeTree, deletion is a higher-version tombstone row, and collapsing happens only at merge time. Expiring the tombstone while an older content-bearing part survives un-merged makes the deleted row visible again.key_retire_atderives only from key-stable inputs, and tombstones always carry a never-retire sentinel, so a tombstone can only outlive the content versions it supersedes — asserted as an invariant.tombstone_is_textlessCHECK constraint makes a tombstone carrying text or a vector a write error rather than a review finding.Scope safety
ClickHouse has no row-level security, so application code is the only fence — not a net over one.
data-schemas, shared with the PostgreSQL tier. This module never re-derives it:fetchCandidatestakes an already-brandedScopeand only gates it withassertScope.predicate.tsrenders that value into ClickHouse{name:Type}bindings and owns only the recordkind. PostgreSQL renders the same value into$npredicates and RLS session settings. Neither re-derives, so the two cannot drift.createScopenormalizes an absent tenant to the base tenant before failing closed — failing on "empty tenant" first would break every non-tenant deployment — then throws on a missing user or on the__SYSTEM__sentinel, which is a query-time wildcard elsewhere in this codebase and must never become one here.Scopeand render the predicate internally. There is no module-level SQL constant with an unfilled scope hole, and no intermediate predicate object to forge — the brand is a module-private symbol, so a structurally identical plain object is rejected.Leak matrix
{text, vector} × {messages, conversations, shared-links}, three principals sharing a user id across two tenants and a tenant across two users, seeded with identical record ids, bodies, and vectors — only the scope predicate can separate them. 49 assertions againstclickhouse-local, plus coverage for absent, forged, and unknown-kind scope.The three properties that had to survive adopting the shared core are each still asserted here: throw-on-unscoped-construction, brand-substitution rejection, and normalize-tenant-before-failing-closed.
Credentials
config.tsreads every DSN and password from the environment with no fallback default; a missing variable throws at startup. Errors name the variable only, never a value, anddescribeTarget()(scheme + host, userinfo stripped) is the only connection detail that may be logged.Verification
Run against this branch combined with #14683 — real shared core, real migration:
sql.spec.tsclickhouse-local26.8leakmatrix.spec.tsclickhouse-local26.8consumer.spec.tsconsumer.integration.spec.tsfrontier/guard/audit/config163 tests, 0 failures. Typecheck and lint clean. The 10 PostgreSQL integration tests run only when
CHAT_SEARCH_TEST_URLis set and the migration file is present; they skip cleanly otherwise.Running against the authoritative migration rather than a local copy immediately caught two divergences, both fixed here: the consumer read
created_at/updated_atwhere the schema hassource_created_at/source_updated_at, and a test fixture used a 2-dimension vector where the column isvector(1024)with NOT NULL model metadata.Two ClickHouse behaviors the DDL work turned up, both now pinned by tests:
argMax(nullable_col, version)skips NULLs and returns an older non-null value — which would resurrect a clearedexpires_at. Every serving query aggregates a single tuple instead.ReplacingMergeTreecollapses duplicate keys within a single insert block, so multi-version tests need separate inserts or they silently prove nothing.Follow-ups (not blocking)
clickhouse-localcovers semantics, not performance. Ingest throughput, ANN recall, and theOPTIMIZE FINALcollapse behavior at scale still need a live ClickHouse server.