From 0a757bf7809f67fbd551a6977cb53e821dcd7ec1 Mon Sep 17 00:00:00 2001 From: 0xTim Date: Wed, 22 Jul 2026 16:50:52 -0700 Subject: [PATCH] fix(entities): thread sourceId through findByTitleFuzzy + skip soft-deleted (#1508) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `findByTitleFuzzy` on both `postgres-engine.ts` and `pglite-engine.ts` has no `source_id` filter and no `deleted_at IS NULL` filter. `tryFuzzyMatch` in `src/core/entities/resolve.ts` got both of those filters via #1436 (v0.41.13.0) for exactly the reasons that apply to its sibling here: fuzzy resolution can suggest cross-source slug candidates that the caller then silently drops at the FK filter (or worse, picks a soft-deleted page). This is the missing twin of #1436. In multi-source brains, the live-mode auto-link resolver invoked from `put_page` (`operations.ts:937`) calls `engine.findByTitleFuzzy` with no scope. When two sources contain pages with similar titles (`people/alice-example` on `source-a`, `people/alice-other` on `source-b`), the fuzzy lookup can return the wrong-source slug, which then fails the downstream `allSlugs` / `addLink` FK filter — the link silently doesn't get created, and from the caller's view the resolver "failed" even though the page existed under the right source. Reproducible with a 2-source PGLite setup + identical-title pages on both sides; the fuzzy call returns a slug whose `source_id` doesn't match the put_page caller's source. - 2-source brains: auto-links between same-title-different-source pages now resolve under the caller's source instead of the wrong neighbor. - Soft-deleted pages can no longer be returned as fuzzy candidates (mirroring the resolve.ts fix from #1436). - 1-source brains: no behavior change. `sourceId` is optional; when omitted the SQL takes the pre-existing unscoped path. - `engine.ts`: add optional 4th `sourceId` param to the `findByTitleFuzzy` interface + JSDoc explaining the scope semantics. - `postgres-engine.ts` / `pglite-engine.ts`: implement the param via a conditional SQL branch that adds `AND source_id = $N AND deleted_at IS NULL` when `sourceId` is set; existing query path unchanged when omitted. - `link-extraction.ts`: add optional `sourceId` to `makeResolver` opts, forward to `findByTitleFuzzy` in step 3 of the resolve chain. - `operations.ts`: pass `opts?.sourceId` to `makeResolver` from the live-mode put_page resolver (the place that already knows the caller's source). - New unit tests in `test/link-extraction.test.ts` (2 cases): - `opts.sourceId` is forwarded to `findByTitleFuzzy` when set. - `opts.sourceId` omitted → `findByTitleFuzzy` receives `undefined` (back-compat). - `bun run typecheck` clean. - `bun test test/link-extraction.test.ts test/entity-resolve.test.ts test/operations.test.ts test/extract.test.ts` — 145/145 pass. Co-authored-by: Time Attakc <89218912+time-attack@users.noreply.github.com> --- src/core/engine.ts | 10 ++++++++++ src/core/link-extraction.ts | 8 ++++++-- src/core/operations.ts | 6 +++++- src/core/pglite-engine.ts | 37 +++++++++++++++++++++++++++--------- src/core/postgres-engine.ts | 35 ++++++++++++++++++++++++++-------- test/link-extraction.test.ts | 37 ++++++++++++++++++++++++++++++++++++ 6 files changed, 113 insertions(+), 20 deletions(-) diff --git a/src/core/engine.ts b/src/core/engine.ts index a389d3091..ef04165b4 100644 --- a/src/core/engine.ts +++ b/src/core/engine.ts @@ -1218,11 +1218,21 @@ export interface BrainEngine { * * Uses the `%` trigram operator (GIN-indexed) + the standard `similarity()` * function. Both engines support pg_trgm (PGLite 0.3+, Postgres always). + * + * `sourceId` constrains the search to a single source and filters out + * soft-deleted pages. Mirrors the same filters `tryFuzzyMatch` in + * `src/core/entities/resolve.ts` got via #1436 (v0.41.13.0). Omit for the + * historical unscoped behavior — live-mode callers that already know + * the source should pass it to avoid cross-source slug suggestions that + * get silently dropped at the FK filter downstream. Batch-mode callers + * (e.g. `gbrain extract`) intentionally omit it to build a cross-source + * resolution map. */ findByTitleFuzzy( name: string, dirPrefix?: string, minSimilarity?: number, + sourceId?: string, ): Promise<{ slug: string; similarity: number } | null>; /** * v0.34.1 (#861 — P0 leak seal): `opts.sourceId` / `opts.sourceIds` diff --git a/src/core/link-extraction.ts b/src/core/link-extraction.ts index 8f27903e6..6ff2f6822 100644 --- a/src/core/link-extraction.ts +++ b/src/core/link-extraction.ts @@ -980,10 +980,14 @@ export function makeResolver( // Step 3: pg_trgm fuzzy title match — both modes. Tries each hint in // order; first hint with a ≥0.55 similarity match wins. If no hints, - // try the whole pages table. + // try the whole pages table. When opts.sourceId is set, the fuzzy + // search is constrained to that source (and skips soft-deleted pages) + // so cross-source slug suggestions don't get silently dropped at the + // FK filter downstream. Mirrors the same scope fix `tryFuzzyMatch` got + // via #1436. const searchHints = hints.length > 0 ? hints : [undefined]; for (const hint of searchHints) { - const match = await engine.findByTitleFuzzy(trimmed, hint, 0.55); + const match = await engine.findByTitleFuzzy(trimmed, hint, 0.55, opts.sourceId); if (match) { cache.set(cacheKey, match.slug); return match.slug; diff --git a/src/core/operations.ts b/src/core/operations.ts index ab2d4cbf1..68f5a56e1 100644 --- a/src/core/operations.ts +++ b/src/core/operations.ts @@ -1153,7 +1153,11 @@ async function runAutoLink( // Live-mode resolver: per-put throwaway cache, pg_trgm + optional search. // Issue #972 (codex [P1]): pass sourceId so basename resolution stays - // within this page's source — no cross-source basename edges. + // within this page's source — no cross-source basename edges. Also scopes + // the fuzzy fallback (findByTitleFuzzy) to the same source the put_page is + // targeting — without it, cross-source slug suggestions get silently dropped + // at the FK filter and the link looks like it failed to resolve. Twin of + // #1436's `tryFuzzyMatch` fix. const resolver = makeResolver(engine, { mode: 'live', sourceId: opts?.sourceId }); // Issue #972: opt-in bare-wikilink basename resolution. Off by default. const globalBasename = await isGlobalBasenameEnabled(engine); diff --git a/src/core/pglite-engine.ts b/src/core/pglite-engine.ts index e11eeb3fb..229322764 100644 --- a/src/core/pglite-engine.ts +++ b/src/core/pglite-engine.ts @@ -2912,22 +2912,41 @@ export class PGLiteEngine implements BrainEngine { name: string, dirPrefix?: string, minSimilarity: number = 0.55, + sourceId?: string, ): Promise<{ slug: string; similarity: number } | null> { // Inline threshold comparison instead of `SET LOCAL pg_trgm.similarity_threshold`. // The GUC only scopes to the current transaction and pglite auto-commits each // .query() call, so the SET LOCAL would be a no-op. Using similarity() >= $N // directly gives predictable behavior. Tie-breaker: sort by slug so re-runs // pick the same winner. + // + // `sourceId` + `deleted_at IS NULL` mirror the filters `tryFuzzyMatch` in + // `src/core/entities/resolve.ts` got via #1436 (v0.41.13.0). Without them, + // fuzzy resolution could suggest cross-source slugs that the caller then + // silently drops at the FK filter — making it look like the match failed + // when in fact it picked the wrong page. const prefixPattern = dirPrefix ? `${dirPrefix}/%` : '%'; - const { rows } = await this.db.query( - `SELECT slug, similarity(title, $1) AS sim - FROM pages - WHERE similarity(title, $1) >= $3 - AND slug LIKE $2 - ORDER BY sim DESC, slug ASC - LIMIT 1`, - [name, prefixPattern, minSimilarity] - ); + const { rows } = sourceId + ? await this.db.query( + `SELECT slug, similarity(title, $1) AS sim + FROM pages + WHERE similarity(title, $1) >= $3 + AND slug LIKE $2 + AND source_id = $4 + AND deleted_at IS NULL + ORDER BY sim DESC, slug ASC + LIMIT 1`, + [name, prefixPattern, minSimilarity, sourceId] + ) + : await this.db.query( + `SELECT slug, similarity(title, $1) AS sim + FROM pages + WHERE similarity(title, $1) >= $3 + AND slug LIKE $2 + ORDER BY sim DESC, slug ASC + LIMIT 1`, + [name, prefixPattern, minSimilarity] + ); if (rows.length === 0) return null; const row = rows[0] as { slug: string; sim: number }; return { slug: row.slug, similarity: row.sim }; diff --git a/src/core/postgres-engine.ts b/src/core/postgres-engine.ts index d3b389293..6deaad784 100644 --- a/src/core/postgres-engine.ts +++ b/src/core/postgres-engine.ts @@ -3082,6 +3082,7 @@ export class PostgresEngine implements BrainEngine { name: string, dirPrefix?: string, minSimilarity: number = 0.55, + sourceId?: string, ): Promise<{ slug: string; similarity: number } | null> { const sql = this.sql; // Use the `similarity()` function directly with an explicit threshold @@ -3094,15 +3095,33 @@ export class PostgresEngine implements BrainEngine { // Tie-breaker: sort by slug after similarity so re-runs return the // same winner when multiple pages score equally (prevents churn // in put_page auto-link reconciliation). + // + // `sourceId` + `deleted_at IS NULL` mirror the filters `tryFuzzyMatch` + // in `src/core/entities/resolve.ts` got via #1436 (v0.41.13.0). Without + // them, fuzzy resolution could suggest cross-source slugs that the + // caller then silently drops at the FK filter in + // `operations.ts:reconcileLinks` (the `allSlugs` filter) — making it + // look like the match failed when in fact it picked the wrong page. const prefixPattern = dirPrefix ? `${dirPrefix}/%` : '%'; - const rows = await sql` - SELECT slug, similarity(title, ${name}) AS sim - FROM pages - WHERE similarity(title, ${name}) >= ${minSimilarity} - AND slug LIKE ${prefixPattern} - ORDER BY sim DESC, slug ASC - LIMIT 1 - `; + const rows = sourceId + ? await sql` + SELECT slug, similarity(title, ${name}) AS sim + FROM pages + WHERE similarity(title, ${name}) >= ${minSimilarity} + AND slug LIKE ${prefixPattern} + AND source_id = ${sourceId} + AND deleted_at IS NULL + ORDER BY sim DESC, slug ASC + LIMIT 1 + ` + : await sql` + SELECT slug, similarity(title, ${name}) AS sim + FROM pages + WHERE similarity(title, ${name}) >= ${minSimilarity} + AND slug LIKE ${prefixPattern} + ORDER BY sim DESC, slug ASC + LIMIT 1 + `; if (rows.length === 0) return null; const row = rows[0] as { slug: string; sim: number }; return { slug: row.slug, similarity: row.sim }; diff --git a/test/link-extraction.test.ts b/test/link-extraction.test.ts index f7c003ab5..9a2bc4f7d 100644 --- a/test/link-extraction.test.ts +++ b/test/link-extraction.test.ts @@ -1236,6 +1236,43 @@ describe('makeResolver — fallback chain', () => { const out = await r.resolveBasenameMatches!('struktura'); expect(out.sort()).toEqual(['notes/struktura', 'struktura']); }); + + test('opts.sourceId is forwarded to findByTitleFuzzy (twin of #1436 fix)', async () => { + // Captures every (name, dirPrefix, minSimilarity, sourceId) call so we + // can assert the resolver threads sourceId through. Without the wire-up, + // findByTitleFuzzy would be called with sourceId=undefined and the SQL + // could return cross-source slug suggestions that the FK filter + // downstream silently drops. + const calls: Array<{ name: string; dirPrefix?: string; minSimilarity?: number; sourceId?: string }> = []; + const engine = { + async getPage() { return null; }, + async findByTitleFuzzy(name: string, dirPrefix?: string, minSimilarity?: number, sourceId?: string) { + calls.push({ name, dirPrefix, minSimilarity, sourceId }); + return null; + }, + async searchKeyword() { return []; }, + } as unknown as BrainEngine; + const r = makeResolver(engine, { mode: 'batch', sourceId: 'src-a' }); + await r.resolve('Alice Example', 'people'); + expect(calls.length).toBeGreaterThan(0); + expect(calls.every(c => c.sourceId === 'src-a')).toBe(true); + }); + + test('opts.sourceId omitted → findByTitleFuzzy receives undefined (back-compat)', async () => { + const calls: Array<{ sourceId?: string }> = []; + const engine = { + async getPage() { return null; }, + async findByTitleFuzzy(_name: string, _dirPrefix?: string, _min?: number, sourceId?: string) { + calls.push({ sourceId }); + return null; + }, + async searchKeyword() { return []; }, + } as unknown as BrainEngine; + const r = makeResolver(engine, { mode: 'batch' }); + await r.resolve('Alice Example', 'people'); + expect(calls.length).toBeGreaterThan(0); + expect(calls.every(c => c.sourceId === undefined)).toBe(true); + }); }); describe('FRONTMATTER_LINK_MAP integrity', () => {