feat(opt): three efficiency levers for NL→SQL workflow
Lever 1: Join-graph via FK logics in annotations + suggest-fks command
- TableAnnotation.foreign_keys field stores curated logical FKs (DWH has no FK constraints)
- tht schema suggest-fks: mine from approved SQL, heuristics (time_key → dim_time),
same-name discovery + explicit --assume flag for multi-owner PKs
- mschema renders 【Foreign keys】 section populated; validation in merge.py
- SKILL.md F4 now reads FKs from mschema-text, no custom data_time_key logic
Lever 2: Context-pack consolidation at kickoff (tht search pack)
- Single embedding of question, reused for schema + evidence + solved searches
- One command: tht search pack <question> --session <id> → retrieval_pack.md
- Graceful degradation when Ollama/vector store unreachable (exit 0, empty sections)
- SKILL.md F1 prescribes as first call; reduces model thinking turns via pre-retrieval
Lever 3: Phase-summary recap v2 auto-construction from session ledger
- tht session show --json includes full decisions ledger
- tht phase meta --json exports 'emits' (substantive decision types per phase)
- Gate appends deterministic 【Decisioni registrate in questa fase】 section (appendLedgerSection)
- Model authors only summary + checks; recap table comes from persisted state (exact by construction)
- SKILL.md Disciplina 6: brief model output, gate fills the rest
Tests: 358 Python (including 10 FK + 3 pack + 1 session-ledger tests) + 111 JS gate tests, all pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -30,3 +30,11 @@ def test_phase_meta_each_phase_carries_id_and_name():
|
||||
assert p["num"] == i
|
||||
assert p["id"], f"phase {i} missing id"
|
||||
assert p["name"], f"phase {i} missing name"
|
||||
|
||||
|
||||
def test_phase_meta_exposes_emits():
|
||||
# Il gate filtra il ledger per fase con `emits` (recap deterministico v2).
|
||||
result = runner.invoke(app, ["phase", "meta", "--json"])
|
||||
data = json.loads(result.stdout)
|
||||
f1 = data["phases"][0]
|
||||
assert f1["emits"] == ["concept_clarified", "ambiguity_open"]
|
||||
|
||||
@@ -0,0 +1,223 @@
|
||||
from datetime import datetime
|
||||
|
||||
import yaml
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from tht.cli import app
|
||||
from tht.mschema.merge import find_orphans
|
||||
from tht.mschema.models import (
|
||||
Annotations,
|
||||
ColumnPhysical,
|
||||
ForeignKey,
|
||||
PhysicalSchema,
|
||||
TableAnnotation,
|
||||
TablePhysical,
|
||||
)
|
||||
from tht.mschema.render import to_mschema_text, to_schema_dict
|
||||
|
||||
|
||||
def _physical():
|
||||
return PhysicalSchema(
|
||||
database="d", schema="s", introspected_at=datetime(2026, 1, 1),
|
||||
tables={
|
||||
"dim_patient": TablePhysical(
|
||||
columns={"cod_paz": ColumnPhysical(type="bigint", pk=True)},
|
||||
),
|
||||
"dim_time": TablePhysical(
|
||||
columns={"day_key": ColumnPhysical(type="integer", pk=True)},
|
||||
),
|
||||
"fact_ablazione": TablePhysical(
|
||||
columns={
|
||||
"cod_paz": ColumnPhysical(type="bigint"),
|
||||
"data_time_key": ColumnPhysical(type="integer"),
|
||||
"esito": ColumnPhysical(type="text"),
|
||||
},
|
||||
),
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
def _annotations_with_fks():
|
||||
return Annotations(
|
||||
tables={
|
||||
"fact_ablazione": TableAnnotation(
|
||||
foreign_keys=[
|
||||
ForeignKey(columns=["cod_paz"], ref_table="dim_patient",
|
||||
ref_columns=["cod_paz"]),
|
||||
ForeignKey(columns=["data_time_key"], ref_table="dim_time",
|
||||
ref_columns=["day_key"]),
|
||||
],
|
||||
)
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def test_mschema_text_renders_annotation_fks():
|
||||
text = to_mschema_text(_physical(), _annotations_with_fks())
|
||||
assert "fact_ablazione.cod_paz=dim_patient.cod_paz" in text
|
||||
assert "fact_ablazione.data_time_key=dim_time.day_key" in text
|
||||
|
||||
|
||||
def test_schema_dict_merges_annotation_fks():
|
||||
d = to_schema_dict(_physical(), _annotations_with_fks())
|
||||
fks = d["fact_ablazione"]["foreign_keys"]
|
||||
assert {"columns": ["cod_paz"], "ref_table": "dim_patient",
|
||||
"ref_columns": ["cod_paz"]} in fks
|
||||
|
||||
|
||||
def test_find_orphans_flags_broken_annotation_fk():
|
||||
ann = Annotations(
|
||||
tables={
|
||||
"fact_ablazione": TableAnnotation(
|
||||
foreign_keys=[
|
||||
ForeignKey(columns=["cod_paz"], ref_table="dim_sparita",
|
||||
ref_columns=["x"]),
|
||||
ForeignKey(columns=["colonna_sparita"], ref_table="dim_time",
|
||||
ref_columns=["day_key"]),
|
||||
],
|
||||
)
|
||||
}
|
||||
)
|
||||
orphans = find_orphans(_physical(), ann)
|
||||
assert "fact_ablazione.fk(cod_paz)->dim_sparita" in orphans
|
||||
assert "fact_ablazione.fk(colonna_sparita)->dim_time" in orphans
|
||||
|
||||
|
||||
def test_find_orphans_ok_with_valid_fk():
|
||||
assert find_orphans(_physical(), _annotations_with_fks()) == []
|
||||
|
||||
|
||||
def _write_workspace(tmp_path):
|
||||
_physical().to_yaml(tmp_path / "artifacts" / "mschema" / "physical.yaml")
|
||||
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/'artifacts'}, indexes: {tmp_path/'i'}, sessions: {tmp_path/'s'}}}\n"
|
||||
)
|
||||
return cfg
|
||||
|
||||
|
||||
def test_suggest_fks_prints_candidates(tmp_path):
|
||||
cfg = _write_workspace(tmp_path)
|
||||
res = CliRunner().invoke(app, ["schema", "suggest-fks", "-c", str(cfg)])
|
||||
assert res.exit_code == 0, res.output
|
||||
data = yaml.safe_load(res.output.rsplit("\n", 2)[0].split("FK candidate")[0])
|
||||
fks = data["tables"]["fact_ablazione"]["foreign_keys"]
|
||||
assert {"columns": ["cod_paz"], "ref_table": "dim_patient",
|
||||
"ref_columns": ["cod_paz"]} in fks
|
||||
assert {"columns": ["data_time_key"], "ref_table": "dim_time",
|
||||
"ref_columns": ["day_key"]} in fks
|
||||
|
||||
|
||||
def test_mine_join_pairs_from_approved_sql():
|
||||
from tht.mschema.fkmine import mine_join_pairs
|
||||
|
||||
sql = """
|
||||
WITH abl AS (
|
||||
SELECT sea.cod_paz, dt.year
|
||||
FROM datawarehouse.fact_ablazione AS sea
|
||||
JOIN datawarehouse.dim_time AS dt ON sea.data_time_key = dt.day_key
|
||||
)
|
||||
SELECT * FROM abl JOIN abl b ON abl.year = b.year;
|
||||
"""
|
||||
pairs = mine_join_pairs(sql, _physical())
|
||||
assert pairs[("fact_ablazione", "data_time_key", "dim_time", "day_key")] == 1
|
||||
# il join CTE-CTE (abl.year=b.year) non produce coppie
|
||||
assert len(pairs) == 1
|
||||
|
||||
|
||||
def test_mine_join_pairs_ignores_non_pk_pairs_and_bad_sql():
|
||||
from tht.mschema.fkmine import mine_join_pairs
|
||||
|
||||
# esito=esito: nessun lato e' PK -> scartato
|
||||
sql = ("SELECT * FROM fact_ablazione a JOIN fact_ablazione b "
|
||||
"ON a.esito = b.esito")
|
||||
assert len(mine_join_pairs(sql, _physical())) == 0
|
||||
assert len(mine_join_pairs("WITH broken (", _physical())) == 0
|
||||
|
||||
|
||||
def test_suggest_fks_skips_generic_and_ambiguous_pks(tmp_path):
|
||||
phys = PhysicalSchema(
|
||||
database="d", schema="s", introspected_at=datetime(2026, 1, 1),
|
||||
tables={
|
||||
"dim_a": TablePhysical(columns={"id": ColumnPhysical(type="int", pk=True)}),
|
||||
"dim_b": TablePhysical(columns={"id": ColumnPhysical(type="int", pk=True)}),
|
||||
"dim_c1": TablePhysical(columns={"cod_x": ColumnPhysical(type="int", pk=True)}),
|
||||
"dim_c2": TablePhysical(columns={"cod_x": ColumnPhysical(type="int", pk=True)}),
|
||||
"fact_f": TablePhysical(
|
||||
columns={
|
||||
"id": ColumnPhysical(type="int"),
|
||||
"cod_x": ColumnPhysical(type="int"),
|
||||
},
|
||||
),
|
||||
},
|
||||
)
|
||||
phys.to_yaml(tmp_path / "artifacts" / "mschema" / "physical.yaml")
|
||||
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/'artifacts'}, indexes: {tmp_path/'i'}, sessions: {tmp_path/'s'}}}\n"
|
||||
)
|
||||
res = CliRunner().invoke(app, ["schema", "suggest-fks", "-c", str(cfg)])
|
||||
assert res.exit_code == 0, res.output
|
||||
assert "nessuna FK da suggerire" in res.output # id generico, cod_x ambigua
|
||||
assert "cod_x" in res.output # segnalata come ambigua saltata
|
||||
|
||||
# --assume disambigua la PK multi-proprietario
|
||||
res2 = CliRunner().invoke(
|
||||
app, ["schema", "suggest-fks", "-c", str(cfg), "--assume", "cod_x=dim_c1"]
|
||||
)
|
||||
assert res2.exit_code == 0, res2.output
|
||||
yaml_text = "\n".join(
|
||||
line for line in res2.output.splitlines() if "FK candidate" not in line
|
||||
)
|
||||
data = yaml.safe_load(yaml_text)
|
||||
fact_fks = data["tables"]["fact_f"]["foreign_keys"]
|
||||
assert {"columns": ["cod_x"], "ref_table": "dim_c1",
|
||||
"ref_columns": ["cod_x"]} in fact_fks
|
||||
# dim_c2.cod_x -> dim_c1 (estensione 1:1), ma NON dim_c1 -> se stessa
|
||||
assert "dim_c1" not in data["tables"] or all(
|
||||
fk["ref_table"] != "dim_c1" for fk in data["tables"].get("dim_c1", {}).get("foreign_keys", [])
|
||||
)
|
||||
|
||||
# --assume con tabella inesistente -> errore chiaro
|
||||
res3 = CliRunner().invoke(
|
||||
app, ["schema", "suggest-fks", "-c", str(cfg), "--assume", "cod_x=nope"]
|
||||
)
|
||||
assert res3.exit_code == 1
|
||||
assert "non valido" in res3.output
|
||||
|
||||
|
||||
def test_suggest_fks_from_sql_mines_joins(tmp_path):
|
||||
cfg = _write_workspace(tmp_path)
|
||||
sqldir = tmp_path / "approved"
|
||||
sqldir.mkdir()
|
||||
(sqldir / "q1.sql").write_text(
|
||||
"SELECT f.esito FROM datawarehouse.fact_ablazione f "
|
||||
"JOIN datawarehouse.dim_patient p ON f.cod_paz = p.cod_paz"
|
||||
)
|
||||
res = CliRunner().invoke(
|
||||
app, ["schema", "suggest-fks", "-c", str(cfg), "--from-sql", str(sqldir)]
|
||||
)
|
||||
assert res.exit_code == 0, res.output
|
||||
assert "Minati 1 equi-join da 1 file SQL" in res.output
|
||||
assert "ref_table: dim_patient" in res.output
|
||||
|
||||
|
||||
def test_suggest_fks_write_merges_and_is_idempotent(tmp_path):
|
||||
cfg = _write_workspace(tmp_path)
|
||||
ann_path = tmp_path / "artifacts" / "mschema" / "annotations.yaml"
|
||||
Annotations(
|
||||
tables={"fact_ablazione": TableAnnotation(description="Ablazioni")}
|
||||
).to_yaml(ann_path)
|
||||
|
||||
res = CliRunner().invoke(app, ["schema", "suggest-fks", "-c", str(cfg), "--write"])
|
||||
assert res.exit_code == 0, res.output
|
||||
ann = Annotations.from_yaml(ann_path)
|
||||
assert ann.tables["fact_ablazione"].description == "Ablazioni" # non distrutta
|
||||
assert len(ann.tables["fact_ablazione"].foreign_keys) == 2
|
||||
|
||||
res2 = CliRunner().invoke(app, ["schema", "suggest-fks", "-c", str(cfg), "--write"])
|
||||
assert "nessuna FK da suggerire" in res2.output
|
||||
ann2 = Annotations.from_yaml(ann_path)
|
||||
assert len(ann2.tables["fact_ablazione"].foreign_keys) == 2
|
||||
@@ -0,0 +1,115 @@
|
||||
import json
|
||||
from datetime import datetime
|
||||
from types import SimpleNamespace
|
||||
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from tht.cli import app
|
||||
from tht.mschema.models import ColumnPhysical, PhysicalSchema, TablePhysical
|
||||
from tht.vectorstore.embeddings import EmbeddingsError
|
||||
|
||||
|
||||
class _FakeEmbedder:
|
||||
def __init__(self):
|
||||
self.calls = 0
|
||||
|
||||
def embed_query(self, text):
|
||||
self.calls += 1
|
||||
return [0.1, 0.2, 0.3]
|
||||
|
||||
|
||||
class _FakeSearcher:
|
||||
def search(self, vec, top_n, kinds=None):
|
||||
if kinds == ["solved_question"]:
|
||||
return [SimpleNamespace(
|
||||
kind="memory", ref="s-1", id="m1", title="q solved",
|
||||
similarity=0.91, content="quanti pazienti nel 2024?",
|
||||
metadata={"session_id": "2026-01-01-000000-x", "sql": "SELECT 1",
|
||||
"tables": ["fact_ablazione"], "question": "quanti pazienti nel 2024?"},
|
||||
)]
|
||||
if kinds == ["schema_table", "schema_column"]:
|
||||
return [SimpleNamespace(
|
||||
kind="schema_table", ref="fact_ablazione", id="t1",
|
||||
title="Tabella fact_ablazione", similarity=0.88,
|
||||
content="Tabella fact_ablazione", metadata={},
|
||||
)]
|
||||
if kinds == ["evidence"]:
|
||||
return [SimpleNamespace(
|
||||
kind="evidence", ref="ev1", id="ev1", title="Dominio ablazione",
|
||||
similarity=0.8, content="L'ablazione e' una procedura...",
|
||||
metadata={"status": "approved"},
|
||||
)]
|
||||
return []
|
||||
|
||||
|
||||
def _workspace(tmp_path, with_session=None):
|
||||
PhysicalSchema(
|
||||
database="d", schema="s", introspected_at=datetime(2026, 1, 1),
|
||||
tables={"fact_ablazione": TablePhysical(
|
||||
comment="Ablazioni", columns={"cod_paz": ColumnPhysical(type="bigint")})},
|
||||
).to_yaml(tmp_path / "artifacts" / "mschema" / "physical.yaml")
|
||||
cfg = tmp_path / "workspace.yaml"
|
||||
cfg.write_text(
|
||||
"database: {database: d, schema: s, user: u, password: p, transport: direct}\n"
|
||||
"vector_db: {database: v, schema: public, user: u, password: p}\n"
|
||||
"embeddings: {base_url: 'http://localhost:11434', model: nomic-embed-text, dim: 8}\n"
|
||||
f"paths: {{artifacts: {tmp_path/'artifacts'}, indexes: {tmp_path/'i'}, "
|
||||
f"sessions: {tmp_path/'sessions'}}}\n"
|
||||
)
|
||||
if with_session:
|
||||
sdir = tmp_path / "sessions" / with_session
|
||||
sdir.mkdir(parents=True)
|
||||
(sdir / "session_manifest.yaml").write_text(
|
||||
f"id: {with_session}\nquestion: q\ndatabase: d\nschema: s\n"
|
||||
"created_at: 2026-01-01T00:00:00+00:00\nstatus: open\n"
|
||||
)
|
||||
return cfg
|
||||
|
||||
|
||||
def _patch(monkeypatch, embedder, searcher):
|
||||
import tht.cli.vector_cmd as vc
|
||||
|
||||
monkeypatch.setattr(vc, "make_embedder", lambda _cfg: embedder)
|
||||
monkeypatch.setattr(vc, "open_searcher", lambda _cfg: searcher)
|
||||
|
||||
|
||||
def test_pack_single_embed_and_sections(tmp_path, monkeypatch):
|
||||
cfg = _workspace(tmp_path)
|
||||
emb = _FakeEmbedder()
|
||||
_patch(monkeypatch, emb, _FakeSearcher())
|
||||
res = CliRunner().invoke(app, ["search", "pack", "quanti pazienti", "-c", str(cfg)])
|
||||
assert res.exit_code == 0, res.output
|
||||
assert emb.calls == 1 # UN solo embedding per le tre ricerche
|
||||
assert "fact_ablazione" in res.output and "Ablazioni" in res.output
|
||||
assert "Dominio ablazione" in res.output
|
||||
assert "SELECT 1" in res.output
|
||||
|
||||
|
||||
def test_pack_json_and_session_file(tmp_path, monkeypatch):
|
||||
sid = "2026-01-01-000000-test"
|
||||
cfg = _workspace(tmp_path, with_session=sid)
|
||||
_patch(monkeypatch, _FakeEmbedder(), _FakeSearcher())
|
||||
res = CliRunner().invoke(
|
||||
app, ["search", "pack", "q", "-c", str(cfg), "--session", sid, "--json"]
|
||||
)
|
||||
assert res.exit_code == 0, res.output
|
||||
data = json.loads(res.output)
|
||||
assert data["tables"][0]["name"] == "fact_ablazione"
|
||||
pack = tmp_path / "sessions" / sid / "retrieval_pack.md"
|
||||
assert pack.exists()
|
||||
assert "Retrieval pack" in pack.read_text()
|
||||
|
||||
|
||||
def test_pack_degrades_gracefully(tmp_path, monkeypatch):
|
||||
cfg = _workspace(tmp_path)
|
||||
|
||||
class _Broken:
|
||||
def embed_query(self, text):
|
||||
raise EmbeddingsError("ollama down")
|
||||
|
||||
_patch(monkeypatch, _Broken(), _FakeSearcher())
|
||||
res = CliRunner().invoke(app, ["search", "pack", "q", "-c", str(cfg), "--json"])
|
||||
assert res.exit_code == 0, res.output
|
||||
data = json.loads(res.output[res.output.index("{"):])
|
||||
assert data["tables"] == [] and data["evidence"] == [] and data["solved"] == []
|
||||
assert any("retrieval non disponibile" in w for w in data["warnings"])
|
||||
@@ -0,0 +1,34 @@
|
||||
import json
|
||||
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from tht.cli import app
|
||||
|
||||
|
||||
def test_session_show_json_includes_ledger(tmp_path):
|
||||
sid = "2026-01-01-000000-test"
|
||||
sdir = tmp_path / "sessions" / sid
|
||||
sdir.mkdir(parents=True)
|
||||
(sdir / "session_manifest.yaml").write_text(
|
||||
f"id: {sid}\nquestion: q\ndatabase: d\nschema: s\n"
|
||||
"created_at: 2026-01-01T00:00:00+00:00\nstatus: open\n"
|
||||
)
|
||||
(sdir / "review_decisions.jsonl").write_text(
|
||||
'{"seq": 1, "ts": "2026-01-01T00:01:00+00:00", "type": "concept_clarified", '
|
||||
'"subject": "ablazione", "detail": "solo transcatetere", '
|
||||
'"rationale": "scelta reviewer", "phase": 1}\n'
|
||||
)
|
||||
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/'artifacts'}, indexes: {tmp_path/'i'}, "
|
||||
f"sessions: {tmp_path/'sessions'}}}\n"
|
||||
)
|
||||
res = CliRunner().invoke(app, ["session", "show", sid, "--json", "-c", str(cfg)])
|
||||
assert res.exit_code == 0, res.output
|
||||
data = json.loads(res.output)
|
||||
assert len(data["decisions"]) == 1
|
||||
d = data["decisions"][0]
|
||||
assert d["type"] == "concept_clarified"
|
||||
assert d["subject"] == "ablazione"
|
||||
assert d["detail"] == "solo transcatetere"
|
||||
Reference in New Issue
Block a user