From a26f16ad792f5389e56ce431b06ebc9207d6cbe8 Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 7 Jul 2026 14:23:28 +0200 Subject: [PATCH] feat(vector): server-side kinds filter for search_similar (legacy fallback) + graceful solved-search degrade Co-Authored-By: Claude Fable 5 --- PROJECT_STATE.md | 16 +-- harness/docs/vector-rest-kinds-migration.md | 101 +++++++++++++++++++ harness/tests/test_search_similar_kinds.py | 104 ++++++++++++++++++++ harness/tests/test_solved_search_cli.py | 85 ++++++++++++++++ harness/tht/cli/memory_cmd.py | 20 +++- harness/tht/vectorstore/reader.py | 7 +- harness/tht/vectorstore/rest_client.py | 30 ++++-- 7 files changed, 340 insertions(+), 23 deletions(-) create mode 100644 harness/docs/vector-rest-kinds-migration.md create mode 100644 harness/tests/test_search_similar_kinds.py create mode 100644 harness/tests/test_solved_search_cli.py diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index f6c916c6..3584b0a1 100644 --- a/PROJECT_STATE.md +++ b/PROJECT_STATE.md @@ -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) diff --git a/harness/docs/vector-rest-kinds-migration.md b/harness/docs/vector-rest-kinds-migration.md new file mode 100644 index 00000000..60850de9 --- /dev/null +++ b/harness/docs/vector-rest-kinds-migration.md @@ -0,0 +1,101 @@ +# Migrazione server: filtro `kinds` nella RPC `search_similar` + +**Contesto.** Il vectorstore ThothII espone la similarity search via PostgREST/Supabase: +`POST /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 + (`_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 "/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 "/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 "" --json` e + `tht memory search "" --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. diff --git a/harness/tests/test_search_similar_kinds.py b/harness/tests/test_search_similar_kinds.py new file mode 100644 index 00000000..5a16be0a --- /dev/null +++ b/harness/tests/test_search_similar_kinds.py @@ -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"] diff --git a/harness/tests/test_solved_search_cli.py b/harness/tests/test_solved_search_cli.py new file mode 100644 index 00000000..2f6fec35 --- /dev/null +++ b/harness/tests/test_solved_search_cli.py @@ -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, + }] diff --git a/harness/tht/cli/memory_cmd.py b/harness/tht/cli/memory_cmd.py index 5ffa21cb..f0ee7571 100644 --- a/harness/tht/cli/memory_cmd.py +++ b/harness/tht/cli/memory_cmd.py @@ -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), diff --git a/harness/tht/vectorstore/reader.py b/harness/tht/vectorstore/reader.py index 7177bb8b..50e852cf 100644 --- a/harness/tht/vectorstore/reader.py +++ b/harness/tht/vectorstore/reader.py @@ -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] diff --git a/harness/tht/vectorstore/rest_client.py b/harness/tht/vectorstore/rest_client.py index d5ff5576..e61c7f03 100644 --- a/harness/tht/vectorstore/rest_client.py +++ b/harness/tht/vectorstore/rest_client.py @@ -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.`: 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, …}`."""