feat(admin): backfill-vector-indexes command to reconcile per-bank vector index coverage (#2645)

[benfrank241]

Addresses #2645 (part 1 of 2).

Problem

Per-(bank, fact_type) partial vector indexes are only created at fresh-bank-creation time. Banks that arrive already populated — via logical restore, cross-version upgrade, or a vector-extension switch (e.g. ScaNN→pgvector) — never hit that path, so their recall silently falls back to the global HNSW + post-filter. As @iRonin measured on a live 40-bank deployment, that's not just a latency hit: it's recall@10 of ~0.63–0.72 (only ~6.3–7.5 of 10 in-bank neighbors returned) — a silent ~30% top-10 miss, i.e. a correctness regression, on any bank in the uncovered state.

This PR — the operator escape hatch

hindsight-admin backfill-vector-indexes reconciles the missing per-bank partial indexes on populated banks — first-classing exactly what @iRonin did by hand (63 indexes across 20 banks).

• CREATE INDEX CONCURRENTLY — never takes ACCESS EXCLUSIVE on the shared memory_units, so backfilling one bank can't stall the fleet's retain/recall/consolidation. (It runs on the admin CLI's raw autocommit connection, since CONCURRENTLY can't run in a txn.)
• Idempotent — pg_indexes existence check + IF NOT EXISTS; safe to re-run.
• Invalid-index cleanup — a failed concurrent build leaves an INVALID index behind; it's dropped with a loud warning so a re-run retries cleanly, and the command exits non-zero listing any failures.
• --dry-run reports what would be built without touching anything.
• Multi-tenant — base schema + all discovered tenant schemas (or --schema).
• Backend guard — no-op for backends that use a single global index (AlloyDB ScaNN, Oracle).
• Reuses the existing index-name scheme (_bank_index_name / _BANK_INDEX_FACT_TYPES / _vector_index_clause) so backfilled indexes are byte-identical to create-time ones.

Deliberately scoped — follow-up (part 2)

Per the design locked in #2645, the ongoing convergence layer (boot/periodic background reconcile as the primary guarantee, plus a lightweight retain-path pg_class existence check as belt-and-suspenders) is a separate PR — it touches the worker/retain hot paths and deserves its own review. This PR is the immediate, self-contained escape hatch that closes the current uncovered state and is directly testable.

Tests

tests/test_backfill_vector_indexes.py (4 tests, deterministic, pg0):

• core regression: a populated bank whose per-bank indexes are dropped (simulating restore/upgrade) gets them rebuilt;
• --dry-run creates nothing;
• re-run is idempotent (no error, no dupes);
• no-op for a backend without per-bank indexes.

cc @iRonin — this is the command; happy to have you point it at the staging copy of your deployment with your recall@K harness for a before/after once CI is green. The boot-reconcile follow-up will land separately.

[benfrank241]

@nicoloboschi flagging you on this one — it's the part-1 escape-hatch PR for #2645 (per-bank vector index coverage never self-heals on restore/upgrade/extension-switch → ~30% recall@10 miss, measured by @iRonin on a live 40-bank deployment).

I built + verified it (admin backfill-vector-indexes: CONCURRENTLY, idempotent, invalid-index cleanup, dry-run, multi-tenant, backend-guarded, reuses the existing _bank_index_name scheme). Wanted your eyes before it lands since it's core index/migration-adjacent territory you own, and because part 2 (boot/periodic reconcile + retain-path existence check) touches the worker/retain hot paths and should be designed with your input.

CI status: our 4 new tests pass (green in test-api (1/3)). The red jobs are all pre-existing/flake — the deterministic TestOracleSetSessionSchema reds fail on main too (unrelated to this diff), plus the usual sentence-transformers-not-installed / pg0-boot / LLM-judge / windows flakes. So nothing red here is from this PR.

@iRonin has offered to run their recall@K harness against this on a staging copy of their deployment for a real before/after — good validation path once you're happy with the shape.

[iRonin]

Ran the command's detection logic as READ-ONLY SELECTs against our live 0.8.4 deployment (the one from #2645 — prod can't run the branch binary; it already carries the manual backfill discussed there).

What validates cleanly:

• The generated DDL is byte-identical to the create-time path (ops_postgresql.create_bank_vector_indexes) apart from CONCURRENTLY: same idx_mu_emb_{worl,expr,obsv}_<internal_id16> names, same WHERE fact_type = … AND bank_id = … predicate, same USING clause.
• Detection maps exactly onto our deployment: 44 banks × 3 fact types = 132 expected → 96 present by name, 36 flagged. Of the flagged, 33 are across 11 small/empty banks (all ≤ ~1k rows — under the row threshold our manual backfill had used), cheap and strictly beneficial to build. On the banks we backfilled by hand, the command correctly reports complete.
• Integrity scan: 0 invalid indexes, 0 predicate/name mismatches, 0 orphan per-bank indexes.
• Autocommit connection for CONCURRENTLY ✓, idempotent re-run ✓, non-zero exit listing failures ✓, tenant discovery matches run-db-migration ✓, ScaNN guard ✓.

Finding 1 (the one I'd block on) — a stale INVALID index is mis-detected as "present", silently defeating the fix for exactly the operator state #2645 targets. Chain: (a) detection is name-based via pg_indexes; (b) pg_indexes doesn't filter on indisvalid — an INVALID index counts as existing; (c) CREATE INDEX CONCURRENTLY IF NOT EXISTS then no-ops on the existing name, so the build path can't repair it either. Net: a bank whose per-bank index is INVALID — left behind by any crashed/interrupted CONCURRENTLY build, precisely what an operator attempting the manual fix from the issue thread can end up with — is reported as fully covered while remaining effectively uncovered forever. The in-run cleanup only handles builds that fail during this run; pre-existing invalids are invisible to it. Suggest: existence check joins pg_class/pg_index and requires indisvalid (and probably indisready); treat invalid as missing → DROP INDEX CONCURRENTLY IF EXISTS + rebuild. Flagging for part 2 as well — if the shared reconcile logic inherits this check, the background job will report full coverage over a stale INVALID on every pass.

Finding 2 — name-only detection double-builds where an operator hand-built equivalents under different names. The remaining 3 of our 36 flagged are on our single largest bank (~118k rows), which we covered by hand before this command existed using non-standard index names (identical predicate + opclass). The command would build 3 duplicate system-named indexes on the biggest table in the run — harmless to correctness, but wasted CONCURRENTLY build time + permanent write amplification in triplicate. Suggest either a coverage-aware check (any valid partial index on memory_units whose pg_get_indexdef predicate matches that bank_id + fact_type) or a doc note that pre-existing operator-built indexes under other names will be duplicated and can be dropped afterward.

Nits: alphabetical build order means the largest, most recall-impacted banks converge last (consider row-count desc); the failure-cleanup DROP INDEX is non-concurrent (brief ACCESS EXCLUSIVE on the shared table — DROP INDEX CONCURRENTLY is safer on a live fleet); optional trailing ANALYZE.

Happy to provide the recall@K before/after on a staging copy — one harness note: the deployment needs plan_cache_mode = force_custom_plan (or equivalent), otherwise prepared-statement generic plans won't match the partial-index predicates and the delta won't show. We hit exactly that on our deployment.

[nicoloboschi]

Closing in favor of #2872 (merged as bd853be), which took a minimal cut of this work.

Thank you @benfrank241 — this PR established the operator-escape-hatch concept and the CREATE INDEX CONCURRENTLY / idempotent / multi-tenant shape that #2872 builds on. #2872 folds the command together with the import-bank leak fix (the one in-app source of uncovered banks) into a single re-runnable repair-bank (--bank | --all).

One substantive change from this PR worth calling out: the coverage check here uses a name-only pg_indexes lookup, which — as @iRonin flagged — treats an INVALID leftover (or a drifted access method after a backend switch) as "already present" and never repairs it. #2872 uses a pg_index.indisvalid/indisready + partial-predicate health check so those are detected as missing and rebuilt.