mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
fix(entities): thread sourceId through findByTitleFuzzy + skip soft-deleted (#1508)
`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>
This commit is contained in:
@@ -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`
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 };
|
||||
|
||||
@@ -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 };
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
Reference in New Issue
Block a user