fix(search): B1/W2 — honest page_embeddings.fts swap + true idempotency (#529)

BLOCKER B1: the fts config swap no longer blindly DROP+ADDs the generated
column. swapEmbeddingsFtsConfig now (1) reads the column's actual generation
expression from pg_catalog and TRUE-no-ops when it already references the target
config (real out-of-band escape hatch), and (2) gates the inline ACCESS
EXCLUSIVE rewrite behind SEARCH_EMBEDDINGS_FTS_INLINE_REWRITE (default 'true';
only a literal 'false' opts out), warning that the operator owns the swap.
up() now creates ru_en only when missing (drop-recreate would fail on the fts
hard dependency on a re-run), so up() is genuinely idempotent. down() guards the
config DROP when fts still references ru_en (gated-off path), staying non-fatal.

W2: corrected the header — Kysely runs EACH migration in its own transaction
(not the whole set in ONE); the single-UPDATE atomicity conclusion is unchanged.
Rewrote the runbook: inline path is a full-table ACCESS EXCLUSIVE rewrite of
page_embeddings; removed the false "IF-EXISTS no-op" claim.

Extends the roundtrip test with idempotent-skip (2nd up() no-op) and env-gate
=false (embeddings rewrite skipped, pages.tsv still swapped) assertions.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

apps/server/src/database/migrations/20260707T120000-search-ru-en-config.ts

@@ -24,31 +24,55 @@ import { type Kysely, sql } from 'kysely';
  * commit. Trigram indexes are LOWER(f_unaccent(...)) and do NOT depend on the FTS
  * config, so they are untouched.
  *
- * REINDEX / LOCK MODEL (deploy-critical). This repo's Kysely Migrator runs the
- * whole pending set inside ONE transaction (migration.service.ts / migrate.ts set
- * no `disableTransactions`), and PostgresJSDialect has transactional DDL — so a
- * migration file CANNOT commit between batches, which means the issue's
- * "procedural batch job OUTSIDE the transaction + dual-config read window +
- * migration_complete gate" is not expressible in-migration here. That machinery
- * exists to bridge a reindex spread over MANY committed batches, during which some
- * rows are still `english` while others are already `ru_en`. Our reindex is ATOMIC
- * (one transaction): at COMMIT every row is `ru_en` at once, so no morphology-
- * desync window exists and no dual-config read path is required — the query config
- * flips to `ru_en` in the very same release. This is the deliberate, correct
- * adaptation to this framework (see the PR notes).
+ * REINDEX / LOCK MODEL (deploy-critical). Kysely runs EACH migration in its OWN
+ * transaction (see sibling 20260706T120000 "Kysely runs each migration in a
+ * transaction"; migrate.ts / migration.service.ts set no `disableTransactions`,
+ * and PostgresJSDialect has transactional DDL). A single migration file therefore
+ * cannot commit between batches, so the issue's "procedural batch job OUTSIDE the
+ * transaction + dual-config read window + migration_complete gate" is not
+ * expressible in-migration here. That machinery bridges a reindex spread over
+ * MANY committed batches, during which some rows are still `english` while others
+ * are already `ru_en`. Our pages.tsv reindex is a SINGLE `UPDATE pages SET tsv`,
+ * ATOMIC within THIS migration's own transaction: at COMMIT every row is `ru_en`
+ * at once, so no morphology-desync window exists and no dual-config read path is
+ * required — the query config flips to `ru_en` in the very same release. This is
+ * the deliberate, correct adaptation to this framework (see the PR notes).
  *
  *   - pages.tsv: swapping the trigger is a cheap catalog change (no table lock),
  *     and the reindex is a single `UPDATE pages SET tsv = <ru_en expr>` — a
- *     ROW-level-lock backfill, NOT an ACCESS EXCLUSIVE rewrite (mirrors the
- *     existing space_id backfill in 20250725T052004).
- *   - page_embeddings.fts is a GENERATED STORED column; Postgres has no way to
- *     change a generated expression without DROP+ADD, which rewrites the table
- *     under ACCESS EXCLUSIVE. On a LARGE tenant run this in a maintenance window,
- *     or do it out-of-band before deploy: add a plain `fts_ruen` column, backfill
- *     with CREATE INDEX CONCURRENTLY-style batched UPDATEs, then swap — the
- *     IF-EXISTS guards here then no-op. Small/typical tenants (this fork's target)
- *     are fine with the in-migration rewrite. Same documented trade-off as the
- *     #443 trgm GIN migration (20260706T120000).
+ *     ROW-level-lock (RowExclusiveLock) backfill, NOT an ACCESS EXCLUSIVE rewrite
+ *     (mirrors the existing space_id backfill in 20250725T052004). On a LARGE
+ *     tenant this still writes every row + its WAL and leaves dead tuples, so it
+ *     can take MINUTES and blocks the startup migrator for that time — but it
+ *     never blocks concurrent reads. It stays inline unconditionally.
+ *
+ *   - page_embeddings.fts is a GENERATED STORED column; Postgres cannot change a
+ *     generated expression without DROP+ADD, which is a full-table ACCESS
+ *     EXCLUSIVE REWRITE of page_embeddings — it blocks ALL reads AND writes on
+ *     that table (including the RAG agent) for the rewrite's duration. That inline
+ *     rewrite is appropriate for small/typical tenants (this fork's target) and
+ *     is the DEFAULT.
+ *
+ *     LARGE TENANTS have two documented escape hatches, either of which makes the
+ *     migration genuinely no-op the rewrite (it is NOT a blind DROP+ADD):
+ *       (a) Set `SEARCH_EMBEDDINGS_FTS_INLINE_REWRITE=false`. The migration then
+ *           SKIPS the embeddings rewrite entirely and logs a WARNING. The operator
+ *           MUST perform the ru_en fts swap out-of-band; until they do, the RAG
+ *           lexical leg stays on `english` while the query config is `ru_en` — a
+ *           documented, operator-owned desync window. (pages.tsv still swaps
+ *           inline — the gate is ONLY the embeddings rewrite.)
+ *       (b) Perform the swap out-of-band BEFORE deploy — add a plain column →
+ *           batched backfill → brief-lock swap → CREATE INDEX CONCURRENTLY — so
+ *           the `fts` column's generated expression already references the TARGET
+ *           config when the migration runs. The migration detects this (it reads
+ *           the column's actual generation expression from pg_catalog) and does a
+ *           TRUE no-op — no DROP, no ADD, no rewrite. This is real idempotency,
+ *           not the old (false) "IF-EXISTS guards no-op" claim: `DROP COLUMN IF
+ *           EXISTS` guards against ABSENCE, not presence, so it would have dropped
+ *           and recreated an existing `fts` regardless. The at-target check is the
+ *           only honest no-op path.
+ *
+ * Same documented trade-off family as the #443 trgm GIN migration (20260706T120000).
  */
 
 // pages.tsv trigger body for a given FTS config — mirrors the latest form
@@ -82,13 +106,65 @@ async function swapPagesConfig(db: Kysely<any>, config: 'ru_en' | 'english') {
   `.execute(db);
 }
 
+// The default in-migration ACCESS EXCLUSIVE rewrite of page_embeddings.fts is
+// ON unless the operator explicitly opts out with the env flag. Parsed strictly
+// (mirrors CLIENT_TELEMETRY_ENABLED / DEBUG_MODE in common/): only a literal
+// (case-insensitive) 'false' disables it; anything else — unset included —
+// keeps the default true.
+function inlineEmbeddingsRewriteEnabled(): boolean {
+  return (
+    (process.env.SEARCH_EMBEDDINGS_FTS_INLINE_REWRITE ?? 'true').toLowerCase() !==
+    'false'
+  );
+}
+
+// Read page_embeddings.fts's ACTUAL generated-column expression from pg_catalog
+// (the generation expression is stored as a column default marked generated).
+// Returns '' when the column is absent.
+async function embeddingsFtsExpr(db: Kysely<any>): Promise<string> {
+  const r = await sql<{ def: string }>`
+    SELECT pg_get_expr(d.adbin, d.adrelid) AS def
+    FROM pg_attrdef d
+    JOIN pg_attribute a ON a.attrelid = d.adrelid AND a.attnum = d.adnum
+    WHERE a.attname = 'fts'
+      AND a.attrelid = 'page_embeddings'::regclass
+      AND NOT a.attisdropped
+  `.execute(db);
+  return r.rows[0]?.def ?? '';
+}
+
 async function swapEmbeddingsFtsConfig(
   db: Kysely<any>,
   config: 'ru_en' | 'english',
 ) {
-  // The generated `fts` expression can only change via DROP+ADD (a table
-  // rewrite; see the lock note in the header). The GIN index depends on the
-  // column, so it is dropped with it and recreated.
+  // 1. TRUE no-op path (real out-of-band escape hatch): if the column's current
+  //    generation expression already references the TARGET config, there is
+  //    nothing to do. An operator who pre-swapped the column out-of-band lands
+  //    here and the migration does NOT rewrite the table. ('ru_en' and 'english'
+  //    are disjoint tokens, neither a substring of the other or of the rest of
+  //    the expression, so a plain contains-check is unambiguous.)
+  const currentExpr = await embeddingsFtsExpr(db);
+  if (currentExpr.includes(config)) return;
+
+  // 2. Env-gated opt-out: large tenants set SEARCH_EMBEDDINGS_FTS_INLINE_REWRITE
+  //    =false to skip the ACCESS EXCLUSIVE rewrite in-migration and own the swap
+  //    out-of-band. Warn loudly so the desync window is not silent.
+  if (!inlineEmbeddingsRewriteEnabled()) {
+    // eslint-disable-next-line no-console
+    console.warn(
+      `[migration 20260707T120000] SEARCH_EMBEDDINGS_FTS_INLINE_REWRITE=false: ` +
+        `SKIPPING the page_embeddings.fts rewrite to '${config}'. The operator MUST ` +
+        `perform this fts swap out-of-band. Until then the RAG lexical leg stays on ` +
+        `its current config while the query config is '${config}' (documented, ` +
+        `operator-owned desync window).`,
+    );
+    return;
+  }
+
+  // 3. Default inline path: the generated `fts` expression can only change via
+  //    DROP+ADD (a full-table ACCESS EXCLUSIVE rewrite; see the lock note in the
+  //    header). The GIN index depends on the column, so it is dropped with it and
+  //    recreated.
   await sql`DROP INDEX IF EXISTS idx_page_embeddings_fts`.execute(db);
   await sql`ALTER TABLE page_embeddings DROP COLUMN IF EXISTS fts`.execute(db);
   await sql`
@@ -102,10 +178,19 @@ async function swapEmbeddingsFtsConfig(
   `.execute(db);
 }
 
-export async function up(db: Kysely<any>): Promise<void> {
-  // Create the ru_en configuration (idempotent: drop first, nothing depends on it
-  // yet at this point in up()).
-  await sql`DROP TEXT SEARCH CONFIGURATION IF EXISTS ru_en`.execute(db);
+async function ruEnConfigExists(db: Kysely<any>): Promise<boolean> {
+  const r = await sql<{ n: number }>`
+    SELECT count(*)::int AS n FROM pg_ts_config WHERE cfgname = 'ru_en'
+  `.execute(db);
+  return (r.rows[0]?.n ?? 0) > 0;
+}
+
+async function ensureRuEnConfig(db: Kysely<any>): Promise<void> {
+  // Idempotent by EXISTENCE, not by drop-recreate. The old `DROP ... IF EXISTS;
+  // CREATE` was safe only on a first run: on a re-run the page_embeddings.fts
+  // generated column already has a hard dependency on ru_en, so dropping the
+  // config would fail. Create only when it is genuinely missing.
+  if (await ruEnConfigExists(db)) return;
   await sql`CREATE TEXT SEARCH CONFIGURATION ru_en (COPY = simple)`.execute(db);
   // Latin token classes → english_stem.
   await sql`
@@ -119,8 +204,14 @@ export async function up(db: Kysely<any>): Promise<void> {
       ALTER MAPPING FOR word, hword, hword_part
       WITH russian_stem
   `.execute(db);
+}
+
+export async function up(db: Kysely<any>): Promise<void> {
+  await ensureRuEnConfig(db);
 
   // Flip both stored sides to ru_en (query-side flips in the same commit).
+  // swapEmbeddingsFtsConfig no-ops when fts already references ru_en, so a
+  // re-run of up() is idempotent and does NOT re-rewrite the embeddings table.
   await swapPagesConfig(db, 'ru_en');
   await swapEmbeddingsFtsConfig(db, 'ru_en');
 }
@@ -132,5 +223,20 @@ export async function down(db: Kysely<any>): Promise<void> {
   // block the DROP with a dependency error).
   await swapEmbeddingsFtsConfig(db, 'english');
   await swapPagesConfig(db, 'english');
+
+  // Drop the config ONLY if nothing still references it. When the embeddings
+  // rewrite was gated off (SEARCH_EMBEDDINGS_FTS_INLINE_REWRITE=false), the fts
+  // column can still reference ru_en — dropping the config would then fail with a
+  // dependency error. Skip + warn so down() stays non-fatal; the operator drops
+  // ru_en after completing the out-of-band english swap.
+  if ((await embeddingsFtsExpr(db)).includes('ru_en')) {
+    // eslint-disable-next-line no-console
+    console.warn(
+      `[migration 20260707T120000] down(): page_embeddings.fts still references ` +
+        `ru_en (inline rewrite was gated off) — leaving the ru_en text-search ` +
+        `configuration in place. Drop it out-of-band once fts is back on 'english'.`,
+    );
+    return;
+  }
   await sql`DROP TEXT SEARCH CONFIGURATION IF EXISTS ru_en`.execute(db);
 }

apps/server/test/integration/search-ru-en-migration.int-spec.ts

@@ -42,10 +42,11 @@ describe('search ru_en config migration [integration]', () => {
   });
 
   afterAll(async () => {
-    // Restore the migrated (ru_en) state for any later suite, then close.
-    if (!(await configExists())) {
-      await migration.up(db);
-    }
+    // Restore the canonical ru_en state for any later suite regardless of where
+    // a test left off. up() is now safe on an existing config (ensureRuEnConfig
+    // no-ops) and re-asserts both stored sides to ru_en.
+    delete process.env.SEARCH_EMBEDDINGS_FTS_INLINE_REWRITE;
+    await migration.up(db);
     await destroyTestDb();
   });
 
@@ -69,4 +70,42 @@ describe('search ru_en config migration [integration]', () => {
     expect(await ftsDef()).toContain('ru_en');
     expect(await triggerSrc()).toContain('ru_en');
   });
+
+  // B1.1 — running up() a SECOND time on an already-migrated DB is a true no-op:
+  // it must NOT throw (the old drop-recreate-config would fail on the fts hard
+  // dependency) and must NOT re-rewrite the embeddings table — the fts column
+  // already references ru_en, so the at-target check short-circuits.
+  it('up() is idempotent: a 2nd run does not error and leaves ru_en intact', async () => {
+    await expect(migration.up(db)).resolves.toBeUndefined();
+    expect(await configExists()).toBe(true);
+    expect(await ftsDef()).toContain('ru_en');
+    expect(await triggerSrc()).toContain('ru_en');
+  });
+
+  // B1.2 — env-gate opt-out: with SEARCH_EMBEDDINGS_FTS_INLINE_REWRITE=false the
+  // embeddings rewrite is SKIPPED (fts stays where it was) but pages.tsv still
+  // swaps inline, and the ru_en config is left in place (fts still depends on it).
+  // Runs down() as the vehicle: english is NOT the current fts config, so the
+  // skip path — not the at-target no-op — is exercised.
+  it('env-gate=false: skips the fts rewrite but still swaps pages.tsv', async () => {
+    // Precondition: fts + trigger on ru_en (from the prior test).
+    expect(await ftsDef()).toContain('ru_en');
+    process.env.SEARCH_EMBEDDINGS_FTS_INLINE_REWRITE = 'false';
+    try {
+      await migration.down(db);
+      // fts rewrite skipped → still ru_en (the ACCESS EXCLUSIVE rewrite avoided).
+      expect(await ftsDef()).toContain('ru_en');
+      expect(await ftsDef()).not.toContain('english');
+      // pages.tsv trigger still swapped inline to english (gate is fts-only).
+      expect(await triggerSrc()).toContain('english');
+      // Config left in place because fts still references it (guarded drop).
+      expect(await configExists()).toBe(true);
+    } finally {
+      // Restore ru_en fully for the afterAll / later suites.
+      delete process.env.SEARCH_EMBEDDINGS_FTS_INLINE_REWRITE;
+      await migration.up(db);
+      expect(await ftsDef()).toContain('ru_en');
+      expect(await triggerSrc()).toContain('ru_en');
+    }
+  });
 });