From cf2deedfc6fd7c875f5d05edb760a0c208c42ecf Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 21 Jul 2026 14:42:17 -0700 Subject: [PATCH] fix(embed): label content_chunks.model with the model that produced the vector (#1717) The embed paths (embedPage, embedAll, embedAllStale) and the inline import/sync embed paths built ChunkInput[] without a model field, so the engines' upsertChunks defaulted content_chunks.model to the hardcoded DEFAULT_EMBEDDING_MODEL instead of the gateway-configured model that actually produced the vector. - New core helper resolveEmbeddingModelLabel() in src/core/embedding.ts (returns the resolved gateway model, undefined when unconfigured). - embed.ts: stamp the label on (re)embedded chunks in all three paths; chunks preserved from a prior embed keep their existing model so a mixed-model page isn't relabeled wholesale. - import-file.ts: stamp the label on inline-embedded markdown chunks and re-embedded code chunks; reused (incremental) code-chunk embeddings carry their existing model label forward. Takeover of PR #1803 (rebased onto master over the pace-mode changes; helper moved into core so import-file.ts can share it). Co-authored-by: harjothkhara Co-Authored-By: Claude Fable 5 --- src/commands/embed.ts | 15 +++++++++- src/core/embedding.ts | 15 ++++++++++ src/core/import-file.ts | 12 +++++++- test/embed.serial.test.ts | 33 ++++++++++++++++++++++ test/import-signature-stamp.serial.test.ts | 17 +++++++++++ 5 files changed, 90 insertions(+), 2 deletions(-) diff --git a/src/commands/embed.ts b/src/commands/embed.ts index 6d8816a6e..e966f7971 100644 --- a/src/commands/embed.ts +++ b/src/commands/embed.ts @@ -1,5 +1,5 @@ import type { BrainEngine } from '../core/engine.ts'; -import { embedBatch, currentEmbeddingSignature } from '../core/embedding.ts'; +import { embedBatch, currentEmbeddingSignature, resolveEmbeddingModelLabel } from '../core/embedding.ts'; import type { ChunkInput } from '../core/types.ts'; import { chunkText } from '../core/chunkers/recursive.ts'; import { createProgress, type ProgressReporter } from '../core/progress.ts'; @@ -581,11 +581,16 @@ async function embedPage( for (let j = 0; j < toEmbed.length; j++) { embeddingMap.set(toEmbed[j].chunk_index, embeddings[j]); } + // #1717: label each (re)embedded chunk with the model that actually + // produced its vector. Preserved chunks (not re-embedded this pass) keep + // their existing model so a mixed-model page isn't relabeled wholesale. + const embedModelLabel = resolveEmbeddingModelLabel(); const updated: ChunkInput[] = chunks.map(c => ({ chunk_index: c.chunk_index, chunk_text: c.chunk_text, chunk_source: c.chunk_source, embedding: embeddingMap.get(c.chunk_index), + model: embeddingMap.has(c.chunk_index) && embedModelLabel ? embedModelLabel : c.model, token_count: c.token_count || Math.ceil(c.chunk_text.length / 4), })); @@ -717,12 +722,16 @@ async function embedAll( for (let j = 0; j < toEmbed.length; j++) { embeddingMap.set(toEmbed[j].chunk_index, embeddings[j]); } + // #1717: stamp the resolved embedding model on (re)embedded chunks; + // preserve the existing model on chunks left untouched. + const embedModelLabel = resolveEmbeddingModelLabel(); // Preserve ALL chunks, only update embeddings for stale ones const updated: ChunkInput[] = chunks.map(c => ({ chunk_index: c.chunk_index, chunk_text: c.chunk_text, chunk_source: c.chunk_source, embedding: embeddingMap.get(c.chunk_index) ?? undefined, + model: embeddingMap.has(c.chunk_index) && embedModelLabel ? embedModelLabel : c.model, token_count: c.token_count || Math.ceil(c.chunk_text.length / 4), })); await observed(pacer, () => engine.upsertChunks(page.slug, updated, pageOpts)); @@ -1012,11 +1021,15 @@ async function embedAllStale( for (let j = 0; j < stale.length; j++) { staleIdxToEmbedding.set(stale[j].chunk_index, embeddings[j]); } + // #1717: label the re-embedded (stale) chunks with the resolved + // model; preserve the existing model on the non-stale chunks. + const embedModelLabel = resolveEmbeddingModelLabel(); const merged: ChunkInput[] = existing.map(c => ({ chunk_index: c.chunk_index, chunk_text: c.chunk_text, chunk_source: c.chunk_source, embedding: staleIdxToEmbedding.get(c.chunk_index) ?? undefined, + model: staleIdxToEmbedding.has(c.chunk_index) && embedModelLabel ? embedModelLabel : c.model, token_count: c.token_count || Math.ceil(c.chunk_text.length / 4), })); await observed(pacer, () => engine.upsertChunks(slug, merged, { sourceId: keySourceId })); diff --git a/src/core/embedding.ts b/src/core/embedding.ts index 108b854f8..ba2582089 100644 --- a/src/core/embedding.ts +++ b/src/core/embedding.ts @@ -113,6 +113,21 @@ export async function embedBatch( return results; } +/** + * Resolve the embedding model label (`provider:model`) to stamp onto + * `content_chunks.model`, so each chunk records the model that actually + * produced its vector instead of the engine's hardcoded default (#1717). + * Returns undefined if the gateway is unconfigured; callers then fall back + * to the chunk's existing model rather than mislabeling it. + */ +export function resolveEmbeddingModelLabel(): string | undefined { + try { + return gatewayGetModel(); + } catch { + return undefined; + } +} + /** Currently-configured embedding model (short form without provider prefix). */ export function getEmbeddingModelName(): string { return gatewayGetModel().split(':').slice(1).join(':') || 'text-embedding-3-large'; diff --git a/src/core/import-file.ts b/src/core/import-file.ts index 4e7722abb..64dd8317d 100644 --- a/src/core/import-file.ts +++ b/src/core/import-file.ts @@ -8,7 +8,7 @@ import { chunkText } from './chunkers/recursive.ts'; import { chunkCodeText, chunkCodeTextFull, detectCodeLanguage, CHUNKER_VERSION } from './chunkers/code.ts'; import { findChunkForOffset } from './chunkers/edge-extractor.ts'; import { extractCodeRefs, imageOfCandidates } from './link-extraction.ts'; -import { embedBatch, embedMultimodal, currentEmbeddingSignature } from './embedding.ts'; +import { embedBatch, embedMultimodal, currentEmbeddingSignature, resolveEmbeddingModelLabel } from './embedding.ts'; import { slugifyPath, slugifyCodePath, isCodeFilePath } from './sync.ts'; import type { ChunkInput, PageInput, PageType } from './types.ts'; import { computeEffectiveDate } from './effective-date.ts'; @@ -716,8 +716,12 @@ export async function importFromContent( ? chunks.map((c) => wrapChunkForEmbedding(c.chunk_text, prefix, c.chunk_source)) : chunks.map((c) => c.chunk_text); const embeddings = await embedBatch(wrappedTexts); + // #1717: label each chunk with the model that actually produced its + // vector, not the engine's hardcoded default. + const embedModelLabel = resolveEmbeddingModelLabel(); for (let i = 0; i < chunks.length; i++) { chunks[i].embedding = embeddings[i]; + if (embedModelLabel) chunks[i].model = embedModelLabel; // token_count tracks the wrapped string length so cost reporting // reflects what we actually sent to the embedder. chunks[i].token_count = Math.ceil(wrappedTexts[i].length / 4); @@ -1141,7 +1145,10 @@ export async function importCodeFile( const matched = existingByKey.get(key); if (matched && matched.embedding) { // Reuse the existing embedding verbatim. No API call, no cost. + // #1717: carry the existing model label along with the reused vector + // so the upsert doesn't relabel it with the engine default. chunks[i]!.embedding = matched.embedding as Float32Array; + chunks[i]!.model = matched.model ?? undefined; chunks[i]!.token_count = matched.token_count ?? undefined; } else { needsEmbedIndexes.push(i); @@ -1153,9 +1160,12 @@ export async function importCodeFile( try { const textsToEmbed = needsEmbedIndexes.map((i) => chunks[i]!.chunk_text); const embeddings = await embedBatch(textsToEmbed); + // #1717: stamp the model that produced these vectors. + const embedModelLabel = resolveEmbeddingModelLabel(); for (let j = 0; j < needsEmbedIndexes.length; j++) { const i = needsEmbedIndexes[j]!; chunks[i]!.embedding = embeddings[j]!; + if (embedModelLabel) chunks[i]!.model = embedModelLabel; chunks[i]!.token_count = Math.ceil(chunks[i]!.chunk_text.length / 4); } } catch (e: unknown) { diff --git a/test/embed.serial.test.ts b/test/embed.serial.test.ts index b4db83beb..bd61ab20b 100644 --- a/test/embed.serial.test.ts +++ b/test/embed.serial.test.ts @@ -37,6 +37,8 @@ mock.module('../src/core/embedding.ts', () => ({ // setPageEmbeddingSignature / invalidateStaleSignatureEmbeddings resolve to // null via the Proxy default, so the signature value is inert here. currentEmbeddingSignature: () => 'test:model:1536', + // #1717: embed paths stamp this label on (re)embedded chunks. + resolveEmbeddingModelLabel: () => 'openai:text-embedding-3-large', })); // Import AFTER mocking. @@ -803,3 +805,34 @@ describe('embedAllStale --source threading (D7)', () => { expect((firstCallOpts as { sourceId?: string }).sourceId).toBe('media-corpus'); }); }); + +// #1717: content_chunks.model must record the model that actually produced +// each vector, not the gateway/engine default. +describe('content_chunks.model labeling (#1717)', () => { + test('stamps the resolved embedding model on re-embedded chunks, preserves it on untouched chunks', async () => { + let upserted: any[] | undefined; + // Chunk 0 is stale (no embedded_at) → gets re-embedded this pass. + // Chunk 1 is already embedded with a DIFFERENT model → must be preserved, + // not relabeled to the current model. + const chunks = [ + { chunk_index: 0, chunk_text: 'a', chunk_source: 'compiled_truth', embedded_at: null, model: 'zeroentropyai:zembed-1', token_count: 1 }, + { chunk_index: 1, chunk_text: 'b', chunk_source: 'compiled_truth', embedded_at: '2026-01-01', embedding: new Float32Array(1536), model: 'voyage:voyage-3', token_count: 1 }, + ]; + const engine = mockEngine({ + getPage: async () => ({ slug: 'notes/x', compiled_truth: 'a', timeline: '', source_id: 'default' }), + getChunks: async () => chunks, + upsertChunks: async (_slug: string, c: any[]) => { upserted = c; }, + setPageEmbeddingSignature: async () => null, + }); + + await runEmbedCore(engine, { slugs: ['notes/x'] }); + + expect(upserted).toBeDefined(); + const byIdx = Object.fromEntries(upserted!.map(c => [c.chunk_index, c])); + // Re-embedded chunk carries the model that produced its vector (was + // mislabeled with the default before the fix). + expect(byIdx[0].model).toBe('openai:text-embedding-3-large'); + // Untouched chunk keeps its original model — no wholesale relabel. + expect(byIdx[1].model).toBe('voyage:voyage-3'); + }); +}); diff --git a/test/import-signature-stamp.serial.test.ts b/test/import-signature-stamp.serial.test.ts index 4614f0fb4..6e5b59c0a 100644 --- a/test/import-signature-stamp.serial.test.ts +++ b/test/import-signature-stamp.serial.test.ts @@ -73,4 +73,21 @@ describe('importFromContent embedding_signature stamping (F1)', () => { await importFromContent(engine, 'concepts/unstamped', '# Unstamped\n\nbody content.', { noEmbed: true }); expect(await signatureOf('concepts/unstamped')).toBeNull(); }); + + // #1717: content_chunks.model must record the model that produced the + // vector (the configured gateway model), not the engine's hardcoded + // default. The gateway here is configured to openai:text-embedding-3-large, + // which differs from DEFAULT_EMBEDDING_MODEL — so this fails without the + // import-path model stamping. + test('inline embed labels content_chunks.model with the configured model (#1717)', async () => { + await importFromContent(engine, 'concepts/labeled', '# Labeled\n\nsome body content to chunk and embed.', {}); + const rows = await engine.executeRaw<{ model: string }>( + `SELECT cc.model FROM content_chunks cc + JOIN pages p ON p.id = cc.page_id + WHERE p.slug = $1 AND p.source_id = 'default'`, + ['concepts/labeled'], + ); + expect(rows.length).toBeGreaterThan(0); + for (const r of rows) expect(r.model).toBe('openai:text-embedding-3-large'); + }); });