From 306fc0e1ef893eab87f214d857d2c540b151c6fd Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 6 May 2026 18:01:18 -0700 Subject: [PATCH] fix(init): error on existing-brain dim mismatch + embedding-migration recipe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds A4 hard-error path: when `gbrain init --embedding-dimensions N` is run against an existing brain whose `content_chunks.embedding` column is a different `vector(M)`, init exits 1 with an inline four-step ALTER recipe and a pointer to docs/embedding-migrations.md. This kills the silent-corruption pattern surfaced by issue #673: the v0.27 schema seeded `('embedding_dimensions', '1536')` regardless of the flag, so users got a config saying 768 but a column at 1536 — first sync write blew up with "expected 1536, got 768." A4's contract: 1. Connect to engine BEFORE saveConfig so we can read the live column type 2. If column exists AND dim != requested, exit 1 (loud failure) 3. If column doesn't exist (fresh init) OR dim matches, proceed normally Recipe in docs/embedding-migrations.md (and inlined in init's error output) covers all four destructive steps codex's plan-review caught: 1. DROP INDEX IF EXISTS idx_chunks_embedding (HNSW won't survive ALTER) 2. ALTER TABLE content_chunks ALTER COLUMN embedding TYPE vector(N) 3. UPDATE content_chunks SET embedding = NULL, embedded_at = NULL 4. CREATE INDEX HNSW *only if N <= 2000* (pgvector cap) Step 4 is conditional: dims > 2000 (e.g. Voyage 4 Large 2048d) cannot be HNSW-indexed in pgvector; the recipe explicitly says "Skip reindex" in that case so the user doesn't paste a CREATE INDEX that crashes. Helper `readContentChunksEmbeddingDim` and message builder `embeddingMismatchMessage` live in src/core/embedding-dim-check.ts so doctor 8b (next commit) can reuse the same source of truth. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/embedding-migrations.md | 105 +++++++++++++++++++++++++++ src/commands/init.ts | 50 +++++++++++++ src/core/embedding-dim-check.ts | 120 +++++++++++++++++++++++++++++++ test/embedding-dim-check.test.ts | 87 ++++++++++++++++++++++ 4 files changed, 362 insertions(+) create mode 100644 docs/embedding-migrations.md create mode 100644 src/core/embedding-dim-check.ts create mode 100644 test/embedding-dim-check.test.ts diff --git a/docs/embedding-migrations.md b/docs/embedding-migrations.md new file mode 100644 index 000000000..f6f692559 --- /dev/null +++ b/docs/embedding-migrations.md @@ -0,0 +1,105 @@ +# Switching embedding models or dimensions on an existing brain + +GBrain stores embeddings in a fixed-dimension `vector(N)` column on +`content_chunks`. If you switch to a model with a different dimension +(e.g. `text-embedding-3-large` 1536 → `voyage-multilingual-large-2` 2048, +or back to a smaller model like `nomic-embed-text` 768), the on-disk +column type doesn't change automatically. + +`gbrain init` and `gbrain doctor` both detect and refuse to silently +proceed in this case. This doc is the recipe they point at. + +## Why we don't do this automatically + +Switching dimensions requires: + +1. Dropping the HNSW vector index (pgvector won't survive an `ALTER COLUMN TYPE`). +2. Altering the column type. +3. Wiping every existing embedding (the old vectors are unusable in the new space). +4. Re-embedding the entire corpus (can take hours on a 50K-page brain and costs $1-100 in API calls depending on model). +5. Conditionally recreating the index (HNSW supports up to 2000 dimensions per pgvector; above that you must use exact scans). + +That's not an upgrade-time auto-run. It's a deliberate, expensive +operation. Run it when you've decided you actually want the new model. + +## Recipe — manual `psql` against your brain + +Replace `` with your target dimension count. + +```sql +BEGIN; + +-- 1. Drop the HNSW index. It can't survive the column type change. +DROP INDEX IF EXISTS idx_chunks_embedding; + +-- 2. Alter the column type. (You can DROP COLUMN + ADD COLUMN instead +-- if the existing data is already gone — same end state.) +ALTER TABLE content_chunks ALTER COLUMN embedding TYPE vector(); + +-- 3. Clear stale embeddings so they don't survive into the new space. +-- Either truncate (faster, drops all chunks) or null out (preserves +-- chunk text so re-embed regenerates without re-chunking): +UPDATE content_chunks SET embedding = NULL, embedded_at = NULL; + +-- 4. Recreate the HNSW index ONLY IF dims <= 2000. Above that, leave it +-- indexless and rely on exact scans (gbrain searchVector handles this +-- automatically — search just gets slower, not broken). +-- For dims <= 2000 (e.g. 1024, 1536, 768): +CREATE INDEX IF NOT EXISTS idx_chunks_embedding + ON content_chunks USING hnsw (embedding vector_cosine_ops); +-- For dims > 2000 (e.g. 2048 Voyage 4 Large): skip step 4. + +COMMIT; +``` + +Then update gbrain's config so it knows the new dim: + +```bash +gbrain config set embedding_model +gbrain config set embedding_dimensions +``` + +And re-embed the corpus: + +```bash +gbrain embed --stale +``` + +## PGLite (local brain) + +Same recipe, but you connect to the embedded database differently: + +```bash +gbrain config get database_url # confirm engine: pglite +# Open a psql-equivalent — for PGLite, the easiest path is to write a small +# script that imports PGLiteEngine and runs the SQL via engine.executeRaw. +# Or migrate to Postgres temporarily (gbrain migrate --to supabase) if you +# want a real psql connection. +``` + +For most PGLite users the simpler path is to **wipe and re-init** if your +corpus is small enough that re-syncing is faster than hand-crafting the +migration: + +```bash +mv ~/.gbrain/brain.pglite ~/.gbrain/brain.pglite.bak +gbrain init --pglite --embedding-dimensions +gbrain sync # re-imports your brain repo from disk +``` + +## Verify + +After the recipe lands, `gbrain doctor --fast` should report green and +`gbrain doctor` (full) should say check 8b passes: + +``` +✓ embedding_provider dim parity: config 768 / column vector(768) / live probe 768 +``` + +If it doesn't, file an issue with the doctor output and the SQL you ran. + +## v0.29+ plans + +`gbrain migrate-embedding-dim --to ` is a tracked TODO. It will run +the recipe above with progress reporting + an explicit confirmation +gate. Until that lands, this manual recipe is the canonical path. diff --git a/src/commands/init.ts b/src/commands/init.ts index 1d1e0a5b0..09b9231f3 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -195,6 +195,32 @@ async function initPGLite(opts: { const engine = await createEngine({ engine: 'pglite' }); try { await engine.connect({ database_path: dbPath, engine: 'pglite' }); + + // v0.28.5 (A4): refuse to silently re-template an existing brain with a + // mismatched embedding dimension. Loud failure beats the v0.27 silent- + // corruption pattern that surfaced as #673. + if (opts.aiOpts?.embedding_dimensions) { + const { readContentChunksEmbeddingDim, embeddingMismatchMessage } = await import('../core/embedding-dim-check.ts'); + const existing = await readContentChunksEmbeddingDim(engine); + if (existing.exists && existing.dims !== null && existing.dims !== opts.aiOpts.embedding_dimensions) { + console.error('\n' + embeddingMismatchMessage({ + currentDims: existing.dims, + requestedDims: opts.aiOpts.embedding_dimensions, + requestedModel: opts.aiOpts.embedding_model, + source: 'init', + }) + '\n'); + if (opts.jsonOutput) { + console.log(JSON.stringify({ + status: 'error', + reason: 'embedding_dim_mismatch', + current_dims: existing.dims, + requested_dims: opts.aiOpts.embedding_dimensions, + })); + } + process.exit(1); + } + } + await engine.initSchema(); const config: GBrainConfig = { @@ -304,6 +330,30 @@ async function initPostgres(opts: { // Non-fatal } + // v0.28.5 (A4): refuse to silently re-template an existing brain with a + // mismatched embedding dimension (mirror of the PGLite path above). + if (opts.aiOpts?.embedding_dimensions) { + const { readContentChunksEmbeddingDim, embeddingMismatchMessage } = await import('../core/embedding-dim-check.ts'); + const existing = await readContentChunksEmbeddingDim(engine); + if (existing.exists && existing.dims !== null && existing.dims !== opts.aiOpts.embedding_dimensions) { + console.error('\n' + embeddingMismatchMessage({ + currentDims: existing.dims, + requestedDims: opts.aiOpts.embedding_dimensions, + requestedModel: opts.aiOpts.embedding_model, + source: 'init', + }) + '\n'); + if (opts.jsonOutput) { + console.log(JSON.stringify({ + status: 'error', + reason: 'embedding_dim_mismatch', + current_dims: existing.dims, + requested_dims: opts.aiOpts.embedding_dimensions, + })); + } + process.exit(1); + } + } + console.log('Running schema migration...'); await engine.initSchema(); diff --git a/src/core/embedding-dim-check.ts b/src/core/embedding-dim-check.ts new file mode 100644 index 000000000..0474f4038 --- /dev/null +++ b/src/core/embedding-dim-check.ts @@ -0,0 +1,120 @@ +/** + * Detect existing-brain embedding-dimension mismatch (v0.28.5 — A4). + * + * `gbrain init --embedding-dimensions N` on an existing brain whose + * `content_chunks.embedding` column is a different `vector(M)` would + * silently create a config/column drift: the config gets templated to N + * but the column stays at M. The first sync write blows up with + * "expected M, got N" — the silent-corruption pattern v0.28.5 is shipped + * to kill. + * + * Loud-failure path: `gbrain init` AND `gbrain doctor` both consult this + * helper. On mismatch they emit the same inline ALTER recipe (see + * `embeddingMismatchMessage`) plus a pointer to `docs/embedding-migrations.md`. + */ + +import type { BrainEngine } from './engine.ts'; +import { PGVECTOR_HNSW_VECTOR_MAX_DIMS } from './vector-index.ts'; + +export interface ColumnDimResult { + /** Whether the `content_chunks.embedding` column exists. False on a fresh brain. */ + exists: boolean; + /** Parsed `vector(N)` dimension if known. null when the column doesn't exist or the type isn't vector. */ + dims: number | null; +} + +/** + * Read the actual dimension of `content_chunks.embedding` from the engine. + * + * Uses information_schema + a vector-specific catalog query. Returns + * { exists: false, dims: null } on a fresh brain that doesn't have the + * column yet. Returns { exists: true, dims: null } on a brain whose + * column type isn't `vector` (shouldn't happen but defensive). + */ +export async function readContentChunksEmbeddingDim(engine: BrainEngine): Promise { + // Probe column existence first to avoid noisy errors on fresh brains. + const existsRows = await engine.executeRaw<{ exists: boolean }>( + `SELECT EXISTS ( + SELECT 1 FROM information_schema.columns + WHERE table_schema = 'public' + AND table_name = 'content_chunks' + AND column_name = 'embedding' + ) AS exists`, + ); + const exists = !!existsRows?.[0]?.exists; + if (!exists) return { exists: false, dims: null }; + + // pgvector stores dim in pg_type.typmod when atttypmod is set; format_type + // returns the human-readable `vector(N)`. We parse N out of that. + const formatRows = await engine.executeRaw<{ formatted: string | null }>( + `SELECT format_type(a.atttypid, a.atttypmod) AS formatted + FROM pg_attribute a + JOIN pg_class c ON c.oid = a.attrelid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname = 'public' + AND c.relname = 'content_chunks' + AND a.attname = 'embedding' + AND NOT a.attisdropped`, + ); + const formatted = formatRows?.[0]?.formatted ?? null; + if (!formatted) return { exists: true, dims: null }; + + const m = formatted.match(/vector\((\d+)\)/i); + return { exists: true, dims: m ? parseInt(m[1], 10) : null }; +} + +/** + * Build the human-readable ALTER recipe printed inline to stderr (or + * delivered via `gbrain doctor` output) when an existing brain's column + * dim doesn't match the requested dim. + * + * Steps cover the four-step contract from `docs/embedding-migrations.md`: + * 1. DROP INDEX (HNSW can't survive ALTER COLUMN TYPE) + * 2. ALTER COLUMN TYPE + * 3. Wipe stale embeddings + * 4. Conditional reindex (HNSW only when dims <= 2000) + */ +export function embeddingMismatchMessage(opts: { + currentDims: number; + requestedDims: number; + requestedModel?: string; + source?: 'init' | 'doctor'; +}): string { + const { currentDims, requestedDims, requestedModel, source } = opts; + const supportsHnsw = requestedDims <= PGVECTOR_HNSW_VECTOR_MAX_DIMS; + const reindexLine = supportsHnsw + ? `CREATE INDEX IF NOT EXISTS idx_chunks_embedding\n ON content_chunks USING hnsw (embedding vector_cosine_ops);` + : `-- Skip reindex. dims=${requestedDims} exceeds pgvector's HNSW cap of ${PGVECTOR_HNSW_VECTOR_MAX_DIMS};\n-- searchVector falls back to exact scan.`; + + const header = source === 'doctor' + ? `Embedding dimension mismatch detected.` + : `Refusing to silently re-template existing brain.`; + + const lines = [ + header, + ``, + ` Existing column: vector(${currentDims})`, + ` Requested: vector(${requestedDims})${requestedModel ? ` (${requestedModel})` : ''}`, + ``, + `Switching dims is destructive: it drops every embedding in your brain and`, + `requires a full re-embed (potentially hours and $1-100 in API calls).`, + ``, + `If you actually want to switch, run this manually against your brain's DB:`, + ``, + ` BEGIN;`, + ` DROP INDEX IF EXISTS idx_chunks_embedding;`, + ` ALTER TABLE content_chunks ALTER COLUMN embedding TYPE vector(${requestedDims});`, + ` UPDATE content_chunks SET embedding = NULL, embedded_at = NULL;`, + ` ${reindexLine.split('\n').join('\n ')}`, + ` COMMIT;`, + ``, + `Then re-embed:`, + ` gbrain config set embedding_dimensions ${requestedDims}`, + requestedModel ? ` gbrain config set embedding_model ${requestedModel}` : '', + ` gbrain embed --stale`, + ``, + `Full guide: docs/embedding-migrations.md`, + ].filter(Boolean); + + return lines.join('\n'); +} diff --git a/test/embedding-dim-check.test.ts b/test/embedding-dim-check.test.ts new file mode 100644 index 000000000..808c7be3f --- /dev/null +++ b/test/embedding-dim-check.test.ts @@ -0,0 +1,87 @@ +/** + * v0.28.5 (A4) — Existing-brain dimension-mismatch detection unit tests. + * + * Pairs with `gbrain init` and `gbrain doctor`'s loud-failure paths. Validates + * that: + * 1. readContentChunksEmbeddingDim correctly reports null on a fresh brain. + * 2. After initSchema, it returns the actual templated dim (1536 default, + * or whatever was passed via gateway config). + * 3. embeddingMismatchMessage produces a recipe that explicitly drops the + * HNSW index, alters the column, wipes embeddings, and conditionally + * reindexes — codex's #8 finding from plan review. + */ + +import { test, expect, describe } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { + readContentChunksEmbeddingDim, + embeddingMismatchMessage, +} from '../src/core/embedding-dim-check.ts'; + +describe('readContentChunksEmbeddingDim', () => { + test('returns { exists: false, dims: null } on a fresh brain', async () => { + const engine = new PGLiteEngine(); + await engine.connect({}); + try { + const result = await readContentChunksEmbeddingDim(engine); + expect(result.exists).toBe(false); + expect(result.dims).toBeNull(); + } finally { + await engine.disconnect(); + } + }, 30000); + + test('returns dims from a migrated brain (default 1536)', async () => { + const engine = new PGLiteEngine(); + await engine.connect({}); + try { + await engine.initSchema(); + const result = await readContentChunksEmbeddingDim(engine); + expect(result.exists).toBe(true); + expect(result.dims).toBe(1536); + } finally { + await engine.disconnect(); + } + }, 30000); +}); + +describe('embeddingMismatchMessage', () => { + test('inlines all four recipe steps for HNSW-eligible dims', () => { + const msg = embeddingMismatchMessage({ + currentDims: 1536, + requestedDims: 768, + requestedModel: 'nomic-embed-text', + source: 'init', + }); + expect(msg).toContain('vector(1536)'); + expect(msg).toContain('vector(768)'); + expect(msg).toContain('DROP INDEX IF EXISTS idx_chunks_embedding'); + expect(msg).toContain('ALTER TABLE content_chunks ALTER COLUMN embedding TYPE vector(768)'); + expect(msg).toContain('UPDATE content_chunks SET embedding = NULL'); + expect(msg).toContain('CREATE INDEX IF NOT EXISTS idx_chunks_embedding'); + expect(msg).toContain('docs/embedding-migrations.md'); + }); + + test('skips HNSW recreate when requested dims exceed pgvector cap', () => { + // Codex finding #8: 2048d (Voyage 4 Large) cannot be HNSW-indexed in pgvector. + // The recipe must NOT instruct a CREATE INDEX HNSW for that dim. + const msg = embeddingMismatchMessage({ + currentDims: 1536, + requestedDims: 2048, + requestedModel: 'voyage-4-large', + source: 'init', + }); + expect(msg).toContain('vector(2048)'); + expect(msg).toContain('Skip reindex'); + expect(msg).toContain('exceeds pgvector\'s HNSW cap'); + // The HNSW CREATE INDEX line must NOT appear in the 2048d recipe. + expect(msg).not.toContain('CREATE INDEX IF NOT EXISTS idx_chunks_embedding\n ON content_chunks USING hnsw'); + }); + + test('source: doctor uses a different header than source: init', () => { + const initMsg = embeddingMismatchMessage({ currentDims: 1536, requestedDims: 768, source: 'init' }); + const doctorMsg = embeddingMismatchMessage({ currentDims: 1536, requestedDims: 768, source: 'doctor' }); + expect(initMsg).toContain('Refusing to silently re-template'); + expect(doctorMsg).toContain('Embedding dimension mismatch detected'); + }); +});