fix(search): S1/W1/W3/W4/S3/S5/S2 — required-term details, CAP note, restored guards (#529)
S1: fetchDetails now builds rank/highlight/matchedFields from the same [...positive, ...required] set matchedTerms uses, so a required-only query (`+кофейня`) no longer reports matchedFields:[] / rank:null. W1: documented that CANDIDATE_CAP bounds pagination reachability (JS-side), not query compute — a SQL LIMIT would corrupt the exact permission-filtered total. S2: one-line note that literal-`%`/`_` search was intentionally dropped (total:0). W3: restored the trgm-index EXPLAIN guard (search-lookup-explain.int-spec) that asserts the coalesce-free substring predicates use idx_pages_*_trgm, not Seq Scan. W4: added an integration ordering test proving title-exact > title-substring > text-only tier dominance under RRF. S3: added a test that an exact-title hit survives a tiny CANDIDATE_CAP window. S5: server pretest now also builds @docmost/mcp so the ToolWriteClass import resolves from clean CI (mirrors editor-ext/prosemirror-markdown). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -385,4 +385,80 @@ describe('SearchService #529 lexical overhaul [integration]', () => {
|
||||
`.execute(db);
|
||||
expect(row.rows[0].m).toBe(true);
|
||||
});
|
||||
|
||||
// W4 — substring-tier dominance under RRF: title-exact (tier 3) > title-
|
||||
// substring (tier 2) > text-only (tier 1). The engine encodes this via
|
||||
// sub_tier DESC → rn_sub → RRF; no test asserted the end-to-end ordering, so
|
||||
// this restores that guarantee. match:'substring' routes the term to the
|
||||
// substring branch (no FTS leg), so sub_tier alone drives the order.
|
||||
it('W4 tier dominance: title-exact > title-substring > text-only', async () => {
|
||||
const exact = await insertPage({ title: 'tierdomxyz' }); // tier 3
|
||||
const titleSub = await insertPage({
|
||||
title: 'prefix tierdomxyz suffix', // tier 2
|
||||
});
|
||||
const textOnly = await insertPage({
|
||||
title: 'w4 unrelated heading',
|
||||
textContent: 'body has tierdomxyz here', // tier 1
|
||||
});
|
||||
const res = await search(buildService(), {
|
||||
query: 'tierdomxyz',
|
||||
match: 'substring',
|
||||
spaceId,
|
||||
});
|
||||
const ids = res.items.map((i: any) => i.id);
|
||||
expect(ids).toContain(exact);
|
||||
expect(ids).toContain(titleSub);
|
||||
expect(ids).toContain(textOnly);
|
||||
// Strict tier order.
|
||||
expect(ids.indexOf(exact)).toBeLessThan(ids.indexOf(titleSub));
|
||||
expect(ids.indexOf(titleSub)).toBeLessThan(ids.indexOf(textOnly));
|
||||
});
|
||||
|
||||
// S3 — an exact-title hit must NOT be lost when the match set exceeds
|
||||
// CANDIDATE_CAP: it ranks first under RRF (tier 3) so it lands in the reachable
|
||||
// window even with a tiny cap. Restores a guarantee the old lookup suite gave.
|
||||
it('S3 exact-title survives the CANDIDATE_CAP window', async () => {
|
||||
const exact = await insertPage({ title: 'capmarkerxyz' }); // tier 3
|
||||
for (let i = 0; i < 6; i++) {
|
||||
await insertPage({
|
||||
title: `cap filler ${i}`,
|
||||
textContent: 'noise capmarkerxyz noise', // tier 1
|
||||
});
|
||||
}
|
||||
process.env.SEARCH_CANDIDATE_CAP = '2';
|
||||
try {
|
||||
const res = await search(buildService(), {
|
||||
query: 'capmarkerxyz',
|
||||
match: 'substring',
|
||||
spaceId,
|
||||
limit: 25,
|
||||
});
|
||||
// More matches than the cap → truncated, but the exact-title survives.
|
||||
expect(res.total).toBeGreaterThan(2);
|
||||
expect(res.truncatedAtCap).toBe(true);
|
||||
expect(res.items.length).toBe(2); // the reachable window
|
||||
expect(res.items.map((i: any) => i.id)).toContain(exact);
|
||||
} finally {
|
||||
delete process.env.SEARCH_CANDIDATE_CAP;
|
||||
}
|
||||
});
|
||||
|
||||
// S1 — a required-only query (`+term`, no bare positive) really matches, so its
|
||||
// hits must report matchedFields / rank / highlight, not [] / null. Before the
|
||||
// fix these detail exprs were built from parsed.positive only.
|
||||
it('S1 required-only query populates matchedFields + rank on a title match', async () => {
|
||||
const page = await insertPage({
|
||||
title: 'Кофейня s1маркер центр',
|
||||
textContent: 'обычный текст без ключевого слова',
|
||||
});
|
||||
const res = await search(buildService(), { query: '+кофейня', spaceId });
|
||||
const hit = res.items.find((i: any) => i.id === page);
|
||||
expect(hit).toBeDefined();
|
||||
// The title matches the required term → matchedFields includes 'title'.
|
||||
expect(hit.matchedFields).toContain('title');
|
||||
// FTS rank is populated (was null before the S1 fix).
|
||||
expect(hit.rank).not.toBeNull();
|
||||
// matchedTerms already echoed the required term; still true.
|
||||
expect(hit.matchedTerms).toContain('кофейня');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,123 @@
|
||||
import { randomUUID } from 'node:crypto';
|
||||
import { Kysely, sql } from 'kysely';
|
||||
import {
|
||||
getTestDb,
|
||||
destroyTestDb,
|
||||
createWorkspace,
|
||||
createSpace,
|
||||
} from './db';
|
||||
|
||||
/**
|
||||
* #443 dead-index guard — EXPLAIN on the REAL DB.
|
||||
*
|
||||
* The lookup mode's substring predicates run a leading-wildcard
|
||||
* `LOWER(f_unaccent(col)) LIKE '%q%'`. Those are only fast when Postgres uses
|
||||
* the GIN trigram indexes:
|
||||
* - idx_pages_title_trgm on (LOWER(f_unaccent(title))) [#348]
|
||||
* - idx_pages_text_content_trgm on (LOWER(f_unaccent(text_content))) [#443]
|
||||
*
|
||||
* Postgres uses a functional index ONLY when the query expression matches the
|
||||
* index expression EXACTLY. The original lookup query wrapped the columns in
|
||||
* `coalesce(col,'')`, which differs from the coalesce-FREE index expression and
|
||||
* silently forced a Seq Scan on pages for EVERY lookup (the MCP client always
|
||||
* sends substring:true). This test locks that in.
|
||||
*
|
||||
* Discriminator: `SET enable_seqscan = off` asks the planner "CAN this predicate
|
||||
* use the index at all?" — which is exactly what the coalesce bug breaks. With
|
||||
* seqscan disabled:
|
||||
* - the coalesce-FREE (fixed) predicate plans a Bitmap Index Scan on the trgm
|
||||
* index (no Seq Scan on pages);
|
||||
* - the coalesce-WRAPPED (buggy) predicate cannot use the index and falls back
|
||||
* to a Seq Scan on pages even though seqscan is disabled.
|
||||
* We assert both to prove the fix and to keep the regression from silently
|
||||
* returning.
|
||||
*/
|
||||
describe('SearchService agent-lookup EXPLAIN — trgm index is live [integration]', () => {
|
||||
let db: Kysely<any>;
|
||||
let workspaceId: string;
|
||||
let spaceId: string;
|
||||
|
||||
async function insertPage(title: string, textContent: string): Promise<void> {
|
||||
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<string> {
|
||||
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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user