feat(atscale_sml): support packages, percentile v1.5 fields, and catalog/model metadata#210
feat(atscale_sml): support packages, percentile v1.5 fields, and catalog/model metadata#210nicosuave wants to merge 5 commits into
Conversation
…log/model metadata - Recognize the package object type (external Git-repo references) as standalone files and inline catalog packages without breaking parsing - Parse percentile compression and custom_quantiles (SML v1.5); use the first custom quantile to drive PERCENTILE_CONT and preserve both fields in metric metadata, with export round-trip - Tolerate catalog hidden_models (v1.2), description (v1.6), and dataset_properties, plus model-level dataset_properties and overrides
![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: e46ae9b2ed
ℹ️ 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".
Imported SML v1.5 custom_quantiles percentiles parse to derived metrics with agg=None and a PERCENTILE_CONT(...) expression, so _export_metric fell through to _export_metric_calc and dropped custom_quantiles and compression. Detect these metrics and export them as percentile metric definitions so the fields round-trip for imported repositories.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbaf91dc2a
ℹ️ 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".
Named percentile metrics (named_quantiles) carrying compression but no custom_quantiles import as derived PERCENTILE_CONT metrics with agg=None, storing compression in metadata only. _export_percentile_metric previously returned None without custom_quantiles, so export degraded to a metric_calc and dropped both the percentile shape and compression. Recover the numeric quantile from the PERCENTILE_CONT expression and re-emit a percentile metric so the shape and compression round-trip.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8315736fd5
ℹ️ 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".
The percentile export helpers used .search() against an end-anchored regex, so a derived metric whose SQL merely contained a percentile sub-expression (e.g. "1 + PERCENTILE_CONT(...)") matched the suffix and was reclassified as a plain percentile metric, silently dropping the prefix. Anchor the pattern at both ends so only a SQL expression that is entirely a percentile is treated as one; prefixed expressions now correctly export as metric_calc.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d05b74475
ℹ️ 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".
Canonical SML package files (version + packages list, no object_type or unique_name, placed in packages/) were silently ignored: only the single-package shape with a top-level name and inline catalog packages were registered. Iterate the top-level packages list for package objects too so each nested reference is tracked.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37b1574782
ℹ️ 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".
1 participant
Summary
Updates the AtScale SML import adapter to cover newer SML spec features that an audit flagged as missing. All changes are conservative additions that preserve existing behavior.
Upstream features now supported
packageobject type (external Git-repo references): standalonepackagefiles are recognized and tracked, and a catalog's inlinepackageslist is registered too. Packages carry no semantic models, so they parse cleanly without materializing anything.compressionandcustom_quantiles(SML v1.5):custom_quantilestakes precedence overnamed_quantilesand drives the generatedPERCENTILE_CONT(...) WITHIN GROUP (...)SQL (first quantile). Both fields are preserved on the metric'smetadataand round-trip on export.hidden_models(v1.2),description(v1.6), anddataset_properties: parsed without error; referenced models stay importable.dataset_propertiesandoverrides: parsed without error; metrics, relationships, and aggregates are unaffected.Notes / limitations
packagereferences are tracked but not resolved/fetched (no Git access at parse time); they intentionally produce no models.description,hidden_models,dataset_properties, and modeloverrides/dataset_propertieshave no direct equivalent in the sidemantic graph today, so they are tolerated rather than mapped. Percentilecompressionandcustom_quantilesare preserved via metric metadata.Tests
packageobject, a percentile metric withcustom_quantiles+compression, and catalog/model fields exercised via the kitchen-sink fixtures.