From d54debddba1b9d460c84908d1b0c2da11f1b73d5 Mon Sep 17 00:00:00 2001 From: jbarol Date: Wed, 10 Jun 2026 15:07:28 -0700 Subject: [PATCH] fix(core): facts alter recipe NULLs embeddings before cross-dimension alters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review: buildFactsAlterRecipe shipped the same defect class this branch fixes for content_chunks 350 lines up — a cross-dimension ALTER ... USING cast that pgvector refuses while rows hold old-width vectors. Dimension changes now wipe first (the facts pipeline re-embeds on next write); same-dim type swaps (halfvec <-> vector) keep the lossless cast and PRESERVE data. Both behaviors pinned by tests. --- src/core/embedding-dim-check.ts | 14 ++++++++++++++ test/embedding-dim-check-facts.test.ts | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/core/embedding-dim-check.ts b/src/core/embedding-dim-check.ts index 084064841..220bcca05 100644 --- a/src/core/embedding-dim-check.ts +++ b/src/core/embedding-dim-check.ts @@ -575,11 +575,25 @@ export function buildFactsAlterRecipe( ): string { const opclass = columnType === 'halfvec' ? 'halfvec_cosine_ops' : 'vector_cosine_ops'; const targetType = columnType === 'halfvec' ? `halfvec(${configuredDims})` : `vector(${configuredDims})`; + const dimsChanged = columnDims !== configuredDims; return [ `-- ALTER ${columnType}(${columnDims}) → ${columnType}(${configuredDims}) on indexed column.`, `-- HOLD a maintenance window: this rewrites every row's embedding.`, `-- Coordinate with any active extract-conversation-facts backfill.`, `DROP INDEX IF EXISTS idx_facts_embedding_hnsw;`, + // Same-dim type swaps (halfvec <-> vector) cast row data losslessly and + // MUST keep it. Cross-dimension changes are different: pgvector refuses + // to cast existing vectors across dimensions ("expected N dimensions, + // not M") and aborts the transaction — and the old-space vectors are + // unusable at the new width anyway. NULL them first; the facts pipeline + // re-embeds on the next write. + ...(dimsChanged + ? [ + `-- Dimension change: NULL embeddings BEFORE the alter — pgvector`, + `-- refuses cross-dimension casts and aborts the transaction.`, + `UPDATE facts SET embedding = NULL;`, + ] + : []), `ALTER TABLE facts ALTER COLUMN embedding TYPE ${targetType}`, ` USING embedding::${targetType};`, `CREATE INDEX idx_facts_embedding_hnsw`, diff --git a/test/embedding-dim-check-facts.test.ts b/test/embedding-dim-check-facts.test.ts index d152e6fd4..2b0a9a8fc 100644 --- a/test/embedding-dim-check-facts.test.ts +++ b/test/embedding-dim-check-facts.test.ts @@ -142,6 +142,27 @@ describe('buildFactsAlterRecipe', () => { const recipe = buildFactsAlterRecipe(1536, 1280, 'halfvec'); expect(recipe).toMatch(/DROP INDEX[\s\S]*ALTER TABLE[\s\S]*CREATE INDEX/); }); + + test('dimension change NULLs embeddings BEFORE the alter (pgvector refuses cross-dim casts)', () => { + // Same defect class fixed for content_chunks in embeddingMismatchMessage: + // pgvector aborts a cross-dimension ALTER while rows still hold old-width + // vectors. The dims-change recipe must wipe first; order pinned. + const recipe = buildFactsAlterRecipe(1536, 1280, 'halfvec'); + const nullIdx = recipe.indexOf('UPDATE facts SET embedding = NULL'); + const alterIdx = recipe.indexOf('ALTER TABLE facts ALTER COLUMN embedding TYPE'); + expect(nullIdx).toBeGreaterThan(-1); + expect(alterIdx).toBeGreaterThan(-1); + expect(nullIdx).toBeLessThan(alterIdx); + }); + + test('same-dim type swap PRESERVES embeddings (no NULL wipe)', () => { + // halfvec(1536) <-> vector(1536): the USING cast is lossless and the + // whole point is keeping the data. A wipe here would destroy valid + // embeddings for no reason. + const recipe = buildFactsAlterRecipe(1536, 1536, 'vector'); + expect(recipe).not.toContain('UPDATE facts SET embedding = NULL'); + expect(recipe).toContain('USING embedding::vector(1536)'); + }); }); describe('FactsEmbeddingDimMismatchError', () => {