From e4e788f1513510fd43f6d9d17270ee8f2ae2914b Mon Sep 17 00:00:00 2001 From: agent_coder Date: Fri, 10 Jul 2026 21:34:53 +0300 Subject: [PATCH 1/2] =?UTF-8?q?feat(search):=20=D0=B0=D0=B3=D0=B5=D0=BD?= =?UTF-8?q?=D1=82=D1=81=D0=BA=D0=B8=D0=B9=20lookup-=D1=80=D0=B5=D0=B6?= =?UTF-8?q?=D0=B8=D0=BC=20=E2=80=94=20substring=20=D0=BF=D0=BE=20=D1=82?= =?UTF-8?q?=D0=BE=D1=87=D0=BA=D0=B0=D0=BC/=D0=B4=D0=B5=D1=84=D0=B8=D1=81?= =?UTF-8?q?=D0=B0=D0=BC/=D1=86=D0=B8=D1=84=D1=80=D0=B0=D0=BC,=20path,=20sn?= =?UTF-8?q?ippet,=20scope=20(#443,=20=D1=87=D0=B0=D1=81=D1=82=D1=8C=201/3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MCP-сервером пользуются LLM-агенты; основной паттерн — lookup: найти страницу по обрывку технической строки и сразу понять, где она лежит и что в ней. Раньше на такой вопрос уходило 3–5 вызовов. Эта часть закрывает `search` (часть 1 из 3; get_tree и get_page_context — следующими PR стопкой). Только чтение. Серверная часть — opt-in, веб-UI байт-в-байт неизменен: - SearchDTO: опциональные substring/parentPageId/titleOnly (default-off; без флагов путь FTS не тронут — guard `if (substring) return searchPageLookup`). - searchPageLookup: один скан pages, WHERE = title LIKE '%q%' OR (если не titleOnly) text LIKE '%q%' OR (если tsquery непустой) tsv @@ ... . LIKE- метасимволы %/_/\ экранируются (escapeLikePattern, ESCAPE '\') — `%`/`_` не матчат всё; substring-ветка работает даже при пустом tsquery (кейс 10.0.12). - Ранжирование тирами (TITLE_EXACT > TITLE_SUBSTRING > TEXT), вторичный сигнал ts_rank / позиция; score∈(0,1] только для сортировки одной выдачи (формула в комментарии). 200-cap упорядочен по SQL-прокси тира ДО среза (иначе Postgres отдаёт произвольные 200 и сильный хит мог выпасть). Пермишен-фильтр к merged-набору ДО limit. path — одна рекурсивная CTE на все хиты (не N+1). - snippet оконный в SQL (~500 символов вокруг первого совпадения). Позиция и срез в ОДНОМ пространстве LOWER(f_unaccent(...)) — f_unaccent не length- preserving (ß→ss, лигатуры, …→...), иначе окно смещалось/пустело. titleOnly → пустой snippet. Компромисс задокументирован. - Миграция: GIN gin_trgm_ops по LOWER(f_unaccent(text_content)); title-trgm индекс #348 переиспользован (IF NOT EXISTS), down() дропает только новый. MCP: схема search (spaceId/parentPageId/titleOnly/limit 1–50, default 10), client.search прокидывает substring:true, filterSearchResult → {pageId, title, path, snippet, score}. Инвариант: наружу только pageId (UUID), slugId/id никогда. Комментарии про намеренное расхождение с in-app hybrid-RRF (не тронут) и про деградацию на stock-upstream/Typesense (substring→plain FTS, без path/ snippet). Проверка на реальном Postgres: server integration 16/16 (вся acceptance-таблица #443 + регрессии на смещение snippet и cap-200), server unit 27/27, mcp node --test 708/708, tsc чисто. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/search/dto/search-response.dto.ts | 19 + apps/server/src/core/search/dto/search.dto.ts | 25 + .../src/core/search/search.controller.ts | 6 + .../core/search/search.service.lookup.spec.ts | 95 ++++ apps/server/src/core/search/search.service.ts | 393 ++++++++++++++- .../20260706T120000-search-lookup-trgm.ts | 56 +++ .../integration/search-lookup.int-spec.ts | 462 ++++++++++++++++++ packages/mcp/src/client.ts | 26 +- packages/mcp/src/index.ts | 55 ++- packages/mcp/src/lib/filters.ts | 32 +- packages/mcp/src/server-instructions.ts | 4 +- packages/mcp/test/unit/filters.test.mjs | 94 +++- 12 files changed, 1237 insertions(+), 30 deletions(-) create mode 100644 apps/server/src/core/search/search.service.lookup.spec.ts create mode 100644 apps/server/src/database/migrations/20260706T120000-search-lookup-trgm.ts create mode 100644 apps/server/test/integration/search-lookup.int-spec.ts diff --git a/apps/server/src/core/search/dto/search-response.dto.ts b/apps/server/src/core/search/dto/search-response.dto.ts index 8f5b343d..e88356ad 100644 --- a/apps/server/src/core/search/dto/search-response.dto.ts +++ b/apps/server/src/core/search/dto/search-response.dto.ts @@ -12,3 +12,22 @@ export class SearchResponseDto { updatedAt: Date; space: Partial; } + +// Response shape for the opt-in agent-lookup mode (#443, `substring: true`). +// Additive to the FTS response: carries the location (`path`), a windowed +// `snippet` around the first match and a per-response sort `score`. The MCP +// layer maps `id → pageId`; `slugId` is never exposed. +export class SearchLookupResponseDto { + id: string; + slugId: string; + title: string; + parentPageId: string | null; + // Ancestor titles from the space root down to the direct parent; [] for a + // root page. + path: string[]; + // ~300–500 chars around the first match (or a leading text window / extended + // ts_headline fallback). + snippet: string; + // 0..1 float, meaningful ONLY for sorting within one response. + score: number; +} diff --git a/apps/server/src/core/search/dto/search.dto.ts b/apps/server/src/core/search/dto/search.dto.ts index f23dd4d5..dd0c0a1b 100644 --- a/apps/server/src/core/search/dto/search.dto.ts +++ b/apps/server/src/core/search/dto/search.dto.ts @@ -30,6 +30,31 @@ export class SearchDTO { @IsOptional() @IsNumber() offset?: number; + + // --- Opt-in agent-lookup mode (#443). ------------------------------------ + // These fields are ADDITIVE and default-off: a web client that sends none of + // them gets byte-identical FTS behaviour and result shape. They are only read + // by the substring/path/snippet code path in SearchService.searchPage. + // + // NOTE (standalone stdio vs stock upstream): stock upstream validates this DTO + // with `whitelist: true`, so an older server silently strips these unknown + // fields and the request degrades gracefully to the plain FTS behaviour. + + // Enables the hybrid substring branch (title + text_content LIKE) merged with + // the existing FTS branch, plus tiered ranking, path and windowed snippet. + @IsOptional() + @IsBoolean() + substring?: boolean; + + // Restrict the search to a page and all of its descendants (inclusive). + @IsOptional() + @IsString() + parentPageId?: string; + + // Match titles only; do not scan text_content. + @IsOptional() + @IsBoolean() + titleOnly?: boolean; } export class SearchShareDTO extends SearchDTO { diff --git a/apps/server/src/core/search/search.controller.ts b/apps/server/src/core/search/search.controller.ts index c968c344..7504174a 100644 --- a/apps/server/src/core/search/search.controller.ts +++ b/apps/server/src/core/search/search.controller.ts @@ -60,6 +60,12 @@ export class SearchController { } } + // #443 graceful degradation: on EE/Typesense instances the request routes to + // the Typesense backend, which does NOT implement the opt-in agent-lookup + // mode. The `substring`/`parentPageId`/`titleOnly` fields are silently ignored + // and the response carries no `path`/`snippet`/`score` and no substring/tier + // ranking — it degrades to plain Typesense FTS. The native lookup mode below + // is Postgres-search-driver only. if (this.environmentService.getSearchDriver() === 'typesense') { return this.searchTypesense(searchDto, { userId: user.id, diff --git a/apps/server/src/core/search/search.service.lookup.spec.ts b/apps/server/src/core/search/search.service.lookup.spec.ts new file mode 100644 index 00000000..46d7f798 --- /dev/null +++ b/apps/server/src/core/search/search.service.lookup.spec.ts @@ -0,0 +1,95 @@ +import { + computeLookupScore, + escapeLikePattern, + SearchLookupTier, +} from './search.service'; + +/** + * Pure-function coverage for the #443 agent-lookup helpers: + * - escapeLikePattern: LIKE-metacharacter escaping so `%`/`_`/`\` are literals + * (the acceptance-table requirement that a query of `%` or `_` does NOT match + * everything); + * - computeLookupScore: the tiered 0..1 ranking score, where a stronger tier + * always outranks a weaker one regardless of the in-tier secondary signal. + * + * The DB-touching branch (substring UNION FTS, path CTE, snippet window) is + * covered by the integration spec against the real schema. + */ +describe('escapeLikePattern', () => { + it('escapes the LIKE metacharacters % _ and \\', () => { + expect(escapeLikePattern('%')).toBe('\\%'); + expect(escapeLikePattern('_')).toBe('\\_'); + expect(escapeLikePattern('\\')).toBe('\\\\'); + }); + + it('escapes the backslash FIRST so it does not double-escape %/_', () => { + // Input `\%` must become `\\` + `\%` = `\\\%`, not `\\%`. + expect(escapeLikePattern('\\%')).toBe('\\\\\\%'); + }); + + it('leaves ordinary technical chars (. - / digits) untouched', () => { + expect(escapeLikePattern('backup-srv.local')).toBe('backup-srv.local'); + expect(escapeLikePattern('10.0.12')).toBe('10.0.12'); + expect(escapeLikePattern('WB-MGE-30D86B')).toBe('WB-MGE-30D86B'); + expect(escapeLikePattern('a/b')).toBe('a/b'); + }); + + it('escapes only the metacharacters in a mixed string', () => { + expect(escapeLikePattern('50%_off.zip')).toBe('50\\%\\_off.zip'); + }); + + it('is null/undefined-safe', () => { + expect(escapeLikePattern(undefined as any)).toBe(''); + expect(escapeLikePattern(null as any)).toBe(''); + }); +}); + +describe('computeLookupScore', () => { + it('keeps every score within (0, 1]', () => { + for (const tier of [ + SearchLookupTier.TITLE_EXACT, + SearchLookupTier.TITLE_SUBSTRING, + SearchLookupTier.TEXT, + ]) { + for (const secondary of [0, 0.001, 1, 100, 1e6]) { + const s = computeLookupScore({ tier, secondary }); + expect(s).toBeGreaterThan(0); + expect(s).toBeLessThanOrEqual(1); + } + } + }); + + it('a stronger tier ALWAYS outranks a weaker tier, whatever the secondary', () => { + // Weak tier with a huge secondary must still lose to a strong tier with a + // tiny secondary — tiers dominate. + const strongLowSecondary = computeLookupScore({ + tier: SearchLookupTier.TITLE_EXACT, + secondary: 0, + }); + const weakHighSecondary = computeLookupScore({ + tier: SearchLookupTier.TEXT, + secondary: 1e9, + }); + expect(strongLowSecondary).toBeGreaterThan(weakHighSecondary); + }); + + it('within a tier a larger secondary sorts higher', () => { + const lo = computeLookupScore({ + tier: SearchLookupTier.TEXT, + secondary: 0.1, + }); + const hi = computeLookupScore({ + tier: SearchLookupTier.TEXT, + secondary: 5, + }); + expect(hi).toBeGreaterThan(lo); + }); + + it('treats a negative/absent secondary as 0', () => { + const zero = computeLookupScore({ tier: SearchLookupTier.TEXT, secondary: 0 }); + expect(computeLookupScore({ tier: SearchLookupTier.TEXT })).toBe(zero); + expect( + computeLookupScore({ tier: SearchLookupTier.TEXT, secondary: -5 }), + ).toBe(zero); + }); +}); diff --git a/apps/server/src/core/search/search.service.ts b/apps/server/src/core/search/search.service.ts index 91e5e5f4..111c5938 100644 --- a/apps/server/src/core/search/search.service.ts +++ b/apps/server/src/core/search/search.service.ts @@ -1,6 +1,9 @@ import { Injectable } from '@nestjs/common'; import { SearchDTO, SearchSuggestionDTO } from './dto/search.dto'; -import { SearchResponseDto } from './dto/search-response.dto'; +import { + SearchLookupResponseDto, + SearchResponseDto, +} from './dto/search-response.dto'; import { InjectKysely } from 'nestjs-kysely'; import { KyselyDB } from '@docmost/db/types/kysely.types'; import { sql } from 'kysely'; @@ -34,6 +37,53 @@ export function buildTsQuery(raw: string): string { return tsquery(cleaned + '*'); } +// Escape the LIKE metacharacters (`%`, `_`, `\`) in a raw user query so every +// character — including `.`, `-`, `_`, `%`, `/` — is matched LITERALLY by a +// `col LIKE '%' || q || '%'` predicate. Without this, a query of `%` or `_` +// would match every row (see the #443 acceptance table). The backslash is the +// escape char (Postgres LIKE default), so it must be escaped first. +export function escapeLikePattern(raw: string): string { + return (raw ?? '') + .replace(/\\/g, '\\\\') + .replace(/%/g, '\\%') + .replace(/_/g, '\\_'); +} + +// Ranking tiers for the agent-lookup mode (#443), highest first. A hit's tier +// is the strongest way it matched; ties inside a tier break on a secondary +// signal (FTS rank, or first-match position). The numeric `score` returned to +// the caller is derived from (tier, secondary) and is meaningful ONLY for +// ordering within a single response. +export enum SearchLookupTier { + // Title equals the query, case-insensitively. + TITLE_EXACT = 3, + // Query is a substring of the title. + TITLE_SUBSTRING = 2, + // Query matched in the text (substring or FTS). + TEXT = 1, +} + +export interface RankableHit { + tier: SearchLookupTier; + // Secondary in-tier signal, higher = better (e.g. ts_rank, or a + // position-derived closeness score). Defaults to 0. + secondary?: number; +} + +// Map (tier, secondary) → a 0..1 float used ONLY to sort one response. +// +// Formula: score = (tier + squash(secondary)) / (maxTier + 1), where +// squash(x) = x / (1 + x) maps any non-negative secondary into [0, 1) +// so a stronger tier ALWAYS outranks a weaker one regardless of the secondary +// value, and within a tier a larger secondary sorts higher. maxTier is the top +// enum value (TITLE_EXACT = 3), so the divisor keeps the result in (0, 1]. +export function computeLookupScore(hit: RankableHit): number { + const maxTier = SearchLookupTier.TITLE_EXACT; + const secondary = Math.max(0, hit.secondary ?? 0); + const squashed = secondary / (1 + secondary); + return (hit.tier + squashed) / (maxTier + 1); +} + @Injectable() export class SearchService { constructor( @@ -50,12 +100,19 @@ export class SearchService { userId?: string; workspaceId: string; }, - ): Promise<{ items: SearchResponseDto[] }> { + ): Promise<{ items: SearchResponseDto[] | SearchLookupResponseDto[] }> { const { query } = searchParams; if (query.length < 1) { return { items: [] }; } + + // Opt-in agent-lookup mode (#443). Guarded by the `substring` flag so the + // web-UI (which never sets it) keeps byte-identical FTS behaviour below. + if (searchParams.substring) { + return this.searchPageLookup(searchParams, opts); + } + const searchQuery = buildTsQuery(query); let queryResults = this.db @@ -175,6 +232,338 @@ export class SearchService { return { items: searchResults }; } + /** + * Agent-lookup search (#443, opt-in via `SearchDTO.substring`). + * + * ADDITIVE to the FTS path: runs a substring branch (title + optionally + * text_content, LIKE with metacharacters escaped) MERGED with the existing + * FTS branch, so technical tokens that the `english` tokenizer mangles + * (`backup-srv.local`, `10.0.12.5`, `WB-MGE-30D86B`) are still found — even + * when `buildTsQuery()` returns '' for a dotted/numeric query. Results carry a + * location (`path`), a windowed `snippet` and a per-response `score`. + * + * The whole method is only reached when `substring: true`; the web-UI never + * sets it, so its behaviour is unchanged. + */ + private async searchPageLookup( + searchParams: SearchDTO, + opts: { userId?: string; workspaceId: string }, + ): Promise<{ items: SearchLookupResponseDto[] }> { + const rawQuery = searchParams.query.trim(); + if (!rawQuery) { + return { items: [] }; + } + + const limit = Math.min(Math.max(searchParams.limit || 10, 1), 50); + + // Normalize the query the same way as the FTS / suggest path: f_unaccent + + // lower, done in SQL. `q` is the escaped LIKE pattern body (literal chars). + const likeBody = escapeLikePattern(rawQuery); + // Compare against `LOWER(f_unaccent(col))`; unaccent+lower the needle too. + const needle = sql`LOWER(f_unaccent(${rawQuery}))`; + const likePattern = sql`LOWER(f_unaccent(${'%' + likeBody + '%'}))`; + const tsQuery = buildTsQuery(rawQuery); + const hasTsQuery = tsQuery.length > 0; + + // --- Resolve the space scope. --------------------------------------------- + // Mirrors searchPage: explicit spaceId, else the authenticated user's member + // spaces. The share path is not exposed to this opt-in mode. + let spaceIds: string[] = []; + if (searchParams.spaceId) { + spaceIds = [searchParams.spaceId]; + } else if (opts.userId) { + spaceIds = await this.spaceMemberRepo.getUserSpaceIds(opts.userId); + } else { + return { items: [] }; + } + if (spaceIds.length === 0) { + return { items: [] }; + } + + // --- Optional parentPageId subtree scope (inclusive). --------------------- + // Reuse the same recursive-descendants pattern used for share-scope. + let descendantIds: string[] | null = null; + if (searchParams.parentPageId) { + const descendants = await this.pageRepo.getPageAndDescendants( + searchParams.parentPageId, + { includeContent: false }, + ); + descendantIds = descendants.map((p: any) => p.id); + if (descendantIds.length === 0) { + return { items: [] }; + } + } + + // --- Candidate query: substring (title + text) UNION FTS. ----------------- + // We compute everything the ranker needs in SQL and pull only small columns + // (never the whole text_content) into Node: + // - titleExact / titleSub: tier signals + // - textMatchPos: 1-based position of the first text match (0 = none) + // - ftsRank: ts_rank for the FTS secondary signal (0 when no tsquery) + // - snippet: windowed ~500 chars around the first text match, or a leading + // text window (title-only hit), or an extended ts_headline fallback. + const N_BEFORE = 60; // chars of context before the first match + const SNIPPET_LEN = 500; + + let candidates = this.db + .selectFrom('pages') + .select([ + 'pages.id as id', + 'pages.slugId as slugId', + 'pages.title as title', + 'pages.parentPageId as parentPageId', + // Tier signals. + sql`LOWER(f_unaccent(coalesce(pages.title, ''))) = ${needle}`.as( + 'titleExact', + ), + sql`LOWER(f_unaccent(coalesce(pages.title, ''))) LIKE ${likePattern} ESCAPE '\\'`.as( + 'titleSub', + ), + // 1-based position of the first text match (0 = no text match). + sql`strpos(LOWER(f_unaccent(coalesce(pages.text_content, ''))), ${needle})`.as( + 'textMatchPos', + ), + // FTS secondary signal (0 when the tsquery is empty). + hasTsQuery + ? sql`ts_rank(pages.tsv, to_tsquery('english', f_unaccent(${tsQuery})))`.as( + 'ftsRank', + ) + : sql`0`.as('ftsRank'), + // Windowed snippet, computed entirely in SQL. Priority: + // 1. window around the first text match; + // 2. otherwise (titleOnly: no snippet; else) a leading window of the + // page text (title-only hit); + // 3. otherwise an extended ts_headline for pure-FTS hits. + // + // #443 snippet-position fix: the match position (`strpos`) is computed in + // the LOWER(f_unaccent(...)) space, but f_unaccent is NOT length- + // preserving (ß→ss, æ→ae, …→..., ½→ 1/2, full-width forms), so slicing + // the ORIGINAL text at that position was misaligned — a single expanding + // char before the match shifted the window (or ran it past end → empty). + // We now slice from the SAME LOWER(f_unaccent(...)) string so position + // and slice share one coordinate space. DELIBERATE trade-off: the snippet + // loses original case/diacritics — acceptable for an agent-facing snippet + // (position accuracy over original-glyph fidelity). The ts_headline branch + // matches over the ORIGINAL text itself, so it is unaffected and kept as-is. + searchParams.titleOnly + ? sql`''`.as('snippet') + : sql` + coalesce( + case + when strpos(LOWER(f_unaccent(coalesce(pages.text_content, ''))), ${needle}) > 0 + then substring( + LOWER(f_unaccent(coalesce(pages.text_content, ''))) + from greatest(1, strpos(LOWER(f_unaccent(coalesce(pages.text_content, ''))), ${needle}) - ${N_BEFORE}) + for ${SNIPPET_LEN} + ) + when coalesce(pages.text_content, '') <> '' + then substring(LOWER(f_unaccent(pages.text_content)) from 1 for 300) + ${ + hasTsQuery + ? sql`else ts_headline('english', coalesce(pages.text_content, ''), to_tsquery('english', f_unaccent(${tsQuery})), 'MinWords=25, MaxWords=40, MaxFragments=3')` + : sql`` + } + end, + '' + ) + `.as('snippet'), + ]) + .where('pages.deletedAt', 'is', null) + .where('pages.spaceId', 'in', spaceIds); + + if (descendantIds) { + candidates = candidates.where('pages.id', 'in', descendantIds); + } + + // 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. + candidates = candidates.where((eb) => { + const ors = [ + eb( + sql`LOWER(f_unaccent(coalesce(pages.title, '')))`, + 'like', + sql`${likePattern} ESCAPE '\\'`, + ), + ]; + if (!searchParams.titleOnly) { + ors.push( + eb( + sql`LOWER(f_unaccent(coalesce(pages.text_content, '')))`, + 'like', + sql`${likePattern} ESCAPE '\\'`, + ), + ); + if (hasTsQuery) { + ors.push( + sql`pages.tsv @@ to_tsquery('english', f_unaccent(${tsQuery}))` as any, + ); + } + } + return eb.or(ors); + }); + + // Pull a generous candidate set (before permission filtering + limit). + // Cap it so a pathological match set cannot blow up memory; 200 >> limit + // (max 50) leaves ample headroom for the post-permission truncation. + // + // #443 cap-ordering fix: the 200-cap MUST be deterministic and relevance- + // biased. Without an ORDER BY, Postgres returns an ARBITRARY 200 rows, so on + // a broad match set (common word / short substring) a strong TITLE_EXACT hit + // could be among the dropped rows while 200 low-tier TEXT hits fill the cap. + // We order by the SAME SQL tier proxies the Node ranker uses — title-exact, + // then title-substring, then fts-rank (nulls last), then earliest text-match + // position — so the cap keeps the strongest candidates. The Node-side final + // tier sort + slice(0, limit) below still runs and stays authoritative; this + // ORDER BY only decides WHICH candidates survive the 200-cap. + // NB: a BARE integer literal in ORDER BY is read by Postgres as an ordinal + // column position (`ORDER BY 0` → "position 0 is not in select list"), so the + // no-tsquery fallback is `0::float`, not `0`. + const ftsRankExpr = hasTsQuery + ? sql`ts_rank(pages.tsv, to_tsquery('english', f_unaccent(${tsQuery})))` + : sql`0::float`; + const candidatesCapped = candidates + // Raw-SQL ORDER BY expressions: pass the full ` ` as ONE arg + // (the two-arg form treats a raw-SQL second arg as an ORDER BY position). + .orderBy( + sql`(LOWER(f_unaccent(coalesce(pages.title, ''))) = ${needle}) desc`, + ) + .orderBy( + sql`(LOWER(f_unaccent(coalesce(pages.title, ''))) LIKE ${likePattern} ESCAPE '\\') desc`, + ) + .orderBy(sql`${ftsRankExpr} desc nulls last`) + // Earlier text match first; strpos returns 0 for "no match", which would + // sort BEFORE a real (>=1) position under plain ASC, so push 0 to the end. + .orderBy( + sql`case when strpos(LOWER(f_unaccent(coalesce(pages.text_content, ''))), ${needle}) = 0 then 2147483647 else strpos(LOWER(f_unaccent(coalesce(pages.text_content, ''))), ${needle}) end asc`, + ); + + let rows: any[] = await candidatesCapped.limit(200).execute(); + + if (rows.length === 0) { + return { items: [] }; + } + + // --- Permissions BEFORE limit. -------------------------------------------- + // Apply the existing page-level post-filter to the MERGED set, then rank and + // only THEN truncate to `limit` — never lose the permission filter. + if (opts.userId) { + const accessibleIds = + await this.pagePermissionRepo.filterAccessiblePageIds({ + pageIds: rows.map((r) => r.id), + userId: opts.userId, + spaceId: searchParams.spaceId, + workspaceId: opts.workspaceId, + }); + const accessibleSet = new Set(accessibleIds); + rows = rows.filter((r) => accessibleSet.has(r.id)); + } + + if (rows.length === 0) { + return { items: [] }; + } + + // --- Tiered ranking + dedup. ---------------------------------------------- + // Rows are already unique by id (single pages scan), so no cross-branch + // dedup is needed here; the tier captures the strongest match reason. + const ranked = rows.map((r) => { + let tier: SearchLookupTier; + let secondary: number; + if (r.titleExact) { + tier = SearchLookupTier.TITLE_EXACT; + secondary = Number(r.ftsRank) || 0; + } else if (r.titleSub) { + tier = SearchLookupTier.TITLE_SUBSTRING; + secondary = Number(r.ftsRank) || 0; + } else { + tier = SearchLookupTier.TEXT; + // Prefer earlier text matches; map position → closeness in (0, 1]. + const pos = Number(r.textMatchPos) || 0; + secondary = + pos > 0 ? 1 / (1 + (pos - 1) / 100) : Number(r.ftsRank) || 0; + } + return { row: r, tier, score: computeLookupScore({ tier, secondary }) }; + }); + + ranked.sort((a, b) => b.score - a.score); + const top = ranked.slice(0, limit); + + // --- Batch ancestor path (ONE recursive CTE, not N+1). -------------------- + const pathById = await this.buildAncestorPaths(top.map((t) => t.row.id)); + + const items: SearchLookupResponseDto[] = top.map((t) => ({ + id: t.row.id, + slugId: t.row.slugId, + title: t.row.title, + parentPageId: t.row.parentPageId ?? null, + path: pathById.get(t.row.id) ?? [], + snippet: (t.row.snippet ?? '') + .replace(/\r\n|\r|\n/g, ' ') + .replace(/\s+/g, ' ') + .trim(), + score: t.score, + })); + + return { items }; + } + + /** + * Batch ancestor-titles helper (#443): ONE recursive CTE seeded with ALL hit + * ids, walking UP parentPageId. Returns a map hitId → ancestor titles ordered + * root → direct parent (the hit's own title is excluded). Root pages map to + * an empty array. Avoids the N+1 of a per-page breadcrumb call. + */ + private async buildAncestorPaths( + hitIds: string[], + ): Promise> { + const result = new Map(); + if (hitIds.length === 0) return result; + + // ancestry(hit_id, page_id, title, parent_page_id, depth): seed one row per + // hit at depth 0 (the hit itself), then walk to parents (increasing depth). + const rows = await this.db + .withRecursive('ancestry', (db) => + db + .selectFrom('pages') + .select([ + 'pages.id as hitId', + 'pages.id as pageId', + 'pages.title as title', + 'pages.parentPageId as parentPageId', + sql`0`.as('depth'), + ]) + .where('pages.id', 'in', hitIds) + .unionAll((exp) => + exp + .selectFrom('pages as p') + .innerJoin('ancestry as a', 'p.id', 'a.parentPageId') + .select([ + 'a.hitId as hitId', + 'p.id as pageId', + 'p.title as title', + 'p.parentPageId as parentPageId', + sql`a.depth + 1`.as('depth'), + ]), + ), + ) + .selectFrom('ancestry') + .select(['hitId', 'title', 'depth']) + // depth 0 is the hit itself — excluded from the path. + .where('depth', '>', 0) + .orderBy('hitId') + // Larger depth = closer to the space root. Ordering DESC gives + // root → parent once collected. + .orderBy('depth', 'desc') + .execute(); + + for (const r of rows as any[]) { + const list = result.get(r.hitId) ?? []; + list.push(r.title); + result.set(r.hitId, list); + } + return result; + } + async searchSuggestions( suggestion: SearchSuggestionDTO, userId: string, diff --git a/apps/server/src/database/migrations/20260706T120000-search-lookup-trgm.ts b/apps/server/src/database/migrations/20260706T120000-search-lookup-trgm.ts new file mode 100644 index 00000000..4e00debf --- /dev/null +++ b/apps/server/src/database/migrations/20260706T120000-search-lookup-trgm.ts @@ -0,0 +1,56 @@ +import { type Kysely, sql } from 'kysely'; + +/** + * #443 — trigram indexes for the opt-in agent-lookup search mode. + * + * The lookup mode adds a substring branch that runs leading-wildcard + * `LOWER(f_unaccent(col)) LIKE '%q%'` predicates on pages.title and + * 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. + * + * - 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 + * on it is the meaningful acceleration for the lookup mode. The lookup search + * is ALWAYS space-scoped (spaceId or the user's member spaces), so on small + * instances a per-space sequential scan is tolerable — but the index turns the + * `%q%` text predicate into a Bitmap Index Scan and removes the only + * unbounded-per-space cost of the feature. We add it. The trade-off is disk + + * write amplification on page edits (GIN trigram indexes are larger and slower + * to update than b-trees); on the small instances this fork targets that cost + * is acceptable and the read win on agent lookups is the priority. + * + * DEPLOY-TIME LOCK WARNING: plain (non-CONCURRENT) CREATE INDEX — Kysely runs + * each migration in a transaction, so CONCURRENTLY is impossible. The build takes + * a SHARE lock that BLOCKS writes on `pages` for its duration. The text_content + * GIN build is the slow one and can take minutes on a large tenant. For large + * installations, run this in a maintenance window or build the index out-of-band + * with CREATE INDEX CONCURRENTLY before deploying (then `IF NOT EXISTS` no-ops + * 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); + + // text_content trigram index — accelerates the lookup-mode text substring + // predicate. Expression matches the predicate in search.service.ts. + await sql` + CREATE INDEX IF NOT EXISTS idx_pages_text_content_trgm + ON pages USING gin ((LOWER(f_unaccent(text_content))) gin_trgm_ops) + `.execute(db); +} + +export async function down(db: Kysely): Promise { + // Only drop the index this migration introduced. idx_pages_title_trgm is owned + // by the #348 perf-indexes migration, so leave it for that migration's down(). + await sql`DROP INDEX IF EXISTS idx_pages_text_content_trgm`.execute(db); +} diff --git a/apps/server/test/integration/search-lookup.int-spec.ts b/apps/server/test/integration/search-lookup.int-spec.ts new file mode 100644 index 00000000..51daa855 --- /dev/null +++ b/apps/server/test/integration/search-lookup.int-spec.ts @@ -0,0 +1,462 @@ +import { randomUUID } from 'node:crypto'; +import { Kysely } from 'kysely'; +import { SearchService } from 'src/core/search/search.service'; +import { PageRepo } from '@docmost/db/repos/page/page.repo'; +import { + getTestDb, + destroyTestDb, + createWorkspace, + createSpace, +} from './db'; + +/** + * #443 — agent-lookup search mode, acceptance on the REAL DB schema. + * + * Exercises SearchService.searchPage(..., { substring: true }) against a + * migrated Postgres: substring matching of technical tokens the FTS tokenizer + * mangles (backup-srv.local, 10.0.12.5, WB-MGE-30D86B, "Теги: Docker"), the + * populated path + snippet, parentPageId subtree scoping, titleOnly, the empty + * result, LIKE-metacharacter escaping (`%`/`_` must NOT match everything), the + * permission post-filter applied BEFORE the limit, and the web-UI path staying + * on the legacy FTS shape when `substring` is absent. + * + * The tsv column is populated by the pages_tsvector_trigger on insert, so the + * FTS branch is exercised too. + */ +describe('SearchService agent-lookup mode [integration]', () => { + let db: Kysely; + let service: SearchService; + let workspaceId: string; + let spaceId: string; + + // Direct page insert (the shared createPage seeder omits text_content / + // parent_page_id, both of which this mode depends on). Returns the id. + async function insertPage(args: { + title: string; + textContent?: string; + parentPageId?: string | null; + spaceId?: string; + }): Promise { + const id = randomUUID(); + await db + .insertInto('pages') + .values({ + id, + slugId: `slug-${id.slice(0, 12)}`, + title: args.title, + textContent: args.textContent ?? null, + parentPageId: args.parentPageId ?? null, + spaceId: args.spaceId ?? spaceId, + workspaceId, + }) + .execute(); + return id; + } + + // Build a SearchService wired to the real DB + a real PageRepo (only its + // recursive-descendants method is used by this mode, and it needs only `db`), + // with lightweight stubs for the space-membership and permission repos so a + // test can drive scope + the permission post-filter explicitly. + function buildService(opts?: { + userSpaceIds?: string[]; + // ids to KEEP after the permission post-filter; undefined = keep all. + accessibleIds?: string[]; + }): SearchService { + const pageRepo = new PageRepo(db as any, null as any, null as any); + const spaceMemberRepo = { + getUserSpaceIds: async () => opts?.userSpaceIds ?? [spaceId], + }; + const pagePermissionRepo = { + filterAccessiblePageIds: async ({ pageIds }: { pageIds: string[] }) => + opts?.accessibleIds + ? pageIds.filter((id) => opts.accessibleIds!.includes(id)) + : pageIds, + }; + return new SearchService( + db as any, + pageRepo as any, + {} as any, // shareRepo — unused by the lookup path + spaceMemberRepo as any, + pagePermissionRepo as any, + ); + } + + beforeAll(async () => { + db = getTestDb(); + workspaceId = (await createWorkspace(db)).id; + spaceId = (await createSpace(db, workspaceId)).id; + service = buildService(); + }); + + afterAll(async () => { + await destroyTestDb(); + }); + + it('finds `backup-srv.local` by the fragment `srv.local`', async () => { + const pageId = await insertPage({ + title: 'backup-srv.local', + textContent: 'A backup server node.', + }); + + const { items } = (await service.searchPage( + { query: 'srv.local', spaceId, substring: true } as any, + { workspaceId }, + )) as any; + + expect(items.map((i: any) => i.id)).toContain(pageId); + const hit = items.find((i: any) => i.id === pageId); + expect(hit.title).toBe('backup-srv.local'); + // slugId must never be part of the server response shape. + expect('slugId' in hit).toBe(true); // server carries it; MCP strips it + }); + + it('finds a page whose TEXT contains `10.0.12.5` by the fragment `10.0.12` (empty-tsquery case)', async () => { + const pageId = await insertPage({ + title: 'Server inventory', + textContent: 'The backup box lives at IP: 10.0.12.5. Debian 12, backups.', + }); + + const { items } = (await service.searchPage( + { query: '10.0.12', spaceId, substring: true } as any, + { workspaceId }, + )) as any; + + const hit = items.find((i: any) => i.id === pageId); + expect(hit).toBeDefined(); + // The windowed snippet must include the matched text. + expect(hit.snippet).toContain('10.0.12.5'); + }); + + it('finds `WB-MGE-30D86B` (alphanumeric token with dashes) by title', async () => { + const pageId = await insertPage({ + title: 'WB-MGE-30D86B', + textContent: 'Device page.', + }); + + const { items } = (await service.searchPage( + { query: 'WB-MGE-30D86B', spaceId, substring: true } as any, + { workspaceId }, + )) as any; + + const hit = items.find((i: any) => i.id === pageId); + expect(hit).toBeDefined(); + // Exact title match → top tier (TITLE_EXACT=3) → score in [0.75, 1]. + expect(hit.score).toBeGreaterThanOrEqual(0.75); + // And it is the top-ranked hit of its own result set. + expect(items[0].id).toBe(pageId); + }); + + it('finds every page whose text literally contains `Теги: Docker`', async () => { + const a = await insertPage({ + title: 'Container host A', + textContent: 'Some notes.\nТеги: Docker, compose\nmore.', + }); + const b = await insertPage({ + title: 'Container host B', + textContent: 'Prelude.\nТеги: Docker\nepilogue.', + }); + const noise = await insertPage({ + title: 'Unrelated', + textContent: 'Теги: Kubernetes', + }); + + const { items } = (await service.searchPage( + { query: 'Теги: Docker', spaceId, substring: true, limit: 50 } as any, + { workspaceId }, + )) as any; + + const ids = items.map((i: any) => i.id); + expect(ids).toContain(a); + expect(ids).toContain(b); + expect(ids).not.toContain(noise); + }); + + it('populates a non-empty `path` for a nested hit and `[]` for a root hit', async () => { + const root = await insertPage({ title: 'Infrastructure' }); + const mid = await insertPage({ title: 'Datacenter A', parentPageId: root }); + const leaf = await insertPage({ + title: 'unique-nested-host', + parentPageId: mid, + }); + + const { items } = (await service.searchPage( + { query: 'unique-nested-host', spaceId, substring: true } as any, + { workspaceId }, + )) as any; + + const hit = items.find((i: any) => i.id === leaf); + expect(hit.path).toEqual(['Infrastructure', 'Datacenter A']); + + const rootHits = (await service.searchPage( + { query: 'Infrastructure', spaceId, substring: true } as any, + { workspaceId }, + )) as any; + const rootHit = rootHits.items.find((i: any) => i.id === root); + expect(rootHit.path).toEqual([]); + }); + + it('scopes to a subtree with parentPageId (cutting off sibling branches)', async () => { + const branchA = await insertPage({ title: 'BranchA-root' }); + const inA = await insertPage({ + title: 'scoped-target-xyz', + parentPageId: branchA, + }); + const branchB = await insertPage({ title: 'BranchB-root' }); + const inB = await insertPage({ + title: 'scoped-target-xyz', + parentPageId: branchB, + }); + + const { items } = (await service.searchPage( + { + query: 'scoped-target-xyz', + spaceId, + substring: true, + parentPageId: branchA, + } as any, + { workspaceId }, + )) as any; + + const ids = items.map((i: any) => i.id); + expect(ids).toContain(inA); + expect(ids).not.toContain(inB); + }); + + it('includes the parent page itself in the parentPageId subtree', async () => { + const parent = await insertPage({ title: 'self-included-parent' }); + await insertPage({ title: 'child-of-self', parentPageId: parent }); + + const { items } = (await service.searchPage( + { + query: 'self-included-parent', + spaceId, + substring: true, + parentPageId: parent, + } as any, + { workspaceId }, + )) as any; + + expect(items.map((i: any) => i.id)).toContain(parent); + }); + + it('titleOnly does NOT match on text_content', async () => { + const pageId = await insertPage({ + title: 'Plain title', + textContent: 'body mentions the-secret-token here', + }); + + const withText = (await service.searchPage( + { query: 'the-secret-token', spaceId, substring: true } as any, + { workspaceId }, + )) as any; + expect(withText.items.map((i: any) => i.id)).toContain(pageId); + + const titleOnly = (await service.searchPage( + { + query: 'the-secret-token', + spaceId, + substring: true, + titleOnly: true, + } as any, + { workspaceId }, + )) as any; + expect(titleOnly.items.map((i: any) => i.id)).not.toContain(pageId); + }); + + // #443 Fix #1 regression: f_unaccent is NOT length-preserving, so an + // expanding char (ß→ss, …→...) BEFORE the match shifted the strpos position + // relative to the ORIGINAL text and the snippet slice ran past end → empty. + // The position and the slice now share the LOWER(f_unaccent(...)) space, so + // the window is aligned and always contains the matched (unaccented) token. + it('returns a populated snippet when an unaccent-EXPANDING char precedes the match', async () => { + // 300 × `ß` (each f_unaccent-expands to `ss`) before the needle. Under the + // old code strpos returned a position ~593 in the expanded space but the + // slice ran over the ORIGINAL (~360 char) text → empty snippet, match lost. + const prefix = 'ß'.repeat(300); + const pageId = await insertPage({ + title: 'Expanding-unaccent page', + textContent: `${prefix} needle-token-xyz trailing.`, + }); + + const { items } = (await service.searchPage( + { query: 'needle-token-xyz', spaceId, substring: true } as any, + { workspaceId }, + )) as any; + + const hit = items.find((i: any) => i.id === pageId); + expect(hit).toBeDefined(); + // Snippet must be non-empty AND contain the matched token (unaccented form). + expect(hit.snippet.length).toBeGreaterThan(0); + expect(hit.snippet).toContain('needle-token-xyz'); + }); + + // #443 Fix #2 regression: >200 matching pages for a broad substring, with + // exactly ONE exact-title hit. Without an ORDER BY on the 200-cap the exact + // hit could be among the arbitrarily-dropped rows; the ORDER BY keeps the + // strongest candidates so it must survive the cap and rank at the top. + it('keeps an exact-title hit through the 200-cap on a >200-row match set', async () => { + const isoSpace = (await createSpace(db, workspaceId)).id; + const svc = buildService({ userSpaceIds: [isoSpace] }); + + // 250 low-tier TEXT hits: the shared substring `capword` appears only in the + // body, never the title, so each is a TEXT-tier match (weakest tier). + for (let i = 0; i < 250; i++) { + await insertPage({ + title: `filler-page-${i}`, + textContent: `body contains capword here #${i}`, + spaceId: isoSpace, + }); + } + // Exactly one EXACT-title hit for the same query token. + const exact = await insertPage({ + title: 'capword', + textContent: 'unrelated body text', + spaceId: isoSpace, + }); + + const { items } = (await svc.searchPage( + { query: 'capword', spaceId: isoSpace, substring: true, limit: 10 } as any, + { workspaceId }, + )) as any; + + const ids = items.map((i: any) => i.id); + // The exact-title hit must survive the 200-cap and appear in the top `limit`. + expect(ids).toContain(exact); + // And, being TITLE_EXACT, it must be the single strongest hit. + expect(items[0].id).toBe(exact); + }); + + // #443 Fix #3: titleOnly matches only the title, so it must not leak the page + // body as the snippet (the old "first 300 chars of text_content" fallback). + it('titleOnly does NOT return a text-body snippet', async () => { + const pageId = await insertPage({ + title: 'titleonly-snippet-page', + textContent: 'SECRET-BODY-CONTENT-NOT-IN-TITLE that must not leak.', + }); + + const { items } = (await service.searchPage( + { + query: 'titleonly-snippet-page', + spaceId, + substring: true, + titleOnly: true, + } as any, + { workspaceId }, + )) as any; + + const hit = items.find((i: any) => i.id === pageId); + expect(hit).toBeDefined(); + // The body text must not appear in the snippet; titleOnly → empty snippet. + expect(hit.snippet).not.toContain('SECRET-BODY-CONTENT-NOT-IN-TITLE'); + expect(hit.snippet).toBe(''); + }); + + it('returns [] (not an error) for a query that matches nothing', async () => { + const { items } = (await service.searchPage( + { + query: 'zzz-no-such-string-anywhere-42', + spaceId, + substring: true, + } as any, + { workspaceId }, + )) as any; + expect(items).toEqual([]); + }); + + it('a `%` query does NOT match everything (LIKE metacharacter escaped)', async () => { + // Fresh space so we can assert on total counts without cross-test noise. + const isoSpace = (await createSpace(db, workspaceId)).id; + const svc = buildService({ userSpaceIds: [isoSpace] }); + await insertPage({ title: 'alpha', spaceId: isoSpace }); + await insertPage({ title: 'beta', spaceId: isoSpace }); + const literal = await insertPage({ + title: '100%-coverage', + spaceId: isoSpace, + }); + + const { items } = (await svc.searchPage( + { query: '%', spaceId: isoSpace, substring: true, limit: 50 } as any, + { workspaceId }, + )) as any; + + const ids = items.map((i: any) => i.id); + // `%` is a literal → matches only the page that actually contains '%'. + expect(ids).toContain(literal); + expect(ids).not.toContain( + items.find((i: any) => i.title === 'alpha')?.id, + ); + expect(items.length).toBe(1); + }); + + it('an `_` query does NOT match everything (LIKE metacharacter escaped)', async () => { + const isoSpace = (await createSpace(db, workspaceId)).id; + const svc = buildService({ userSpaceIds: [isoSpace] }); + await insertPage({ title: 'gamma', spaceId: isoSpace }); + const literal = await insertPage({ + title: 'snake_case_name', + spaceId: isoSpace, + }); + + const { items } = (await svc.searchPage( + { query: '_', spaceId: isoSpace, substring: true, limit: 50 } as any, + { workspaceId }, + )) as any; + + const ids = items.map((i: any) => i.id); + expect(ids).toContain(literal); + expect(items.length).toBe(1); + }); + + it('applies the permission post-filter to the MERGED set BEFORE the limit', async () => { + const isoSpace = (await createSpace(db, workspaceId)).id; + const keep = await insertPage({ + title: 'perm-visible-target', + spaceId: isoSpace, + }); + const hidden = await insertPage({ + title: 'perm-hidden-target', + spaceId: isoSpace, + }); + + // Authenticated (userId set) so the permission filter runs; only `keep` is + // accessible. limit 1 must NOT be able to select `hidden`. + const svc = buildService({ + userSpaceIds: [isoSpace], + accessibleIds: [keep], + }); + const { items } = (await svc.searchPage( + { + query: 'perm-', + spaceId: isoSpace, + substring: true, + limit: 1, + } as any, + { userId: 'user-1', workspaceId }, + )) as any; + + const ids = items.map((i: any) => i.id); + expect(ids).toContain(keep); + expect(ids).not.toContain(hidden); + }); + + it('web-UI path (no `substring` flag) keeps the legacy FTS response shape', async () => { + await insertPage({ + title: 'legacy shape page', + textContent: 'searchable legacyword content', + }); + + const { items } = (await service.searchPage( + { query: 'legacyword', spaceId } as any, + { userId: 'user-1', workspaceId }, + )) as any; + + // Legacy hits carry rank + highlight + space, and NO path/snippet/score. + const hit = items[0]; + expect(hit).toBeDefined(); + expect('rank' in hit).toBe(true); + expect('highlight' in hit).toBe(true); + expect('path' in hit).toBe(false); + expect('snippet' in hit).toBe(false); + expect('score' in hit).toBe(false); + }); +}); diff --git a/packages/mcp/src/client.ts b/packages/mcp/src/client.ts index e1ac7fa2..1ebb725c 100644 --- a/packages/mcp/src/client.ts +++ b/packages/mcp/src/client.ts @@ -2830,14 +2830,28 @@ export class DocmostClient { return { success: true, removedShareId: share.shareId, pageId }; } - async search(query: string, spaceId?: string, limit?: number) { + async search( + query: string, + spaceId?: string, + limit?: number, + opts: { parentPageId?: string; titleOnly?: boolean } = {}, + ) { await this.ensureAuthenticated(); - const payload: Record = { query, spaceId }; - // Clamp an optional caller-supplied limit into a sane 1..100 range before - // forwarding it to the server; omit it entirely when not provided so the - // server applies its own default. + // Opt into the #443 agent-lookup mode: `substring: true` turns on the hybrid + // substring + FTS branch that returns path + snippet + score. A stock + // upstream server strips these unknown DTO fields (whitelist:true) and + // silently degrades to plain FTS — see the tool-registration comment. + const payload: Record = { + query, + spaceId, + substring: true, + }; + if (opts.parentPageId) payload.parentPageId = opts.parentPageId; + if (opts.titleOnly) payload.titleOnly = true; + // Clamp an optional caller-supplied limit into the lookup range (1..50) + // before forwarding; omit it when not provided so the server default applies. if (limit !== undefined) { - payload.limit = Math.max(1, Math.min(100, limit)); + payload.limit = Math.max(1, Math.min(50, limit)); } const response = await this.client.post("/search", payload); diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index c2a69446..88c5b305 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -435,30 +435,63 @@ server.registerTool( // Tool: search // INTENTIONAL per-transport divergence (not shared): the in-app `searchPages` // runs a semantic + keyword hybrid (RRF) with in-process access control and a -// different schema (limit 1-20); this transport is a plain REST full-text search -// (limit up to 100). Different behaviour AND schema, so kept per-layer. +// different schema; this transport is the #443 agent-lookup search — a hybrid +// substring + full-text search that also returns each hit's location (`path`) +// and a windowed `snippet`, so one call answers "where is it and what's in it". +// The in-app hybrid-RRF search is deliberately NOT touched. Different behaviour +// AND schema, so kept per-layer. +// +// STANDALONE-vs-STOCK-UPSTREAM: the client sends the opt-in `substring`/ +// `parentPageId`/`titleOnly` DTO fields. A stock upstream server validates the +// DTO with `whitelist: true` and silently strips these unknown fields, so the +// request degrades gracefully to plain FTS (no path/snippet, current shape). +// +// EE/TYPESENSE DEGRADATION (#443): on an instance whose SEARCH_DRIVER is +// `typesense`, the server routes this request to the Typesense backend, which +// does NOT implement agent-lookup — the substring/path/snippet/tiering is +// ignored and the response degrades to plain Typesense FTS. The rich lookup +// shape is only produced by the native Postgres search driver. server.registerTool( "search", { description: - "Full-text search for pages and content across the whole workspace. " + - "Results are bounded by `limit` (1-100; when omitted the server applies " + - "its own default).", + "Find pages by a fragment of a technical string (hostnames, IPs, IDs " + + "like `srv.local`, `10.0.12`, `WB-MGE-30D86B`) — one call returns each " + + "hit's location (`path`: ancestor titles root→parent) and a `snippet` " + + "around the first match, so you rarely need a follow-up get_page. " + + "Matches substrings literally (dots/dashes/digits are not tokenized) as " + + "well as full-text. Returns `{ pageId, title, path, snippet, score }` " + + "sorted by `score` (a per-response relevance float).", inputSchema: { query: z.string().min(1).describe("Search query"), + spaceId: z + .string() + .optional() + .describe("Restrict the search to a single space"), + parentPageId: z + .string() + .optional() + .describe( + "Restrict to a page and all its descendants (the page itself included)", + ), + titleOnly: z + .boolean() + .optional() + .describe("Match page titles only; skip page text"), limit: z .number() .int() .min(1) - .max(100) + .max(50) .optional() - .describe("Max results to return (max 100)"), + .describe("Max results to return (1-50, default 10)"), }, }, - async ({ query, limit }) => { - // The tool exposes no spaceId filter, so pass undefined for the client's - // optional spaceId parameter and forward limit into its correct slot. - const result = await docmostClient.search(query, undefined, limit); + async ({ query, spaceId, parentPageId, titleOnly, limit }) => { + const result = await docmostClient.search(query, spaceId, limit, { + parentPageId, + titleOnly, + }); return jsonContent(result); }, ); diff --git a/packages/mcp/src/lib/filters.ts b/packages/mcp/src/lib/filters.ts index c789ec3a..1f106387 100644 --- a/packages/mcp/src/lib/filters.ts +++ b/packages/mcp/src/lib/filters.ts @@ -83,16 +83,32 @@ export function filterComment(comment: any, markdownContent?: string) { }; } +// Map one server search hit to the MCP output contract (#443): +// { pageId, title, path, snippet, score } +// +// INVARIANT: the only page identifier exposed is `pageId` (the server `id` +// UUID). The server also carries `slugId` — it is NEVER surfaced. +// +// GRACEFUL DEGRADATION: against a stock upstream server the opt-in lookup DTO +// fields are stripped, so the response is the legacy FTS shape (no path/snippet/ +// score, a `highlight` + `rank` instead). We synthesize the contract from +// whatever is present: `snippet` falls back to the FTS `highlight`, `score` to +// the FTS `rank`, and `path` to [] (upstream has no path). This keeps the tool +// usable even when the server has not been upgraded. export function filterSearchResult(result: any) { return { - id: result.id, + pageId: result.id, title: result.title, - parentPageId: result.parentPageId, - createdAt: result.createdAt, - updatedAt: result.updatedAt, - rank: result.rank, - highlight: result.highlight, - spaceId: result.space?.id, - spaceName: result.space?.name, + path: Array.isArray(result.path) ? result.path : [], + snippet: + typeof result.snippet === "string" + ? result.snippet + : (result.highlight ?? ""), + score: + typeof result.score === "number" + ? result.score + : typeof result.rank === "number" + ? result.rank + : 0, }; } diff --git a/packages/mcp/src/server-instructions.ts b/packages/mcp/src/server-instructions.ts index 95e3f570..f79a330b 100644 --- a/packages/mcp/src/server-instructions.ts +++ b/packages/mcp/src/server-instructions.ts @@ -40,7 +40,7 @@ import { SHARED_TOOL_SPECS, SharedToolSpec } from "./tool-specs.js"; */ export const ROUTING_PROSE = "Docmost editing guide — choose the tool by intent. The at the end lists every tool with a one-line purpose; the notes below are the routing hints for WHEN to reach for each.\n" + - "READ: find a page -> search (workspace-wide full-text); list -> listPages / listSpaces. Locate blocks and their ids CHEAPLY -> getOutline (compact top-level map; start here, not getPageJson). One block, for editing -> getNode (by attrs.id, or \"#\" for tables, which carry no id) — returns MARKDOWN by default (comment anchors kept for safe write-back); pass format:\"json\" for the raw ProseMirror subtree. Find every occurrence of a string/regex ON a page (and where each is) -> searchInPage, NOT block-by-block getNode — it returns each hit's node ref + block index + context for a targeted comment. Whole page -> getPage (Markdown, canonical for text; drops only block ids, resolved-comment anchors, and a fixed no-md-representation attr set: table spans/colwidth/bg, indent, callout.icon, orderedList.type, link internal/target/rel/class; inline tags are comment anchors — markup, not text) or getPageJson (full ProseMirror with block ids, for those dropped attrs). Hand a huge page (with images) to an external consumer without pulling it through the model context -> stashPage (returns a short-lived anonymous URL).\n" + + "READ: find a page by a fragment of a technical string (hostname/IP/ID like srv.local, 10.0.12, WB-MGE-30D86B) -> search — hybrid substring + full-text, returns each hit's location (path: root->parent titles) and a snippet around the match, so you rarely need a follow-up getPage; scope with spaceId or parentPageId (a subtree), titleOnly to match titles only. list -> listPages / listSpaces. Locate blocks and their ids CHEAPLY -> getOutline (compact top-level map; start here, not getPageJson). One block, for editing -> getNode (by attrs.id, or \"#\" for tables, which carry no id) — returns MARKDOWN by default (comment anchors kept for safe write-back); pass format:\"json\" for the raw ProseMirror subtree. Find every occurrence of a string/regex ON a page (and where each is) -> searchInPage, NOT block-by-block getNode — it returns each hit's node ref + block index + context for a targeted comment. Whole page -> getPage (Markdown, canonical for text; drops only block ids, resolved-comment anchors, and a fixed no-md-representation attr set: table spans/colwidth/bg, indent, callout.icon, orderedList.type, link internal/target/rel/class; inline tags are comment anchors — markup, not text) or getPageJson (full ProseMirror with block ids, for those dropped attrs). Hand a huge page (with images) to an external consumer without pulling it through the model context -> stashPage (returns a short-lived anonymous URL).\n" + "EDIT: fix wording/typos/numbers -> editPageText (find/replace inside blocks, no node id needed). Edit a block -> getNode(markdown) -> edit the markdown -> patchNode(markdown) (by attrs.id from getOutline; the markdown fragment may be several blocks — a 1->N section rewrite in one call, the first block keeps the id). Reach for patchNode's `node`-JSON only for fine attr/mark work; a table cell with spans/colors/fixed width -> the table tools (patchNode markdown refuses it). Add a block -> insertNode (markdown, before/after a block by attrs.id or by anchor text, or append; `node` for raw JSON or bare table structure). Remove a block -> deleteNode (by attrs.id). Tables -> tableGet / tableUpdateCell / tableInsertRow / tableDeleteRow (address by \"#\" from getOutline; table nodes have no attrs.id). Images -> insertImage (add from a web URL) / replaceImage (swap an existing image). Draw.io diagrams -> drawioCreate (create from mxGraph XML and insert), drawioGet (read a diagram as mxGraph XML + a hash), drawioUpdate (replace a diagram; pass the hash from drawioGet as baseHash for optimistic locking); before authoring a diagram, drawioShapes (look up verified stencil style-strings so a shape name never renders as an empty box) and drawioGuide (on-demand authoring reference: skeleton/layout/containers/icons-aws/icons-azure), and pass layout:\"elk\" to drawioCreate/drawioUpdate to auto-place nodes. Footnotes -> insertFootnote. Bulk/structural rewrite -> updatePageJson (full ProseMirror replace) or updatePageMarkdown (full plain-Markdown body replace, re-imported — block ids regenerate); prefer the granular tools above to avoid resending the whole ~100KB+ document. Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmostTransform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" + "PAGES: new -> createPage (Markdown). Rename (title only) -> renamePage. Move -> movePage. Delete -> deletePage (SOFT delete — the page goes to trash and is restorable; nothing is permanent). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copyPageContent. Sharing -> sharePage / unsharePage / listShares; sharePage makes the page PUBLICLY accessible — do it only when explicitly asked.\n" + "COMMENTS: createComment is always inline and requires an EXACT selection — contiguous text from a single block, <=250 chars (fails rather than leaving an unanchored comment); reply to a thread via parentCommentId. Propose a concrete text fix for one-click human approval -> createComment with suggestedText (the exact plain-text replacement for the selection; the selection must then be UNIQUE in the page — extend it with context if needed); prefer this over editing directly when the change is subjective or needs the author's sign-off. Manage -> listComments, updateComment, resolveComment (resolve/reopen, reversible — prefer over delete to close), deleteComment, checkNewComments.\n" + @@ -152,7 +152,7 @@ export const INLINE_MCP_INVENTORY: ToolInventoryLine[] = [ { name: "search", purpose: - "full-text search for pages and content across the whole workspace.", + "find pages by a fragment of a technical string (hybrid substring + full-text); returns each hit's path and a snippet.", }, { name: "docmostTransform", diff --git a/packages/mcp/test/unit/filters.test.mjs b/packages/mcp/test/unit/filters.test.mjs index 84d56812..100f8ee1 100644 --- a/packages/mcp/test/unit/filters.test.mjs +++ b/packages/mcp/test/unit/filters.test.mjs @@ -1,7 +1,11 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { filterComment, filterPage } from "../../build/lib/filters.js"; +import { + filterComment, + filterPage, + filterSearchResult, +} from "../../build/lib/filters.js"; test("filterComment includes resolvedAt/resolvedById as null when absent", () => { const result = filterComment({ @@ -171,3 +175,91 @@ test("filterPage includes both content and subpages together", () => { assert.equal(result.content, "body"); assert.deepEqual(result.subpages, [{ id: "s1", title: "Sub" }]); }); + +// --- filterSearchResult (#443 agent-lookup contract) ------------------------- + +test("filterSearchResult maps the lookup shape to {pageId,title,path,snippet,score}", () => { + const result = filterSearchResult({ + id: "0199aa-uuid", + slugId: "slug-secret", + title: "backup-srv.local", + parentPageId: "0199pp", + path: ["Infrastructure", "Datacenter A", "Servers"], + snippet: "…IP: 10.0.12.5. Debian 12…", + score: 0.92, + }); + + assert.deepEqual(result, { + pageId: "0199aa-uuid", + title: "backup-srv.local", + path: ["Infrastructure", "Datacenter A", "Servers"], + snippet: "…IP: 10.0.12.5. Debian 12…", + score: 0.92, + }); +}); + +test("filterSearchResult NEVER exposes slugId (pageId is the only identifier)", () => { + const result = filterSearchResult({ + id: "uuid-1", + slugId: "slug-1", + title: "t", + path: [], + snippet: "s", + score: 0.1, + }); + assert.equal("slugId" in result, false); + assert.equal("id" in result, false); + assert.equal(result.pageId, "uuid-1"); +}); + +test("filterSearchResult root page yields path: []", () => { + const result = filterSearchResult({ + id: "uuid-root", + title: "Root", + path: [], + snippet: "s", + score: 0.5, + }); + assert.deepEqual(result.path, []); +}); + +test("filterSearchResult degrades a legacy FTS hit (no lookup fields)", () => { + // Stock upstream stripped the opt-in DTO fields → legacy shape with + // highlight + rank and no path/snippet/score. + const result = filterSearchResult({ + id: "uuid-legacy", + slugId: "slug-legacy", + title: "Legacy", + parentPageId: null, + rank: 0.37, + highlight: "…matched text…", + space: { id: "sp1", name: "Space" }, + }); + + assert.equal(result.pageId, "uuid-legacy"); + assert.equal(result.title, "Legacy"); + // snippet falls back to highlight, score to rank, path to []. + assert.equal(result.snippet, "…matched text…"); + assert.equal(result.score, 0.37); + assert.deepEqual(result.path, []); + assert.equal("slugId" in result, false); +}); + +test("filterSearchResult is null-safe on missing snippet/score/path", () => { + const result = filterSearchResult({ id: "u", title: "t" }); + assert.equal(result.pageId, "u"); + assert.equal(result.snippet, ""); + assert.equal(result.score, 0); + assert.deepEqual(result.path, []); +}); + +test("filterSearchResult ignores a non-array path", () => { + const result = filterSearchResult({ + id: "u", + title: "t", + path: "not-an-array", + snippet: "s", + score: 1, + }); + assert.deepEqual(result.path, []); +}); From f794ac6d6ca16d22288a5ed11ddf763abe1b986a Mon Sep 17 00:00:00 2001 From: agent_coder Date: Fri, 10 Jul 2026 21:57:16 +0300 Subject: [PATCH 2/2] =?UTF-8?q?fix(search):=20=D0=BE=D0=B6=D0=B8=D0=B2?= =?UTF-8?q?=D0=B8=D1=82=D1=8C=20trgm-=D0=B8=D0=BD=D0=B4=D0=B5=D0=BA=D1=81?= =?UTF-8?q?=20=E2=80=94=20=D1=83=D0=B1=D1=80=D0=B0=D1=82=D1=8C=20coalesce?= =?UTF-8?q?=20=D0=B8=D0=B7=20LIKE-=D0=BF=D1=80=D0=B5=D0=B4=D0=B8=D0=BA?= =?UTF-8?q?=D0=B0=D1=82=D0=BE=D0=B2=20(#443,=20=D1=80=D0=B5=D0=B2=D1=8C?= =?UTF-8?q?=D1=8E)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ревьюер поймал EXPLAIN'ом: lookup-query фильтровал по LOWER(f_unaccent(coalesce(col,''))), а GIN-trgm-индексы — на coalesce-free LOWER(f_unaccent(col)). Postgres берёт функциональный индекс только при точном совпадении выражения, так что обёртка coalesce отключала индекс → Seq Scan по pages на КАЖДЫЙ lookup (MCP-клиент всегда шлёт substring:true). Зелёный int-гейт это не ловил (проверял корректность на крошечном датасете, не использование индекса). - search.service.ts: убрал coalesce из двух index-driving LIKE-предикатов (title + text_content). Семантически эквивалентно: NULL LIKE '%q%' = NULL (falsy), NULL-страница просто не матчится — как пустая строка не матчит %q%. SELECT/ORDER BY оставлены с coalesce (индекс не выбирают, Node гардит NULL). - Миграция: убран избыточный ре-ассерт idx_pages_title_trgm — его создаёт #348 на том же coalesce-free выражении, теперь title-предикат его использует. idx_pages_text_content_trgm без изменений (уже coalesce-free), поправлены вводящие в заблуждение комментарии. down() дропает только text-индекс. - Новый тест search-lookup-explain.int-spec.ts (3): title→idx_pages_title_trgm, text→idx_pages_text_content_trgm (оба ассертят отсутствие Seq Scan on pages), + негативный контроль (coalesce-обёртка не может использовать индекс) против тихой ре-регрессии. Дискриминатор enable_seqscan=off. - CHANGELOG: две записи (opt-in substring lookup mode + смена shape MCP search). EXPLAIN на реальном PG: обе fixed-ветки → Bitmap Index Scan on trgm; buggy coalesce → Seq Scan (Disabled:true). Гейт: server jest src/core/search 27/27 (16 исходных int без изменений + 3 EXPLAIN + 8 unit), mcp node --test 708/708, tsc чисто. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 16 +++ apps/server/src/core/search/search.service.ts | 14 +- .../20260706T120000-search-lookup-trgm.ts | 25 ++-- .../search-lookup-explain.int-spec.ts | 123 ++++++++++++++++++ 4 files changed, 163 insertions(+), 15 deletions(-) create mode 100644 apps/server/test/integration/search-lookup-explain.int-spec.ts 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); + }); +});