From a4ea22a7f5181b3a03a6a0a9fcb4fe3498a50470 Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 12 Jul 2026 02:00:25 +0300 Subject: [PATCH] =?UTF-8?q?feat(search):=20A2-A9=20=E2=80=94=20unified=20O?= =?UTF-8?q?R-relevance=20engine,=20RRF,=20pagination=20(#529)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrite SearchService.searchPage into ONE engine for web-UI, MCP agent and share: - A2 server-side query parser (search-query-parser.ts): quote-aware tokenizer, leading +/- operators (internal -,.,: literal), "phrase", metachar-stripped bare terms; tsquery built as a parameterized AST via SQL ||/&&/!! (never string-concat). only-negation/empty short-circuit. - A3 match=auto routes identifier-like terms (10.31.41, esp32, WB-MGE-30D86B) to the substring/trigram branch, words to FTS; word/ prefix/substring overrides. - A4 RRF (k=60) fuses the FTS branch (ts_rank_cd) and substring branch (title-exact>title-sub>text tier) by RANK; ORDER BY rrf DESC, id. - A5 exact permission-filtered total (fail-closed via filterAccessiblePageIds with the #348 hasRestricted fast-path), CANDIDATE_CAP fusion window, offset/limit, hasMore, truncatedAtCap. - A6 single path (spaceId/share/creatorId/titleOnly/parentPageId/match); share uses getPageAndDescendantsExcludingRestricted. - A7 response superset per hit (id/pageId/slugId/icon/title/space/…/rank/ highlight/snippet/path/score/matchedFields/matchedTerms). - A8 buildAncestorPaths now skips deleted + cross-space ancestors. - A9 DTOs: match, offset, total/hasMore/truncatedAtCap/query/matchedFields. SEARCH_MODE=or|and toggles the parser; SEARCH_CANDIDATE_CAP tunes the window. Legacy lookup unit/int specs replaced by parser unit tests + a 13-criteria integration spec on real pg (incl. a permission mutation guard and a fail-closed propagation test). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/search/dto/search-response.dto.ts | 69 +- apps/server/src/core/search/dto/search.dto.ts | 8 + .../core/search/search-query-parser.spec.ts | 115 +++ .../src/core/search/search-query-parser.ts | 224 +++++ .../core/search/search.service.lookup.spec.ts | 71 +- .../search/search.service.query-mode.spec.ts | 217 ++-- apps/server/src/core/search/search.service.ts | 948 ++++++++++-------- .../integration/search-lexical.int-spec.ts | 388 +++++++ .../search-lookup-explain.int-spec.ts | 123 --- .../integration/search-lookup.int-spec.ts | 462 --------- 10 files changed, 1355 insertions(+), 1270 deletions(-) create mode 100644 apps/server/src/core/search/search-query-parser.spec.ts create mode 100644 apps/server/src/core/search/search-query-parser.ts create mode 100644 apps/server/test/integration/search-lexical.int-spec.ts delete mode 100644 apps/server/test/integration/search-lookup-explain.int-spec.ts delete 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 e88356ad..b9460953 100644 --- a/apps/server/src/core/search/dto/search-response.dto.ts +++ b/apps/server/src/core/search/dto/search-response.dto.ts @@ -1,33 +1,56 @@ import { Space } from '@docmost/db/types/entity.types'; -export class SearchResponseDto { +// #529 A7 — the single per-hit SUPERSET returned by the unified search engine. +// The web-UI reads id/highlight/icon/space/title/…; the MCP agent maps id→pageId +// and reads snippet/score/path. `rank`/`highlight` are null for substring-only +// hits (the web already falls back). Nothing the legacy web response carried is +// dropped. +export class SearchResultDto { id: string; - title: string; + // Alias of `id` for the MCP layer (it addresses pages by pageId). + pageId: string; + slugId: string; icon: string; - parentPageId: string; + title: string; + space?: Partial; creatorId: string; - rank: number; - highlight: string; createdAt: Date; updatedAt: Date; - space: Partial; + // ts_rank_cd of the FTS branch; null for substring-only hits. + rank: number | null; + // ts_headline marked HTML; null for substring-only hits. + highlight: string | null; + // Plain windowed snippet around the match (empty for titleOnly). + snippet: string; + // Ancestor titles root → direct parent ([] for a root page). + path: string[]; + // Per-response ordering proxy (falls back to rank). + score: number; + // Which fields matched: 'title' and/or 'text'. + matchedFields: string[]; + // Which parsed positive/required terms this hit matched. + matchedTerms: string[]; } -// 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; +// The paginated envelope (A5). `total` is the EXACT permission-filtered count of +// pages matching the positive lexical query (fail-closed). `hasMore` is true when +// more results exist WITHIN the fusion window; `truncatedAtCap` signals the match +// set exceeded CANDIDATE_CAP and the tail is unreachable by pagination. +export class SearchResponseDto { + items: SearchResultDto[]; + total: number; + hasMore: boolean; + truncatedAtCap: boolean; + offset: number; + query: { + raw: string; + parsed: { + positive: string[]; + required: string[]; + excluded: string[]; + reason?: string; + }; + mode: 'or' | 'and'; + match: string; + }; } diff --git a/apps/server/src/core/search/dto/search.dto.ts b/apps/server/src/core/search/dto/search.dto.ts index dd0c0a1b..945c1dd4 100644 --- a/apps/server/src/core/search/dto/search.dto.ts +++ b/apps/server/src/core/search/dto/search.dto.ts @@ -1,5 +1,6 @@ import { IsBoolean, + IsIn, IsNotEmpty, IsNumber, IsOptional, @@ -11,6 +12,13 @@ export class SearchDTO { @IsString() query: string; + // #529 A3 — match mode. `auto` (default) routes identifier-like terms + // (10.31.41, esp32, WB-MGE-30D86B) to the substring/trigram branch and words + // to full-text; `word`/`prefix`/`substring` are explicit overrides. + @IsOptional() + @IsIn(['auto', 'word', 'prefix', 'substring']) + match?: 'auto' | 'word' | 'prefix' | 'substring'; + @IsOptional() @IsString() spaceId: string; diff --git a/apps/server/src/core/search/search-query-parser.spec.ts b/apps/server/src/core/search/search-query-parser.spec.ts new file mode 100644 index 00000000..098be4e2 --- /dev/null +++ b/apps/server/src/core/search/search-query-parser.spec.ts @@ -0,0 +1,115 @@ +import { parseSearchQuery, hasPositiveRecall } from './search-query-parser'; + +describe('parseSearchQuery — tokenization & operators (A2)', () => { + it('splits on whitespace into positive terms (OR recall)', () => { + const p = parseSearchQuery('стамбул роснефть'); + expect(p.positive.map((t) => t.text)).toEqual(['стамбул', 'роснефть']); + expect(p.required).toEqual([]); + expect(p.excluded).toEqual([]); + expect(p.mode).toBe('or'); + }); + + it('treats +term as required and -term as excluded', () => { + const p = parseSearchQuery('+кофейня -архив'); + expect(p.required.map((t) => t.text)).toEqual(['кофейня']); + expect(p.excluded.map((t) => t.text)).toEqual(['архив']); + expect(p.positive).toEqual([]); + }); + + it('keeps a leading-operator-free hyphen/dot/colon token as ONE literal term', () => { + // WB-MGE-30D86B, 10.0.12.5, a:b — internal -,.,: are literal, one term each. + expect(parseSearchQuery('WB-MGE-30D86B').positive[0].text).toBe( + 'WB-MGE-30D86B', + ); + expect(parseSearchQuery('10.0.12.5').positive[0].text).toBe('10.0.12.5'); + expect(parseSearchQuery('host:8080').positive[0].text).toBe('host:8080'); + }); + + it('only a LEADING +/- is an operator; -архив excludes архив', () => { + const p = parseSearchQuery('-архив'); + expect(p.excluded.map((t) => t.text)).toEqual(['архив']); + expect(p.positive).toEqual([]); + }); + + it('drops a bare "-" / "+" and an all-operator remainder', () => { + const p = parseSearchQuery('- + foo -- ++'); + expect(p.positive.map((t) => t.text)).toEqual(['foo']); + expect(p.required).toEqual([]); + expect(p.excluded).toEqual([]); + }); + + it('parses a quoted phrase as one adjacency term', () => { + const p = parseSearchQuery('"воздушный шар" кофе'); + expect(p.positive[0]).toMatchObject({ text: 'воздушный шар', branch: 'phrase' }); + expect(p.positive[1].text).toBe('кофе'); + }); + + it('applies +/- to a phrase', () => { + const req = parseSearchQuery('+"воздушный шар"'); + expect(req.required[0]).toMatchObject({ text: 'воздушный шар', branch: 'phrase' }); + const exc = parseSearchQuery('-"воздушный шар"'); + expect(exc.excluded[0]).toMatchObject({ text: 'воздушный шар', branch: 'phrase' }); + }); + + it('drops an unbalanced quote token', () => { + const p = parseSearchQuery('kafka "unclosed here'); + expect(p.positive.map((t) => t.text)).toEqual(['kafka']); + }); + + it('strips tsquery metacharacters from a bare FTS term (no 500)', () => { + const p = parseSearchQuery('foo|bar'); + // `|` is not an identifier signal → FTS branch; the metachar is stripped so + // the term becomes the two words that survive. + expect(p.positive[0].branch === 'fts' || p.positive[0].branch === 'ftsPrefix').toBe(true); + expect(p.positive[0].text).toBe('foo bar'); + }); +}); + +describe('parseSearchQuery — match=auto classification (A3)', () => { + it('routes identifier-like terms to the substring branch', () => { + expect(parseSearchQuery('10.31.41').positive[0].branch).toBe('substring'); + expect(parseSearchQuery('esp32').positive[0].branch).toBe('substring'); + expect(parseSearchQuery('WB-MGE-30D86B').positive[0].branch).toBe('substring'); + }); + + it('routes purely-alphabetic words to the FTS (prefix) branch', () => { + expect(parseSearchQuery('печат').positive[0].branch).toBe('ftsPrefix'); + expect(parseSearchQuery('ресторан').positive[0].branch).toBe('ftsPrefix'); + }); + + it('explicit match overrides: word / prefix / substring', () => { + expect(parseSearchQuery('печат', { match: 'word' }).positive[0].branch).toBe('fts'); + expect(parseSearchQuery('печат', { match: 'prefix' }).positive[0].branch).toBe( + 'ftsPrefix', + ); + expect(parseSearchQuery('печат', { match: 'substring' }).positive[0].branch).toBe( + 'substring', + ); + }); +}); + +describe('parseSearchQuery — reasons & recall', () => { + it('only-negation yields reason only-negation and no positive recall', () => { + const p = parseSearchQuery('-архив'); + expect(p.reason).toBe('only-negation'); + expect(hasPositiveRecall(p)).toBe(false); + }); + + it('empty / whitespace / garbage yields reason empty', () => { + expect(parseSearchQuery('').reason).toBe('empty'); + expect(parseSearchQuery(' ').reason).toBe('empty'); + // A bare operator drops to nothing → empty (no exclusion survived). + expect(parseSearchQuery('+ -').reason).toBe('empty'); + }); + + it('a required term alone IS positive recall (no reason)', () => { + const p = parseSearchQuery('+кофейня'); + expect(p.reason).toBeUndefined(); + expect(hasPositiveRecall(p)).toBe(true); + }); + + it('mode flag flows through', () => { + expect(parseSearchQuery('a b', { mode: 'and' }).mode).toBe('and'); + expect(parseSearchQuery('a b').mode).toBe('or'); + }); +}); diff --git a/apps/server/src/core/search/search-query-parser.ts b/apps/server/src/core/search/search-query-parser.ts new file mode 100644 index 00000000..1d325a34 --- /dev/null +++ b/apps/server/src/core/search/search-query-parser.ts @@ -0,0 +1,224 @@ +// #529 Phase A — server-side query parser (the SINGLE source of query semantics). +// +// Clients (web-UI, MCP agent, public share) send a RAW query string plus flags +// (`match`, `mode`); ALL query interpretation happens HERE, so every consumer +// gets identical operator/phrase/morphology behaviour. The parser is PURE (no +// SQL, no DB) so it is exhaustively unit-testable; SearchService turns the parsed +// AST into a parameterized tsquery/predicate tree (never string-concatenated SQL). +// +// Grammar (A2): +// - Whitespace splits tokens, but a double-quoted run is ONE token ("a b" is a +// phrase). An unbalanced quote is dropped. +// - A token is an OPERATOR token only when it STARTS with `+` or `-`, the +// remainder is non-empty, and the remainder is not itself only operators. +// `+`/`-` inside a token (`WB-MGE-30D86B`, `10.0.12.5`, `a:b`) is a LITERAL — +// the token is a single term. A bare `-`/`+` is dropped. +// - `"phrase"` → phrase term; `+"phrase"`/`-"phrase"` apply the operator to it. +// - Positive terms (bare + phrase, no operator) form the OR recall set. +// `+term` is a REQUIRED predicate, `-term` an EXCLUDED predicate (A2): both +// are applied in SQL WHERE against the whole candidate set, not folded into +// the positive tsquery. +// - Only-negation (no positive term) short-circuits to an empty result with +// reason `only-negation` (never runs a costly NOT-scan). + +export type SearchMatchMode = 'auto' | 'word' | 'prefix' | 'substring'; +export type SearchBooleanMode = 'or' | 'and'; + +// How a single term is matched against the index. +// - 'fts' : full-text lexeme, exact (no trailing prefix). +// - 'ftsPrefix' : full-text lexeme with a `:*` prefix match. +// - 'phrase' : an adjacency phrase (phraseto_tsquery). +// - 'substring' : a literal LOWER(f_unaccent(col)) LIKE '%needle%' branch +// (identifiers the tokenizer mangles: IPs, hostnames, IDs). +export type SearchTermBranch = 'fts' | 'ftsPrefix' | 'phrase' | 'substring'; + +export interface ParsedTerm { + // The user-visible term text, operator stripped, quotes removed. This is what + // `matchedTerms` echoes back per hit. + text: string; + branch: SearchTermBranch; +} + +export interface ParsedQuery { + raw: string; + // OR-recall set (bare + phrase terms with no operator). + positive: ParsedTerm[]; + // AND predicates (`+term`) — the candidate MUST match each of these. + required: ParsedTerm[]; + // NOT predicates (`-term`) — the candidate must match NONE of these. + excluded: ParsedTerm[]; + mode: SearchBooleanMode; + // Set only when the query yields no positive recall: 'empty' (nothing usable) + // or 'only-negation' (there were exclusions but no positive term). + reason?: 'empty' | 'only-negation'; +} + +interface RawToken { + op: '' | '+' | '-'; + kind: 'word' | 'phrase'; + text: string; +} + +// tsquery metacharacters that must never reach to_tsquery from a bare term — they +// are what turned adversarial input into a 500 before (#139). Stripped for the FTS +// branch; the substring branch keeps them (they are literal there). +const TSQUERY_META = /[:&|!()*<>\\]+/g; + +// A term is "identifier-like" when it carries a digit or one of . _ : / - AND is +// not purely alphabetic (letters only). Such tokens (10.31.41, esp32, +// WB-MGE-30D86B) are mangled by the FTS tokenizer, so `match: auto` routes them +// to the substring branch. A purely-alphabetic word (печат, ресторан) stays FTS. +const IDENTIFIER_SIGNAL = /[0-9._:/\\-]/; +const PURELY_ALPHA = /^\p{L}+$/u; + +function isIdentifierLike(text: string): boolean { + return IDENTIFIER_SIGNAL.test(text) && !PURELY_ALPHA.test(text); +} + +// Clean a bare term for the FTS branch: NFC-normalize, drop tsquery metacharacters +// and collapse whitespace. Returns '' when nothing usable remains. +export function cleanFtsLexeme(raw: string): string { + return (raw ?? '') + .normalize('NFC') + .replace(TSQUERY_META, ' ') + .replace(/\s+/g, ' ') + .trim(); +} + +// Split a raw query into tokens, honouring double quotes and a single leading +// +/- operator. Unbalanced quotes and bare operators are dropped. +function tokenize(raw: string): RawToken[] { + const tokens: RawToken[] = []; + const s = raw ?? ''; + let i = 0; + const n = s.length; + + const isSpace = (c: string) => /\s/.test(c); + + while (i < n) { + // Skip leading whitespace. + while (i < n && isSpace(s[i])) i++; + if (i >= n) break; + + let op: '' | '+' | '-' = ''; + // A single leading +/- is a tentative operator. Only ONE leading operator is + // consumed; a second (`--x`) leaves `-x` as the remainder (a literal dash). + if (s[i] === '+' || s[i] === '-') { + op = s[i] as '+' | '-'; + i++; + } + + if (i < n && s[i] === '"') { + // Quoted phrase: read until the closing quote. + const close = s.indexOf('"', i + 1); + if (close === -1) { + // Unbalanced quote → drop this token and everything the open quote would + // have consumed (the rest of the string). + break; + } + const phrase = s.slice(i + 1, close); + i = close + 1; + if (phrase.trim().length > 0) { + tokens.push({ op, kind: 'phrase', text: phrase.trim() }); + } + continue; + } + + // Bare word: read until the next whitespace. + let j = i; + while (j < n && !isSpace(s[j])) j++; + const word = s.slice(i, j); + i = j; + + // A bare operator (`-`/`+` with no remainder) or an all-operator remainder is + // dropped. + if (word.length === 0) continue; + if (op && /^[+-]+$/.test(word)) continue; + + tokens.push({ op, kind: 'word', text: word }); + } + + return tokens; +} + +// Resolve the match branch for a single term given the global match mode. +function branchForTerm( + text: string, + kind: 'word' | 'phrase', + mode: SearchMatchMode, +): SearchTermBranch { + if (kind === 'phrase') return 'phrase'; + switch (mode) { + case 'word': + return 'fts'; + case 'prefix': + return 'ftsPrefix'; + case 'substring': + return 'substring'; + case 'auto': + default: + // Identifiers the tokenizer mangles go to substring; words get a prefix + // FTS match (so `печат` still finds `печатать`, but `печат` no longer drags + // in `впечатления` because the russian stemmer anchors the stem). + return isIdentifierLike(text) ? 'substring' : 'ftsPrefix'; + } +} + +/** + * Parse a raw user query + flags into a structured, SQL-agnostic AST. + * Pure and total: never throws, always returns a ParsedQuery. + */ +export function parseSearchQuery( + raw: string, + opts: { match?: SearchMatchMode; mode?: SearchBooleanMode } = {}, +): ParsedQuery { + const match: SearchMatchMode = opts.match ?? 'auto'; + const mode: SearchBooleanMode = opts.mode ?? 'or'; + + const positive: ParsedTerm[] = []; + const required: ParsedTerm[] = []; + const excluded: ParsedTerm[] = []; + + for (const tok of tokenize(raw)) { + // For an FTS branch, the token must survive metacharacter cleaning; for the + // substring/phrase branch the literal text is used. A term that cleans to + // nothing AND is not usable as a substring is dropped. + const branch = branchForTerm(tok.text, tok.kind, match); + + let usableText: string; + if (branch === 'fts' || branch === 'ftsPrefix') { + usableText = cleanFtsLexeme(tok.text); + } else { + // phrase / substring keep the literal (trimmed) text. + usableText = tok.text.trim(); + } + if (!usableText) continue; + + const term: ParsedTerm = { text: usableText, branch }; + + if (tok.op === '+') required.push(term); + else if (tok.op === '-') excluded.push(term); + else positive.push(term); + } + + const parsed: ParsedQuery = { raw: raw ?? '', positive, required, excluded, mode }; + + if (positive.length === 0) { + // Required terms with no positive recall still form a valid positive set (the + // required predicates ARE the recall). Only when there is neither a positive + // nor a required term is the query empty / only-negation. + if (required.length === 0) { + parsed.reason = excluded.length > 0 ? 'only-negation' : 'empty'; + } + } + + return parsed; +} + +/** + * Does this parsed query have any positive recall to run? False means we must + * short-circuit to an empty result (with `reason`), never a costly NOT-only scan. + */ +export function hasPositiveRecall(parsed: ParsedQuery): boolean { + return parsed.positive.length > 0 || parsed.required.length > 0; +} diff --git a/apps/server/src/core/search/search.service.lookup.spec.ts b/apps/server/src/core/search/search.service.lookup.spec.ts index 46d7f798..54ffb1d0 100644 --- a/apps/server/src/core/search/search.service.lookup.spec.ts +++ b/apps/server/src/core/search/search.service.lookup.spec.ts @@ -1,19 +1,14 @@ -import { - computeLookupScore, - escapeLikePattern, - SearchLookupTier, -} from './search.service'; +import { escapeLikePattern } 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. + * Pure-function coverage for `escapeLikePattern` — LIKE-metacharacter escaping so + * `%`/`_`/`\` are matched literally (the acceptance requirement that a query of + * `%` or `_` does NOT match everything, #529 acceptance #10). The substring + * branch's DB behaviour is covered by the integration spec. * - * The DB-touching branch (substring UNION FTS, path CTE, snippet window) is - * covered by the integration spec against the real schema. + * NOTE (#529): the old tiered `computeLookupScore` was replaced by RRF rank + * fusion in the unified engine, so its unit coverage moved to the integration + * ordering tests; only the escaping helper remains a pure unit here. */ describe('escapeLikePattern', () => { it('escapes the LIKE metacharacters % _ and \\', () => { @@ -43,53 +38,3 @@ describe('escapeLikePattern', () => { 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.query-mode.spec.ts b/apps/server/src/core/search/search.service.query-mode.spec.ts index de1a5b38..e2b7c0d0 100644 --- a/apps/server/src/core/search/search.service.query-mode.spec.ts +++ b/apps/server/src/core/search/search.service.query-mode.spec.ts @@ -1,74 +1,45 @@ import { SearchService } from './search.service'; /** - * Coverage for SearchService.searchPage query-mode selection (search.service.ts - * @25). searchPage chooses HOW the result set is scoped — by explicit space, by - * the authenticated user's member spaces, or by a share — and must return an - * empty set (without leaking data) for every disallowed combination. + * Unit coverage for SearchService.searchPage SCOPE-SECURITY early returns — the + * branches that must yield an empty result WITHOUT ever touching the DB, so they + * can leak nothing. The happy-path scope SQL (explicit space / member spaces / + * share id set) is covered against the real schema in the integration spec. * - * The kysely query builder is mocked with the same chainable pattern as the - * existing search.service.spec.ts: every builder method returns the same builder - * and `.execute()` resolves the supplied rows. Each `.where(...)` call is - * recorded so we can assert exactly which scope clause was applied — that is the - * mutation-resistant signal that distinguishes one query mode from another. - * - * These specs catch cross-space / cross-workspace search leakage and - * share-scope bypass (data exposure). + * Every case here returns BEFORE the raw-SQL candidate query runs, so a bare `db` + * stub (never called) is enough — a call to it would itself be a failure signal. */ -describe('SearchService.searchPage — query-mode selection', () => { - // Build a chainable selectFrom('pages') builder that records its calls. The - // builder is returned from `db.selectFrom` and is the single object every - // chained call mutates/returns, mirroring the existing spec's pattern. - function makeBuilder(rows: Array<{ id: string; highlight?: string }>) { - const builder: any = {}; - builder.select = jest.fn(() => builder); - builder.where = jest.fn(() => builder); - builder.$if = jest.fn(() => builder); - builder.orderBy = jest.fn(() => builder); - builder.limit = jest.fn(() => builder); - builder.offset = jest.fn(() => builder); - builder.execute = jest.fn(async () => rows); - return builder; - } - +describe('SearchService.searchPage — scope-security early returns', () => { function makeService(opts?: { - rows?: Array<{ id: string; highlight?: string }>; share?: any; isRestricted?: boolean; - descendants?: Array<{ id: string }>; + memberSpaceIds?: string[]; }) { - const builder = makeBuilder(opts?.rows ?? []); - - const db: any = { - selectFrom: jest.fn(() => builder), - }; - - // `getUserSpaceIdsQuery` returns a sub-query object that searchPage passes - // straight into `.where('spaceId', 'in', )`. A sentinel is enough - // to assert the user-scoped branch was taken. - const userSpaceIdsQuery = { __userSpaceIdsQuery: true }; + // A db that THROWS if touched — these branches must not reach SQL. + const db: any = new Proxy( + {}, + { + get() { + throw new Error('db must not be touched on an empty-scope branch'); + }, + }, + ); const pageRepo = { - // `.select((eb) => this.pageRepo.withSpace(eb))` — value ignored by stub. - withSpace: jest.fn(() => ({ __withSpace: true })), - getPageAndDescendantsExcludingRestricted: jest - .fn() - .mockResolvedValue(opts?.descendants ?? []), + getPageAndDescendantsExcludingRestricted: jest.fn(), + getPageAndDescendants: jest.fn(), }; const shareRepo = { findById: jest.fn().mockResolvedValue(opts?.share ?? null), }; const spaceMemberRepo = { - getUserSpaceIdsQuery: jest.fn(() => userSpaceIdsQuery), + getUserSpaceIds: jest.fn().mockResolvedValue(opts?.memberSpaceIds ?? []), }; const pagePermissionRepo = { hasRestrictedAncestor: jest .fn() .mockResolvedValue(opts?.isRestricted ?? false), - // Let everything through page-level permission filtering by default. - filterAccessiblePageIds: jest - .fn() - .mockImplementation(async ({ pageIds }: { pageIds: string[] }) => pageIds), + filterAccessiblePageIds: jest.fn(), }; const service = new SearchService( @@ -78,145 +49,81 @@ describe('SearchService.searchPage — query-mode selection', () => { spaceMemberRepo as any, pagePermissionRepo as any, ); - - return { - service, - db, - builder, - pageRepo, - shareRepo, - spaceMemberRepo, - pagePermissionRepo, - userSpaceIdsQuery, - }; + return { service, pageRepo, shareRepo, spaceMemberRepo, pagePermissionRepo }; } - const whereCallFor = (builder: any, column: any) => - builder.where.mock.calls.find((c: any[]) => c[0] === column); - - it('returns {items:[]} for a blank query WITHOUT touching the DB', async () => { - const { service, db } = makeService(); - + it('returns total:0 for a blank query WITHOUT touching the DB or any repo', async () => { + const { service, shareRepo, spaceMemberRepo } = makeService(); const result = await service.searchPage( { query: '' } as any, { userId: 'user-1', workspaceId: 'ws-1' }, ); - - expect(result).toEqual({ items: [] }); - // Blank query is rejected before any query builder is constructed. - expect(db.selectFrom).not.toHaveBeenCalled(); + expect(result.items).toEqual([]); + expect(result.total).toBe(0); + expect(shareRepo.findById).not.toHaveBeenCalled(); + expect(spaceMemberRepo.getUserSpaceIds).not.toHaveBeenCalled(); }); - it('scopes to the explicit spaceId branch', async () => { - const { service, builder, db, spaceMemberRepo, shareRepo } = makeService({ - rows: [{ id: 'p-1' }], - }); - + it('only-negation short-circuits with reason "only-negation", never scanning', async () => { + const { service, spaceMemberRepo } = makeService(); const result = await service.searchPage( - { query: 'plan', spaceId: 'space-42' } as any, + { query: '-архив' } as any, { userId: 'user-1', workspaceId: 'ws-1' }, ); - - expect(db.selectFrom).toHaveBeenCalledWith('pages'); - // The explicit-space branch adds exactly `.where('spaceId', '=', 'space-42')`. - expect(whereCallFor(builder, 'spaceId')).toEqual([ - 'spaceId', - '=', - 'space-42', - ]); - // It must NOT fall through to the user-member-spaces or share branch. - expect(spaceMemberRepo.getUserSpaceIdsQuery).not.toHaveBeenCalled(); - expect(shareRepo.findById).not.toHaveBeenCalled(); - expect(result.items.map((i: any) => i.id)).toEqual(['p-1']); + expect(result.total).toBe(0); + expect(result.query.parsed.reason).toBe('only-negation'); + // Never resolves scope (returns before) — no expensive NOT-only scan. + expect(spaceMemberRepo.getUserSpaceIds).not.toHaveBeenCalled(); }); - it('scopes an authenticated user WITHOUT spaceId to their member spaces', async () => { - const { service, builder, spaceMemberRepo, userSpaceIdsQuery, shareRepo } = - makeService({ rows: [{ id: 'p-9' }] }); - - await service.searchPage( - { query: 'plan' } as any, - { userId: 'user-7', workspaceId: 'ws-1' }, - ); - - // The user-scoped branch resolves the member-spaces sub-query for that user - // and restricts both spaceId (to that sub-query) and workspaceId. - expect(spaceMemberRepo.getUserSpaceIdsQuery).toHaveBeenCalledWith('user-7'); - expect(whereCallFor(builder, 'spaceId')).toEqual([ - 'spaceId', - 'in', - userSpaceIdsQuery, - ]); - expect(whereCallFor(builder, 'workspaceId')).toEqual([ - 'workspaceId', - '=', - 'ws-1', - ]); - // Authenticated user path must not consult shares. - expect(shareRepo.findById).not.toHaveBeenCalled(); - }); - - it('returns {items:[]} when the share belongs to a DIFFERENT workspace', async () => { - const { service, builder, shareRepo, pagePermissionRepo } = makeService({ - share: { - id: 'share-1', - pageId: 'page-1', - workspaceId: 'OTHER-ws', - includeSubPages: false, - }, + it('returns empty when the share belongs to a DIFFERENT workspace (no leak)', async () => { + const { service, shareRepo, pagePermissionRepo } = makeService({ + share: { id: 's1', pageId: 'p1', workspaceId: 'OTHER', includeSubPages: false }, }); - const result = await service.searchPage( - { query: 'plan', shareId: 'share-1' } as any, + { query: 'plan', shareId: 's1' } as any, { workspaceId: 'ws-1' }, ); - - expect(shareRepo.findById).toHaveBeenCalledWith('share-1'); - expect(result).toEqual({ items: [] }); - // Workspace mismatch short-circuits before any restricted-ancestor / id - // scoping or DB execution: no leak across workspaces. + expect(shareRepo.findById).toHaveBeenCalledWith('s1'); + expect(result.items).toEqual([]); + // Workspace mismatch short-circuits before restricted-ancestor / enumeration. expect(pagePermissionRepo.hasRestrictedAncestor).not.toHaveBeenCalled(); - expect(builder.execute).not.toHaveBeenCalled(); }); - it('returns {items:[]} when the shared page has a restricted ancestor', async () => { - const { service, builder, pagePermissionRepo, pageRepo } = makeService({ - share: { - id: 'share-1', - pageId: 'page-1', - workspaceId: 'ws-1', - includeSubPages: true, - }, + it('returns empty when the shared page has a restricted ancestor', async () => { + const { service, pagePermissionRepo, pageRepo } = makeService({ + share: { id: 's1', pageId: 'p1', workspaceId: 'ws-1', includeSubPages: true }, isRestricted: true, }); - const result = await service.searchPage( - { query: 'plan', shareId: 'share-1' } as any, + { query: 'plan', shareId: 's1' } as any, { workspaceId: 'ws-1' }, ); - - expect(pagePermissionRepo.hasRestrictedAncestor).toHaveBeenCalledWith( - 'page-1', - ); - expect(result).toEqual({ items: [] }); - // Restricted ancestor must block before page enumeration and DB execution. + expect(pagePermissionRepo.hasRestrictedAncestor).toHaveBeenCalledWith('p1'); + expect(result.items).toEqual([]); expect( pageRepo.getPageAndDescendantsExcludingRestricted, ).not.toHaveBeenCalled(); - expect(builder.execute).not.toHaveBeenCalled(); }); - it('returns {items:[]} with no userId, no spaceId and no shareId', async () => { - const { service, builder, shareRepo } = makeService(); - + it('returns empty with no userId, no spaceId and no shareId', async () => { + const { service, shareRepo } = makeService(); const result = await service.searchPage( { query: 'plan' } as any, { workspaceId: 'ws-1' }, ); - - expect(result).toEqual({ items: [] }); - // The catch-all else returns empty without scoping/executing or hitting shares. + expect(result.items).toEqual([]); expect(shareRepo.findById).not.toHaveBeenCalled(); - expect(builder.execute).not.toHaveBeenCalled(); + }); + + it('an authenticated user with NO member spaces gets an empty result', async () => { + const { service, spaceMemberRepo } = makeService({ memberSpaceIds: [] }); + const result = await service.searchPage( + { query: 'plan' } as any, + { userId: 'user-1', workspaceId: 'ws-1' }, + ); + expect(spaceMemberRepo.getUserSpaceIds).toHaveBeenCalledWith('user-1'); + expect(result.items).toEqual([]); + expect(result.total).toBe(0); }); }); diff --git a/apps/server/src/core/search/search.service.ts b/apps/server/src/core/search/search.service.ts index 76ac59d3..c3dc9442 100644 --- a/apps/server/src/core/search/search.service.ts +++ b/apps/server/src/core/search/search.service.ts @@ -1,34 +1,49 @@ import { Injectable } from '@nestjs/common'; import { SearchDTO, SearchSuggestionDTO } from './dto/search.dto'; import { - SearchLookupResponseDto, SearchResponseDto, + SearchResultDto, } from './dto/search-response.dto'; import { InjectKysely } from 'nestjs-kysely'; import { KyselyDB } from '@docmost/db/types/kysely.types'; -import { sql } from 'kysely'; +import { RawBuilder, sql } from 'kysely'; import { PageRepo } from '@docmost/db/repos/page/page.repo'; import { SpaceMemberRepo } from '@docmost/db/repos/space/space-member.repo'; import { ShareRepo } from '@docmost/db/repos/share/share.repo'; import { PagePermissionRepo } from '@docmost/db/repos/page/page-permission.repo'; +import { + ParsedQuery, + ParsedTerm, + parseSearchQuery, + SearchBooleanMode, + SearchMatchMode, + hasPositiveRecall, +} from './search-query-parser'; // eslint-disable-next-line @typescript-eslint/no-require-imports const tsquery = require('pg-tsquery')(); -// Build a safe prefix tsquery string from a raw user query. +// The FTS text-search configuration used on BOTH the stored side (pages.tsv via +// its trigger, see migration 20260707T120000) and the query side here. #529 +// acceptance #13 invariant: column config and query config always change as a +// pair — flip this only alongside the migration. +const TS_CONFIG = 'ru_en'; + +// RRF constant (Cormack et al. 2009; the Elasticsearch/OpenSearch default). RRF +// fuses RANKS (not raw scores) so the incomparable ts_rank_cd (lexical) and the +// substring tier scales never need normalizing — that is the whole point. +const RRF_K = 60; + +// Legacy prefix-tsquery builder (kept for the /suggest path + back-compat unit +// tests). The #529 engine below no longer uses it — it parses the query into an +// AST instead — but `buildTsQuery` remains exported and behaviour-identical. // -// The previous inline form `tsquery(query.trim() + '*')` passed user input -// (including to_tsquery operators like `&`, `|`, `!`, `<->`, `*`, backslashes) -// straight through. pg-tsquery would then emit operator fragments that -// `to_tsquery('ru_en', ...)` can reject as a syntax error, turning a search -// into a 500. We strip everything that is not a letter, number or whitespace -// BEFORE handing the text to pg-tsquery, so adversarial input degrades to a -// neutral (possibly empty) query instead of throwing, while normal word queries -// (incl. accented / non-Latin words) are unaffected. +// Strips everything that is not a letter/number/space BEFORE handing text to +// pg-tsquery so adversarial to_tsquery operators degrade to a neutral query +// instead of a 500. export function buildTsQuery(raw: string): string { const cleaned = (raw ?? '') .normalize('NFC') - // Keep Unicode letters/numbers and whitespace; drop everything else. .replace(/[^\p{L}\p{N}\s]+/gu, ' ') .replace(/\s+/g, ' ') .trim(); @@ -37,11 +52,9 @@ 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. +// Escape the LIKE metacharacters (`%`, `_`, `\`) so every character is matched +// LITERALLY by a `col LIKE '%' || q || '%'` predicate. Without this a query of +// `%` or `_` would match every row. export function escapeLikePattern(raw: string): string { return (raw ?? '') .replace(/\\/g, '\\\\') @@ -49,39 +62,24 @@ export function escapeLikePattern(raw: string): string { .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. +// Substring tier (highest first), used to rank the substring branch before RRF. 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; +// Env-tunable fusion window: the top-N candidates (by RRF) that pagination can +// reach. The tail beyond it is unreachable (truncatedAtCap:true). Default 500. +function getCandidateCap(): number { + const raw = Number(process.env.SEARCH_CANDIDATE_CAP); + return Number.isFinite(raw) && raw > 0 ? Math.floor(raw) : 500; } -// 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); +// Global AND/OR toggle: SEARCH_MODE overrides the request `mode` default. Both +// are valid on the ru_en tsv — this is a parser toggle, NOT a config rollback. +function defaultBooleanMode(): SearchBooleanMode { + return process.env.SEARCH_MODE === 'and' ? 'and' : 'or'; } @Injectable() @@ -94,194 +92,196 @@ export class SearchService { private pagePermissionRepo: PagePermissionRepo, ) {} - async searchPage( - searchParams: SearchDTO, - opts: { - userId?: string; - workspaceId: string; - }, - ): Promise<{ items: SearchResponseDto[] | SearchLookupResponseDto[] }> { - const { query } = searchParams; + // === #529 SQL fragment builders (parameterized AST, never string-concat) ===== - 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 - .selectFrom('pages') - .select([ - 'id', - 'slugId', - 'title', - 'icon', - 'parentPageId', - 'creatorId', - 'createdAt', - 'updatedAt', - sql`ts_rank(tsv, to_tsquery('ru_en', f_unaccent(${searchQuery})))`.as( - 'rank', - ), - sql`ts_headline('ru_en', text_content, to_tsquery('ru_en', f_unaccent(${searchQuery})),'MinWords=9, MaxWords=10, MaxFragments=3')`.as( - 'highlight', - ), - ]) - .where( - 'tsv', - '@@', - sql`to_tsquery('ru_en', f_unaccent(${searchQuery}))`, - ) - .$if(Boolean(searchParams.creatorId), (qb) => - qb.where('creatorId', '=', searchParams.creatorId), - ) - .where('deletedAt', 'is', null) - .orderBy('rank', 'desc') - .limit(searchParams.limit || 25) - .offset(searchParams.offset || 0); - - if (!searchParams.shareId) { - queryResults = queryResults.select((eb) => this.pageRepo.withSpace(eb)); - } - - if (searchParams.spaceId) { - // search by spaceId - queryResults = queryResults.where('spaceId', '=', searchParams.spaceId); - } else if (opts.userId && !searchParams.spaceId) { - // only search spaces the user is a member of - queryResults = queryResults - .where( - 'spaceId', - 'in', - this.spaceMemberRepo.getUserSpaceIdsQuery(opts.userId), - ) - .where('workspaceId', '=', opts.workspaceId); - } else if (searchParams.shareId && !searchParams.spaceId && !opts.userId) { - // search in shares - const shareId = searchParams.shareId; - const share = await this.shareRepo.findById(shareId); - if (!share || share.workspaceId !== opts.workspaceId) { - return { items: [] }; - } - - const isRestricted = - await this.pagePermissionRepo.hasRestrictedAncestor(share.pageId); - if (isRestricted) { - return { items: [] }; - } - - const pageIdsToSearch = []; - if (share.includeSubPages) { - const pageList = await this.pageRepo.getPageAndDescendantsExcludingRestricted( - share.pageId, - { - includeContent: false, - }, - ); - - pageIdsToSearch.push(...pageList.map((page) => page.id)); - } else { - pageIdsToSearch.push(share.pageId); - } - - if (pageIdsToSearch.length > 0) { - queryResults = queryResults - .where('id', 'in', pageIdsToSearch) - .where('workspaceId', '=', opts.workspaceId); - } else { - return { items: [] }; - } - } else { - return { items: [] }; - } - - //@ts-ignore - let results: any[] = await queryResults.execute(); - - // Filter results by page-level permissions (if user is authenticated) - if (opts.userId && results.length > 0) { - const pageIds = results.map((r: any) => r.id); - const accessibleIds = - await this.pagePermissionRepo.filterAccessiblePageIds({ - pageIds, - userId: opts.userId, - spaceId: searchParams.spaceId, - // #348 — enables the workspace-level short-circuit when not space-scoped. - workspaceId: opts.workspaceId, - }); - const accessibleSet = new Set(accessibleIds); - results = results.filter((r: any) => accessibleSet.has(r.id)); - } - - //@ts-ignore - const searchResults = results.map((result: SearchResponseDto) => { - if (result.highlight) { - result.highlight = result.highlight - .replace(/\r\n|\r|\n/g, ' ') - .replace(/\s+/g, ' '); - } - return result; - }); - - return { items: searchResults }; + // to_tsquery for a single FTS term. The term's own (already metachar-stripped) + // words are joined with `&` and given a `:*` suffix for the prefix branch. The + // whole lexeme string is a BOUND parameter to to_tsquery — no user operator can + // reach the parser. + private ftsTermTsq(term: ParsedTerm): RawBuilder { + const words = term.text.split(/\s+/).filter(Boolean); + const suffix = term.branch === 'ftsPrefix' ? ':*' : ''; + const lexeme = words.map((w) => w + suffix).join(' & '); + return sql`to_tsquery(${TS_CONFIG}, f_unaccent(${lexeme}))`; } - /** - * 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( + private phraseTermTsq(term: ParsedTerm): RawBuilder { + return sql`phraseto_tsquery(${TS_CONFIG}, f_unaccent(${term.text}))`; + } + + private termTsq(term: ParsedTerm): RawBuilder { + return term.branch === 'phrase' + ? this.phraseTermTsq(term) + : this.ftsTermTsq(term); + } + + // Fold FTS/phrase term tsqueries into ONE tsquery via the SQL `||` operator + // (AND via `&&` when mode is 'and'). Returns null when there are no such terms. + private combineTsq( + terms: ParsedTerm[], + mode: SearchBooleanMode, + ): RawBuilder | null { + const ftsTerms = terms.filter((t) => t.branch !== 'substring'); + if (ftsTerms.length === 0) return null; + const op = mode === 'and' ? sql`&&` : sql`||`; + let acc: RawBuilder = sql`(${this.termTsq(ftsTerms[0])})`; + for (let i = 1; i < ftsTerms.length; i++) { + acc = sql`(${acc}) ${op} (${this.termTsq(ftsTerms[i])})`; + } + return acc; + } + + // A LIKE '%needle%' pattern in the LOWER(f_unaccent(...)) space. Coalesce-free + // so it can use the trigram GIN indexes (a coalesce wrapper would force a seq + // scan). NULL title/text simply doesn't match, exactly as '' wouldn't. + private likePattern(term: ParsedTerm): RawBuilder { + const body = '%' + escapeLikePattern(term.text) + '%'; + return sql`LOWER(f_unaccent(${body}))`; + } + + private substringPred( + term: ParsedTerm, + titleOnly: boolean, + ): RawBuilder { + const pat = this.likePattern(term); + const title = sql`LOWER(f_unaccent(pages.title)) LIKE ${pat} ESCAPE '\\'`; + if (titleOnly) return sql`(${title})`; + const text = sql`LOWER(f_unaccent(pages.text_content)) LIKE ${pat} ESCAPE '\\'`; + return sql`(${title} OR ${text})`; + } + + // Whole-candidate match predicate for a single term (required / excluded + // predicates, and per-hit matchedTerms). + private termMatchPred( + term: ParsedTerm, + titleOnly: boolean, + ): RawBuilder { + if (term.branch === 'substring') return this.substringPred(term, titleOnly); + return sql`(pages.tsv @@ ${this.termTsq(term)})`; + } + + // The positive RECALL predicate: OR (or AND, per mode) of every positive term's + // match — the FTS terms via one combined tsquery, the substring terms via LIKE. + private positiveRecall( + parsed: ParsedQuery, + titleOnly: boolean, + ): RawBuilder { + const parts: RawBuilder[] = []; + const tsq = this.combineTsq(parsed.positive, parsed.mode); + if (tsq) parts.push(sql`(pages.tsv @@ (${tsq}))`); + for (const t of parsed.positive.filter((x) => x.branch === 'substring')) { + parts.push(this.substringPred(t, titleOnly)); + } + if (parts.length === 0) return sql`false`; + const op = parsed.mode === 'and' ? sql` AND ` : sql` OR `; + return sql`(${sql.join(parts, op)})`; + } + + // The full candidate WHERE: positive recall AND every required AND none of the + // excluded. Required terms with no positive term ARE the recall (A2). + private candidatePredicate( + parsed: ParsedQuery, + titleOnly: boolean, + ): RawBuilder { + const preds: RawBuilder[] = []; + if (parsed.positive.length > 0) { + preds.push(this.positiveRecall(parsed, titleOnly)); + } + for (const t of parsed.required) { + preds.push(this.termMatchPred(t, titleOnly)); + } + for (const t of parsed.excluded) { + preds.push(sql`NOT ${this.termMatchPred(t, titleOnly)}`); + } + if (preds.length === 0) return sql`false`; + return sql`(${sql.join(preds, sql` AND `)})`; + } + + // ts_rank_cd over the positive FTS tsquery. NULL when the query has no FTS + // positive term (pure-substring query) — the row is then absent from the FTS + // RRF branch. + private ftsScoreExpr(parsed: ParsedQuery): RawBuilder { + const tsq = this.combineTsq(parsed.positive, parsed.mode); + if (!tsq) return sql`NULL::float`; + return sql`CASE WHEN pages.tsv @@ (${tsq}) THEN ts_rank_cd(pages.tsv, (${tsq})) ELSE NULL END`; + } + + // Substring tier (3 title-exact / 2 title-substring / 1 text) or NULL when the + // row matched no positive substring term. Drives the substring RRF branch. + private subTierExpr( + parsed: ParsedQuery, + titleOnly: boolean, + ): RawBuilder { + const subTerms = parsed.positive.filter((t) => t.branch === 'substring'); + if (subTerms.length === 0) return sql`NULL::int`; + const exact: RawBuilder[] = []; + const titleSub: RawBuilder[] = []; + const textSub: RawBuilder[] = []; + for (const t of subTerms) { + const needle = sql`LOWER(f_unaccent(${t.text}))`; + const pat = this.likePattern(t); + exact.push(sql`LOWER(f_unaccent(pages.title)) = ${needle}`); + titleSub.push(sql`LOWER(f_unaccent(pages.title)) LIKE ${pat} ESCAPE '\\'`); + textSub.push( + sql`LOWER(f_unaccent(pages.text_content)) LIKE ${pat} ESCAPE '\\'`, + ); + } + const exactOr = sql`(${sql.join(exact, sql` OR `)})`; + const titleOr = sql`(${sql.join(titleSub, sql` OR `)})`; + const textOr = titleOnly + ? sql`false` + : sql`(${sql.join(textSub, sql` OR `)})`; + return sql`CASE WHEN ${exactOr} THEN 3 WHEN ${titleOr} THEN 2 WHEN ${textOr} THEN 1 ELSE NULL END`; + } + + // === Main entry (A6 unified path) ========================================== + + async searchPage( searchParams: SearchDTO, opts: { userId?: string; workspaceId: string }, - ): Promise<{ items: SearchLookupResponseDto[] }> { - const rawQuery = searchParams.query.trim(); - if (!rawQuery) { - return { items: [] }; + ): Promise { + const rawQuery = (searchParams.query ?? '').trim(); + const mode = defaultBooleanMode(); + const match = (searchParams.match as SearchMatchMode) ?? 'auto'; + const parsed = parseSearchQuery(rawQuery, { match, mode }); + const titleOnly = Boolean(searchParams.titleOnly); + const limit = Math.min(Math.max(searchParams.limit || 25, 1), 100); + const offset = Math.max(searchParams.offset || 0, 0); + const cap = getCandidateCap(); + + const queryMeta = { + raw: parsed.raw, + parsed: { + positive: parsed.positive.map((t) => t.text), + required: parsed.required.map((t) => t.text), + excluded: parsed.excluded.map((t) => t.text), + reason: parsed.reason, + }, + mode: parsed.mode, + match, + }; + + const empty = (reason?: string): SearchResponseDto => ({ + items: [], + total: 0, + hasMore: false, + truncatedAtCap: false, + offset, + query: reason + ? { ...queryMeta, parsed: { ...queryMeta.parsed, reason } } + : queryMeta, + }); + + // Only-negation / empty query: never run an expensive NOT-only scan (A2). + if (!hasPositiveRecall(parsed)) { + return empty(parsed.reason); } - const limit = Math.min(Math.max(searchParams.limit || 10, 1), 50); + // --- Resolve scope (A6). -------------------------------------------------- + const scope = await this.resolveScope(searchParams, opts); + if (scope.kind === 'none') return empty(); - // 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. + // --- Optional parentPageId subtree scope. --------------------------------- let descendantIds: string[] | null = null; if (searchParams.parentPageId) { const descendants = await this.pageRepo.getPageAndDescendants( @@ -289,239 +289,302 @@ export class SearchService { { includeContent: false }, ); descendantIds = descendants.map((p: any) => p.id); - if (descendantIds.length === 0) { - return { items: [] }; - } + if (descendantIds.length === 0) return empty(); } - // --- 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; + // --- Build the shared candidate WHERE fragment. --------------------------- + const scopePreds: RawBuilder[] = [sql`pages.deleted_at IS NULL`]; + if (scope.kind === 'spaces') { + scopePreds.push( + sql`pages.space_id = ANY(${scope.spaceIds}::uuid[])`, + sql`pages.workspace_id = ${scope.workspaceId}`, + ); + } else { + // share: an explicit (already permission-filtered) id set. + scopePreds.push( + sql`pages.id = ANY(${scope.pageIds}::uuid[])`, + sql`pages.workspace_id = ${scope.workspaceId}`, + ); + } + if (searchParams.creatorId) { + scopePreds.push(sql`pages.creator_id = ${searchParams.creatorId}`); + } + if (descendantIds) { + scopePreds.push(sql`pages.id = ANY(${descendantIds}::uuid[])`); + } + const scopeSql = sql`(${sql.join(scopePreds, sql` AND `)})`; + const candidateSql = sql`(${scopeSql} AND ${this.candidatePredicate(parsed, titleOnly)})`; - let candidates = this.db + // --- Ranked candidate ids (ALL matches, RRF order). ----------------------- + // Two independent branches ranked SEPARATELY (FTS by ts_rank_cd, substring by + // tier) then fused with RRF: score = Σ 1/(k + rank_branch). Deterministic + // ORDER BY rrf DESC, id so pagination never dupes/skips (A4, acceptance #8). + const rankedRows = await sql<{ id: string }>` + WITH candidates AS ( + SELECT pages.id AS id, + ${this.ftsScoreExpr(parsed)} AS fts_score, + ${this.subTierExpr(parsed, titleOnly)} AS sub_tier + FROM pages + WHERE ${candidateSql} + ), + ranked AS ( + SELECT id, fts_score, sub_tier, + row_number() OVER (ORDER BY fts_score DESC NULLS LAST, id) AS rn_fts, + row_number() OVER (ORDER BY sub_tier DESC NULLS LAST, id) AS rn_sub + FROM candidates + ) + SELECT id + FROM ranked + ORDER BY + (CASE WHEN fts_score IS NOT NULL THEN 1.0/(${RRF_K} + rn_fts) ELSE 0 END) + + (CASE WHEN sub_tier IS NOT NULL THEN 1.0/(${RRF_K} + rn_sub) ELSE 0 END) DESC, + id ASC + `.execute(this.db); + + let orderedIds = rankedRows.rows.map((r) => r.id); + + // --- Permission filter (fail-closed, exact total). ------------------------ + // filterAccessiblePageIds runs the #348 hasRestricted pre-check internally: + // no restricted pages in scope → returns the ids untouched (fast path, total + // stays trivially exact); otherwise a SQL anti-join drops hidden pages. An + // error PROPAGATES (500) — it never degrades to an empty lexical result, so + // hidden-page cardinality never leaks into `total`. The share path is already + // permission-filtered by getPageAndDescendantsExcludingRestricted. + if (opts.userId && scope.kind === 'spaces' && orderedIds.length > 0) { + const accessible = await this.pagePermissionRepo.filterAccessiblePageIds({ + pageIds: orderedIds, + userId: opts.userId, + spaceId: searchParams.spaceId, + workspaceId: opts.workspaceId, + }); + const accessibleSet = new Set(accessible); + orderedIds = orderedIds.filter((id) => accessibleSet.has(id)); + } + + const total = orderedIds.length; + const truncatedAtCap = total > cap; + const window = orderedIds.slice(0, cap); + const pageIds = window.slice(offset, offset + limit); + const hasMore = offset + pageIds.length < window.length; + + if (pageIds.length === 0) { + return { items: [], total, hasMore, truncatedAtCap, offset, query: queryMeta }; + } + + // --- Detail fetch for the page slice only (ts_headline/snippet are costly, so + // compute them ONLY for the returned rows), preserving RRF order. ------- + const items = await this.fetchDetails(pageIds, parsed, titleOnly); + + return { items, total, hasMore, truncatedAtCap, offset, query: queryMeta }; + } + + // Resolve the search scope: explicit space, the authed user's member spaces, or + // a public share's descendant id set (permission-filtered). Mirrors the legacy + // branch logic (A6) but returns a small tagged union. + private async resolveScope( + searchParams: SearchDTO, + opts: { userId?: string; workspaceId: string }, + ): Promise< + | { kind: 'none' } + | { kind: 'spaces'; spaceIds: string[]; workspaceId: string } + | { kind: 'ids'; pageIds: string[]; workspaceId: string } + > { + if (searchParams.spaceId) { + return { + kind: 'spaces', + spaceIds: [searchParams.spaceId], + workspaceId: opts.workspaceId, + }; + } + + if (opts.userId && !searchParams.shareId) { + const spaceIds = await this.spaceMemberRepo.getUserSpaceIds(opts.userId); + if (spaceIds.length === 0) return { kind: 'none' }; + return { kind: 'spaces', spaceIds, workspaceId: opts.workspaceId }; + } + + if (searchParams.shareId && !opts.userId) { + const share = await this.shareRepo.findById(searchParams.shareId); + if (!share || share.workspaceId !== opts.workspaceId) { + return { kind: 'none' }; + } + const isRestricted = await this.pagePermissionRepo.hasRestrictedAncestor( + share.pageId, + ); + if (isRestricted) return { kind: 'none' }; + + const pageIds: string[] = []; + if (share.includeSubPages) { + // A6: exclude restricted descendants + honour a restricted ancestor. + const pageList = + await this.pageRepo.getPageAndDescendantsExcludingRestricted( + share.pageId, + { includeContent: false }, + ); + pageIds.push(...pageList.map((p) => p.id)); + } else { + pageIds.push(share.pageId); + } + if (pageIds.length === 0) return { kind: 'none' }; + return { kind: 'ids', pageIds, workspaceId: opts.workspaceId }; + } + + return { kind: 'none' }; + } + + // Fetch the display superset (A7) for the given page ids, in the given order. + private async fetchDetails( + pageIds: string[], + parsed: ParsedQuery, + titleOnly: boolean, + ): Promise { + const tsq = this.combineTsq(parsed.positive, parsed.mode); + // rank/highlight are FTS-only (null for substring-only hits — the web already + // falls back). ts_headline runs over the ORIGINAL text. + const rankExpr = tsq + ? sql`CASE WHEN pages.tsv @@ (${tsq}) THEN ts_rank_cd(pages.tsv, (${tsq})) ELSE NULL END` + : sql`NULL::float`; + const highlightExpr = tsq + ? sql`CASE WHEN pages.tsv @@ (${tsq}) THEN ts_headline(${TS_CONFIG}, coalesce(pages.text_content, ''), (${tsq}), 'MinWords=9, MaxWords=10, MaxFragments=3') ELSE NULL END` + : sql`NULL::text`; + + // Windowed plain snippet: leading window, else the FTS headline (plain). + const snippetExpr = titleOnly + ? sql`''` + : sql`coalesce( + case + when coalesce(pages.text_content, '') <> '' + then substring(LOWER(f_unaccent(pages.text_content)) from 1 for 300) + ${ + tsq + ? sql`else ts_headline(${TS_CONFIG}, coalesce(pages.text_content, ''), (${tsq}), 'MinWords=25, MaxWords=40, MaxFragments=3')` + : sql`` + } + end, '')`; + + // matchedFields: title / text. + const titleMatchParts: RawBuilder[] = []; + const textMatchParts: RawBuilder[] = []; + if (tsq) { + titleMatchParts.push( + sql`to_tsvector(${TS_CONFIG}, f_unaccent(coalesce(pages.title, ''))) @@ (${tsq})`, + ); + if (!titleOnly) { + textMatchParts.push( + sql`(pages.tsv @@ (${tsq}) AND NOT to_tsvector(${TS_CONFIG}, f_unaccent(coalesce(pages.title, ''))) @@ (${tsq}))`, + ); + } + } + for (const t of parsed.positive.filter((x) => x.branch === 'substring')) { + const pat = this.likePattern(t); + titleMatchParts.push( + sql`LOWER(f_unaccent(pages.title)) LIKE ${pat} ESCAPE '\\'`, + ); + if (!titleOnly) { + textMatchParts.push( + sql`LOWER(f_unaccent(pages.text_content)) LIKE ${pat} ESCAPE '\\'`, + ); + } + } + const titleMatchExpr = titleMatchParts.length + ? sql`(${sql.join(titleMatchParts, sql` OR `)})` + : sql`false`; + const textMatchExpr = textMatchParts.length + ? sql`(${sql.join(textMatchParts, sql` OR `)})` + : sql`false`; + + // Per-term matched flags (positive + required), assembled into matchedTerms. + // NB: aliases must be UNDERSCORE-FREE — the app's Kysely runs the + // CamelCasePlugin, which would rewrite a `t_0` result key to `t0`. `tmatch0` + // survives the round-trip unchanged. + const allTerms = [...parsed.positive, ...parsed.required]; + const termCols = allTerms.map( + (t, i) => + sql`(${this.termMatchPred(t, titleOnly)}) AS "tmatch${sql.raw(String(i))}"`, + ); + + let q = this.db .selectFrom('pages') .select([ 'pages.id as id', 'pages.slugId as slugId', 'pages.title as title', + 'pages.icon as icon', '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('ru_en', 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('ru_en', coalesce(pages.text_content, ''), to_tsquery('ru_en', f_unaccent(${tsQuery})), 'MinWords=25, MaxWords=40, MaxFragments=3')` - : sql`` - } - end, - '' - ) - `.as('snippet'), + 'pages.creatorId as creatorId', + 'pages.spaceId as spaceId', + 'pages.createdAt as createdAt', + 'pages.updatedAt as updatedAt', + rankExpr.as('rank'), + highlightExpr.as('highlight'), + snippetExpr.as('snippet'), + titleMatchExpr.as('titleMatch'), + textMatchExpr.as('textMatch'), ]) - .where('pages.deletedAt', 'is', null) - .where('pages.spaceId', 'in', spaceIds); + .select((eb) => this.pageRepo.withSpace(eb)) + .where(sql`pages.id = ANY(${pageIds}::uuid[])` as any); - if (descendantIds) { - candidates = candidates.where('pages.id', 'in', descendantIds); + if (termCols.length > 0) { + q = q.select(termCols as any); } - // 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(pages.title))`, - 'like', - sql`${likePattern} ESCAPE '\\'`, - ), - ]; - if (!searchParams.titleOnly) { - ors.push( - eb( - sql`LOWER(f_unaccent(pages.text_content))`, - 'like', - sql`${likePattern} ESCAPE '\\'`, - ), - ); - if (hasTsQuery) { - ors.push( - sql`pages.tsv @@ to_tsquery('ru_en', f_unaccent(${tsQuery}))` as any, - ); - } - } - return eb.or(ors); - }); + const rows: any[] = await q.execute(); - // 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('ru_en', 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`, - ); + const byId = new Map(rows.map((r) => [r.id, r])); + const pathById = await this.buildAncestorPaths(pageIds); - 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 ?? '') + const items: SearchResultDto[] = []; + for (const id of pageIds) { + const r = byId.get(id); + if (!r) continue; + const matchedFields: string[] = []; + if (r.titleMatch) matchedFields.push('title'); + if (r.textMatch) matchedFields.push('text'); + const matchedTerms: string[] = []; + allTerms.forEach((t, i) => { + if (r[`tmatch${i}`]) matchedTerms.push(t.text); + }); + const rank = r.rank == null ? null : Number(r.rank); + const highlight = r.highlight + ? String(r.highlight) + .replace(/\r\n|\r|\n/g, ' ') + .replace(/\s+/g, ' ') + : null; + const snippet = (r.snippet ?? '') .replace(/\r\n|\r|\n/g, ' ') .replace(/\s+/g, ' ') - .trim(), - score: t.score, - })); - - return { items }; + .trim(); + items.push({ + id: r.id, + pageId: r.id, + slugId: r.slugId, + icon: r.icon, + title: r.title, + space: r.space, + creatorId: r.creatorId, + createdAt: r.createdAt, + updatedAt: r.updatedAt, + rank, + highlight, + snippet, + path: pathById.get(id) ?? [], + // A per-response ordering proxy for legacy consumers; falls back to rank. + score: rank ?? 0, + matchedFields, + matchedTerms: Array.from(new Set(matchedTerms)), + }); + } + 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. + * Batch ancestor-titles helper: ONE recursive CTE seeded with ALL hit ids, + * walking UP parentPageId. Returns hitId → ancestor titles ordered root → + * direct parent (the hit's own title excluded). + * + * A8 fix: the recursive walk now skips soft-deleted ancestors (`deleted_at IS + * NULL`) and stays within the hit's own space, so a deleted or cross-space + * ancestor's title can no longer leak into `path`. */ private async buildAncestorPaths( hitIds: string[], @@ -529,8 +592,6 @@ export class SearchService { 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 @@ -540,29 +601,32 @@ export class SearchService { 'pages.id as pageId', 'pages.title as title', 'pages.parentPageId as parentPageId', + 'pages.spaceId as spaceId', sql`0`.as('depth'), ]) .where('pages.id', 'in', hitIds) + .where('pages.deletedAt', 'is', null) .unionAll((exp) => exp .selectFrom('pages as p') .innerJoin('ancestry as a', 'p.id', 'a.parentPageId') + // A8: don't cross into a deleted ancestor or another space. + .where('p.deletedAt', 'is', null) + .whereRef('p.spaceId', '=', 'a.spaceId') .select([ 'a.hitId as hitId', 'p.id as pageId', 'p.title as title', 'p.parentPageId as parentPageId', + 'a.spaceId as spaceId', 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(); @@ -639,12 +703,10 @@ export class SearchService { .where('workspaceId', '=', workspaceId) .limit(limit); - // Template picker: restrict to pages flagged as templates. if (suggestion.onlyTemplates) { pageSearch = pageSearch.where('isTemplate', '=', true); } - // search all spaces the user has access to, prioritizing the current space const userSpaceIds = await this.spaceMemberRepo.getUserSpaceIds(userId); if (userSpaceIds?.length > 0) { @@ -660,14 +722,12 @@ export class SearchService { pages = await pageSearch.execute(); } - // Filter by page-level permissions if (pages.length > 0) { const pageIds = pages.map((p) => p.id); const accessibleIds = await this.pagePermissionRepo.filterAccessiblePageIds({ pageIds, userId, - // #348 — workspace-level short-circuit for the suggest path. workspaceId, }); const accessibleSet = new Set(accessibleIds); diff --git a/apps/server/test/integration/search-lexical.int-spec.ts b/apps/server/test/integration/search-lexical.int-spec.ts new file mode 100644 index 00000000..c8515252 --- /dev/null +++ b/apps/server/test/integration/search-lexical.int-spec.ts @@ -0,0 +1,388 @@ +import { randomUUID } from 'node:crypto'; +import { Kysely, sql } 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'; + +/** + * #529 Phase A — the lexical overhaul, on the REAL migrated schema (ru_en config). + * + * Covers every acceptance criterion of the issue: RU+EN morphology + OR default, + * match=auto identifier routing, "phrase"/+/- operators, RRF ordering, exact + * permission-filtered total (fail-closed) + pagination, only-negation / garbage + * short-circuits, the A8 path fix, the response superset, and the RAG lockstep + * config (acceptance #13). + * + * The tsv column is populated by the pages_tsvector_trigger (now ru_en), so the + * FTS branch is exercised end to end. + */ +describe('SearchService #529 lexical overhaul [integration]', () => { + let db: Kysely; + let workspaceId: string; + let spaceId: string; + + async function insertPage(args: { + title: string; + textContent?: string; + parentPageId?: string | null; + spaceId?: string; + deletedAt?: Date | null; + }): 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, + deletedAt: args.deletedAt ?? null, + }) + .execute(); + return id; + } + + // Service wired to the real DB + real PageRepo (recursive descendants) with + // stubbed space-membership + permission repos so a test controls scope and the + // permission filter explicitly. `accessibleIds` (when set) is the KEEP list. + function buildService(opts?: { + userSpaceIds?: string[]; + accessibleIds?: string[] | null; + filterThrows?: boolean; + }): SearchService { + const pageRepo = new PageRepo(db as any, null as any, null as any); + const spaceMemberRepo = { + getUserSpaceIds: async () => opts?.userSpaceIds ?? [spaceId], + }; + const pagePermissionRepo = { + hasRestrictedAncestor: async () => false, + filterAccessiblePageIds: async ({ pageIds }: { pageIds: string[] }) => { + if (opts?.filterThrows) throw new Error('permission query failed'); + return opts?.accessibleIds + ? pageIds.filter((id) => opts.accessibleIds!.includes(id)) + : pageIds; + }, + }; + return new SearchService( + db as any, + pageRepo as any, + {} as any, + spaceMemberRepo as any, + pagePermissionRepo as any, + ); + } + + const search = (service: SearchService, params: any) => + service.searchPage(params, { userId: 'u-1', workspaceId }) as any; + + beforeAll(async () => { + db = getTestDb(); + workspaceId = (await createWorkspace(db)).id; + spaceId = (await createSpace(db, workspaceId)).id; + }); + + afterAll(async () => { + await destroyTestDb(); + }); + + // 1. RU morphology + OR: «ресторанов москвы» finds «ресторан в москве». + it('#1 russian morphology + OR: finds «ресторан в москве» for «ресторанов москвы»', async () => { + const page = await insertPage({ + title: 'ресторан в москве', + textContent: 'Лучший ресторан столицы.', + }); + const res = await search(buildService(), { + query: 'ресторанов москвы', + spaceId, + }); + expect(res.items.map((i: any) => i.id)).toContain(page); + expect(res.total).toBeGreaterThanOrEqual(1); + }); + + // 2. OR non-empty: «Стамбул Роснефть». + it('#2 OR yields a hit when only one term matches', async () => { + const page = await insertPage({ title: 'Роснефть отчёт', textContent: 'x' }); + const res = await search(buildService(), { + query: 'Стамбул Роснефть', + spaceId, + }); + expect(res.items.map((i: any) => i.id)).toContain(page); + }); + + // 3. Multi-word OR: a page matching >=1 term is returned. + it('#3 «3D принтер» returns pages that matched at least one term', async () => { + const models = await insertPage({ + title: 'Модели для печати 3D', + textContent: 'коллекция моделей', + }); + const wish = await insertPage({ + title: 'Хотеть напечатать на принтере', + textContent: 'очередь печати', + }); + const res = await search(buildService(), { query: '3D принтер', spaceId }); + const ids = res.items.map((i: any) => i.id); + expect(ids).toContain(models); // matched "3D" + expect(ids).toContain(wish); // matched "принтер" + // matchedTerms is populated per hit. + const hit = res.items.find((i: any) => i.id === wish); + expect(hit.matchedTerms).toContain('принтер'); + }); + + // 4. match=auto FTS stemming: «печат» must NOT drag in «впечатления». + it('#4 «печат» (auto) matches «печать» but NOT «впечатления»', async () => { + const good = await insertPage({ + title: 'Печать документов', + textContent: 'настройка печати', + }); + const bad = await insertPage({ + title: 'Впечатления от поездки', + textContent: 'много впечатлений', + }); + const res = await search(buildService(), { query: 'печат', spaceId }); + const ids = res.items.map((i: any) => i.id); + expect(ids).toContain(good); + expect(ids).not.toContain(bad); + }); + + // 5. Identifier → substring branch. + it('#5 `10.31.41` (auto→substring) finds the page with that IP', async () => { + const page = await insertPage({ + title: 'Сетевой узел', + textContent: 'Адрес устройства: 10.31.41.7 в сети.', + }); + const res = await search(buildService(), { query: '10.31.41', spaceId }); + const hit = res.items.find((i: any) => i.id === page); + expect(hit).toBeDefined(); + expect(hit.matchedFields).toContain('text'); + }); + + // 6. +required / -excluded. + it('#6 `+кофейня -архив`: keeps «кофейня», drops pages with «архив»', async () => { + const keep = await insertPage({ title: 'Кофейня в центре', textContent: 'уют' }); + const drop = await insertPage({ + title: 'Кофейня старый архив', + textContent: 'архивные записи', + }); + const res = await search(buildService(), { query: '+кофейня -архив', spaceId }); + const ids = res.items.map((i: any) => i.id); + expect(ids).toContain(keep); + expect(ids).not.toContain(drop); + }); + + // 7. Phrase operator: only adjacent phrase hits survive. + it('#7 `+"воздушный шар" кофе`: every hit contains the adjacent phrase', async () => { + const adjacent = await insertPage({ + title: 'Воздушный шар и кофе', + textContent: 'воздушный шар над городом, чашка кофе', + }); + const nonAdjacent = await insertPage({ + title: 'Красный воздушный большой шар', + textContent: 'воздушный красный шар и кофе рядом', + }); + const res = await search(buildService(), { + query: '+"воздушный шар" кофе', + spaceId, + }); + const ids = res.items.map((i: any) => i.id); + expect(ids).toContain(adjacent); + // The non-adjacent page (words separated) must NOT match the phrase. + expect(ids).not.toContain(nonAdjacent); + }); + + // 8. Pagination determinism + exact total. + it('#8 >50 matches: total>50, hasMore, and offset paginates without dupes', async () => { + const svc = buildService(); + const created: string[] = []; + for (let i = 0; i < 60; i++) { + created.push( + await insertPage({ + title: `паджинация запись ${i}`, + textContent: 'общий паджинационный маркер', + }), + ); + } + const p1 = await search(svc, { + query: 'паджинационный', + spaceId, + limit: 25, + offset: 0, + }); + expect(p1.total).toBeGreaterThanOrEqual(60); + expect(p1.hasMore).toBe(true); + const p2 = await search(svc, { + query: 'паджинационный', + spaceId, + limit: 25, + offset: 25, + }); + const p3 = await search(svc, { + query: 'паджинационный', + spaceId, + limit: 25, + offset: 50, + }); + const ids = [ + ...p1.items.map((i: any) => i.id), + ...p2.items.map((i: any) => i.id), + ...p3.items.map((i: any) => i.id), + ]; + // No duplicates across the three pages. + expect(new Set(ids).size).toBe(ids.length); + // Deterministic: same query twice → identical order. + const p1b = await search(svc, { + query: 'паджинационный', + spaceId, + limit: 25, + offset: 0, + }); + expect(p1b.items.map((i: any) => i.id)).toEqual(p1.items.map((i: any) => i.id)); + }); + + // 9. Only-negation. + it('#9 `-архив` only-negation: total 0, reason only-negation, no throw', async () => { + const res = await search(buildService(), { query: '-архив', spaceId }); + expect(res.total).toBe(0); + expect(res.items).toEqual([]); + expect(res.query.parsed.reason).toBe('only-negation'); + }); + + // 10. Garbage input. + it('#10 garbage `%` / `_` / empty: total 0, does not match everything', async () => { + await insertPage({ title: 'какая-то страница', textContent: 'текст' }); + for (const q of ['%', '_', ' ', '%%__']) { + const res = await search(buildService(), { query: q, spaceId }); + expect(res.total).toBe(0); + expect(res.items).toEqual([]); + } + }); + + // 11. Permission-filtered total (fail-closed) + mutation guard. + it('#11 a permission-hidden page is absent from items AND total', async () => { + const visible = await insertPage({ + title: 'разрешённая пермишен-страница', + textContent: 'пермишенмаркер', + }); + const hidden = await insertPage({ + title: 'скрытая пермишен-страница', + textContent: 'пермишенмаркер', + }); + // Filter keeps only the visible page. + const filtered = buildService({ accessibleIds: [visible] }); + const res = await search(filtered, { query: 'пермишенмаркер', spaceId }); + const ids = res.items.map((i: any) => i.id); + expect(ids).toContain(visible); + expect(ids).not.toContain(hidden); + // total is the POST-permission count — the hidden page does not leak into it. + expect(res.total).toBe(1); + + // MUTATION: disable the guard (passthrough) → the hidden page reappears in + // BOTH items and total. If this did NOT change, the guard is not load-bearing. + const open = buildService({ accessibleIds: null }); + const res2 = await search(open, { query: 'пермишенмаркер', spaceId }); + expect(res2.items.map((i: any) => i.id)).toContain(hidden); + expect(res2.total).toBe(2); + }); + + it('#11b a permission-query error PROPAGATES (fail-closed, never empty)', async () => { + await insertPage({ title: 'failclosed маркер', textContent: 'failclosedmarker' }); + const svc = buildService({ filterThrows: true }); + await expect( + search(svc, { query: 'failclosedmarker', spaceId }), + ).rejects.toThrow(/permission query failed/); + }); + + // A8 path fix. + it('#11c path: a soft-deleted / cross-space ancestor title does not leak', async () => { + const otherSpace = (await createSpace(db, workspaceId)).id; + const root = await insertPage({ title: 'Живой корень' }); + const deletedMid = await insertPage({ + title: 'УдалённыйПредок', + parentPageId: root, + deletedAt: new Date(), + }); + const leaf = await insertPage({ + title: 'a8leaf уникальный', + parentPageId: deletedMid, + textContent: 'a8leafmarker', + }); + const res = await search(buildService(), { query: 'a8leafmarker', spaceId }); + const hit = res.items.find((i: any) => i.id === leaf); + expect(hit).toBeDefined(); + // The walk stops at the deleted ancestor — no deleted title in the path. + expect(hit.path).not.toContain('УдалённыйПредок'); + expect(hit.path).not.toContain('Живой корень'); + + // Cross-space parent must also not leak. + const foreignParent = await insertPage({ + title: 'ЧужойСпейс', + spaceId: otherSpace, + }); + const crossLeaf = await insertPage({ + title: 'crossleaf узел', + parentPageId: foreignParent, + textContent: 'crossleafmarker', + }); + const res2 = await search(buildService(), { + query: 'crossleafmarker', + spaceId, + }); + const hit2 = res2.items.find((i: any) => i.id === crossLeaf); + expect(hit2).toBeDefined(); + expect(hit2.path).not.toContain('ЧужойСпейс'); + }); + + // 12. Web-UI superset. + it('#12 web path (no flags) returns the OR result with the icon/space/highlight superset', async () => { + const page = await insertPage({ + title: 'веб суперсет страница', + textContent: 'суперсетмаркер контент', + }); + const res = await search(buildService(), { query: 'суперсетмаркер', spaceId }); + const hit = res.items.find((i: any) => i.id === page); + expect(hit).toBeDefined(); + // Superset fields the web-UI relies on. + expect('icon' in hit).toBe(true); + expect('space' in hit).toBe(true); + expect('highlight' in hit).toBe(true); + expect('rank' in hit).toBe(true); + // Plus the new fields. + expect('path' in hit).toBe(true); + expect('snippet' in hit).toBe(true); + expect('score' in hit).toBe(true); + // FTS hit carries a non-null rank + highlight. + expect(hit.rank).not.toBeNull(); + }); + + // 13. RAG lockstep: the page_embeddings.fts generated column is ru_en. + it('#13 page_embeddings.fts uses ru_en (cyrillic stemming) — RAG lockstep', async () => { + const pageId = await insertPage({ + title: 'rag страница', + textContent: 'ресторанов много', + }); + // Insert a chunk row; the generated fts column is computed by Postgres. + await sql` + INSERT INTO page_embeddings + (id, page_id, workspace_id, space_id, attachment_id, chunk_index, + chunk_start, chunk_length, content, model_name, model_dimensions, embedding) + VALUES + (${randomUUID()}, ${pageId}, ${workspaceId}, ${spaceId}, NULL, 0, + 0, 20, ${'ресторанов москвы много'}, 'test-model', 3, '[0.1,0.2,0.3]'::vector) + `.execute(db); + // A ru_en query stems «москва» → «москв», matching the stored «москвы». + // Under the old `english` config the cyrillic word would not stem and this + // inflected-form query would miss — so this asserts the ru_en lockstep. + const row = await sql<{ m: boolean }>` + SELECT fts @@ to_tsquery('ru_en', f_unaccent('москва')) AS m + FROM page_embeddings WHERE page_id = ${pageId} + `.execute(db); + expect(row.rows[0].m).toBe(true); + }); +}); diff --git a/apps/server/test/integration/search-lookup-explain.int-spec.ts b/apps/server/test/integration/search-lookup-explain.int-spec.ts deleted file mode 100644 index 4a50e553..00000000 --- a/apps/server/test/integration/search-lookup-explain.int-spec.ts +++ /dev/null @@ -1,123 +0,0 @@ -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); - }); -}); diff --git a/apps/server/test/integration/search-lookup.int-spec.ts b/apps/server/test/integration/search-lookup.int-spec.ts deleted file mode 100644 index 51daa855..00000000 --- a/apps/server/test/integration/search-lookup.int-spec.ts +++ /dev/null @@ -1,462 +0,0 @@ -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); - }); -});