Import metric reference entries in Holistics datasets#228
Conversation
The reusable-metric-store shorthand `metric name: standalone_metric` references a top-level Metric by name instead of redefining it inline. The parser represents that reference as a property rather than a block, so the dataset-block loop dropped it. A Dataset that extends a PartialDataset whose only contribution is such a reference then produced no model at all. Resolve metric-reference property entries against the standalone Metric registry (renamed to the dataset-local key) so the dataset still surfaces with the referenced metric attached.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f53b3671a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
When a Dataset/PartialDataset authored inside a module references a bare standalone metric name via the reusable-metric-store shorthand, resolve the context-qualified name (e.g. finance.revenue) before the bare name. Previously the unqualified candidate was tried first, so a finance partial referencing revenue resolved to a same-named root metric instead of the local one, giving the extending dataset the wrong SQL.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18b824932d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| referenced = standalone_metrics.get(candidate) | ||
| if referenced is not None: | ||
| resolved = referenced.model_copy(deep=True) | ||
| resolved.name = item.key |
There was a problem hiding this comment.
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 👍 / 👎.
1 participant
Follow-up to #204.
What & why
The Holistics reusable-metric-store shorthand
metric name: standalone_metricreferences a top-levelMetricby name instead of redefining it inline. The parser represents that reference as a property (not a block), so the dataset-block item loop insidemantic/adapters/holistics.pydropped it viacontinue.As a result, a
Dataset d = base.extend(partial_metrics)whose partial only contributes such metric references produced nodmodel at all, even though the referenced standaloneMetricexisted.PartialDataset.extendonly worked when every reusable metric was rewritten as a full inline block.This change resolves metric-reference property entries against the standalone
Metricregistry (copied and renamed to the dataset-local key) so the dataset still surfaces as a model carrying the referenced metric. Standalone metrics are now resolved before datasets so the reference can be resolved at dataset-parse time. Genuine dataset properties (label,description,models,relationships) are untouched because only values that resolve to a known standalone metric are pulled in.Known limitations
usealiases; a reference that cannot be matched to a known standalone metric is left as before (no spurious metric created).