Compare commits

..
Author SHA1 Message Date
Garry TanandClaude Fable 5 89579780e0 fix(embed): extend #1717 model labeling to the embed-backfill stale path
src/core/embed-stale.ts (used by the embed-backfill minion handler) built
its merged ChunkInput without a model field, so every chunk on a touched
page — re-embedded AND preserved — was relabeled to the engine default on
each backfill pass. Mirror the embed.ts semantics: stamp the resolved
gateway label on re-embedded chunks, carry the existing label on untouched
ones. Pinned by a PGLite test that fails without the fix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-22 11:06:16 -07:00
cf2deedfc6 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>
2026-07-21 14:42:17 -07:00
11 changed files with 152 additions and 106 deletions
+14 -1
View File
@@ -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 }));
+5 -15
View File
@@ -43,7 +43,7 @@ import {
} from '../core/link-extraction.ts';
import { createProgress } from '../core/progress.ts';
import { getCliOptions, cliOptsToProgressOptions } from '../core/cli-options.ts';
import { pathToSlug, slugifyPath, pruneDir, isSyncable } from '../core/sync.ts';
import { pathToSlug, pruneDir, isSyncable } from '../core/sync.ts';
// v0.41.18.0: withRetry + isRetryableConnError + WithRetryOpts moved to
// src/core/retry.ts as the canonical primitive. Engine methods
// (addLinksBatch/addTimelineEntriesBatch/upsertChunks) now self-retry via
@@ -269,24 +269,14 @@ export function extractMarkdownLinks(content: string): { name: string; relTarget
export function resolveSlug(fileDir: string, relTarget: string, allSlugs: Set<string>): string | null {
const targetNoExt = relTarget.endsWith('.md') ? relTarget.slice(0, -3) : relTarget;
// Issue #1964: wikilinks carry raw Obsidian paths (`[[llm-wiki/entities/AI 3.0]]`)
// but allSlugs holds sync-slugified slugs (`llm-wiki/entities/ai-3.0`). Try the
// raw candidate first (back-compat), then the sync-consistent slugified form.
const hit = (candidate: string): string | null => {
if (allSlugs.has(candidate)) return candidate;
const slugified = slugifyPath(candidate);
if (slugified !== candidate && allSlugs.has(slugified)) return slugified;
return null;
};
const s1 = hit(join(fileDir, targetNoExt));
if (s1) return s1;
const s1 = join(fileDir, targetNoExt);
if (allSlugs.has(s1)) return s1;
const parts = fileDir.split('/').filter(Boolean);
for (let strip = 1; strip <= parts.length; strip++) {
const ancestor = parts.slice(0, parts.length - strip).join('/');
const candidate = hit(ancestor ? join(ancestor, targetNoExt) : targetNoExt);
if (candidate) return candidate;
const candidate = ancestor ? join(ancestor, targetNoExt) : targetNoExt;
if (allSlugs.has(candidate)) return candidate;
}
return null;
+7
View File
@@ -20,6 +20,7 @@
import type { BrainEngine } from './engine.ts';
import type { ChunkInput } from './types.ts';
import { embedBatchWithBackoff } from '../commands/embed.ts';
import { resolveEmbeddingModelLabel } from './embedding.ts';
import { type DbPacer, createNoopPacer, observed } from './db-pacer.ts';
import { AbortError } from './abort-check.ts';
@@ -200,11 +201,17 @@ export async function embedStaleForSource(
for (let j = 0; j < stale.length; j++) {
staleIdxToEmbedding.set(stale[j].chunk_index, embeddings[j]);
}
// #1717: label re-embedded chunks with the model that produced the
// vector; preserved chunks keep their existing model. Without this,
// upsertChunks falls back to DEFAULT_EMBEDDING_MODEL for every chunk
// (the same mislabel the embed.ts paths fixed).
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),
// Carry through per-chunk metadata. upsertChunks writes these as
// EXCLUDED.<col> (not COALESCE), so omitting them here resets image
+15
View File
@@ -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
View File
@@ -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) {
+4 -20
View File
@@ -14,7 +14,6 @@
import type { BrainEngine } from './engine.ts';
import type { PageType } from './types.ts';
import { ensureWellFormed } from './text-safe.ts';
import { slugifyPath } from './sync.ts';
/**
* v0.42.7 — link-extraction version stamp. Bump this ISO timestamp whenever the
@@ -483,29 +482,14 @@ export async function extractPageLinks(
// pre-v0.40.8.2 behavior of dropping bare wikilinks outside
// DIR_PATTERN.
if (ref.needsResolution) {
if (typeof resolver.resolveBasenameMatches !== 'function') continue;
if (!opts.globalBasename || typeof resolver.resolveBasenameMatches !== 'function') {
continue;
}
// Issue #972 (codex): resolve by the wikilink TARGET (ref.slug — the
// text inside `[[...]]` before any `|`), NOT the display alias
// (ref.name = match[2]). `[[struktura|the project]]` must resolve
// `struktura`, not "the project". The display text is for context only.
//
// Issue #1964: a dir-qualified wikilink (`[[llm-wiki/entities/AI 3.0]]`)
// carries a raw Obsidian path while page slugs are sync-slugified
// (`llm-wiki/entities/ai-3.0`). Slugify the path the same way sync does,
// then match by exact slug or path-suffix (wiki-root-relative authoring).
// This runs regardless of global_basename — it's dir-qualified, so the
// cross-dir false-positive risk the flag guards against doesn't apply.
// Mirrors the FS path's resolveSlug ancestor walk. Bare `[[name]]`
// wikilinks still require the global_basename flag.
let matches: string[] = [];
const slugified = ref.slug.includes('/') ? slugifyPath(ref.slug) : '';
if (slugified.includes('/')) {
const tail = slugified.slice(slugified.lastIndexOf('/') + 1);
matches = (await resolver.resolveBasenameMatches(tail))
.filter(m => m === slugified || m.endsWith(`/${slugified}`));
} else if (opts.globalBasename) {
matches = await resolver.resolveBasenameMatches(ref.slug);
}
const matches = await resolver.resolveBasenameMatches(ref.slug);
if (matches.length === 0) continue;
const idx = content.indexOf(ref.slug);
const context = idx >= 0 ? excerpt(content, idx, 240) : ref.name;
+46
View File
@@ -15,6 +15,7 @@ import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:tes
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
import { resetPgliteState } from './helpers/reset-pglite.ts';
import { embedStaleForSource } from '../src/core/embed-stale.ts';
import { configureGateway, resetGateway } from '../src/core/ai/gateway.ts';
import type { ChunkInput } from '../src/core/types.ts';
let engine: PGLiteEngine;
@@ -276,4 +277,49 @@ describe('embedStaleForSource', () => {
// The stale text row actually got its embedding.
expect(txtRow.embedded_at).not.toBeNull();
});
// #1717: the backfill path must label re-embedded chunks with the model
// that produced the vector, and preserve the existing label on chunks it
// did not touch (before the fix, both were reset to the engine default).
test('labels re-embedded chunks with the gateway model, preserves untouched labels (#1717)', async () => {
configureGateway({
embedding_model: 'openai:text-embedding-3-large',
env: { OPENAI_API_KEY: 'sk-test-embed-stale-1717' },
});
try {
await engine.putPage('notes/model-label', {
type: 'note',
title: 'model-label',
compiled_truth: '# model-label\n\nseeded',
});
await engine.upsertChunks('notes/model-label', [
{
chunk_index: 0,
chunk_text: 'already embedded elsewhere',
chunk_source: 'compiled_truth',
embedding: new Float32Array(1536).fill(0.01),
model: 'voyage:voyage-3',
token_count: 4,
},
{
chunk_index: 1,
chunk_text: 'stale chunk needing embed',
chunk_source: 'compiled_truth',
token_count: 5,
embedding: undefined, // stale
},
]);
const result = await embedStaleForSource(engine, 'default', { embedFn: fakeEmbedFn });
expect(result.embedded).toBe(1);
const after = await engine.getChunks('notes/model-label');
const preserved = after.find((c) => c.chunk_index === 0)!;
const reembedded = after.find((c) => c.chunk_index === 1)!;
expect(reembedded.model).toBe('openai:text-embedding-3-large');
expect(preserved.model).toBe('voyage:voyage-3');
} finally {
resetGateway();
}
});
});
+33
View File
@@ -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');
});
});
-27
View File
@@ -389,33 +389,6 @@ describe('resolveSlugAll', () => {
});
});
// ─── issue #1964: cross-directory wikilinks — slug/path mismatch ──────────
describe('issue #1964: raw Obsidian wikilink paths resolve to sync-slugified slugs', () => {
test('resolveSlug slugifies the candidate (sync-consistent), no flag needed', () => {
const all = new Set(['llm-wiki/entities/ai-3.0']);
// Wikilink literal `[[llm-wiki/entities/AI 3.0]]` — spaces + uppercase.
expect(resolveSlug('llm-wiki/notes', 'llm-wiki/entities/AI 3.0.md', all))
.toBe('llm-wiki/entities/ai-3.0');
});
test('resolveSlug slugifies raw (unslugified) fileDir too', () => {
const all = new Set(['llm-wiki/entities/ai-3.0']);
// fileDir comes from dirname(relPath) — the raw on-disk directory.
expect(resolveSlug('LLM Wiki/Notes', 'entities/AI 3.0.md', all))
.toBe('llm-wiki/entities/ai-3.0');
});
test('extractLinksFromFile resolves cross-directory wikilink with flag OFF as a typed edge', async () => {
const allSlugs = new Set(['llm-wiki/entities/ai-3.0', 'llm-wiki/notes/roadmap']);
const content = '---\ntitle: Roadmap\ntype: concept\n---\n\nSee [[llm-wiki/entities/AI 3.0]].\n';
const links = await extractLinksFromFile(content, 'llm-wiki/notes/roadmap.md', allSlugs);
expect(links.map(l => l.to_slug)).toEqual(['llm-wiki/entities/ai-3.0']);
// Dir-qualified path resolution is exact, NOT the basename fallback.
expect(links[0].link_type).not.toBe('wikilink_basename');
});
});
describe('issue #972 repro: bare wikilinks resolve when flag is on', () => {
// End-to-end: reproduces the issue's exact repro inside a tempdir +
// PGLite, then asserts edge count under both flag states.
@@ -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');
});
});
-42
View File
@@ -9,8 +9,6 @@ import {
parseTimelineEntries,
isAutoLinkEnabled,
FRONTMATTER_LINK_MAP,
buildBasenameIndex,
queryBasenameIndex,
type SlugResolver,
} from '../src/core/link-extraction.ts';
import type { BrainEngine } from '../src/core/engine.ts';
@@ -426,46 +424,6 @@ describe('extractPageLinks', () => {
expect(strk!.linkType).toBe('wikilink_basename');
});
// ─── issue #1964: dir-qualified wikilinks with raw Obsidian paths ────────
test('#1964: dir-qualified wikilink resolves via sync-consistent slugification (flag OFF)', async () => {
// `[[llm-wiki/entities/AI 3.0]]` is a raw Obsidian path; the page slug
// is the sync-slugified `llm-wiki/entities/ai-3.0`. Must resolve WITHOUT
// global_basename (it's dir-qualified) and must NOT leak to a same-tail
// page in a different directory.
const resolver: SlugResolver = {
resolve: async () => null,
resolveBasenameMatches: async (name) =>
name === 'ai-3.0' ? ['other/ai-3.0', 'llm-wiki/entities/ai-3.0'] : [],
};
const { candidates } = await extractPageLinks(
'llm-wiki/notes/roadmap',
'See [[llm-wiki/entities/AI 3.0]] for the model.',
{}, 'concept', resolver,
// opts.globalBasename omitted (= false) — path is dir-qualified
);
expect(candidates.map(c => c.targetSlug)).toEqual(['llm-wiki/entities/ai-3.0']);
expect(candidates[0].linkType).toBe('wikilink_basename');
expect(candidates[0].linkSource).toBe('wikilink-resolved');
});
test('#1964: path-suffix match resolves wiki-root-relative paths against a real index', async () => {
// Author writes `[[llm-wiki/entities/AI 3.0]]` but the brain nests the
// wiki under a vault dir. Suffix match rescues it; queried through the
// REAL basename index so the tail-key lookup is exercised end to end.
const idx = buildBasenameIndex(['vault/llm-wiki/entities/ai-3.0', 'people/ai-3.0']);
const resolver: SlugResolver = {
resolve: async () => null,
resolveBasenameMatches: async (name) => queryBasenameIndex(idx, name),
};
const { candidates } = await extractPageLinks(
'vault/llm-wiki/notes/roadmap',
'See [[llm-wiki/entities/AI 3.0]].',
{}, 'concept', resolver,
);
expect(candidates.map(c => c.targetSlug)).toEqual(['vault/llm-wiki/entities/ai-3.0']);
});
test('opts.skipFrontmatter suppresses the frontmatter pass', async () => {
// Real resolver shape that WOULD resolve frontmatter source: too,
// but skipFrontmatter blocks the path entirely.