diff --git a/src/keboola_agent_cli/services/_semantic_layer_internals.py b/src/keboola_agent_cli/services/_semantic_layer_internals.py index cd461046..81936d5b 100644 --- a/src/keboola_agent_cli/services/_semantic_layer_internals.py +++ b/src/keboola_agent_cli/services/_semantic_layer_internals.py @@ -994,6 +994,54 @@ def _normalize_field_type(basetype: str) -> str: return _FIELD_TYPE_MAP.get(head, "string") +# Aggregation heuristic for auto-generated per-measure metrics. Mirrors the +# semantic-layer toolkit's name-based guess, with two deliberate refinements: +# percentage / ratio columns aggregate as AVG (not SUM -- summing a rate is +# almost always wrong, and would trip the SUM_ON_PCT validation), and a +# ``count_*`` column stays SUM (additive daily counts sum to a total; COUNT() +# of an already-aggregated column would be meaningless). +_AGG_AVG_TOKENS = ("AVG", "AVERAGE", "MEAN", "RATE", "PCT", "PERCENT", "RATIO", "SHARE") +_AGG_MAX_TOKENS = ("MAX", "MAXIMUM", "PEAK") +_AGG_MIN_TOKENS = ("MIN", "MINIMUM") + +# metric-name prefix per aggregate. +_AGG_NAME_PREFIX = {"AVG": "avg", "MAX": "max", "MIN": "min", "SUM": "total"} + + +def _estimate_metric_aggregation(col_name: str) -> str: + """Guess an aggregate for a measure column from keywords in its name. + + Defaults to SUM (additive measures). See ``_AGG_*_TOKENS`` for the rules. + """ + upper = col_name.upper() + if any(tok in upper for tok in _AGG_AVG_TOKENS): + return "AVG" + if any(tok in upper for tok in _AGG_MAX_TOKENS): + return "MAX" + if any(tok in upper for tok in _AGG_MIN_TOKENS): + return "MIN" + return "SUM" + + +def _metric_snake_case(text: str) -> str: + """Lowercase snake_case slug safe for a metric name.""" + slug = re.sub(r"[^a-z0-9]+", "_", text.lower()).strip("_") + return slug or "metric" + + +def _dedupe_name(name: str, seen: set[str]) -> str: + """Return ``name`` (or a ``_2``/``_3``… suffixed variant) unused in ``seen``.""" + if name not in seen: + seen.add(name) + return name + counter = 2 + while f"{name}_{counter}" in seen: + counter += 1 + unique = f"{name}_{counter}" + seen.add(unique) + return unique + + def heuristic_generate_model( *, schemas: dict[str, dict[str, Any]], @@ -1003,10 +1051,11 @@ def heuristic_generate_model( ) -> dict[str, Any]: """Deterministic stand-in for the AI generator (see ``build_model``). - Builds: one dataset per table (with classified fields[]), one - COUNT(*) metric per dataset as a placeholder, no relationships - (cross-table FKs are not inferrable from columns alone), an empty - constraints list, and a glossary entry per dataset. + Builds: one dataset per table (with classified fields[]); one metric per + ``measure`` field (aggregate guessed from the column name -- see + ``_estimate_metric_aggregation``) plus a ``COUNT(*)`` row-count metric per + dataset; no relationships (cross-table FKs are not inferrable from columns + alone); an empty constraints list; and a glossary entry per dataset. Accepts the FQN-derivation and role-classification helpers as callables so the helper module stays free of import-time coupling @@ -1015,6 +1064,9 @@ def heuristic_generate_model( datasets: list[dict[str, Any]] = [] metrics: list[dict[str, Any]] = [] glossary: list[dict[str, Any]] = [] + # Metric names must be unique across the whole model (validate_basic flags + # duplicates), so dedupe against one model-wide set. + seen_metric_names: set[str] = set() for tid, detail in schemas.items(): ds_name = ( @@ -1022,6 +1074,7 @@ def heuristic_generate_model( .replace(" ", "_") .lower() ) + fqn = derive_fqn(tid) fields: list[dict[str, Any]] = [] for col in detail.get("column_details", []) or []: cname = col.get("name", "") @@ -1043,15 +1096,33 @@ def heuristic_generate_model( { "name": ds_name, "tableId": tid, - "fqn": derive_fqn(tid), + "fqn": fqn, "fields": fields, "description": detail.get("description", "") or "", } ) + # One metric per measure field: aggregate guessed from the name. + for field in fields: + if field["role"] != "measure": + continue + col = field["name"] + agg = _estimate_metric_aggregation(col) + name = _dedupe_name( + _metric_snake_case(f"{_AGG_NAME_PREFIX[agg]}_{col}"), seen_metric_names + ) + metrics.append( + { + "name": name, + "sql": f'{agg}("{col}") FROM {fqn}', + "dataset": tid, + "description": f"{agg} of {col}.", + } + ) + # Always add a row-count metric as a safe baseline. metrics.append( { - "name": f"{ds_name}_row_count", - "sql": f"COUNT(*) FROM {derive_fqn(tid)}", + "name": _dedupe_name(f"{ds_name}_row_count", seen_metric_names), + "sql": f"COUNT(*) FROM {fqn}", "dataset": tid, "description": f"Row count of {ds_name}.", } diff --git a/tests/test_semantic_layer_service.py b/tests/test_semantic_layer_service.py index e463ff0f..af9c3326 100644 --- a/tests/test_semantic_layer_service.py +++ b/tests/test_semantic_layer_service.py @@ -2085,7 +2085,8 @@ def test_heuristic_fallback(self, tmp_path: Path) -> None: mock.post_item.side_effect = [ {"id": "new-model"}, # the model {"id": "d1"}, # dataset - {"id": "m1"}, # metric (count(*)) + {"id": "m1"}, # measure metric (SUM of AMOUNT) + {"id": "m2"}, # row-count metric {"id": "g1"}, # glossary ] with patch( @@ -2099,7 +2100,12 @@ def test_heuristic_fallback(self, tmp_path: Path) -> None: result = service.build_model("prod", table_ids=["out.c.t"]) assert result["fallback_used"] == "heuristic" assert len(result["generated"]["datasets"]) == 1 - assert len(result["generated"]["metrics"]) == 1 + # One metric per measure field (AMOUNT -> SUM) + a row-count baseline. + metrics = result["generated"]["metrics"] + assert len(metrics) == 2 + measure_metric = next(m for m in metrics if m["name"] != "fact_orders_row_count") + assert measure_metric["name"] == "total_amount" + assert measure_metric["sql"] == 'SUM("AMOUNT") FROM "KEBOOLA"."out.c"."t"' assert len(result["generated"]["glossary"]) == 1 assert result["validated"] is True # Pin the warehouse → metastore type normalization: Storage hands us @@ -2870,3 +2876,112 @@ def test_registry_entries(self) -> None: assert OPERATION_REGISTRY["semantic-layer.reference-data.get"] == "read" assert OPERATION_REGISTRY["semantic-layer.reference-data.set"] == "write" assert OPERATION_REGISTRY["semantic-layer.reference-data.delete"] == "destructive" + + +# --------------------------------------------------------------------------- +# build: one metric per measure field (sl-toolkit-style), aggregate guessed +# --------------------------------------------------------------------------- + + +class TestEstimateMetricAggregation: + """Name-based aggregate guess for auto-generated metrics.""" + + def test_default_is_sum(self) -> None: + from keboola_agent_cli.services._semantic_layer_internals import ( + _estimate_metric_aggregation, + ) + + assert _estimate_metric_aggregation("total_ppc_revenue") == "SUM" + # Additive daily counts sum to a total -- NOT COUNT(). + assert _estimate_metric_aggregation("count_marketplace_orders") == "SUM" + + def test_rate_and_percent_are_avg(self) -> None: + from keboola_agent_cli.services._semantic_layer_internals import ( + _estimate_metric_aggregation, + ) + + assert _estimate_metric_aggregation("conversion_rate") == "AVG" + assert _estimate_metric_aggregation("offers_75_100_pct_pricelist") == "AVG" + assert _estimate_metric_aggregation("avg_order_value") == "AVG" + + def test_min_max(self) -> None: + from keboola_agent_cli.services._semantic_layer_internals import ( + _estimate_metric_aggregation, + ) + + assert _estimate_metric_aggregation("max_price") == "MAX" + assert _estimate_metric_aggregation("min_price") == "MIN" + + +class TestHeuristicMetricPerMeasure: + """`heuristic_generate_model` emits one metric per measure + a row count.""" + + @staticmethod + def _identity_fqn(tid: str) -> str: + return f"FQN({tid})" + + @staticmethod + def _role(name: str, basetype: str) -> str: + # Minimal stand-in classifier for the test: numeric measure-ish names. + if basetype.lower() in ("numeric", "integer", "decimal") and any( + t in name.lower() for t in ("revenue", "count", "rate") + ): + return "measure" + return "dimension" + + def test_metric_per_measure_plus_row_count(self) -> None: + from keboola_agent_cli.services._semantic_layer_internals import ( + heuristic_generate_model, + ) + + schemas = { + "in.c-x.t": { + "display_name": "t", + "column_details": [ + {"name": "total_revenue", "type": "NUMERIC"}, + {"name": "conversion_rate", "type": "NUMERIC"}, + {"name": "category_id", "type": "INTEGER"}, # dimension + ], + } + } + model = heuristic_generate_model( + schemas=schemas, + model_name="m", + derive_fqn=self._identity_fqn, + classify_role=self._role, + ) + metrics = {m["name"]: m for m in model["metrics"]} + # 2 measures -> 2 metrics, + 1 row count = 3 + assert len(metrics) == 3 + assert metrics["total_total_revenue"]["sql"] == 'SUM("total_revenue") FROM FQN(in.c-x.t)' + assert metrics["avg_conversion_rate"]["sql"] == 'AVG("conversion_rate") FROM FQN(in.c-x.t)' + assert "t_row_count" in metrics + # The dimension column produced no metric. + assert not any("category_id" in name for name in metrics) + + def test_duplicate_metric_names_deduped_across_tables(self) -> None: + from keboola_agent_cli.services._semantic_layer_internals import ( + heuristic_generate_model, + ) + + # Same measure column name in two tables -> names must not collide. + schemas = { + "in.c-a.t": { + "display_name": "a", + "column_details": [{"name": "revenue", "type": "NUMERIC"}], + }, + "in.c-b.t": { + "display_name": "b", + "column_details": [{"name": "revenue", "type": "NUMERIC"}], + }, + } + model = heuristic_generate_model( + schemas=schemas, + model_name="m", + derive_fqn=self._identity_fqn, + classify_role=self._role, + ) + names = [m["name"] for m in model["metrics"]] + assert len(names) == len(set(names)), f"duplicate metric names: {names}" + assert "total_revenue" in names + assert "total_revenue_2" in names