mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-28 14:59:47 +00:00
reland: fix(embed): stamp gateway-resolved model in content_chunks.model, not compiled default (#2846) (#3343)
* fix(embed): stamp gateway-resolved model in content_chunks.model, not compiled default (#2846) upsertChunks fell back to the compile-time DEFAULT_EMBEDDING_MODEL ('zeroentropyai:zembed-1') when a ChunkInput carried no explicit `model`. The embed pipeline (src/commands/embed.ts) builds ChunkInputs without a `model` field, so rows whose vectors were produced by the config-resolved model (e.g. openai:text-embedding-3-large) were mislabeled with the hardcoded default — corrupting the provenance that signature-drift staleness and dimension-migration logic depend on. Both engines now resolve the gateway's runtime embedding model once per upsert and use it as the fallback, mirroring the existing resolve-then- default pattern used for schema sizing. Regression test added (pglite); verified via negative control that it fails against the old fallback. This is a write-path change (upsertChunks), not a search-path change, so retrieval eval replay is not applicable. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * test: Lane A.7 pins gateway-resolved chunk model, not compiled default #2846 changed upsertChunks' fallback from DEFAULT_EMBEDDING_MODEL to the gateway-resolved runtime model. Lane A.7 still pinned the old fallback, and the test preload (test/helpers/legacy-embedding-preload.ts) pins the gateway to openai:text-embedding-3-large for every test process — so the original #2846 landing failed this test deterministically and got batch- reverted. The test now asserts the resolved model (the intended #2846 semantics) while keeping the CDX2-4 bare-literal regression guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: SailorJoe6 <SailorJoe6@Gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Garry Tan <garrytan@gmail.com>
This commit is contained in:
co-authored by
Claude Fable 5
SailorJoe6
Garry Tan
parent
5e665c1c06
commit
e1919fab9f
@@ -2312,6 +2312,18 @@ export class PGLiteEngine implements BrainEngine {
|
||||
const params: unknown[] = [];
|
||||
let paramIdx = 1;
|
||||
|
||||
// Provenance fallback for chunks without an explicit `model`: resolve the
|
||||
// gateway's runtime model, not the compile-time DEFAULT_EMBEDDING_MODEL.
|
||||
// See postgres-engine.ts _upsertChunksOnce for the full rationale — pglite
|
||||
// mirrors it for parity.
|
||||
let resolvedModel: string = DEFAULT_EMBEDDING_MODEL;
|
||||
try {
|
||||
const gw = await import('./ai/gateway.ts');
|
||||
resolvedModel = gw.getEmbeddingModel() || resolvedModel;
|
||||
} catch {
|
||||
// Gateway unconfigured (unit tests / pre-connect): keep the default.
|
||||
}
|
||||
|
||||
for (const chunk of chunks) {
|
||||
const embeddingStr = chunk.embedding
|
||||
? '[' + Array.from(chunk.embedding).join(',') + ']'
|
||||
@@ -2344,7 +2356,7 @@ export class PGLiteEngine implements BrainEngine {
|
||||
if (embeddingImageStr) params.push(embeddingImageStr);
|
||||
params.push(
|
||||
pageId, chunk.chunk_index, chunk.chunk_text, chunk.chunk_source,
|
||||
chunk.model || DEFAULT_EMBEDDING_MODEL, chunk.token_count || null,
|
||||
chunk.model || resolvedModel, chunk.token_count || null,
|
||||
chunk.language || null, chunk.symbol_name || null, chunk.symbol_type || null,
|
||||
chunk.start_line ?? null, chunk.end_line ?? null,
|
||||
parentPath, chunk.doc_comment || null, chunk.symbol_name_qualified || null,
|
||||
|
||||
@@ -2438,6 +2438,23 @@ export class PostgresEngine implements BrainEngine {
|
||||
const params: unknown[] = [];
|
||||
let paramIdx = 1;
|
||||
|
||||
// Provenance fallback for chunks that don't carry an explicit `model`:
|
||||
// resolve the model the gateway ACTUALLY uses at runtime, not the
|
||||
// compile-time DEFAULT_EMBEDDING_MODEL constant. Callers like `embed`
|
||||
// build ChunkInputs without a `model` field (src/commands/embed.ts), so
|
||||
// the old `chunk.model || DEFAULT_EMBEDDING_MODEL` fallback stamped the
|
||||
// hardcoded default (e.g. zeroentropyai:zembed-1) onto rows whose vectors
|
||||
// were produced by a different, config-resolved model — corrupting the
|
||||
// provenance that signature-drift staleness + dim-migration logic trust.
|
||||
// Mirrors the resolve-then-fallback pattern used for schema sizing above.
|
||||
let resolvedModel: string = DEFAULT_EMBEDDING_MODEL;
|
||||
try {
|
||||
const gw = await import('./ai/gateway.ts');
|
||||
resolvedModel = gw.getEmbeddingModel() || resolvedModel;
|
||||
} catch {
|
||||
// Gateway unconfigured (unit tests / pre-connect): keep the default.
|
||||
}
|
||||
|
||||
for (const chunk of chunks) {
|
||||
const embeddingStr = chunk.embedding
|
||||
? '[' + Array.from(chunk.embedding).join(',') + ']'
|
||||
@@ -2467,7 +2484,7 @@ export class PostgresEngine implements BrainEngine {
|
||||
if (embeddingImageStr) params.push(embeddingImageStr);
|
||||
params.push(
|
||||
pageId, chunk.chunk_index, chunk.chunk_text, chunk.chunk_source,
|
||||
chunk.model || DEFAULT_EMBEDDING_MODEL, chunk.token_count || null,
|
||||
chunk.model || resolvedModel, chunk.token_count || null,
|
||||
chunk.language || null, chunk.symbol_name || null, chunk.symbol_type || null,
|
||||
chunk.start_line ?? null, chunk.end_line ?? null,
|
||||
parentPath, chunk.doc_comment || null, chunk.symbol_name_qualified || null,
|
||||
|
||||
@@ -216,6 +216,44 @@ describe('hybridSearch + resolver — unknown column at entry (D11)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('upsertChunks — model provenance uses gateway-resolved model, not compiled default', () => {
|
||||
// Regression (zbrain-rfi): when a caller builds ChunkInputs without an
|
||||
// explicit `model` (as src/commands/embed.ts does), the engine used to
|
||||
// stamp the compile-time DEFAULT_EMBEDDING_MODEL ('zeroentropyai:zembed-1')
|
||||
// onto content_chunks.model — even though the vector was produced by the
|
||||
// config-resolved model. That corrupted provenance the signature-drift +
|
||||
// dim-migration logic trusts. The engine must fall back to the model the
|
||||
// gateway ACTUALLY resolves at write time.
|
||||
test('unspecified chunk.model records the resolved model, not zeroentropyai:zembed-1', async () => {
|
||||
configureGateway({
|
||||
embedding_model: 'openai:text-embedding-3-large',
|
||||
embedding_dimensions: 1536,
|
||||
env: { OPENAI_API_KEY: 'sk-test' },
|
||||
});
|
||||
|
||||
await engine.putPage('docs/provenance-page', {
|
||||
type: 'concept',
|
||||
title: 'Provenance test page',
|
||||
compiled_truth: 'Chunk whose model column must reflect the resolved model.',
|
||||
});
|
||||
// No `model` field on the input — the write-side fallback must fill it.
|
||||
await engine.upsertChunks('docs/provenance-page', [
|
||||
{ chunk_index: 0, chunk_text: 'provenance chunk', chunk_source: 'compiled_truth' },
|
||||
]);
|
||||
|
||||
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 = 'docs/provenance-page'`,
|
||||
);
|
||||
expect(rows.length).toBe(1);
|
||||
expect(rows[0].model).toBe('openai:text-embedding-3-large');
|
||||
expect(rows[0].model).not.toBe('zeroentropyai:zembed-1');
|
||||
|
||||
resetGateway();
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildVectorCastFragment — engine SQL composer (D3)', () => {
|
||||
test('vector descriptor emits $1::vector', () => {
|
||||
const r: ResolvedColumn = {
|
||||
|
||||
@@ -30,11 +30,15 @@ import { configureGateway, resetGateway, __setEmbedTransportForTests } from '../
|
||||
import { withEnv } from './helpers/with-env.ts';
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────
|
||||
// Lane A.7 — Chunk-row INSERT model default tracks defaults.ts constant
|
||||
// (not stale OpenAI literal). Pre-fix `chunk.model || 'text-embedding-3-large'`
|
||||
// in both engines; post-fix `chunk.model || DEFAULT_EMBEDDING_MODEL`.
|
||||
// Lane A.7 — Chunk-row INSERT model default tracks the gateway-resolved
|
||||
// model (not a stale OpenAI literal, not the compile-time constant).
|
||||
// Pre-fix `chunk.model || 'text-embedding-3-large'` in both engines;
|
||||
// v0.37 fix `chunk.model || DEFAULT_EMBEDDING_MODEL`; #2846 tightened it
|
||||
// to the gateway's runtime model so provenance matches the vector's
|
||||
// actual producer (falls back to DEFAULT_EMBEDDING_MODEL only when the
|
||||
// gateway is unconfigured).
|
||||
// ─────────────────────────────────────────────────────────────────────
|
||||
describe('Lane A.7 — chunk-row INSERT default tracks ai/defaults.ts constant', () => {
|
||||
describe('Lane A.7 — chunk-row INSERT default tracks the gateway-resolved model', () => {
|
||||
let engine: PGLiteEngine;
|
||||
|
||||
beforeAll(async () => {
|
||||
@@ -47,8 +51,11 @@ describe('Lane A.7 — chunk-row INSERT default tracks ai/defaults.ts constant',
|
||||
await engine.disconnect();
|
||||
});
|
||||
|
||||
test('upsertChunks without explicit model: row stores DEFAULT_EMBEDDING_MODEL', async () => {
|
||||
const { DEFAULT_EMBEDDING_MODEL } = await import('../src/core/ai/defaults.ts');
|
||||
test('upsertChunks without explicit model: row stores the gateway-resolved model', async () => {
|
||||
// The test preload (test/helpers/legacy-embedding-preload.ts) pins the
|
||||
// gateway to 'openai:text-embedding-3-large', so that's what the write
|
||||
// site must stamp — NOT the compile-time DEFAULT_EMBEDDING_MODEL (#2846).
|
||||
const { getEmbeddingModel } = await import('../src/core/ai/gateway.ts');
|
||||
await engine.putPage('test/a7', { type: 'note', title: 'A.7', compiled_truth: 'hello' });
|
||||
await engine.upsertChunks('test/a7', [
|
||||
{ chunk_index: 0, chunk_text: 'hello', chunk_source: 'compiled_truth' },
|
||||
@@ -57,9 +64,9 @@ describe('Lane A.7 — chunk-row INSERT default tracks ai/defaults.ts constant',
|
||||
const rows = await engine.executeRaw<{ model: string }>(
|
||||
`SELECT model FROM content_chunks WHERE chunk_index = 0 LIMIT 1`,
|
||||
);
|
||||
expect(rows[0]?.model).toBe(DEFAULT_EMBEDDING_MODEL);
|
||||
expect(rows[0]?.model).toBe(getEmbeddingModel());
|
||||
// CDX2-4 regression: would have been 'text-embedding-3-large'
|
||||
// (a literal pre-fix; production write site that was never tested).
|
||||
// (a bare literal pre-fix; provider-prefixed form is required).
|
||||
expect(rows[0]?.model).not.toBe('text-embedding-3-large');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user