diff --git a/sidemantic/adapters/holistics.py b/sidemantic/adapters/holistics.py index e8459663..f523fac9 100644 --- a/sidemantic/adapters/holistics.py +++ b/sidemantic/adapters/holistics.py @@ -1313,10 +1313,21 @@ def resolve_named_block(name: str) -> AmlBlock | None: return resolved_blocks[name] return None + # Standalone Metric blocks become graph-level metrics. Resolved before + # datasets so a dataset/partial that references one via the reusable + # "metric name: standalone_metric" shorthand can pull it onto the dataset. + standalone_metrics: dict[str, Metric] = {} + for name, (block, context) in metric_blocks.items(): + metric_block = AmlBlock(kind="metric", name=block.name, items=block.items) + metric = _parse_measure_block(metric_block, constants, context) + if metric: + metric.name = name + standalone_metrics[name] = metric + dataset_models: dict[str, Model] = {} for name, (block, context) in dataset_blocks.items(): - model = _parse_dataset_block(block, constants, context) + model = _parse_dataset_block(block, constants, context, standalone_metrics) if model and model.name not in existing_models: dataset_models[model.name] = model @@ -1335,32 +1346,73 @@ def resolve_named_block(name: str) -> AmlBlock | None: continue block_with_name = AmlBlock(kind="Dataset", name=name, items=block.items) assignment_dataset_blocks.append((block_with_name, context)) - model = _parse_dataset_block(block_with_name, constants, context) + model = _parse_dataset_block(block_with_name, constants, context, standalone_metrics) if model and model.name not in existing_models: dataset_models[model.name] = model - # Standalone Metric blocks become graph-level metrics. - standalone_metrics: dict[str, Metric] = {} - for name, (block, context) in metric_blocks.items(): - metric_block = AmlBlock(kind="metric", name=block.name, items=block.items) - metric = _parse_measure_block(metric_block, constants, context) - if metric: - metric.name = name - standalone_metrics[name] = metric - return dataset_models, standalone_metrics, assignment_dataset_blocks -def _parse_dataset_block(block: AmlBlock, constants: dict[str, AmlValue], context: _FileContext) -> Model | None: +def _resolve_standalone_metric_reference( + item: AmlProperty, + standalone_metrics: dict[str, Metric], + context: _FileContext, +) -> Metric | None: + """Resolve a `metric name: standalone_metric` shorthand reference. + + The reusable-metric-store shorthand parses to a property whose value is a + bare identifier/reference naming a top-level Metric. When it resolves to a + known standalone metric, return a copy renamed to the property key (the + dataset-local name); otherwise return None so genuine properties such as + `label`/`description`/`models` are left untouched. + """ + if not standalone_metrics: + return None + + value = item.value + if isinstance(value, Identifier): + name = value.name + elif isinstance(value, Reference): + name = ".".join(value.parts) + else: + return None + + # Prefer the name qualified against the referencing file's module prefix / + # `use` aliases before the bare name, matching how standalone metrics are + # keyed and how other resolver paths qualify in context first. This lets a + # local same-named metric (e.g. finance.revenue) win over a root metric of + # the same name instead of silently resolving to the global one. + candidates = [_qualify_name(name, context), name] + for candidate in candidates: + referenced = standalone_metrics.get(candidate) + if referenced is not None: + resolved = referenced.model_copy(deep=True) + resolved.name = item.key + return resolved + return None + + +def _parse_dataset_block( + block: AmlBlock, + constants: dict[str, AmlValue], + context: _FileContext, + standalone_metrics: dict[str, Metric] | None = None, +) -> Model | None: """Parse a Dataset block's dataset-level dimension/metric blocks into a Model. Dataset dimensions and metrics use cross-model AQL definitions (the only style Holistics allows in datasets). They are surfaced on a synthetic model named after the dataset so they are not silently dropped. + + `standalone_metrics` lets the reusable-metric-store shorthand + (`metric name: standalone_metric`) resolve the referenced top-level Metric + onto the dataset instead of dropping the reference. """ if not block.name: return None + standalone_metrics = standalone_metrics or {} + properties = _properties_from_items(block.items) label = _value_as_string(properties.get("label"), constants, context) description = _value_as_string(properties.get("description"), constants, context) @@ -1369,6 +1421,16 @@ def _parse_dataset_block(block: AmlBlock, constants: dict[str, AmlValue], contex metrics: list[Metric] = [] for item in block.items: + # Reusable-metric-store shorthand: `metric name: standalone_metric` + # parses to a property whose value names a top-level Metric. Resolve the + # referenced standalone metric onto the dataset (renamed to the local + # key) instead of dropping it, so a dataset that only references + # standalone metrics still produces a model. + if isinstance(item, AmlProperty): + referenced = _resolve_standalone_metric_reference(item, standalone_metrics, item.context or context) + if referenced is not None: + metrics.append(referenced) + continue if not isinstance(item, AmlBlock): continue # A child field carries its own defining context when the dataset was diff --git a/tests/adapters/holistics/test_parsing.py b/tests/adapters/holistics/test_parsing.py index 5c0782c2..fb972c94 100644 --- a/tests/adapters/holistics/test_parsing.py +++ b/tests/adapters/holistics/test_parsing.py @@ -417,6 +417,120 @@ def test_holistics_partial_dataset_assignment_extend(): temp_path.unlink() +def test_holistics_partial_dataset_metric_reference_shorthand(): + """The reusable-metric-store shorthand `metric name: standalone_metric` + references a top-level Metric by name rather than redefining it inline. The + parser represents that reference as a property (not a block), so a Dataset + that extends a PartialDataset whose only contribution is such a reference + must still surface as a model carrying the resolved standalone metric, + instead of producing no model at all.""" + aml_content = """ +Model base_orders { + type: 'table' + table_name: 'orders' + dimension order_id { type: 'number' } + measure order_count { type: 'number' aggregation_type: 'count' } +} + +Metric total_orders { + label: 'Total Orders' + type: 'number' + definition: @aql count(base_orders.order_id) ;; +} + +PartialDataset partial_metrics = PartialDataset { + metric total_orders: total_orders +} + +Dataset base_ds { + label: 'Base DS' + models: [base_orders] +} + +Dataset d = base_ds.extend(partial_metrics) +""" + + with tempfile.NamedTemporaryFile(mode="w", suffix=".aml", delete=False) as f: + f.write(aml_content) + temp_path = Path(f.name) + + try: + graph = HolisticsAdapter().parse(temp_path) + + # The standalone metric registers at graph scope. + assert "total_orders" in graph.metrics + + # The extending dataset must surface as a model even though its only + # field is a reference to a standalone metric. + assert "d" in graph.models + d_model = graph.models["d"] + # The referenced standalone metric is resolved onto the dataset. + assert d_model.get_metric("total_orders").sql == "COUNT(base_orders.order_id)" + finally: + temp_path.unlink() + + +def test_holistics_metric_reference_shorthand_prefers_local_metric(tmp_path): + """The reusable-metric-store shorthand `metric name: standalone_metric` must + resolve the reference against the referencing file's module context before + falling back to a bare global name. When both a root `Metric revenue` and a + `modules/finance` `Metric revenue` exist, a finance PartialDataset that + references `revenue` must pull in the local `finance.revenue`, not the + same-named root metric.""" + finance_dir = tmp_path / "modules" / "finance" + finance_dir.mkdir(parents=True) + + # Both modules define a standalone `revenue` metric with different SQL, and the + # finance partial references the bare name `revenue` via the shorthand. + (finance_dir / "rev.aml").write_text( + """ +Metric revenue { + label: 'Finance Revenue' + type: 'number' + definition: @aql sum(base_orders.amount) ;; +} + +PartialDataset finance_part = PartialDataset { + metric revenue: revenue +} +""" + ) + + # The root file declares its own same-named standalone `revenue` and builds + # the consuming dataset by extending the finance partial. + (tmp_path / "root.aml").write_text( + """ +use finance { finance_part } + +Model base_orders { + type: 'table' + table_name: 'orders' + dimension order_id { type: 'number' } + dimension amount { type: 'number' } +} + +Metric revenue { + label: 'Root Revenue' + type: 'number' + definition: @aql count(base_orders.order_id) ;; +} + +Dataset base_ds { + label: 'Base DS' + models: [base_orders] +} + +Dataset combined = base_ds.extend(finance_part) +""" + ) + + graph = HolisticsAdapter().parse(tmp_path) + + assert "combined" in graph.models + # The local finance metric wins over the root metric of the same name. + assert graph.models["combined"].get_metric("revenue").sql == "SUM(base_orders.amount)" + + def test_holistics_extend_partial_preserves_defining_context(tmp_path): """A Dataset that extends a PartialDataset declared in another module must resolve the partial's field definitions against the partial's own file. A