Files
gbrain/test/postgres-engine.test.ts
T
SilverNineandClaude Fable 5 0621687276 fix(search): port the CJK keyword fallback to the Postgres engine (PGLite parity)
websearch_to_tsquery('english') can't tokenize CJK, so the keyword arm
of hybrid search degrades badly for Korean/Chinese/Japanese queries on
the Postgres engine — particle-suffixed Korean often matches nothing.
PGLite has had the hasCJK() -> ILIKE + occurrence-count-ranking fallback
since v0.32.7; the Postgres engine never got it.

Port _searchKeywordCJK to postgres-engine.ts at both FTS sites
(searchKeyword dedup path + searchKeywordChunks), keeping the host
engine's conventions (sql.begin + SET LOCAL statement_timeout, named
limit params, its extra-filter set including types/exclude_slugs).
scoreExpr, ILIKE ESCAPE discipline, empty-query guard, and the
dedup/chunk-grain split are copied from PGLite exactly.

Measured on a private production Korean corpus (74k chunks):
particle-suffixed queries returned 7 rows via english FTS vs 152 via
the fallback; a common two-word query 21 vs 847 — a 20-40x recall gap
this closes.

Tests: 6 source-guardrail tests in test/postgres-engine.test.ts +
3 Postgres-vs-PGLite parity e2e tests with synthetic Korean fixtures.
bun run verify 31/31.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-05 12:51:40 +09:00

191 lines
8.7 KiB
TypeScript

/**
* postgres-engine.ts source-level guardrails.
*
* Live Postgres coverage for search paths lives in test/e2e/search-quality.test.ts.
* This file stays fast and DB-free: it inspects the source of
* src/core/postgres-engine.ts to lock in decisions that protect the
* shared connection pool from per-request GUC leaks.
*
* Regression: R6-F006 / R4-F002.
* searchKeyword and searchVector used to call bare
* await sql`SET statement_timeout = '8s'`
* ...query...
* finally { await sql`SET statement_timeout = '0'` }
* against the shared pool. Each tagged template picks an arbitrary
* connection, so the SET, the query, and the reset could all land on
* DIFFERENT connections. Worst case: the 8s GUC sticks on some pooled
* connection and clips the next caller's long-running query; or the
* reset to 0 lands on a connection that other code expected to be
* protected. The fix wraps each query in sql.begin() and uses
* SET LOCAL so the GUC is transaction-scoped and auto-resets on
* COMMIT/ROLLBACK, regardless of error path.
*/
import { describe, test, expect } from 'bun:test';
import { readFileSync } from 'fs';
import { join } from 'path';
const SRC = readFileSync(
join(import.meta.dir, '..', 'src', 'core', 'postgres-engine.ts'),
'utf-8',
);
describe('postgres-engine / search path timeout isolation', () => {
test('no bare `SET statement_timeout` statement survives', () => {
// Strip comments so the commentary mentioning the anti-pattern does
// not trigger a false positive. Block-comment + line-comment strip.
const stripped = SRC
.replace(/\/\*[\s\S]*?\*\//g, '')
.replace(/(^|\s)\/\/[^\n]*/g, '$1');
// Match a tagged-template statement of the form
// sql`SET statement_timeout = ...`
// that is NOT preceded by LOCAL. This is the exact shape that bleeds
// onto pooled connections; SET LOCAL is safe inside a transaction.
const bare = stripped.match(
/sql`\s*SET\s+(?!LOCAL\s)statement_timeout\b[^`]*`/gi,
);
expect(bare).toBeNull();
});
test('searchKeyword wraps its query in sql.begin()', () => {
const fn = extractMethod(SRC, 'searchKeyword');
expect(fn).toMatch(/sql\.begin\s*\(\s*async\s+sql\s*=>/);
});
test('searchVector wraps its query in sql.begin()', () => {
const fn = extractMethod(SRC, 'searchVector');
expect(fn).toMatch(/sql\.begin\s*\(\s*async\s+sql\s*=>/);
});
test('both search methods use SET LOCAL for the timeout', () => {
const keyword = extractMethod(SRC, 'searchKeyword');
const vector = extractMethod(SRC, 'searchVector');
expect(keyword).toMatch(/SET\s+LOCAL\s+statement_timeout/);
expect(vector).toMatch(/SET\s+LOCAL\s+statement_timeout/);
});
test('connect() with poolSize honors resolvePrepare (PgBouncer regression guard)', () => {
// Regression: worker-instance pools were NOT honoring the prepare decision
// before v0.15.4. Module singleton connect() in db.ts was fixed by #284 but
// PostgresEngine.connect({poolSize}) (the branch used by `gbrain jobs work`)
// silently ignored it — agents running background work against Supabase
// pooler URLs still hit `prepared statement "..." does not exist` under
// load. Source-level grep is enough: runtime mocking of postgres.js's
// tagged-template interface is painful under bun ESM and the wiring is
// simple enough that if `resolvePrepare` name appears and a conditional
// `prepare` key appears in the options literal, the wire-up is live.
const stripped = stripComments(SRC);
expect(stripped).toMatch(/db\.resolvePrepare\s*\(\s*url\s*\)/);
expect(stripped).toMatch(/typeof\s+prepare\s*===\s*['"]boolean['"]/);
});
test('neither search method clears the timeout with `SET statement_timeout = 0`', () => {
// The reset-to-zero pattern was the other half of the leak: if SET
// LOCAL is in play, COMMIT handles the reset and an explicit
// `SET statement_timeout = '0'` would itself leak the GUC change
// onto the returned connection. Strip comments first so the
// commentary in the method itself (which quotes the anti-pattern
// to explain it) does not trigger a false positive.
const keyword = stripComments(extractMethod(SRC, 'searchKeyword'));
const vector = stripComments(extractMethod(SRC, 'searchVector'));
expect(keyword).not.toMatch(/SET\s+statement_timeout\s*=\s*['"]?0/);
expect(vector).not.toMatch(/SET\s+statement_timeout\s*=\s*['"]?0/);
});
});
// v0.32.7 CJK branch (Postgres parity) guardrails. Live-DB coverage of the
// fallback lives in test/e2e/engine-parity.test.ts (Korean parity case);
// this block stays DB-free and locks in the source-level invariants that
// make the port faithful to pglite-engine.ts `_searchKeywordCJK`.
describe('postgres-engine / CJK keyword fallback (v0.32.7 parity)', () => {
test('searchKeyword routes CJK queries to _searchKeywordCJK', () => {
const fn = stripComments(extractMethod(SRC, 'searchKeyword'));
expect(fn).toMatch(/if\s*\(\s*hasCJK\s*\(\s*query\s*\)\s*\)/);
expect(fn).toMatch(/this\._searchKeywordCJK\s*\(/);
});
test('searchKeywordChunks routes CJK queries to _searchKeywordCJK (chunk-grain, no dedup)', () => {
const fn = stripComments(extractMethod(SRC, 'searchKeywordChunks'));
expect(fn).toMatch(/if\s*\(\s*hasCJK\s*\(\s*query\s*\)\s*\)/);
expect(fn).toMatch(/this\._searchKeywordCJK\s*\(/);
expect(fn).toMatch(/dedup:\s*false/);
});
test('_searchKeywordCJK matches via ILIKE with explicit ESCAPE, never tsquery', () => {
const fn = extractCJKHelper(SRC);
// ILIKE '%' || $1 || '%' ESCAPE '\' — the escaped-pattern binding.
expect(fn).toMatch(/ILIKE\s+'%'\s*\|\|\s*\$1\s*\|\|\s*'%'\s+ESCAPE/);
// The fallback must not route back through the english tokenizer it
// exists to bypass.
expect(stripComments(fn)).not.toMatch(/websearch_to_tsquery/);
});
test('_searchKeywordCJK keeps the two-binding discipline (qLike escaped, qRaw for ranking)', () => {
const fn = stripComments(extractCJKHelper(SRC));
// $1 comes from escapeLikePattern; $2 is the raw string used by the
// REPLACE/POSITION occurrence-count scoring. Escaped chars cannot be
// reused as ranking substrings (codex outside-voice C8).
expect(fn).toMatch(/escapeLikePattern\s*\(\s*qRaw\s*\)/);
expect(fn).toMatch(/REPLACE\(cc\.chunk_text,\s*\$2/);
expect(fn).toMatch(/POSITION\(\$2\s+IN\s+cc\.chunk_text\)/);
});
test('_searchKeywordCJK honors the pool-safety contract (sql.begin + SET LOCAL)', () => {
const fn = extractCJKHelper(SRC);
expect(fn).toMatch(/sql\.begin\s*\(\s*async\s+sql\s*=>/);
expect(fn).toMatch(/SET\s+LOCAL\s+statement_timeout/);
expect(stripComments(fn)).not.toMatch(/SET\s+statement_timeout\s*=\s*['"]?0/);
});
test('_searchKeywordCJK seals source-isolation on the fallback path (#861 parity)', () => {
const fn = stripComments(extractCJKHelper(SRC));
expect(fn).toMatch(/p\.source_id = ANY\(\$\$\{params\.length\}::text\[\]\)/);
expect(fn).toMatch(/opts\?\.sourceId/);
});
});
function stripComments(s: string): string {
return s
.replace(/\/\*[\s\S]*?\*\//g, '')
.replace(/(^|\s)\/\/[^\n]*/g, '$1');
}
// _searchKeywordCJK's signature contains an object-type literal (the ctx
// param), so extractMethod's first-brace matcher would stop at the type.
// Slice from the declaration to the method's 2-space-indent closing brace
// instead (nested blocks all sit deeper).
function extractCJKHelper(source: string): string {
const start = source.indexOf('private async _searchKeywordCJK(');
if (start < 0) throw new Error('_searchKeywordCJK not found in postgres-engine.ts');
const end = source.indexOf('\n }', start);
if (end < 0) throw new Error('no closing brace for _searchKeywordCJK');
return source.slice(start, end + 4);
}
// extractMethod grabs the body of a class method by brace-matching from
// its opening line. Returns the method body up to the matching closing
// brace. Good enough for the small number of methods in this file.
function extractMethod(source: string, name: string): string {
// Find "async <name>(" at method-definition indentation (2 spaces).
const openRe = new RegExp(`^\\s+async\\s+${name}\\s*\\(`, 'm');
const match = openRe.exec(source);
if (!match) {
throw new Error(`method ${name} not found in postgres-engine.ts`);
}
// Scan forward balancing braces.
let i = source.indexOf('{', match.index);
if (i < 0) throw new Error(`no opening brace for ${name}`);
const start = i;
let depth = 0;
for (; i < source.length; i++) {
const c = source[i];
if (c === '{') depth++;
else if (c === '}') {
depth--;
if (depth === 0) return source.slice(start, i + 1);
}
}
throw new Error(`unbalanced braces in ${name}`);
}