mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-31 04:07:52 +00:00
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 <harjothkhara@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
harjothkhara
Claude Fable 5
parent
0612b0daa8
commit
cf2deedfc6
+14
-1
@@ -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 }));
|
||||
|
||||
@@ -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';
|
||||
|
||||
+11
-1
@@ -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) {
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user