Fix #172: suggested_links 500 — HNSW approximate-NN, not a full-corpus self-join scan

[mkreyman]

Summary

Fixes #172 — GET /api/v1/knowledge/articles/:id/suggested_links still 500'd in prod after #170.

Root cause (corrected from #170): #170 removed the %Pgvector{} re-interpolation (the original #168 500) but rewrote the query to a column-to-column self-join that read the target vector as a column (a.embedding <=> t.embedding). A column right-operand can't use the HNSW index, so the query did a per-row cosine + full ORDER BY ... <=> sort over every published-embedded article. Negligible on a handful of test rows; at prod scale (~76k articles) it exceeds the statement timeout → generic 500 on 100% of calls.

Fix: pass the already-fetched target embedding as a bound [float()] list param — the exact shape search_semantic and distant_pairs ship in production. ORDER BY embedding <=> $const LIMIT k is the HNSW-indexable form (articles_embedding_idx, vector_cosine_ops), so the planner does an approximate-NN index scan instead of a full-corpus sort. Converting the stored %Pgvector{} to a plain list (not binding the struct) keeps the #168 fix intact.

The query stays a bare AdminRepo.all — no transaction, matching search_semantic/distant_pairs, which deliberately avoid holding one of the 3 AdminRepo connections (see the distant_pairs pool-starvation note in the same module).

Enhanced review (team + adversarial, Opus)

Both rounds ran; every agent claim was independently verified against the code/tests.

• Round 1 (team): flagged that a single HNSW ef_search batch could let the already-linked anti-join under-fill the result below limit. An initial fix added SET LOCAL hnsw.iterative_scan in a transaction.
• Round 2 (adversarial): three independent reviewers (security, architect, skeptical-BA) converged that the transaction wrapper was worse than the bug it fixed — pool starvation on the 3-connection admin pool (the hottest knowledge endpoint), a 20k-tuple max_scan_tuples walk on the common sparse-article case, and a new SET LOCAL failure surface. Resolution: reverted it. The under-fill it targeted is by-design and identical to the production-proven search_semantic (architect-rated LOW in both rounds; negligible at the default limit 5 vs ef_search 40), now documented honestly on suggest_links/3.
• Disproven adversarial attacks (verified empirically, not fixed): MatchError on the transaction result (raises propagate; only rollback yields {:error}); "version-skew 500" (Postgres treats two-part GUC names as placeholders → graceful no-op). Both moot after the revert.

Tests

• EXPLAIN index-eligibility guard (the issue's "test that exhibits scale" ask): with seq-scan and sort disabled, the plan must satisfy the ORDER BY via articles_embedding_idx with no Sort node — catches the broad "reintroduced full-corpus scan" regression class (self-join column-operand, wrapped order expr, dropped index) that a small-data behavioral test can't, deterministically at any row count.
• Structural SQL-shape guard (bound list param, single articles table, no self-join).
• Behavioral ACs: 200 + ranked candidates; excludes self + already-linked (either direction); honors threshold/limit; exclusion-under-limit (linked near-neighbors excluded, result filled from the unlinked remainder); published-only; tenant isolation; read-only.

Gate: compile --warnings-as-errors, format, credo --strict, dialyzer, 2532 tests — all green.

Post-deploy verification (operator gate before marking #172 fully verified)

The prod-only 500 can't be reproduced at test scale, so before closing #172 as verified-in-prod, on the deployed build: hit the 5 repro ids from the issue and confirm 200 + ranked candidates, and (optionally) capture a prod EXPLAIN ANALYZE showing the index scan. These need production access and are not deferrals of any code change.

Closes #172