Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 71 additions & 12 deletions sidemantic/adapters/holistics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -1335,32 +1346,70 @@ 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

# Try the name as written, then qualified against the referencing file's
# module prefix / `use` aliases, matching how standalone metrics are keyed.
candidates = [name, _qualify_name(name, context)]
Comment thread
nicosuave marked this conversation as resolved.
Outdated
for candidate in candidates:
referenced = standalone_metrics.get(candidate)
if referenced is not None:
resolved = referenced.model_copy(deep=True)
resolved.name = item.key

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve standalone metric names when copying references

When a module metric is referenced through this shorthand with a local alias that matches a real root standalone metric, this rename lets the copied dataset metric occupy the root graph key before standalone metrics are added. For example, with root Metric revenue and module finance.revenue, a finance partial containing metric revenue: revenue makes graph.metrics["revenue"] point to the finance SQL, and the actual root standalone Metric revenue is skipped by the later if metric.name not in graph.metrics check. This only affects collisions between dataset-local aliases and standalone metric names, but it changes graph-level metric resolution for existing root metrics.

Useful? React with 👍 / 👎.

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)
Expand All @@ -1369,6 +1418,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
Expand Down
53 changes: 53 additions & 0 deletions tests/adapters/holistics/test_parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,59 @@ 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_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
Expand Down
Loading