diff --git a/tests/test_mart_multi_year.py b/tests/test_mart_multi_year.py index 467014b..4b76a19 100644 --- a/tests/test_mart_multi_year.py +++ b/tests/test_mart_multi_year.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import re from pathlib import Path import pytest @@ -64,3 +65,198 @@ def test_mart_multi_year_on_project_example(project_example: Path) -> None: tables = metadata.get("tables") or [] assert any(t.get("name") == "clean_union" for t in tables), "clean_union missing from metadata" assert any(t.get("years") == [2022, 2023] for t in tables), "years missing from metadata" + + +def test_mart_only_multi_year(project_example: Path) -> None: + """Solo tabelle multi-year: il run per-anno non deve fallire (issue #445). + + Regressione: prima del fix, un candidate con TUTTE le tabelle mart + dichiarate ``years`` falliva la validazione per-anno (Missing required + MART tables) e il passaggio multi-year non partiva mai. + """ + config_path = project_example / "dataset.yml" + sql_dir = project_example / "sql" / "multi_year" + sql_dir.mkdir(parents=True, exist_ok=True) + (sql_dir / "solo_multi.sql").write_text( + "\n".join( + [ + "select", + " anno,", + " count(*) as righe", + "from clean_input", + "group by anno", + ] + ), + encoding="utf-8", + ) + + config_text = config_path.read_text(encoding="utf-8") + config_data = yaml.safe_load(config_text) + config_data["dataset"]["years"] = [2022, 2023] + # Rimuove tutte le tabelle per-anno esistenti: solo tabelle multi-year + config_data["mart"] = { + "tables": [ + { + "name": "solo_multi", + "sql": "sql/multi_year/solo_multi.sql", + "years": [2022, 2023], + } + ], + "required_tables": ["solo_multi"], + "validate": { + "table_rules": { + "solo_multi": { + "required_columns": ["anno", "righe"], + "primary_key": ["anno"], + "min_rows": 1, + } + } + }, + } + config_path.write_text( + yaml.dump(config_data, default_flow_style=False, allow_unicode=True, sort_keys=False), + encoding="utf-8", + ) + + # Run all years + multi-year mart: deve passare (prima falliva) + run_cmd(step="all", config=str(config_path)) + + # Output a livello dataset + mart_dir = project_example / "_smoke_out" / "data" / "mart" / "project_example" + assert (mart_dir / "solo_multi.parquet").exists(), "multi-year parquet should exist" + + # La validazione multi-year deve essere applicata (issue #445 gap 2): + # il metadata registra l'esito della validazione delle tabelle multi-year. + metadata = json.loads((mart_dir / "metadata.json").read_text(encoding="utf-8")) + validation = metadata.get("validation") or {} + assert validation.get("passed") is True, ( + f"multi-year validation failed: {validation.get('errors')}" + ) + assert validation.get("errors_count") == 0, ( + f"multi-year validation errors: {validation.get('errors')}" + ) + + +def test_mart_output_paths_multi_year_resolve_to_dataset_level(tmp_path: Path) -> None: + """Le tabelle multi-year risolvono a livello dataset, non per-anno (issue #445). + + Regressione: il path resolver elencava TUTTI gli output mart nel dir + per-anno, quindi readiness/summary segnalavano mart_outputs_missing + per le tabelle multi-year (scritte a data/mart/{dataset}/{name}.parquet). + """ + from toolkit.core.config import load_config + from toolkit.domain.path_resolver import payload_for_year + + root = tmp_path / "out" + (root / "data" / "raw" / "demo_ds" / "2022").mkdir(parents=True) + cfg_path = tmp_path / "dataset.yml" + cfg_path.write_text( + "\n".join( + [ + f'root: "{root.as_posix()}"', + "dataset:", + ' name: "demo_ds"', + " years: [2022]", + "raw:", + " sources:", + " - type: local_file", + " args:", + ' path: "."', + ' filename: "dummy.csv"', + "mart:", + " tables:", + ' - name: "mart_per_anno"', + ' sql: "sql/mart_per_anno.sql"', + ' - name: "mart_multi"', + ' sql: "sql/mart_multi.sql"', + " years: [2022]", + ] + ), + encoding="utf-8", + ) + + cfg = load_config(str(cfg_path), strict_config=False) + payload = payload_for_year(cfg, 2022) + outputs = payload["paths"]["mart"]["outputs"] + + # mart_per_anno -> nel dir per-anno + assert any(o.endswith("data/mart/demo_ds/2022/mart_per_anno.parquet") for o in outputs), ( + f"per-year mart should be in year dir: {outputs}" + ) + # mart_multi -> a livello dataset + assert any(o.endswith("data/mart/demo_ds/mart_multi.parquet") for o in outputs), ( + f"multi-year mart should be at dataset level: {outputs}" + ) + + +def test_mart_only_multi_year_validation_failure(project_example: Path) -> None: + """Validazione multi-year fallita: table_rules violata blocca il run (issue #445). + + Regressione: il ramo di errore di _validate_multi_year_tables non era + coperto — la validazione multi-year applica le table_rules ma il + fallimento (validation_passed=False) deve far fallire il run quando + fail_on_error è attivo. + """ + config_path = project_example / "dataset.yml" + sql_dir = project_example / "sql" / "multi_year" + sql_dir.mkdir(parents=True, exist_ok=True) + (sql_dir / "solo_multi_viol.sql").write_text( + "\n".join( + [ + "select", + " anno,", + " count(*) as righe", + "from clean_input", + "group by anno", + ] + ), + encoding="utf-8", + ) + + config_text = config_path.read_text(encoding="utf-8") + config_data = yaml.safe_load(config_text) + config_data["dataset"]["years"] = [2022, 2023] + config_data["mart"] = { + "tables": [ + { + "name": "solo_multi_viol", + "sql": "sql/multi_year/solo_multi_viol.sql", + "years": [2022, 2023], + } + ], + "required_tables": ["solo_multi_viol"], + "validate": { + # required_columns include una colonna che la query non produce: + # la validazione multi-year deve fallire. + "table_rules": { + "solo_multi_viol": { + "required_columns": ["anno", "colonna_inesistente"], + "primary_key": ["anno"], + "min_rows": 1, + } + } + }, + } + config_path.write_text( + yaml.dump(config_data, default_flow_style=False, allow_unicode=True, sort_keys=False), + encoding="utf-8", + ) + + # fail_on_error attivo (default): il run deve fallire con la validazione + # multi-year segnalata come errore. Il logger rich spezza le righe lunghe + # (ancora cmd_run.py:NNNN / run.py:NNNN in mezzo): tollerare con regex. + from typer.testing import CliRunner + from toolkit.cli.app import app + + runner = CliRunner() + result = runner.invoke(app, ["run", "--config", str(config_path)]) + assert result.exit_code != 0, "run should fail when multi-year validation fails" + normalized = re.sub(r"\s+", " ", result.output) + assert re.search(r"MART multi-year validation failed", normalized), normalized + + # Il metadata registra l'esito della validazione fallita. + mart_dir = project_example / "_smoke_out" / "data" / "mart" / "project_example" + metadata = json.loads((mart_dir / "metadata.json").read_text(encoding="utf-8")) + validation = metadata.get("validation") or {} + assert validation.get("passed") is False, "multi-year validation should have failed" + assert len(validation.get("errors") or []) > 0, "errors should be recorded" diff --git a/tests/test_run_dry_run.py b/tests/test_run_dry_run.py index fad237a..d42ed86 100644 --- a/tests/test_run_dry_run.py +++ b/tests/test_run_dry_run.py @@ -75,7 +75,9 @@ def test_run_dry_run_fails_on_clean_sql_syntax_error(tmp_path: Path, runner) -> # Il logger rich spezza le righe e inserisce il path del file: # "CLEAN SQL cmd_run.py:986 dry-run failed (...)". Verifichiamo le parti. normalized = _normalized(result.output) - assert "CLEAN SQL" in normalized + # Il logger rich spezza le righe lunghe (ancora cmd_run.py:NNNN in mezzo): + # verificare che entrambe le parole compaiano, non come stringa contigua. + assert "CLEAN" in normalized and "SQL" in normalized, normalized assert "dry-run failed" in normalized assert "Parser Error" in normalized @@ -546,8 +548,11 @@ def test_run_all_fails_with_bootstrap_hint_when_clean_sql_missing( result = runner.invoke(app, ["run", "--config", str(config_path)]) assert result.exit_code != 0 - assert "CLEAN SQL" in result.output - assert "toolkit run raw" in result.output + # Il logger rich spezza le righe lunghe (ancora cmd_run.py:NNNN in mezzo): + # verificare che entrambe le parole compaiano, non come stringa contigua. + normalized = _normalized(result.output) + assert "CLEAN" in normalized and "SQL" in normalized, normalized + assert "toolkit run raw" in normalized # ── Probe step contract tests ──────────────────────────────────────────────── diff --git a/toolkit/cli/cmd_run.py b/toolkit/cli/cmd_run.py index f57f786..3104483 100644 --- a/toolkit/cli/cmd_run.py +++ b/toolkit/cli/cmd_run.py @@ -336,11 +336,42 @@ def _execute_layer(layer_name: str, target, *args, **kwargs) -> bool: source_id=source_id, smoke=sampling_active, ) + elif "mart" in layers_to_run and cfg.has_multi_year_mart: + _skip_mart_validation(cfg, year, context, validations) context.complete_run(success_with_warnings=run_has_validation_warnings) return context +def _skip_mart_validation(cfg, year: int, context, validations: dict) -> None: + """Registra una validazione mart 'skippata' quando tutte le tabelle sono + multi-year (issue #445). + + Il layer mart per-anno non ha nulla da eseguire/validare quando tutte le + tabelle hanno ``years`` (vengono prodotte e validate da + run_mart_multi_year a livello dataset). Senza questo, la validazione + mart per-anno resterebbe vuota e il run risulterebbe fallito. + """ + skipped_summary: dict[str, Any] = { + "passed": True, + "errors_count": 0, + "warnings_count": 0, + "quality_score": None, + "quality_verdict": "skipped", + "errors": [], + "warnings": [], + "checks": [], + "summary": { + "dir": str(layer_year_dir(cfg.root, "mart", cfg.dataset, year)), + "skipped": True, + "reason": "all mart tables are multi-year (mart.tables[].years) — " + "validated at dataset level by run_mart_multi_year", + }, + } + validations["mart"] = skipped_summary + context.set_validation("mart", skipped_summary) + + def _maybe_run_multi_year_mart( cfg, selected_years: list[int], @@ -376,7 +407,7 @@ def _maybe_run_multi_year_mart( ",".join(str(y) for y in selected_years), ) try: - run_mart_multi_year( + result = run_mart_multi_year( cfg.dataset, selected_years, cfg.root, @@ -388,6 +419,16 @@ def _maybe_run_multi_year_mart( source_id=cfg.source_id, smoke=sampling_active, ) + # La validazione delle tabelle multi-year può fallire senza eccezione: + # il runner registra validation_passed=False nel risultato. In quel + # caso comportarsi come un fallimento di validazione. + if not (result or {}).get("validation_passed", True): + errors = (result or {}).get("validation_errors") or [ + "multi-year mart validation failed" + ] + if fail_on_error: + raise ValidationGateError(f"Multi-year MART validation failed: {errors}") + logger.warning("Multi-year MART validation failed (non-fatal): %s", errors) except Exception as exc: if fail_on_error: raise ValidationGateError(f"Multi-year MART failed: {exc}") diff --git a/toolkit/domain/path_resolver.py b/toolkit/domain/path_resolver.py index 61d6674..b0ec394 100644 --- a/toolkit/domain/path_resolver.py +++ b/toolkit/domain/path_resolver.py @@ -15,6 +15,7 @@ METADATA, RAW_PROFILE_DIR, RAW_SUGGESTED_READ, + layer_dataset_dir, layer_year_dir, ) from toolkit.core.run_records import get_run_dir, latest_run @@ -44,16 +45,27 @@ def _clean_paths(root: Path, dataset: str, year: int) -> dict[str, str | None]: } -def _mart_output_paths(root: Path, year_dir: Path, tables: list[Any]) -> list[Path]: +def _mart_output_paths(root: Path, year_dir: Path, dataset: str, tables: list[Any]) -> list[Path]: result: list[Path] = [] + # Le tabelle multi-year (mart.tables[].years) vengono scritte a livello + # dataset (data/mart/{dataset}/{name}.parquet), NON nel dir per-anno: + # il path deve rifletterlo, altrimenti readiness/summary segnalano + # mart_outputs_missing anche quando gli output esistono (issue #445). + dataset_mart_dir = layer_dataset_dir(root, "mart", dataset) for table in tables: if isinstance(table, dict): name = table.get("name") + is_multi_year = bool(table.get("years")) elif hasattr(table, "name"): name = table.name + is_multi_year = bool(getattr(table, "years", None)) else: continue - if name: + if not name: + continue + if is_multi_year: + result.append(dataset_mart_dir / f"{name}.parquet") + else: result.append(year_dir / f"{name}.parquet") return result @@ -64,7 +76,7 @@ def _mart_paths( mart_dir = layer_year_dir(root, "mart", dataset, year) return { "dir": str(mart_dir), - "outputs": [str(path) for path in _mart_output_paths(root, mart_dir, tables)], + "outputs": [str(path) for path in _mart_output_paths(root, mart_dir, dataset, tables)], "metadata": str(mart_dir / METADATA), "validation": None, } diff --git a/toolkit/mart/run.py b/toolkit/mart/run.py index 33105b1..9d82a63 100644 --- a/toolkit/mart/run.py +++ b/toolkit/mart/run.py @@ -36,6 +36,52 @@ _CLEAN_INPUT_TOKEN_RE = re.compile(rf"\b{CLEAN_INPUT_VIEW}\b", re.IGNORECASE) +def _validate_multi_year_tables( + mart_dir: Path, + mart_cfg: dict[str, Any], + *, + root: str | Path | None, + multi_year_tables: list[dict[str, Any]], +) -> dict[str, Any]: + """Valida le tabelle multi-year prodotte a livello dataset (issue #445). + + Applica le table_rules e required_tables relative alle tabelle con + ``years``. Prima del fix queste regole non venivano mai applicate: + la validazione per-anno le escludeva (tabelle non nel dir per-anno) e + il passaggio multi-year non validava nulla. + """ + multi_year_names: set[str] = {str(t.get("name")) for t in multi_year_tables if t.get("name")} + validate_rules = (mart_cfg.get("validate") or {}).get("table_rules") or {} + required_tables = (mart_cfg.get("required_tables") or []) or [] + + multi_year_rules = { + name: rule for name, rule in validate_rules.items() if name in multi_year_names + } + multi_year_required = [t for t in required_tables if t in multi_year_names] + + # Import locale: evita di caricare validate_mart al module-import di run.py + # (il dry-run importa run.py e non deve tirare su l'intero stack di + # validazione mart). + from toolkit.mart.validate import validate_mart + + result = validate_mart( + mart_dir, + required_tables=multi_year_required or None, + root=root, + table_rules=multi_year_rules, + declared_tables=list(multi_year_names), + ) + + return { + "passed": result.ok, + "errors_count": len(result.errors), + "warnings_count": len(result.warnings), + "errors": result.errors, + "warnings": result.warnings, + "summary": result.summary, + } + + # --------------------------------------------------------------------------- # Multi-year mart tables (assorbe ex-cross_year) # --------------------------------------------------------------------------- @@ -145,6 +191,17 @@ def run_mart_multi_year( } ) + # Validazione delle tabelle multi-year a livello dataset (issue #445): + # le table_rules/required_tables delle tabelle con years vengono + # applicate qui, dopo la produzione. Prima questa validazione non + # esisteva: le regole multi-year non venivano mai applicate. + validation_result = _validate_multi_year_tables( + multi_year_dir, + mart_cfg, + root=root_dir, + multi_year_tables=multi_year_tables, + ) + outputs = [file_record(p) for p in written] metadata_payload: dict[str, Any] = { "layer": "mart_multi_year", @@ -154,6 +211,7 @@ def run_mart_multi_year( "outputs": outputs, "output_paths": [serialize_metadata_path(p, root_dir) for p in written], "tables": executed, + "validation": validation_result, } if source_id: metadata_payload["source_id"] = source_id @@ -173,11 +231,17 @@ def run_mart_multi_year( or None ) logger.info("MART multi-year -> %s (%d tables)", multi_year_dir, len(written)) + if not validation_result["passed"]: + # La validazione delle tabelle multi-year è fallita: segnalare nel + # ritorno per far fallire il run (fail_on_error gestito dal chiamante). + logger.error("MART multi-year validation failed: %s", validation_result["errors"]) return { "output_rows": total_rows, "output_bytes": total_bytes, "tables_count": len(written), "col_count": col_count, + "validation_passed": validation_result["passed"], + "validation_errors": validation_result["errors"], } diff --git a/toolkit/mart/validate.py b/toolkit/mart/validate.py index 7b63219..938a226 100644 --- a/toolkit/mart/validate.py +++ b/toolkit/mart/validate.py @@ -189,10 +189,18 @@ def run_mart_validation(cfg, year: int, logger, *, sample_mode: bool = False) -> declared_tables = [t.name for t in cfg.mart.tables if t.name] validate_rules = cfg.mart.validate.to_dict() if cfg.mart.validate else {} + + # Le tabelle multi-year (mart.tables[].years) vengono eseguite da + # run_mart_multi_year() e scritte a livello dataset, NON nel dir + # per-anno. Escluderle dalla validazione per-anno, altrimenti + # `Missing required MART tables` fallisce sempre il run (issue #445). + multi_year_names = {t.name for t in cfg.mart.tables if t.years} + per_year_required = [t for t in (cfg.mart.required_tables or []) if t not in multi_year_names] + spec = ( MartValidationSpec.from_dict( { - "required_tables": cfg.mart.required_tables, + "required_tables": per_year_required, **validate_rules, } )