mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-28 14:59:47 +00:00
fix(core): facts alter recipe NULLs embeddings before cross-dimension alters
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.
This commit is contained in:
@@ -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`,
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
Reference in New Issue
Block a user