mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
v0.41.12.0 fix(ze-switch): preserve multimodal column dimensions + restore partial WHERE clause (#1450)
* fix: ze-switch preserves embedding_image column dimensions runSchemaTransition was dropping and recreating embedding_image at the same target dimension as the text embedding column. This silently breaks multimodal search when the image model (e.g. voyage-multimodal-3 at 1024d) uses different dimensions than the text model. Example: switching from OpenAI text-embedding-3-large (1536d) to ZeroEntropy zembed-1 (1280d) would also change embedding_image from vector(1024) to vector(1280), creating a dimension mismatch that prevents voyage-multimodal-3 from writing image embeddings. Fix: only transition the primary 'embedding' column. Leave embedding_image untouched (rebuild its HNSW index for safety but preserve its existing dimensions). * fix(ze-switch): restore partial WHERE clause, schema-qualify probe, add regression tests Eng-review wave on top of PR #1443's column-dim fix: - Restore WHERE embedding_image IS NOT NULL on idx_chunks_embedding_image recreation. Matches src/schema.sql:258-260 and pglite-schema.ts:198-200. Pre-fix the recreated index covered every row including NULLs, wasting HNSW memory proportional to total chunk count on brains with few image chunks. - Scope the information_schema.columns EXISTS probe by table_schema = 'public' so it cannot false-positive against same-named tables in other schemas. - Widen the function-leading comment to name embedding_multimodal (migration v78) alongside embedding_image. Same "separate multimodal model, separate dim" rationale applies. - Add three regression tests pinning the column-preservation invariant: embedding_image AND embedding_multimodal stay vector(1024) post-switch, the partial WHERE clause survives index recreation, and the EXISTS guard short-circuits cleanly on fresh brains lacking the column. * chore: bump version and changelog (v0.41.12.0) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: root <root@localhost> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
co-authored by
root
Claude Opus 4.7
parent
552ff4ed82
commit
25fbae3e18
@@ -2,6 +2,37 @@
|
||||
|
||||
All notable changes to GBrain will be documented in this file.
|
||||
|
||||
## [0.41.12.0] - 2026-05-25
|
||||
|
||||
**`gbrain ze-switch` no longer silently breaks multimodal search on brains that mix text and image embeddings.**
|
||||
|
||||
If you had a brain with image content embedded via Voyage multimodal-3 (1024 dims) and you switched the TEXT embedding model from OpenAI (1536 dims) to ZeroEntropy (1280 dims), the ze-switch transition would silently rewrite the `embedding_image` column to 1280 dims too. Voyage then could not write into it. Image search just stopped returning results. No error. No log line. The column was the wrong shape.
|
||||
|
||||
The fix carves the multimodal columns (`embedding_image` and `embedding_multimodal`) out of the schema transition entirely. Only the primary text `embedding` column moves to the new dim. The HNSW index on `embedding_image` is rebuilt to keep the search path warm, but the column type is preserved.
|
||||
|
||||
The same wave restored the partial `WHERE embedding_image IS NOT NULL` predicate on the recreated index (matches `src/schema.sql:258-260` verbatim, so the HNSW footprint scales with image-chunk count instead of total-chunk count), and tightened the `information_schema` probe to scope by `table_schema = 'public'`.
|
||||
|
||||
To take advantage of v0.41.12.0:
|
||||
|
||||
```bash
|
||||
gbrain upgrade
|
||||
```
|
||||
|
||||
No manual action needed. If you already ran `gbrain ze-switch` on a brain with image embeddings before this fix landed and your image search has been broken since, restore by re-embedding the image content (`gbrain embed --stale` or rerun your multimodal ingest path).
|
||||
|
||||
### Itemized changes
|
||||
|
||||
#### Fixed
|
||||
- `gbrain ze-switch` preserves `embedding_image` and `embedding_multimodal` column dimensions during schema transition. Pre-fix, both columns were dropped and recreated at the new text-embedding target dim, breaking the multimodal provider's writes. Cherry-picked from community PR #1443 (originally authored by `@garrytan-agents`).
|
||||
- `idx_chunks_embedding_image` is recreated with the canonical `WHERE embedding_image IS NOT NULL` partial-index predicate (matches `src/schema.sql:258-260`). Without it, ze-switch silently turned a sparse partial index into a full HNSW index, wasting space proportional to total chunk count on brains with few image chunks.
|
||||
- `runSchemaTransition`'s `information_schema.columns` probe now scopes by `table_schema = 'public'` so the EXISTS check can't false-positive against same-named tables in other schemas.
|
||||
|
||||
#### Added
|
||||
- Three regression tests in `test/retrieval-upgrade-planner.test.ts` pinning the multimodal-column preservation invariant: dim is still `vector(1024)` post-switch on both `embedding_image` and `embedding_multimodal`, the partial WHERE clause survives the index recreation, and the EXISTS guard short-circuits cleanly on fresh brains that lack the column.
|
||||
|
||||
### For contributors
|
||||
|
||||
PR #1443 was incorporated through the standard fix-wave workflow: the original fork PR from `@garrytan-agents` was cherry-picked into a base-repo branch (fork PRs from non-collaborator accounts do not inherit base-repo CI secrets), eng-reviewed via `/plan-eng-review`, and the partial-WHERE-clause + schema-qualified-probe corrections were bundled into the same ship to keep the change atomic and bisect-friendly. The original PR is closed; attribution is preserved in the commit trailer.
|
||||
## [0.41.11.1] - 2026-05-25
|
||||
|
||||
**CI got twice as fast. Every PR now finishes in about four and a half
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "gbrain",
|
||||
"version": "0.41.11.1",
|
||||
"version": "0.41.12.0",
|
||||
"description": "Postgres-native personal knowledge brain with hybrid RAG search",
|
||||
"type": "module",
|
||||
"main": "src/core/index.ts",
|
||||
|
||||
@@ -553,19 +553,48 @@ export async function undoRetrievalUpgrade(engine: BrainEngine): Promise<
|
||||
* `--resume`.
|
||||
*/
|
||||
async function runSchemaTransition(engine: BrainEngine, targetDim: number): Promise<void> {
|
||||
// v0.41 fix: only transition the primary text embedding column.
|
||||
// The embedding_image (v0.27.1) and embedding_multimodal (v0.36 / migration
|
||||
// v78) columns use SEPARATE multimodal models (e.g. voyage-multimodal-3 at
|
||||
// 1024d) whose dimensions are independent of the text embedding model.
|
||||
// Dropping and recreating either at targetDim silently breaks multimodal
|
||||
// search by creating a dimension mismatch between the column and the
|
||||
// multimodal provider's output.
|
||||
//
|
||||
// Before this fix, switching text embeddings from OpenAI (1536d) to
|
||||
// ZeroEntropy (1280d) would also change embedding_image from 1024d to
|
||||
// 1280d, making voyage-multimodal-3 unable to write to it. The same
|
||||
// class of bug applies to embedding_multimodal — leave both untouched.
|
||||
await engine.transaction(async (tx) => {
|
||||
// Text embedding column — transition to target dim.
|
||||
await tx.executeRaw(`DROP INDEX IF EXISTS idx_chunks_embedding`);
|
||||
await tx.executeRaw(`DROP INDEX IF EXISTS idx_chunks_embedding_image`);
|
||||
await tx.executeRaw(`ALTER TABLE content_chunks DROP COLUMN IF EXISTS embedding`);
|
||||
await tx.executeRaw(`ALTER TABLE content_chunks ADD COLUMN embedding vector(${targetDim})`);
|
||||
await tx.executeRaw(`ALTER TABLE content_chunks DROP COLUMN IF EXISTS embedding_image`);
|
||||
await tx.executeRaw(`ALTER TABLE content_chunks ADD COLUMN embedding_image vector(${targetDim})`);
|
||||
await tx.executeRaw(
|
||||
`CREATE INDEX IF NOT EXISTS idx_chunks_embedding ON content_chunks USING hnsw (embedding vector_cosine_ops)`,
|
||||
);
|
||||
await tx.executeRaw(
|
||||
`CREATE INDEX IF NOT EXISTS idx_chunks_embedding_image ON content_chunks USING hnsw (embedding_image vector_cosine_ops)`,
|
||||
|
||||
// Image/multimodal embedding column — rebuild index but preserve
|
||||
// existing dimension. Only create it if it doesn't already exist
|
||||
// (fresh brains may not have it yet). Partial WHERE clause matches
|
||||
// schema.sql:258-260 and pglite-schema.ts:198-200: HNSW footprint
|
||||
// scales with image-chunk count, not table size.
|
||||
const hasImageCol = await tx.executeRaw<{ exists: boolean }>(
|
||||
`SELECT EXISTS (
|
||||
SELECT 1 FROM information_schema.columns
|
||||
WHERE table_schema = 'public'
|
||||
AND table_name = 'content_chunks'
|
||||
AND column_name = 'embedding_image'
|
||||
) AS exists`,
|
||||
);
|
||||
if (hasImageCol[0]?.exists) {
|
||||
await tx.executeRaw(`DROP INDEX IF EXISTS idx_chunks_embedding_image`);
|
||||
await tx.executeRaw(
|
||||
`CREATE INDEX IF NOT EXISTS idx_chunks_embedding_image
|
||||
ON content_chunks USING hnsw (embedding_image vector_cosine_ops)
|
||||
WHERE embedding_image IS NOT NULL`,
|
||||
);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -252,6 +252,108 @@ describe('applyRetrievalUpgrade — state machine + atomicity (D12, D18)', () =>
|
||||
expect(names).toContain('idx_chunks_embedding');
|
||||
expect(names).toContain('idx_chunks_embedding_image');
|
||||
});
|
||||
|
||||
// v0.41 regression — PR #1443. runSchemaTransition must NOT widen the
|
||||
// embedding_image or embedding_multimodal columns to targetDim. Both use
|
||||
// separate multimodal models whose dimensions are independent of the text
|
||||
// embedding model. The pre-fix code dropped + recreated embedding_image
|
||||
// at targetDim, silently breaking voyage-multimodal-3 (1024d). The same
|
||||
// bug class applies to embedding_multimodal — pin both.
|
||||
test('runSchemaTransition preserves embedding_image dimension (1024) post-switch', async () => {
|
||||
await setLegacyDefaultConfig();
|
||||
await seedPages(150);
|
||||
const plan = await planRetrievalUpgrade(engine);
|
||||
await applyRetrievalUpgrade(engine, plan);
|
||||
|
||||
// Probe column type via format_type so we see the parameterized dim
|
||||
// (the typmod). udt_name alone reports just 'vector' without the dim.
|
||||
const rows = await engine.executeRaw<{ column_name: string; col_type: string }>(
|
||||
`SELECT a.attname AS column_name,
|
||||
format_type(a.atttypid, a.atttypmod) AS col_type
|
||||
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 IN ('embedding', 'embedding_image', 'embedding_multimodal')
|
||||
AND a.attnum > 0
|
||||
ORDER BY a.attname`,
|
||||
);
|
||||
const byName = new Map(rows.map(r => [r.column_name, r.col_type]));
|
||||
|
||||
// Primary text column transitioned to target dim.
|
||||
expect(byName.get('embedding')).toBe(`vector(${ZE_TARGET_EMBEDDING_DIM})`);
|
||||
// Multimodal columns preserved at their original 1024d shape.
|
||||
expect(byName.get('embedding_image')).toBe('vector(1024)');
|
||||
expect(byName.get('embedding_multimodal')).toBe('vector(1024)');
|
||||
});
|
||||
|
||||
test('runSchemaTransition restores partial WHERE on idx_chunks_embedding_image', async () => {
|
||||
await setLegacyDefaultConfig();
|
||||
await seedPages(150);
|
||||
const plan = await planRetrievalUpgrade(engine);
|
||||
await applyRetrievalUpgrade(engine, plan);
|
||||
|
||||
// pg_indexes.indexdef carries the partial predicate verbatim when present.
|
||||
const rows = await engine.executeRaw<{ indexdef: string }>(
|
||||
`SELECT indexdef FROM pg_indexes
|
||||
WHERE schemaname = 'public'
|
||||
AND tablename = 'content_chunks'
|
||||
AND indexname = 'idx_chunks_embedding_image'`,
|
||||
);
|
||||
expect(rows.length).toBe(1);
|
||||
// Match schema.sql:258-260 / pglite-schema.ts:198-200 canonical shape.
|
||||
expect(rows[0].indexdef).toMatch(/WHERE\s+\(?embedding_image IS NOT NULL\)?/i);
|
||||
});
|
||||
|
||||
test('runSchemaTransition EXISTS guard short-circuits cleanly when embedding_image column is absent', async () => {
|
||||
// Simulate a (hypothetical) pre-v0.27.1 brain with no embedding_image
|
||||
// column. The fix's EXISTS probe must skip the image branch without
|
||||
// error. We can't actually run the full ze-switch flow without the
|
||||
// column (other code paths assume it), but we can directly drop the
|
||||
// column and re-invoke the schema transition via resume to verify the
|
||||
// probe doesn't throw on the missing-column branch.
|
||||
await setLegacyDefaultConfig();
|
||||
await seedPages(150);
|
||||
|
||||
// Drop both multimodal columns to simulate a brain that never had them.
|
||||
await engine.executeRaw(
|
||||
`ALTER TABLE content_chunks DROP COLUMN IF EXISTS embedding_image`,
|
||||
);
|
||||
await engine.executeRaw(
|
||||
`ALTER TABLE content_chunks DROP COLUMN IF EXISTS embedding_multimodal`,
|
||||
);
|
||||
|
||||
// Resume path also calls runSchemaTransition. Mark as requested so the
|
||||
// resume actually runs the schema work rather than short-circuiting.
|
||||
await engine.setConfig(KEY_REQUESTED, 'true');
|
||||
const result = await resumeRetrievalUpgrade(engine);
|
||||
|
||||
// The probe should have short-circuited the image branch and the rest
|
||||
// of the transition should have completed without error.
|
||||
expect(result.status).toBe('applied');
|
||||
|
||||
// Primary column still landed at target dim.
|
||||
const rows = await engine.executeRaw<{ col_type: string }>(
|
||||
`SELECT format_type(a.atttypid, a.atttypmod) AS col_type
|
||||
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'`,
|
||||
);
|
||||
expect(rows[0]?.col_type).toBe(`vector(${ZE_TARGET_EMBEDDING_DIM})`);
|
||||
|
||||
// Image index should NOT exist — the EXISTS guard correctly skipped it.
|
||||
const idxRows = await engine.executeRaw<{ indexname: string }>(
|
||||
`SELECT indexname FROM pg_indexes
|
||||
WHERE schemaname = 'public'
|
||||
AND tablename = 'content_chunks'
|
||||
AND indexname = 'idx_chunks_embedding_image'`,
|
||||
);
|
||||
expect(idxRows.length).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Three-key state machine transitions (D12)', () => {
|
||||
|
||||
Reference in New Issue
Block a user