Skip to content

Commit 88ef112

Browse files
committed
Fix: raise error when retention model inference is ambiguous
1 parent d495baf commit 88ef112

2 files changed

Lines changed: 61 additions & 3 deletions

File tree

sidemantic/sql/generator.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2301,13 +2301,21 @@ def _generate_retention_query(
23012301
model = m
23022302
break
23032303
if not model:
2304+
matching_models = []
23042305
for m_name, m in self.graph.models.items():
23052306
for dim in m.dimensions:
23062307
if dim.name == metric.entity:
2307-
model = m
2308+
matching_models.append(m_name)
23082309
break
2309-
if model:
2310-
break
2310+
if len(matching_models) == 1:
2311+
model = self.graph.get_model(matching_models[0])
2312+
elif len(matching_models) > 1:
2313+
raise ValueError(
2314+
f"Ambiguous model for retention metric '{metric_name}': "
2315+
f"entity dimension '{metric.entity}' found in multiple models: "
2316+
f"{', '.join(matching_models)}. "
2317+
f"Use a model-qualified metric name (e.g., 'model_name.{metric_name}') to disambiguate."
2318+
)
23112319

23122320
if not model:
23132321
raise ValueError(f"No model found for retention metric {metric_name}")

tests/metrics/test_retention.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,3 +818,53 @@ def test_retention_offset_in_sql():
818818
# Without offset, OFFSET should not appear
819819
sql_no_offset = generator.generate(metrics=["retention"], dimensions=[], limit=5)
820820
assert "OFFSET" not in sql_no_offset
821+
822+
823+
def test_retention_ambiguous_model_raises():
824+
"""Test that graph-level retention metric with entity in multiple models raises ValueError."""
825+
orders = Model(
826+
name="orders",
827+
sql="SELECT 1 AS user_id, 'purchase' AS event, '2024-01-01'::DATE AS ts",
828+
primary_key="user_id",
829+
dimensions=[
830+
Dimension(name="user_id", sql="user_id", type="categorical"),
831+
Dimension(name="event", sql="event", type="categorical"),
832+
Dimension(name="ts", sql="ts", type="time"),
833+
],
834+
metrics=[],
835+
)
836+
837+
sessions = Model(
838+
name="sessions",
839+
sql="SELECT 1 AS user_id, 'visit' AS event, '2024-01-01'::DATE AS ts",
840+
primary_key="user_id",
841+
dimensions=[
842+
Dimension(name="user_id", sql="user_id", type="categorical"),
843+
Dimension(name="event", sql="event", type="categorical"),
844+
Dimension(name="ts", sql="ts", type="time"),
845+
],
846+
metrics=[],
847+
)
848+
849+
retention = Metric(
850+
name="retention",
851+
type="retention",
852+
entity="user_id",
853+
cohort_event="event = 'purchase'",
854+
periods=7,
855+
retention_granularity="day",
856+
)
857+
858+
graph = SemanticGraph()
859+
graph.add_model(orders)
860+
graph.add_model(sessions)
861+
graph.add_metric(retention)
862+
863+
generator = SQLGenerator(graph)
864+
with pytest.raises(ValueError, match="Ambiguous model for retention metric") as exc_info:
865+
generator.generate(metrics=["retention"], dimensions=[])
866+
867+
# Verify error mentions both model names
868+
err_msg = str(exc_info.value)
869+
assert "orders" in err_msg
870+
assert "sessions" in err_msg

0 commit comments

Comments
 (0)