diff --git a/src/commands/embed.ts b/src/commands/embed.ts index 7b3c227aa..5760155f4 100644 --- a/src/commands/embed.ts +++ b/src/commands/embed.ts @@ -19,6 +19,26 @@ import { } from '../core/pace-mode.ts'; import { tryAcquireDbLock, type DbLockHandle } from '../core/db-lock.ts'; import { embedBackfillLockId } from '../core/embed-backfill-lock.ts'; +import { wrapChunkTextsForStoredMode } from '../core/embedding-context.ts'; +import { titleTierCorpusGeneration } from '../core/contextual-retrieval-service.ts'; +import type { Page } from '../core/types.ts'; + +/** + * #3507 — after a plain re-embed fully re-embedded a `per_chunk_synopsis` + * page at the title-only tier (see wrapChunkTextsForStoredMode), restamp the + * page's CR state to 'title' so `contextual_retrieval_mode` keeps describing + * the vectors actually in the column. The reindex sweep restores the synopsis + * tier later. No-op for every other mode. + */ +export async function restampIfDemotedToTitleTier( + engine: BrainEngine, + page: Pick | null | undefined, + slug: string, + sourceId: string, +): Promise { + if (page?.contextual_retrieval_mode !== 'per_chunk_synopsis') return; + await engine.updatePageContextualRetrievalState(slug, sourceId, 'title', titleTierCorpusGeneration()); +} export interface EmbedOpts { /** Embed ALL pages (every chunk). */ @@ -599,7 +619,11 @@ async function embedPage( return; } - const embeddings = await embedBatch(toEmbed.map(c => c.chunk_text), { abortSignal: signal }); + // #3507: embed with the page's STORED wrapping convention (title-tier + // contextual prefix when the page was embedded wrapped), not raw + // chunk_text — otherwise a re-embed silently strips the contextual + // prefixes the sync path applied. fenced_code chunks stay unwrapped. + const embeddings = await embedBatch(wrapChunkTextsForStoredMode(page, toEmbed), { abortSignal: signal }); const embeddingMap = new Map(); for (let j = 0; j < toEmbed.length; j++) { embeddingMap.set(toEmbed[j].chunk_index, embeddings[j]); @@ -622,6 +646,9 @@ async function embedPage( // such a page and then stamps it. if (toEmbed.length === chunks.length) { await engine.setPageEmbeddingSignature(slug, { sourceId, signature: currentEmbeddingSignature() }); + // #3507: a fully re-embedded per_chunk_synopsis page landed at the + // title tier — keep the stamped mode honest. + await restampIfDemotedToTitleTier(engine, page, slug, page.source_id); } result.embedded += toEmbed.length; result.pages_processed++; @@ -763,7 +790,8 @@ async function embedAll( } try { - const embeddings = await embedBatch(toEmbed.map(c => c.chunk_text)); + // #3507: reproduce the page's stored wrapping convention (see embedPage). + const embeddings = await embedBatch(wrapChunkTextsForStoredMode(page, toEmbed)); // Build a map of new embeddings by chunk_index const embeddingMap = new Map(); for (let j = 0; j < toEmbed.length; j++) { @@ -785,6 +813,11 @@ async function embedAll( await observed(pacer, () => engine.setPageEmbeddingSignature(page.slug, { sourceId: pageSourceId, signature }), ); + // #3507: --all fully re-embeds; a per_chunk_synopsis page landed at + // the title tier — keep the stamped mode honest. + await observed(pacer, () => + restampIfDemotedToTitleTier(engine, page, page.slug, pageSourceId), + ); result.embedded += toEmbed.length; } catch (e: unknown) { serr(`\n Error embedding ${page.slug}: ${e instanceof Error ? e.message : e}`); @@ -1098,7 +1131,13 @@ async function embedAllStale( const keySourceId = stale[0]?.source_id ?? 'default'; const slug = stale[0].slug; try { - const embeddings = await embedBatchWithBackoff(stale.map(c => c.chunk_text), { abortSignal: effectiveSignal }); + // #3507: fetch the page row for its title + stored CR mode so the + // re-embed reproduces the page's wrapping convention instead of + // silently stripping contextual prefixes — `embed --stale` is the + // NORMAL post-model-migration path, so raw-text embedding here + // quietly converted whole corpora to the unwrapped convention. + const pageRow = await observed(pacer, () => engine.getPage(slug, { sourceId: keySourceId })); + const embeddings = await embedBatchWithBackoff(wrapChunkTextsForStoredMode(pageRow, stale), { abortSignal: effectiveSignal }); // Re-fetch existing chunks and merge to avoid deleting non-stale chunks. const existing = await observed(pacer, () => engine.getChunks(slug, { sourceId: keySourceId })); const staleIdxToEmbedding = new Map(); @@ -1126,6 +1165,14 @@ async function embedAllStale( engine.setPageEmbeddingSignature(slug, { sourceId: keySourceId, signature }), ); } + // #3507: a FULLY re-embedded per_chunk_synopsis page landed at the + // title tier — keep the stamped mode honest. Partially-stale pages + // stay stamped as-is (mixed provenance; reindex sweeps fix them). + if (stale.length === existing.length) { + await observed(pacer, () => + restampIfDemotedToTitleTier(engine, pageRow, slug, keySourceId), + ); + } result.embedded += stale.length; } catch (e: unknown) { // Budget/abort-fired cancellations are expected on the way out; don't diff --git a/src/core/contextual-retrieval-service.ts b/src/core/contextual-retrieval-service.ts index ed3e19ebb..ab1f34ec1 100644 --- a/src/core/contextual-retrieval-service.ts +++ b/src/core/contextual-retrieval-service.ts @@ -145,6 +145,19 @@ export function computeCorpusGeneration(args: { return h.digest('hex').slice(0, 16); } +/** + * #3507 — the corpus_generation a page lands on when a plain re-embed path + * (`embed --stale` and friends) re-embeds a `per_chunk_synopsis` page at the + * title-only tier (the D14 fallback tier; synopsis re-generation is a paid + * backfill concern). Callers restamp + * `updatePageContextualRetrievalState(slug, sourceId, 'title', titleTierCorpusGeneration())` + * so the stamped mode keeps describing the vectors actually in the column. + * Matches what the inline import path writes for its title-tier pages. + */ +export function titleTierCorpusGeneration(): string { + return computeCorpusGeneration({ crMode: 'title', haikuModel: DEFAULT_HAIKU_MODEL }); +} + /** * Compute source_text_hash for D27 P1-4 cache key composition. The * synopsis cache invalidates correctly when adjacent text changes (page diff --git a/src/core/embed-stale.ts b/src/core/embed-stale.ts index 474a3ea03..94dfae727 100644 --- a/src/core/embed-stale.ts +++ b/src/core/embed-stale.ts @@ -19,7 +19,8 @@ import type { BrainEngine } from './engine.ts'; import type { ChunkInput } from './types.ts'; -import { embedBatchWithBackoff } from '../commands/embed.ts'; +import { embedBatchWithBackoff, restampIfDemotedToTitleTier } from '../commands/embed.ts'; +import { wrapChunkTextsForStoredMode } from './embedding-context.ts'; import { type DbPacer, createNoopPacer, observed } from './db-pacer.ts'; import { AbortError } from './abort-check.ts'; @@ -189,8 +190,15 @@ export async function embedStaleForSource( const keySourceId = stale[0]?.source_id ?? sourceId; const slug = stale[0].slug; try { + // #3507: fetch the page row for its title + stored CR mode so the + // re-embed reproduces the page's wrapping convention instead of + // silently stripping contextual prefixes (mirrors + // src/commands/embed.ts:embedAllStale). + const pageRow = await observed(pacer, () => + engine.getPage(slug, { sourceId: keySourceId }), + ); const embeddings = await embedFn( - stale.map((c) => c.chunk_text), + wrapChunkTextsForStoredMode(pageRow, stale), { abortSignal: signal }, ); const existing = await observed(pacer, () => @@ -233,6 +241,13 @@ export async function embedStaleForSource( engine.setPageEmbeddingSignature(slug, { sourceId: keySourceId, signature }), ); } + // #3507: a FULLY re-embedded per_chunk_synopsis page landed at the + // title tier — keep the stamped mode honest (mixed pages stay as-is). + if (stale.length === existing.length) { + await observed(pacer, () => + restampIfDemotedToTitleTier(engine, pageRow, slug, keySourceId), + ); + } result.embedded += stale.length; result.pagesProcessed += 1; } catch (e: unknown) { diff --git a/src/core/embedding-context.ts b/src/core/embedding-context.ts index 7c3b4b8e0..97c1d08a7 100644 --- a/src/core/embedding-context.ts +++ b/src/core/embedding-context.ts @@ -186,3 +186,41 @@ export function modeRequiresHaiku(mode: CRMode): boolean { export function modeRequiresWrapper(mode: CRMode): boolean { return mode !== 'none'; } + +/** + * #3507 — build the embedding inputs for a re-embed of EXISTING chunk rows, + * reproducing the wrapping convention the page's vectors were originally + * built under (recorded in `pages.contextual_retrieval_mode`). + * + * Used by every plain re-embed path (`embed `, `embed --all`, + * `embed --stale`, the embed-backfill Minion loop). Before this helper those + * paths embedded raw `chunk_text`, so any re-embed — including the NORMAL + * post-model-migration `embed --stale` — silently replaced context-wrapped + * vectors with unwrapped ones, degrading retrieval with no signature change + * to show for it. + * + * Convention rules (embed PRESERVES conventions; changing them is + * sync/reindex's job): + * - mode NULL/undefined/'none' → raw chunk_text (status quo). + * - mode 'title' → title-only prefix (pure string concat). + * - mode 'per_chunk_synopsis' → title-only prefix. Re-generating Haiku + * synopses is a paid backfill concern; title-only is the service's own + * documented fallback tier (D14). Callers that fully re-embed a page + * this way should restamp the page to 'title' so the column stays + * honest (see contextual-retrieval-service.ts:titleTierCorpusGeneration). + * - `fenced_code` chunks are NEVER wrapped (D20-T4), same as sync. + */ +export function wrapChunkTextsForStoredMode( + page: + | { title?: string | null; contextual_retrieval_mode?: CRMode | null } + | null + | undefined, + chunks: ReadonlyArray<{ chunk_text: string; chunk_source?: string | null }>, +): string[] { + const mode = page?.contextual_retrieval_mode; + if (mode == null || !modeRequiresWrapper(mode)) { + return chunks.map((c) => c.chunk_text); + } + const prefix = buildContextualPrefix(page?.title ?? '', null); + return chunks.map((c) => wrapChunkForEmbedding(c.chunk_text, prefix, c.chunk_source)); +} diff --git a/src/core/pglite-engine.ts b/src/core/pglite-engine.ts index ab562db23..df2896102 100644 --- a/src/core/pglite-engine.ts +++ b/src/core/pglite-engine.ts @@ -420,8 +420,10 @@ export class PGLiteEngine implements BrainEngine { let model: string = DEFAULT_EMBEDDING_MODEL; try { const gw = await import('./ai/gateway.ts'); + // Both accessors THROW when the gateway is unconfigured (they never + // return falsy), so the catch below is the only fallback path (#3461). dims = gw.getEmbeddingDimensions(); - model = gw.getEmbeddingModel() || model; + model = gw.getEmbeddingModel(); } catch { /* gateway not configured — use defaults */ } await this.db.exec(getPGLiteSchema(dims, model)); @@ -979,7 +981,8 @@ export class PGLiteEngine implements BrainEngine { const { rows } = await this.db.query( `SELECT id, source_id, slug, type, title, compiled_truth, timeline, frontmatter, content_hash, created_at, updated_at, deleted_at, effective_date, effective_date_source, - source_kind, source_uri, ingested_via, ingested_at + source_kind, source_uri, ingested_via, ingested_at, + contextual_retrieval_mode FROM pages WHERE ${where.join(' AND ')} LIMIT 1`, params ); @@ -2320,15 +2323,26 @@ export class PGLiteEngine implements BrainEngine { // 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; + // #3461: getEmbeddingModel() THROWS when unconfigured (never returns + // falsy) — on the throw path fall back to the brain's own + // `config.embedding_model` row, then the compile-time default as the + // last resort. See postgres-engine.ts _upsertChunksOnce for the full + // rationale — pglite mirrors it for parity. + let resolvedModel: string | null = null; try { const gw = await import('./ai/gateway.ts'); - resolvedModel = gw.getEmbeddingModel() || resolvedModel; + resolvedModel = gw.getEmbeddingModel(); } catch { - // Gateway unconfigured (unit tests / pre-connect): keep the default. + try { + const cfg = await this.db.query( + `SELECT value FROM config WHERE key = 'embedding_model'`, + ); + resolvedModel = ((cfg.rows[0] as { value?: string } | undefined)?.value) ?? null; + } catch { + // config table unreadable — fall through to the compile-time default. + } } + if (!resolvedModel) resolvedModel = DEFAULT_EMBEDDING_MODEL; for (const chunk of chunks) { const embeddingStr = chunk.embedding @@ -2381,6 +2395,9 @@ export class PGLiteEngine implements BrainEngine { // Code-chunk metadata columns follow the same chunk_text-gated CASE pattern as `embedding` // (#769). Re-chunk trusts EXCLUDED outright; pure re-embed COALESCEs so a caller carrying // only embedding-shaped fields doesn't clobber metadata to NULL. + // + // #3461: `model` mirrors the `embedding` CASE branch-for-branch so the label always + // describes whichever vector wins the upsert. See postgres-engine.ts for rationale. await this.db.query( `INSERT INTO content_chunks ${cols} VALUES ${rowParts.join(', ')} ON CONFLICT (page_id, chunk_index) DO UPDATE SET @@ -2394,7 +2411,14 @@ export class PGLiteEngine implements BrainEngine { THEN EXCLUDED.embedding ELSE content_chunks.embedding END, - model = COALESCE(EXCLUDED.model, content_chunks.model), + model = CASE + WHEN EXCLUDED.chunk_text != content_chunks.chunk_text THEN EXCLUDED.model + WHEN content_chunks.embedding IS NULL THEN EXCLUDED.model + WHEN EXCLUDED.embedded_at IS NOT NULL + AND (content_chunks.embedded_at IS NULL OR EXCLUDED.embedded_at > content_chunks.embedded_at) + THEN EXCLUDED.model + ELSE content_chunks.model + END, token_count = EXCLUDED.token_count, embedded_at = CASE WHEN EXCLUDED.chunk_text != content_chunks.chunk_text AND EXCLUDED.embedding IS NULL THEN NULL diff --git a/src/core/postgres-engine.ts b/src/core/postgres-engine.ts index 07608f3a4..cb0244c49 100644 --- a/src/core/postgres-engine.ts +++ b/src/core/postgres-engine.ts @@ -382,8 +382,10 @@ export class PostgresEngine implements BrainEngine { let model: string = DEFAULT_EMBEDDING_MODEL; try { const gw = await import('./ai/gateway.ts'); + // Both accessors THROW when the gateway is unconfigured (they never + // return falsy), so the catch below is the only fallback path (#3461). dims = gw.getEmbeddingDimensions(); - model = gw.getEmbeddingModel() || model; + model = gw.getEmbeddingModel(); } catch { /* gateway not yet configured — use defaults */ } const sqlText = getPostgresSchema(dims, model); @@ -1031,7 +1033,8 @@ export class PostgresEngine implements BrainEngine { const rows = await tx` SELECT id, source_id, slug, type, title, compiled_truth, timeline, frontmatter, content_hash, created_at, updated_at, deleted_at, effective_date, effective_date_source, - source_kind, source_uri, ingested_via, ingested_at + source_kind, source_uri, ingested_via, ingested_at, + contextual_retrieval_mode FROM pages WHERE slug = ${slug} ${sourceCondition} ${deletedCondition} LIMIT 1 @@ -2437,14 +2440,28 @@ export class PostgresEngine implements BrainEngine { // 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; + // + // #3461: getEmbeddingModel() THROWS when the gateway is unconfigured — + // it never returns falsy — so an `||` guard here is dead code and the + // catch path used to stamp the compile-time default onto rows whose + // vectors came from the config-resolved provider. On the throw path we + // now fall back to the brain's own `config.embedding_model` row (kept + // current by init / migrate / retrieval-upgrade), which names the model + // that actually produced this brain's vectors. The compile-time default + // is the LAST resort (fresh brain whose config row doesn't exist yet). + let resolvedModel: string | null = null; try { const gw = await import('./ai/gateway.ts'); - resolvedModel = gw.getEmbeddingModel() || resolvedModel; + resolvedModel = gw.getEmbeddingModel(); } catch { - // Gateway unconfigured (unit tests / pre-connect): keep the default. + try { + const cfg = await sql`SELECT value FROM config WHERE key = 'embedding_model'`; + resolvedModel = (cfg[0]?.value as string | undefined) ?? null; + } catch { + // config table unreadable — fall through to the compile-time default. + } } + if (!resolvedModel) resolvedModel = DEFAULT_EMBEDDING_MODEL; for (const chunk of chunks) { const embeddingStr = chunk.embedding @@ -2508,6 +2525,11 @@ export class PostgresEngine implements BrainEngine { // pure re-embed (chunk_text unchanged) COALESCEs so a caller that only carries embedding // doesn't clobber metadata to NULL. Without this, every embed --stale pass nuked code-def's // primary index for thousands of chunks at once. + // + // #3461: `model` mirrors the `embedding` CASE branch-for-branch — the label must + // describe whichever vector WINS the upsert. The old COALESCE(EXCLUDED.model, …) + // relabeled preserved (older-model) vectors with the current gateway model on every + // partial re-embed, corrupting provenance without changing the vector. await sql.unsafe( `INSERT INTO content_chunks ${cols} VALUES ${rows.join(', ')} ON CONFLICT (page_id, chunk_index) DO UPDATE SET @@ -2521,7 +2543,14 @@ export class PostgresEngine implements BrainEngine { THEN EXCLUDED.embedding ELSE content_chunks.embedding END, - model = COALESCE(EXCLUDED.model, content_chunks.model), + model = CASE + WHEN EXCLUDED.chunk_text != content_chunks.chunk_text THEN EXCLUDED.model + WHEN content_chunks.embedding IS NULL THEN EXCLUDED.model + WHEN EXCLUDED.embedded_at IS NOT NULL + AND (content_chunks.embedded_at IS NULL OR EXCLUDED.embedded_at > content_chunks.embedded_at) + THEN EXCLUDED.model + ELSE content_chunks.model + END, token_count = EXCLUDED.token_count, embedded_at = CASE WHEN EXCLUDED.chunk_text != content_chunks.chunk_text AND EXCLUDED.embedding IS NULL THEN NULL diff --git a/src/core/utils.ts b/src/core/utils.ts index 5f79d8cf7..09d4eb632 100644 --- a/src/core/utils.ts +++ b/src/core/utils.ts @@ -110,6 +110,12 @@ export function rowToPage(row: Record): Page { const sourceUri = row.source_uri === undefined ? undefined : (row.source_uri as string | null); const ingestedVia = row.ingested_via === undefined ? undefined : (row.ingested_via as string | null); const ingestedAt = readOptionalDate(row.ingested_at); + // #3507: the CR tier the page was last embedded under (three-state, same + // pattern as the provenance columns above). Re-embed paths (`embed --stale` + // and friends) read this to reproduce the page's stored wrapping convention. + const contextualRetrievalMode = row.contextual_retrieval_mode === undefined + ? undefined + : (row.contextual_retrieval_mode as Page['contextual_retrieval_mode']); return { id: row.id as number, slug: row.slug as string, @@ -135,6 +141,7 @@ export function rowToPage(row: Record): Page { ...(sourceUri !== undefined && { source_uri: sourceUri }), ...(ingestedVia !== undefined && { ingested_via: ingestedVia }), ...(ingestedAt !== undefined && { ingested_at: ingestedAt }), + ...(contextualRetrievalMode !== undefined && { contextual_retrieval_mode: contextualRetrievalMode }), // v0.31.12: propagate source_id so downstream callers (embed, reconcile-links) // can thread it through getChunks / upsertChunks without defaulting to 'default'. // v0.32.8: Page.source_id is required. Every SELECT feeding rowToPage now diff --git a/test/e2e/embedding-column-pglite.test.ts b/test/e2e/embedding-column-pglite.test.ts index 7254806cf..19a0f8cb1 100644 --- a/test/e2e/embedding-column-pglite.test.ts +++ b/test/e2e/embedding-column-pglite.test.ts @@ -252,6 +252,87 @@ describe('upsertChunks — model provenance uses gateway-resolved model, not com resetGateway(); }); + + // #3461: getEmbeddingModel() THROWS when the gateway is unconfigured — it + // never returns falsy — so the reland's `|| resolvedModel` guard was dead + // code and the catch path still stamped the compile-time default onto rows + // whose vectors came from the config-resolved provider. The engine must + // fall back to the brain's own `config.embedding_model` row instead. + test('#3461: unconfigured gateway falls back to the brain config model, never the compiled default', async () => { + await engine.setConfig('embedding_model', 'voyage:voyage-3-large'); + // The preload's beforeEach re-configures the gateway before every test, + // so the reset must happen INSIDE the test body. + resetGateway(); + + await engine.putPage('docs/provenance-throw-path', { + type: 'concept', + title: 'Provenance throw-path page', + compiled_truth: 'Chunk written while the gateway is unconfigured.', + }); + await engine.upsertChunks('docs/provenance-throw-path', [ + { chunk_index: 0, chunk_text: 'throw-path 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-throw-path'`, + ); + expect(rows.length).toBe(1); + expect(rows[0].model).toBe('voyage:voyage-3-large'); + + // Restore the value initSchema wrote for the rest of the file. + await engine.setConfig('embedding_model', 'openai:text-embedding-3-large'); + }); + + // #3461 sibling: on a partial re-upsert that carries NO new embedding (the + // exact shape `embed --stale` produces for a page's non-stale chunks), the + // preserved vector must KEEP its original model label. The old + // COALESCE(EXCLUDED.model, …) relabeled it with the current gateway model. + test('#3461: preserved vector keeps its original model label on a no-embedding re-upsert', async () => { + configureGateway({ + embedding_model: 'openai:text-embedding-3-large', + embedding_dimensions: 1536, + env: { OPENAI_API_KEY: 'sk-test' }, + }); + + await engine.putPage('docs/provenance-preserve', { + type: 'concept', + title: 'Provenance preserve page', + compiled_truth: 'Chunk embedded under model A, re-upserted under model B.', + }); + await engine.upsertChunks('docs/provenance-preserve', [ + { + chunk_index: 0, + chunk_text: 'stable chunk text', + chunk_source: 'compiled_truth', + embedding: new Float32Array(VEC1536_A), + }, + ]); + + // Model swap: the gateway now resolves a different model, and the + // re-upsert (same chunk_text) carries no new embedding. + configureGateway({ + embedding_model: 'voyage:voyage-3-large', + embedding_dimensions: 1536, + env: { VOYAGE_API_KEY: 'test' }, + }); + await engine.upsertChunks('docs/provenance-preserve', [ + { chunk_index: 0, chunk_text: 'stable chunk text', chunk_source: 'compiled_truth' }, + ]); + + const rows = await engine.executeRaw<{ model: string; has_embedding: boolean }>( + `SELECT cc.model, cc.embedding IS NOT NULL AS has_embedding + FROM content_chunks cc + JOIN pages p ON p.id = cc.page_id + WHERE p.slug = 'docs/provenance-preserve'`, + ); + expect(rows.length).toBe(1); + expect(rows[0].has_embedding).toBe(true); // vector preserved… + expect(rows[0].model).toBe('openai:text-embedding-3-large'); // …and its label still describes it + + resetGateway(); + }); }); describe('buildVectorCastFragment — engine SQL composer (D3)', () => { diff --git a/test/embed-stale.serial.test.ts b/test/embed-stale.serial.test.ts index 9c3a88221..8ef583159 100644 --- a/test/embed-stale.serial.test.ts +++ b/test/embed-stale.serial.test.ts @@ -277,3 +277,79 @@ describe('embedStaleForSource', () => { expect(txtRow.embedded_at).not.toBeNull(); }); }); + +// ──────────────────────────────────────────────────────────────── +// #3507 — re-embed must reproduce the page's STORED contextual-retrieval +// wrapping convention. Before the fix, every plain re-embed (including the +// normal post-model-migration `embed --stale`) embedded raw chunk_text, +// silently replacing context-wrapped vectors with unwrapped ones. +// ──────────────────────────────────────────────────────────────── + +describe('contextual-retrieval wrapping on re-embed (#3507)', () => { + /** embedFn that records every text it is asked to embed. */ + function capturingEmbedFn(seen: string[]) { + return (texts: string[]): Promise => { + seen.push(...texts); + return fakeEmbedFn(texts); + }; + } + + async function seedWrappablePage(slug: string, title: string): Promise { + await engine.putPage(slug, { type: 'note', title, compiled_truth: 'seeded' }); + await engine.upsertChunks(slug, [ + { chunk_index: 0, chunk_text: 'prose chunk about widgets', chunk_source: 'compiled_truth', token_count: 4 }, + { chunk_index: 1, chunk_text: 'const x = 1;', chunk_source: 'fenced_code', token_count: 4 }, + ]); + } + + test('title-mode page: stale re-embed sends title-wrapped texts; fenced_code stays raw', async () => { + await seedWrappablePage('wrapped-page', 'Widget Notes'); + await engine.updatePageContextualRetrievalState('wrapped-page', 'default', 'title', 'gen-title'); + + const seen: string[] = []; + const result = await embedStaleForSource(engine, 'default', { embedFn: capturingEmbedFn(seen) }); + expect(result.embedded).toBe(2); + + expect(seen).toContain('Widget Notes\n\nprose chunk about widgets'); + expect(seen).toContain('const x = 1;'); // fenced_code is NEVER wrapped (D20-T4) + + // D20-T1: the canonical chunk_text is NOT rewritten — wrapping is embed-input-only. + const chunks = await engine.getChunks('wrapped-page'); + expect(chunks.map((c) => c.chunk_text).sort()).toEqual(['const x = 1;', 'prose chunk about widgets']); + // Mode stamp unchanged for title-tier pages. + const rows = await engine.executeRaw<{ contextual_retrieval_mode: string }>( + `SELECT contextual_retrieval_mode FROM pages WHERE slug = 'wrapped-page'`, + ); + expect(rows[0].contextual_retrieval_mode).toBe('title'); + }); + + test('per_chunk_synopsis page: re-embed applies the title-tier wrapper and restamps honestly', async () => { + await seedWrappablePage('synopsis-page', 'Synopsis Notes'); + await engine.updatePageContextualRetrievalState('synopsis-page', 'default', 'per_chunk_synopsis', 'gen-synopsis'); + + const seen: string[] = []; + const result = await embedStaleForSource(engine, 'default', { embedFn: capturingEmbedFn(seen) }); + expect(result.embedded).toBe(2); + + // Synopsis re-generation is a paid backfill concern; the plain re-embed + // lands at the title tier (the service's own D14 fallback tier)… + expect(seen).toContain('Synopsis Notes\n\nprose chunk about widgets'); + // …and the stamped mode is updated so it keeps describing the vectors. + const rows = await engine.executeRaw<{ contextual_retrieval_mode: string }>( + `SELECT contextual_retrieval_mode FROM pages WHERE slug = 'synopsis-page'`, + ); + expect(rows[0].contextual_retrieval_mode).toBe('title'); + }); + + test('unstamped page (NULL mode) embeds raw chunk_text — convention preserved', async () => { + await seedWrappablePage('plain-page', 'Plain Notes'); + // No updatePageContextualRetrievalState call: pre-CR page. + + const seen: string[] = []; + const result = await embedStaleForSource(engine, 'default', { embedFn: capturingEmbedFn(seen) }); + expect(result.embedded).toBe(2); + + expect(seen).toContain('prose chunk about widgets'); + expect(seen.some((t) => t.startsWith(''))).toBe(false); + }); +}); diff --git a/test/embed.serial.test.ts b/test/embed.serial.test.ts index a0bf89bea..b27700a8e 100644 --- a/test/embed.serial.test.ts +++ b/test/embed.serial.test.ts @@ -907,3 +907,76 @@ describe('runEmbed preserves code-chunk metadata across re-embed (regression for expect(metadataOf(upsertChunkArgs![0])).toEqual(metadataOf(fullCodeChunk)); }); }); + +// ──────────────────────────────────────────────────────────────── +// #3507 — `embed --stale` must reproduce the page's STORED +// contextual-retrieval wrapping convention instead of embedding raw +// chunk_text (which silently stripped contextual prefixes on every +// re-embed, including the normal post-model-migration path). +// ──────────────────────────────────────────────────────────────── + +describe('embed --stale contextual-retrieval wrapping (#3507)', () => { + const wrapChunks = [ + { chunk_index: 0, chunk_text: 'prose chunk', chunk_source: 'compiled_truth', embedded_at: null, token_count: 1 }, + { chunk_index: 1, chunk_text: 'const x = 1;', chunk_source: 'fenced_code', embedded_at: null, token_count: 1 }, + ]; + const wrapStale = [ + { slug: 'wrapped', chunk_index: 0, chunk_text: 'prose chunk', chunk_source: 'compiled_truth' as const, model: null, token_count: 1, source_id: 'default', page_id: 1 }, + { slug: 'wrapped', chunk_index: 1, chunk_text: 'const x = 1;', chunk_source: 'fenced_code' as any, model: null, token_count: 1, source_id: 'default', page_id: 1 }, + ]; + + function wrappingHarness(mode: string | null) { + const seen: string[] = []; + const restamps: any[][] = []; + embedBatchBehavior = async (texts: string[]) => { + seen.push(...texts); + return texts.map(() => new Float32Array(1536)); + }; + const engine = mockEngine({ + countStaleChunks: async () => 2, + listStaleChunks: async () => wrapStale, + getPage: async () => ({ + slug: 'wrapped', + title: 'Widget Notes', + source_id: 'default', + compiled_truth: 'x', + timeline: '', + contextual_retrieval_mode: mode, + }), + getChunks: async () => wrapChunks, + upsertChunks: async () => {}, + updatePageContextualRetrievalState: async (...args: any[]) => { restamps.push(args); }, + }); + return { engine, seen, restamps }; + } + + test('title-mode page: stale re-embed wraps prose with the title prefix; fenced_code stays raw', async () => { + const { engine, seen, restamps } = wrappingHarness('title'); + const result = await runEmbedCore(engine, { stale: true }); + expect(result.embedded).toBe(2); + expect(seen).toContain('Widget Notes\n\nprose chunk'); + expect(seen).toContain('const x = 1;'); + expect(restamps).toHaveLength(0); // title tier: stamp already honest + }); + + test('per_chunk_synopsis page: fully re-embedded page restamps to the title tier', async () => { + const { engine, seen, restamps } = wrappingHarness('per_chunk_synopsis'); + const result = await runEmbedCore(engine, { stale: true }); + expect(result.embedded).toBe(2); + expect(seen).toContain('Widget Notes\n\nprose chunk'); + expect(restamps).toHaveLength(1); + const [slug, sourceId, newMode] = restamps[0]; + expect(slug).toBe('wrapped'); + expect(sourceId).toBe('default'); + expect(newMode).toBe('title'); + }); + + test('page with no stored CR mode embeds raw chunk_text (convention preserved)', async () => { + const { engine, seen, restamps } = wrappingHarness(null); + const result = await runEmbedCore(engine, { stale: true }); + expect(result.embedded).toBe(2); + expect(seen).toContain('prose chunk'); + expect(seen.some((t) => t.startsWith(''))).toBe(false); + expect(restamps).toHaveLength(0); + }); +});