feat(vector): server-side kinds filter for search_similar (legacy fallback) + graceful solved-search degrade

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-07 14:23:28 +02:00
parent 8d427a2ccb
commit a26f16ad79
7 changed files with 340 additions and 23 deletions
@@ -105,13 +105,15 @@ pre-selected and persists selected/declined correctly, (b) finalize indexes the
 (c) `tht memory solved-search` returns it with sql + tables.

 **Fast-follow:**
- RestSearcher top-k dilution: `memory` and `solved_question` share the pgvector table and
-  `search_similar` has no server-side kind filter — with many solved records the client-side
-  filter can starve `tht memory search` results (and vice versa); fix = over-fetch (top_n*3)
-  when kinds is a strict subset, or kind param in the RPC — da fare prima che i solved record
-  si accumulino.
- `tht memory solved-search` muore con traceback grezzo se il vectordb è irraggiungibile
-  (SKILL.md la prescrive in F4/F6/F7) — degradare a messaggio di una riga.
+- RestSearcher top-k dilution — **client-side DONE** (2026-07-07): `search_similar` manda
+  `kinds` alla RPC (filtro server-side esatto) con fallback automatico su server legacy
+  (404 → retry senza filtro, post-filter client). **Resta la migrazione server** della
+  funzione SQL `search_similar` (+`kinds text[] DEFAULT NULL`): istruzioni pronte in
+  `harness/docs/vector-rest-kinds-migration.md`; l'ordine di deploy è libero, ma fino
+  alla migrazione il filtro resta client-side e la diluizione persiste.
+- ~~`tht memory solved-search` muore con traceback grezzo se il vectordb è irraggiungibile~~
+  **DONE** (2026-07-07): degrada a warning di una riga su stderr, stdout puro (`[]` in
+  --json), exit 0 — copre VectorRestError/EmbeddingsError/OperationalError.

 ## Review gates v2 — payload strutturati + viewer dedicati — COMPLETE (2026-07-07)

@@ -0,0 +1,101 @@
+# Migrazione server: filtro `kinds` nella RPC `search_similar`
+
+**Contesto.** Il vectorstore ThothII espone la similarity search via PostgREST/Supabase:
+`POST <base_url>/rpc/search_similar` con payload `{query_embedding, limit_count, table_name}`.
+Da quando `memory` e `solved_question` condividono la tabella `vectors.memory` (feature
+"active memory", 2026-07-07), il top-k della tabella mista diluisce i risultati: il filtro
+per kind avviene client-side DOPO il taglio a `limit_count`. Serve il filtro nel `WHERE`
+della funzione SQL.
+
+**Lato client: già pronto.** Il harness manda `kinds` nel payload quando filtra
+(`tht memory search`, `tht memory solved-search`); se il server risponde 404
+(funzione a 3 argomenti, pre-migrazione) ritenta senza `kinds` e filtra client-side.
+Quindi **l'ordine di deploy è libero** — ma finché la migrazione non è fatta il filtro
+resta client-side e la diluizione persiste.
+
+## Cosa fare (istruzioni per il Claude del server)
+
+Obiettivo: aggiungere a `search_similar` un quarto parametro `kinds text[] DEFAULT NULL`
+che filtra `kind = ANY(kinds)` quando valorizzato, preservando il comportamento attuale
+quando assente.
+
+1. **Recupera la definizione corrente** (non riscriverla a memoria):
+   ```sql
+   SELECT pg_get_functiondef(oid)
+   FROM pg_proc
+   WHERE proname = 'search_similar';
+   ```
+   Prendi nota di: schema della funzione, `LANGUAGE`, `SECURITY DEFINER` e `SET search_path`
+   se presenti, shape del risultato (le righe devono restare `{id, similarity, metadata}`),
+   e come viene usato `table_name` (quasi certamente SQL dinamico con `EXECUTE format(...)`).
+
+2. **Sostituisci la funzione in una sola transazione.** In Postgres non si può aggiungere
+   un parametro con ALTER: `CREATE OR REPLACE` con firma diversa creerebbe un **overload**,
+   e due overload con lo stesso nome mandano PostgREST in ambiguità (PGRST203) per le
+   chiamate senza `kinds`. Quindi:
+   ```sql
+   BEGIN;
+   DROP FUNCTION search_similar(vector, integer, text);  -- adatta la firma esatta trovata al punto 1
+   CREATE FUNCTION search_similar(
+       query_embedding vector,
+       limit_count integer,
+       table_name text,
+       kinds text[] DEFAULT NULL
+   ) RETURNS ... -- stessa shape di prima
+   ...
+   COMMIT;
+   ```
+   Nel corpo, la condizione deve essere **null-safe** così i chiamanti legacy (senza
+   `kinds`) mantengono il comportamento attuale:
+   ```sql
+   WHERE (kinds IS NULL OR t.kind = ANY(kinds))
+   ```
+   Se il corpo usa SQL dinamico, passa `kinds` come parametro (`USING`), non interpolarlo:
+   ```sql
+   EXECUTE format(
+     'SELECT metadata, 1 - (embedding <=> $1) AS similarity
+        FROM vectors.%I
+       WHERE ($3::text[] IS NULL OR kind = ANY($3))
+       ORDER BY embedding <=> $1
+       LIMIT $2', table_name)
+   USING query_embedding, limit_count, kinds;
+   ```
+   (Adatta al corpo reale: l'esempio mostra solo dove inserire la condizione.)
+
+3. **Ripristina i GRANT.** Il DROP cancella i grant della funzione: ri-esegui gli
+   `GRANT EXECUTE` che la definizione originale aveva (ruolo reader della REST, es.
+   `vector_reader`, e il ruolo con cui PostgREST esegue le RPC). Verifica con:
+   ```sql
+   SELECT proacl FROM pg_proc WHERE proname = 'search_similar';
+   ```
+
+4. **Ricarica lo schema cache di PostgREST** — senza questo la nuova firma risponde 404:
+   ```sql
+   NOTIFY pgrst, 'reload schema';
+   ```
+   (oppure riavvia il servizio PostgREST).
+
+5. **Verifica indice.** La colonna `kind` dovrebbe già avere un indice
+   (`<table>_kind_idx`); se manca sulla tabella `memory`:
+   ```sql
+   CREATE INDEX IF NOT EXISTS vectors_memory_kind_idx ON vectors.memory (kind);
+   ```
+
+6. **Test end-to-end** (con la API key reader):
+   ```bash
+   # senza kinds (comportamento legacy) — deve rispondere come prima
+   curl -s -X POST "<base_url>/rpc/search_similar" -H "X-API-Key: $KEY" \
+     -H "Content-Type: application/json" \
+     -d '{"query_embedding": [...], "limit_count": 3, "table_name": "memory"}'
+   # con kinds — deve restituire SOLO righe con metadata.kind = "solved_question"
+   curl -s -X POST "<base_url>/rpc/search_similar" -H "X-API-Key: $KEY" \
+     -H "Content-Type: application/json" \
+     -d '{"query_embedding": [...], "limit_count": 3, "table_name": "memory",
+          "kinds": ["solved_question"]}'
+   ```
+   Poi dal workstation: `tht memory solved-search "<domanda>" --json` e
+   `tht memory search "<domanda>" --json` devono continuare a funzionare (il secondo
+   ora senza righe solved nei top-k).
+
+**Non toccare** `existing_vector_hashes` e `upsert_vector_records` (path di scrittura):
+già ricevono `kinds`/`kind` e non sono coinvolte.
@@ -0,0 +1,104 @@
+"""L1: filtro `kinds` server-side su search_similar (fast-follow post active-memory).
+
+`memory` e `solved_question` condividono la tabella pgvector: senza filtro nel
+`WHERE` della RPC, il top-k della tabella mista puo' affamare la ricerca memorie
+(e viceversa) perche' il filtro per kind avveniva solo client-side DOPO il taglio
+a top_n. Questi test fissano il contratto client:
+- il client manda `kinds` nel payload della RPC quando richiesto (filtro esatto);
+- su un server legacy (funzione a 3 argomenti -> PostgREST 404) ritenta senza
+  `kinds`, lasciando il filtro al post-filter client-side esistente;
+- RestSearcher inoltra i kinds alla RPC.
+"""
+import pytest
+
+from tht.config import RestConfig
+from tht.vectorstore.reader import RestSearcher
+from tht.vectorstore.rest_client import VectorRestClient, VectorRestError
+
+
+class _Resp:
+    def __init__(self, status_code=200, payload=None, text=""):
+        self.status_code = status_code
+        self._payload = [] if payload is None else payload
+        self.text = text or ("[]" if status_code == 200 else text)
+
+    @property
+    def ok(self):
+        return self.status_code < 400
+
+    def json(self):
+        if not self.ok:
+            return {"message": self.text}
+        return self._payload
+
+
+def _client() -> VectorRestClient:
+    return VectorRestClient(RestConfig(base_url="https://v/", api_key="K-READ"))
+
+
+def test_search_similar_sends_kinds_in_rpc_payload(monkeypatch):
+    seen = []
+
+    def fake_post(url, json=None, **kw):
+        seen.append(json)
+        return _Resp(payload=[{"similarity": 0.9, "metadata": {"kind": "memory"}}])
+
+    monkeypatch.setattr("tht.vectorstore.rest_client.requests.post", fake_post)
+    rows = _client().search_similar("memory", [0.1] * 4, 5, kinds=["memory"])
+    assert len(rows) == 1
+    assert seen[0]["kinds"] == ["memory"]
+    assert seen[0]["table_name"] == "memory"
+    assert seen[0]["limit_count"] == 5
+
+
+def test_search_similar_omits_kinds_when_none(monkeypatch):
+    seen = []
+
+    def fake_post(url, json=None, **kw):
+        seen.append(json)
+        return _Resp()
+
+    monkeypatch.setattr("tht.vectorstore.rest_client.requests.post", fake_post)
+    _client().search_similar("memory", [0.1] * 4, 5)
+    assert "kinds" not in seen[0]
+
+
+def test_search_similar_falls_back_without_kinds_on_legacy_404(monkeypatch):
+    # Server legacy: la funzione a 4 argomenti non esiste -> PostgREST 404 (PGRST202).
+    # Il client ritenta senza `kinds`; il filtro resta al post-filter client-side.
+    seen = []
+
+    def fake_post(url, json=None, **kw):
+        seen.append(json)
+        if "kinds" in json:
+            return _Resp(status_code=404, text="Could not find the function (PGRST202)")
+        return _Resp(payload=[{"similarity": 0.8, "metadata": {"kind": "memory"}}])
+
+    monkeypatch.setattr("tht.vectorstore.rest_client.requests.post", fake_post)
+    rows = _client().search_similar("memory", [0.1] * 4, 5, kinds=["memory"])
+    assert len(rows) == 1
+    assert len(seen) == 2
+    assert "kinds" in seen[0] and "kinds" not in seen[1]
+
+
+def test_search_similar_reraises_non_404_with_kinds(monkeypatch):
+    def fake_post(url, json=None, **kw):
+        return _Resp(status_code=500, text="boom")
+
+    monkeypatch.setattr("tht.vectorstore.rest_client.requests.post", fake_post)
+    with pytest.raises(VectorRestError, match="HTTP 500"):
+        _client().search_similar("memory", [0.1] * 4, 5, kinds=["memory"])
+
+
+def test_rest_searcher_forwards_kinds_to_client():
+    calls = []
+
+    class FakeClient:
+        def search_similar(self, table_name, query_vec, top_n, kinds=None):
+            calls.append((table_name, top_n, kinds))
+            return [{"similarity": 0.7, "metadata": {"kind": "solved_question",
+                                                     "record_key": "solved:s1"}}]
+
+    hits = RestSearcher(FakeClient()).search([0.1] * 4, top_n=3, kinds=["solved_question"])
+    assert calls == [("memory", 3, ["solved_question"])]
+    assert [h.kind for h in hits] == ["solved_question"]
@@ -0,0 +1,85 @@
+"""L1: `tht memory solved-search` — degrado gentile e mapping dei risultati.
+
+SKILL.md prescrive solved-search in F4/F6/F7 di OGNI sessione: a vectordb
+irraggiungibile (VPN giu', Ollama spento) il comando non deve morire con un
+traceback grezzo ma degradare a un avviso di una riga su stderr, con stdout
+puro (`[]` in modalita' --json) ed exit 0, cosi' il modello prosegue senza
+exemplar. Il finalize-hook gestisce gia' lo stesso scenario in modo analogo.
+"""
+import json
+
+from typer.testing import CliRunner
+
+from tht.cli import app
+from tht.vectorstore.rest_client import VectorRestError
+from tht.vectorstore.store import VectorHit
+
+
+def _cfg(tmp_path):
+    cfg = tmp_path / "workspace.yaml"
+    cfg.write_text(
+        "database: {database: d, schema: s, user: u, password: p, transport: direct}\n"
+        f"paths: {{artifacts: {tmp_path/'a'}, indexes: {tmp_path/'i'}, sessions: {tmp_path/'se'}}}\n"
+        "vector_db: {database: v, schema: vectors, user: u, password: p, transport: direct}\n"
+        "embeddings: {base_url: 'http://localhost:11434', model: nomic-embed-text, dim: 8}\n"
+    )
+    return cfg
+
+
+def test_solved_search_degrades_when_vectordb_unreachable(tmp_path, monkeypatch):
+    def boom(cfg):
+        raise VectorRestError("Vector REST non raggiungibile su https://v/ (rpc search_similar)")
+
+    monkeypatch.setattr("tht.cli.vector_cmd.open_searcher", boom)
+    res = CliRunner().invoke(
+        app, ["memory", "solved-search", "quante ablazioni", "--json", "-c", str(_cfg(tmp_path))]
+    )
+    assert res.exit_code == 0, res.output
+    assert json.loads(res.stdout) == []          # stdout puro: JSON valido
+    assert "exemplar non disponibili" in res.stderr
+
+
+def test_solved_search_degrades_human_mode(tmp_path, monkeypatch):
+    def boom(cfg):
+        raise VectorRestError("Vector REST non raggiungibile")
+
+    monkeypatch.setattr("tht.cli.vector_cmd.open_searcher", boom)
+    res = CliRunner().invoke(
+        app, ["memory", "solved-search", "quante ablazioni", "-c", str(_cfg(tmp_path))]
+    )
+    assert res.exit_code == 0, res.output
+    assert "exemplar non disponibili" in res.stderr
+    assert "Traceback" not in res.stderr
+
+
+def test_solved_search_json_maps_hit_metadata(tmp_path, monkeypatch):
+    hit = VectorHit(
+        id="solved:s1", kind="solved_question", ref="s1", title="quante ablazioni nel 2023",
+        content="quante ablazioni nel 2023",
+        metadata={
+            "session_id": "s1", "question": "quante ablazioni nel 2023",
+            "sql": "SELECT 1", "tables": ["fact_seeablazione"],
+        },
+        similarity=0.91,
+    )
+
+    class FakeSearcher:
+        def search(self, vec, top_n=10, kinds=None):
+            assert kinds == ["solved_question"]
+            return [hit]
+
+    class FakeEmbedder:
+        def embed_query(self, text):
+            return [0.1] * 8
+
+    monkeypatch.setattr("tht.cli.vector_cmd.open_searcher", lambda cfg: FakeSearcher())
+    monkeypatch.setattr("tht.cli.vector_cmd.make_embedder", lambda e: FakeEmbedder())
+    res = CliRunner().invoke(
+        app, ["memory", "solved-search", "quante ablazioni", "--json", "-c", str(_cfg(tmp_path))]
+    )
+    assert res.exit_code == 0, res.output
+    data = json.loads(res.stdout)
+    assert data == [{
+        "session_id": "s1", "question": "quante ablazioni nel 2023",
+        "sql": "SELECT 1", "tables": ["fact_seeablazione"], "score": 0.91,
+    }]
@@ -516,12 +516,26 @@ def solved_search_cmd(

    from tht.cli.vector_cmd import make_embedder, open_searcher
    from tht.solved import SOLVED_KIND
+    from tht.vectorstore.embeddings import EmbeddingsError
+    from tht.vectorstore.rest_client import VectorRestError

    cfg = _load_config_or_exit(config)
    require_vector_cfg(cfg)
-    searcher = open_searcher(cfg)
-    embedder = make_embedder(cfg.embeddings)
-    hits = searcher.search(embedder.embed_query(question), top_n=top, kinds=[SOLVED_KIND])
+    # Degrado gentile: SKILL.md prescrive solved-search in F4/F6/F7 di ogni sessione,
+    # quindi vectordb/Ollama irraggiungibili non devono produrre un traceback grezzo
+    # nel transcript: avviso di una riga su stderr, stdout puro ([] in --json), exit 0.
+    try:
+        searcher = open_searcher(cfg)
+        embedder = make_embedder(cfg.embeddings)
+        hits = searcher.search(embedder.embed_query(question), top_n=top, kinds=[SOLVED_KIND])
+    except (VectorRestError, EmbeddingsError, OperationalError) as e:
+        typer.secho(
+            f"ATTENZIONE: exemplar non disponibili ({e}). Prosegui senza.",
+            fg=typer.colors.YELLOW, err=True,
+        )
+        if json_out:
+            typer.echo("[]")
+        return
    results = [
        {
            "session_id": h.metadata.get("session_id", h.ref),
@@ -45,10 +45,11 @@ class RestSearcher:
    ) -> list[VectorHit]:
        hits: list[VectorHit] = []
        for table in tables_for_kinds(kinds):
-            for row in self.client.search_similar(table, query_vec, top_n):
+            for row in self.client.search_similar(table, query_vec, top_n, kinds=kinds):
                hits.append(hit_from_metadata(row.get("similarity", 0.0), row.get("metadata")))
-        # schema_records contiene sia schema_table sia schema_column: la RPC non filtra
-        # per kind, quindi lo facciamo lato client per parita' col path diretto (#25).
+        # Il filtro per kind avviene server-side (RPC con `kinds`); il post-filter resta
+        # come difesa per il fallback legacy (server pre-migrazione: 404 -> query senza
+        # filtro) e per parita' col path diretto (#25).
        if kinds:
            allowed = set(kinds)
            hits = [h for h in hits if h.kind in allowed]
@@ -58,18 +58,28 @@ class VectorRestClient:
        return resp.json()

    def search_similar(
-        self, table_name: str, query_embedding: list[float], limit_count: int
+        self, table_name: str, query_embedding: list[float], limit_count: int,
+        kinds: list[str] | None = None,
    ) -> list[dict]:
        """Ricerca per similarità coseno su `vectors.<table_name>`: ritorna le righe
-        `{id, similarity, metadata}` ordinate per similarity decrescente."""
-        return self._call(
-            "search_similar",
-            {
-                "query_embedding": query_embedding,
-                "limit_count": limit_count,
-                "table_name": table_name,
-            },
-        ) or []
+        `{id, similarity, metadata}` ordinate per similarity decrescente. Con `kinds`
+        il filtro avviene server-side nel WHERE della RPC (evita la diluizione del
+        top-k quando piu' kind condividono la tabella, es. memory/solved_question).
+        Su un server legacy senza il parametro (PostgREST 404) ritenta senza filtro:
+        resta il post-filter client-side di RestSearcher."""
+        args = {
+            "query_embedding": query_embedding,
+            "limit_count": limit_count,
+            "table_name": table_name,
+        }
+        if kinds is not None:
+            try:
+                return self._call("search_similar", {**args, "kinds": kinds}) or []
+            except VectorRestError as e:
+                if "HTTP 404" not in str(e):
+                    raise
+                # funzione a 3 argomenti (pre-migrazione kinds): fallback senza filtro
+        return self._call("search_similar", args) or []

    def list_tables(self) -> list[dict]:
        """Tabelle vettoriali disponibili: `{table_name, vector_dimensions, …}`."""