diff --git a/CHANGELOG.md b/CHANGELOG.md index d9ad3d09..7b44878a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -251,6 +251,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 by physical key position and matched against the commands; genuine Cyrillic search terms keep priority over remapped candidates, and short wrong-layout prefixes match by command title. (#283, #285, #287) +- **Opt-in substring "lookup" search mode for agents.** `/api/search` gains an + additive, opt-in mode (guarded by a new `substring` flag) that matches literal + substrings of page titles and body text — so technical tokens the full-text + tokenizer mangles (`backup-srv.local`, `10.0.12.5`, `WB-MGE-30D86B`) are found + even when the FTS query is empty. It returns a location `path`, a windowed + `snippet` and a per-response relevance `score`, supports `titleOnly` and a + `parentPageId` subtree scope, and applies the page-level permission filter + before the limit. The web UI never sets `substring`, so its full-text search + behaviour is byte-for-byte unchanged. The leading-wildcard `LIKE` predicates + are backed by GIN trigram indexes on `LOWER(f_unaccent(title))` and + `LOWER(f_unaccent(text_content))` so lookups use a bitmap index scan instead of + a sequential scan. (#443) +- **MCP `search` tool returns richer, agent-oriented results.** The external MCP + `search` response shape changes for the agent surface: each hit now carries + `pageId` (renamed from `id`), plus `path`, `snippet` and `score`; the + UI-oriented `spaceId`, `rank` and `highlight` fields are dropped. (#443) ### Changed diff --git a/apps/server/src/core/search/search.service.ts b/apps/server/src/core/search/search.service.ts index 111c5938..0d125d58 100644 --- a/apps/server/src/core/search/search.service.ts +++ b/apps/server/src/core/search/search.service.ts @@ -378,10 +378,20 @@ export class SearchService { // Match predicate: title substring OR (unless titleOnly) text substring OR // (unless titleOnly) FTS. The substring branch runs even when the tsquery is // empty — that is the dotted/numeric-token case the FTS path misses. + // + // #443 dead-index fix: these two LIKE predicates MUST match the GIN trgm + // index expressions EXACTLY for Postgres to use them. The indexes are on the + // coalesce-FREE expressions `LOWER(f_unaccent(title))` (#348's + // idx_pages_title_trgm) and `LOWER(f_unaccent(text_content))` (this PR's + // idx_pages_text_content_trgm). A `coalesce(col,'')` wrapper here would make + // the query expression differ from the index expression and force a Seq Scan + // on pages for every lookup. Dropping coalesce is SEMANTICALLY EQUIVALENT: + // `NULL LIKE '%q%'` is NULL (falsy), so a NULL title/text simply doesn't + // match — exactly as an empty string wouldn't match `%q%`. candidates = candidates.where((eb) => { const ors = [ eb( - sql`LOWER(f_unaccent(coalesce(pages.title, '')))`, + sql`LOWER(f_unaccent(pages.title))`, 'like', sql`${likePattern} ESCAPE '\\'`, ), @@ -389,7 +399,7 @@ export class SearchService { if (!searchParams.titleOnly) { ors.push( eb( - sql`LOWER(f_unaccent(coalesce(pages.text_content, '')))`, + sql`LOWER(f_unaccent(pages.text_content))`, 'like', sql`${likePattern} ESCAPE '\\'`, ), diff --git a/apps/server/src/database/migrations/20260706T120000-search-lookup-trgm.ts b/apps/server/src/database/migrations/20260706T120000-search-lookup-trgm.ts index 4e00debf..c28f4cf0 100644 --- a/apps/server/src/database/migrations/20260706T120000-search-lookup-trgm.ts +++ b/apps/server/src/database/migrations/20260706T120000-search-lookup-trgm.ts @@ -8,11 +8,12 @@ import { type Kysely, sql } from 'kysely'; * pages.text_content. A leading wildcard cannot use a b-tree index, so without a * GIN trigram index each such predicate is a sequential scan. * - * - TITLE: the title predicate is IDENTICAL to the one added for /search/suggest - * (#348), which already created `idx_pages_title_trgm` on - * `(LOWER(f_unaccent(title))) gin_trgm_ops`. We re-assert it here with - * `IF NOT EXISTS` so a fresh DB that somehow lacks it still gets it, and so - * this migration is self-describing. On an existing DB it is a no-op. + * - TITLE: the lookup-mode title predicate is `LOWER(f_unaccent(title)) LIKE + * '%q%'` (coalesce-free, so it can use a functional index), which is IDENTICAL + * to the one added for /search/suggest (#348). #348's perf-indexes migration + * already created `idx_pages_title_trgm` on `(LOWER(f_unaccent(title))) + * gin_trgm_ops`, so the title predicate is already covered — we do NOT + * re-create that index here (it would be redundant). * * - TEXT_CONTENT: NEW. The substring branch scans text_content when the query * is not titleOnly. text_content is the large column, so a GIN trigram index @@ -34,15 +35,13 @@ import { type Kysely, sql } from 'kysely'; * here). Small/typical tenants are unaffected. */ export async function up(db: Kysely): Promise { - // Title trigram index — matches the lookup-mode title predicate exactly. - // Already present from #348 on existing DBs; re-asserted for freshness. - await sql` - CREATE INDEX IF NOT EXISTS idx_pages_title_trgm - ON pages USING gin ((LOWER(f_unaccent(title))) gin_trgm_ops) - `.execute(db); + // The title predicate is served by #348's idx_pages_title_trgm — see header. + // Only the text_content index is introduced here. - // text_content trigram index — accelerates the lookup-mode text substring - // predicate. Expression matches the predicate in search.service.ts. + // text_content trigram index. Its expression is coalesce-free — + // `LOWER(f_unaccent(text_content))` — to EXACTLY match the coalesce-free + // lookup-mode text substring predicate in search.service.ts, so Postgres can + // use it (a `coalesce(...)` mismatch would silently fall back to a Seq Scan). await sql` CREATE INDEX IF NOT EXISTS idx_pages_text_content_trgm ON pages USING gin ((LOWER(f_unaccent(text_content))) gin_trgm_ops) diff --git a/apps/server/test/integration/search-lookup-explain.int-spec.ts b/apps/server/test/integration/search-lookup-explain.int-spec.ts new file mode 100644 index 00000000..4a50e553 --- /dev/null +++ b/apps/server/test/integration/search-lookup-explain.int-spec.ts @@ -0,0 +1,123 @@ +import { randomUUID } from 'node:crypto'; +import { Kysely, sql } from 'kysely'; +import { + getTestDb, + destroyTestDb, + createWorkspace, + createSpace, +} from './db'; + +/** + * #443 dead-index guard — EXPLAIN on the REAL DB. + * + * The lookup mode's substring predicates run a leading-wildcard + * `LOWER(f_unaccent(col)) LIKE '%q%'`. Those are only fast when Postgres uses + * the GIN trigram indexes: + * - idx_pages_title_trgm on (LOWER(f_unaccent(title))) [#348] + * - idx_pages_text_content_trgm on (LOWER(f_unaccent(text_content))) [#443] + * + * Postgres uses a functional index ONLY when the query expression matches the + * index expression EXACTLY. The original lookup query wrapped the columns in + * `coalesce(col,'')`, which differs from the coalesce-FREE index expression and + * silently forced a Seq Scan on pages for EVERY lookup (the MCP client always + * sends substring:true). This test locks that in. + * + * Discriminator: `SET enable_seqscan = off` asks the planner "CAN this predicate + * use the index at all?" — which is exactly what the coalesce bug breaks. With + * seqscan disabled: + * - the coalesce-FREE (fixed) predicate plans a Bitmap Index Scan on the trgm + * index (no Seq Scan on pages); + * - the coalesce-WRAPPED (buggy) predicate cannot use the index and falls back + * to a Seq Scan on pages even though seqscan is disabled. + * We assert both to prove the fix and to keep the regression from silently + * returning. + */ +describe('SearchService agent-lookup EXPLAIN — trgm index is live [integration]', () => { + let db: Kysely; + let workspaceId: string; + let spaceId: string; + + async function insertPage(title: string, textContent: string): Promise { + const id = randomUUID(); + await db + .insertInto('pages') + .values({ + id, + slugId: `slug-${id.slice(0, 12)}`, + title, + textContent, + spaceId, + workspaceId, + }) + .execute(); + } + + // Run EXPLAIN (no ANALYZE — we only inspect the chosen plan) and return the + // concatenated plan text. + async function explain(query: string): Promise { + const rows = await sql<{ 'QUERY PLAN': string }>`EXPLAIN ${sql.raw(query)}`.execute( + db, + ); + return (rows.rows as any[]).map((r) => r['QUERY PLAN']).join('\n'); + } + + beforeAll(async () => { + db = getTestDb(); + workspaceId = (await createWorkspace(db)).id; + spaceId = (await createSpace(db, workspaceId)).id; + + // Seed enough rows that a trigram index is a plausible plan. The content is + // varied so the '%needle%' pattern is selective. + for (let i = 0; i < 200; i++) { + await insertPage( + `seed-title-${i}`, + `seed body content number ${i} lorem ipsum dolor sit amet ${i}`, + ); + } + await insertPage('backup-srv.local', 'the needle-token-xyz lives here'); + + // Keep the trgm indexes' stats fresh so the planner costs them correctly. + await sql`ANALYZE pages`.execute(db); + }); + + afterAll(async () => { + await destroyTestDb(); + }); + + // Force the planner to answer "can the index be used?" rather than "is it + // cheaper than a seq scan on this size?". Restored after each test. + beforeEach(async () => { + await sql`SET enable_seqscan = off`.execute(db); + }); + afterEach(async () => { + await sql`RESET enable_seqscan`.execute(db); + }); + + it('title predicate (coalesce-FREE, as fixed) uses idx_pages_title_trgm, not a Seq Scan', async () => { + const plan = await explain( + `SELECT id FROM pages WHERE LOWER(f_unaccent(title)) LIKE '%srv.local%'`, + ); + expect(plan).toContain('idx_pages_title_trgm'); + expect(plan).not.toMatch(/Seq Scan on pages/i); + }); + + it('text_content predicate (coalesce-FREE, as fixed) uses idx_pages_text_content_trgm, not a Seq Scan', async () => { + const plan = await explain( + `SELECT id FROM pages WHERE LOWER(f_unaccent(text_content)) LIKE '%needle-token%'`, + ); + expect(plan).toContain('idx_pages_text_content_trgm'); + expect(plan).not.toMatch(/Seq Scan on pages/i); + }); + + // Negative control: the OLD coalesce-wrapped predicate must NOT be able to use + // the index — even with seqscan disabled it can only Seq Scan pages. If this + // ever stops seq-scanning, the coalesce/index expressions have re-aligned and + // the guard above is no longer meaningful. + it('coalesce-WRAPPED text predicate (the bug) cannot use the index — falls to Seq Scan', async () => { + const plan = await explain( + `SELECT id FROM pages WHERE LOWER(f_unaccent(coalesce(text_content,''))) LIKE '%needle-token%'`, + ); + expect(plan).not.toContain('idx_pages_text_content_trgm'); + expect(plan).toMatch(/Seq Scan on pages/i); + }); +});