Import Snowflake Cortex Analyst measures and enrichment keys#200
Import Snowflake Cortex Analyst measures and enrichment keys#200nicosuave wants to merge 27 commits into
Conversation
Read the table-level measures key as a legacy alias of facts so current Cortex Analyst files (e.g. the revenue_timeseries tutorial) no longer import with zero metrics. Also import synonyms on dimensions/measures/ metrics, sample_values and cortex_search_service(_name) on dimensions, top-level verified_queries/custom_instructions/module_custom_instructions, and preserve newer per-field keys (access_modifier, is_enum, unique, labels, tags, non_additive_dimensions, using_relationships) in metadata. Export round-trips the new fields. Adds synonyms/sample_values/ cortex_search_service_name fields to core Dimension/Metric.
![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: e985426c7e
ℹ️ 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".
Graph-level metrics (top-level metrics with no owning table) were passed
through _qualify_columns(), which corrupted their model.field references
into {model}.model.field. They were also never serialized by export(),
so they silently disappeared on parse/export round-trips.
Parse graph-level metric expressions without {model} qualification and
re-export graph.metrics into the top-level metrics section.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7f01278c0
ℹ️ 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".
…tive YAML Snowflake export now skips metrics it cannot represent: model-owned metrics auto-registered at graph level (time_comparison/conversion) were being double-exported as top-level stubs, and any metric type without a Snowflake expr produced invalid stubs that failed to re-parse. Also wire the Cortex enrichment fields (dimension synonyms/sample_values/ cortex_search_service_name and metric synonyms) into the native Sidemantic adapter so export-native round-trips them instead of silently dropping them.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a14c81f4c
ℹ️ 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".
…ynonyms Snowflake sample_values can be numeric/time scalars per the Cortex spec; coerce them to str so import does not reject valid files. Also emit synonyms when exporting graph-level metrics so native round-trips no longer drop them.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b19d987727
ℹ️ 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".
SidemanticAdapter now round-trips graph.metadata as a top-level metadata key, so Snowflake Cortex top-level sections (verified_queries and the custom-instruction blocks) survive import snowflake -> export-native -> re-parse. The Snowflake export falls back to graph.metadata['snowflake'] when the dynamic attributes are absent, so re-export to Snowflake still emits those sections.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7180510cb
ℹ️ 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 Snowflake relationship name referenced by a metric using_relationships was discarded (Relationship.name holds the related model), so the metric referenced a relationship that no longer existed after export. Stash the original name in relationship metadata and re-emit it. A simple aggregate metric carrying using_relationships was also routed through facts, which do not collect that metric-only key on re-parse; export such metrics through the metrics block so the reference survives.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6640a93040
ℹ️ 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".
…rics export The top-level metric export skipped any graph metric whose name matched a model-local metric, silently dropping a distinct top-level metric that merely shared a name. Match by object identity instead, so auto-registered model-owned metrics are still skipped while genuine graph-level metrics round-trip.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 228441757e
ℹ️ 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 Python native adapter now emits a root metadata block plus dimension synonyms/sample_values/cortex_search_service_name and metric synonyms. The Rust native loader uses deny_unknown_fields, so add these optional fields to SidemanticConfig/DimensionConfig/MetricConfig to keep native YAML produced by export-native loadable by the Rust/wasm parser.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55c8517d0e
ℹ️ 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".
# Conflicts: # sidemantic-rs/src/config/schema.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 642cde340d
ℹ️ 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 CLI-first load_from_directory path only copied _tmdl_ passthrough attrs into layer.graph, dropping graph.metadata and the Snowflake dynamic top-level attributes (verified_queries / custom instructions). Merge those too so import snowflake -> export-native re-emits the root metadata block and the sections can be re-exported to Snowflake.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a322a3945f
ℹ️ 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".
… metadata - Route simple aggregate metrics carrying non_additive_dimensions through the metrics block (not facts) so the metric-only key keeps them metrics. - Map access_modifier private_access onto Dimension/Metric public=False on import and re-emit access_modifier on export, so private Cortex fields are hidden in info/catalog/native YAML while round-tripping. - Deep-merge Snowflake graph metadata across directory-loaded files so multi-file Cortex projects accumulate verified_queries and custom instructions instead of overwriting.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b68e9e232a
ℹ️ 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".
- Strip the {model} placeholder when exporting table-scoped metrics so
Snowflake receives bare column references instead of unresolvable tokens.
- Preserve model.field qualifiers when exporting graph-level ratio metrics
so Snowflake can resolve cross-table members in view-level metrics.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71d521603e
ℹ️ 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".
Parsing a directory attached a top-level metric to its table only if the table file was visited first; otherwise the metric fell into the graph-level branch and lost its table context on export. Collect top-level metric definitions across all files and resolve them after every table is loaded so attachment no longer depends on traversal order.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cf5af001a
ℹ️ 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 CLI loader parses each Snowflake file separately, so a top-level metric referencing a table defined in another file could not attach. Mark such metrics with a pending table reference during parse, detect Cortex files (tables + base_table) before the generic MetricFlow heuristic, and re-attach the metric to its model once every file is loaded, re-qualifying its SQL.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da115d3185
ℹ️ 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".
A Cortex project may split top-level metrics (table + expr, no tables section) into their own file. Such a file was never routed to the Snowflake adapter, so the metrics silently dropped during load_from_directory. Detect these files by their Cortex-only table/expr shape and route them to Snowflake so the metrics defer and attach to tables defined in sibling files.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87ea415518
ℹ️ 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 metrics-only Cortex detector rejected the whole file when any metric lacked a table, dropping mixed files that combine table-scoped metrics with graph-level view metrics. Require expr and no MetricFlow markers on every metric and at least one table reference, so mixed files route to Snowflake while tableless-only files are left to native detection.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c684a99b47
ℹ️ 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".
A split Cortex file with only view-level metrics (no table) plus Snowflake top-level sections such as verified_queries matched neither native nor Snowflake detection, so it was skipped and its metrics/metadata dropped. Treat a Snowflake-only top-level section as a Cortex signal so these sidecar files route to the Snowflake adapter.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d20f85dfb9
ℹ️ 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".
A Cortex sidecar holding only verified_queries/custom_instructions and no metrics returned early from the metrics-file detector before the Snowflake-section check, so it was skipped and its metadata lost. Treat the metrics key as optional so files carrying only Snowflake-only top-level sections route to the Snowflake adapter.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ef649c3f3
ℹ️ 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".
…in Rust - Hold Snowflake table-scoped metrics whose table is in another file in a table-qualified pending list instead of the graph-level name map, so two tables defining a same-named scoped metric no longer overwrite each other; unresolved entries fall back to graph-level metrics. - Replace the Rust directory loader's OSI-only metadata merge with a generic deep merge so root metadata.snowflake (verified queries / custom instructions) from Python export-native files survives directory loads.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fca46034b1
ℹ️ 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".
A split Cortex project may keep top-level relationships in their own file. The detector now recognizes Snowflake-shaped relationship-only sidecars, the adapter defers relationships until all tables load (and stashes unresolved ones), and the directory loader applies pending relationships once every referenced table is loaded, so declared joins survive the CLI-first flow.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ffb3a8e2e
ℹ️ 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".
- Apply Snowflake relationships from sidecar files before foreign-key inference so an explicit Cortex join wins over a guessed one for the same table pair, de-duplicating existing edges. - Detect a tableless Cortex metrics sidecar by Snowflake-only per-metric keys (access_modifier / labels / tags / non_additive_dimensions / using_relationships), so files native detection rejects still route to Snowflake instead of being dropped.
Resolve loaders.py conflict: keep the new ThoughtSpot TML Model detection branch from #199 and drop main's duplicate Snowflake Cortex elif, which the snowflake branch already moved earlier in the chain (before the MetricFlow metrics+type heuristic).
Resolve tests/core/test_directory_loaders.py by unioning both sides' added test functions (main's Hex/validate tests + snowflake's Cortex sidecar tests), keeping the 5 shared baseline tests once. loaders.py / schema.json / metric.py auto-merged cleanly.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cb7c3077e
ℹ️ 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".
apply_pending_relationships de-duplicated by target table alone, dropping every named relationship after the first when a split Cortex project declared two joins between the same two tables. De-dup by the preserved Snowflake relationship name (or join columns) instead so alternate joins survive and using_relationships references keep resolving.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 892f84ea5d
ℹ️ 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".
A split Cortex sidecar with a root name and only tableless view metrics (no per-metric Snowflake key, no verified_queries/instructions/relationships) was skipped by load_from_directory: native detection rejects the root name and the Snowflake predicate found no Cortex signal, so the view metric was dropped. Treat a root name alongside Cortex-shaped metrics as a Snowflake signal.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 976b382369
ℹ️ 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".
Snowflake export: when public is false, write access_modifier private_access unconditionally so a stale public_access value carried in metadata no longer keeps the field public. Applies to dimensions, facts, and metrics. Native SQL frontmatter: strip root-only metadata before deciding whether a .sql frontmatter is a model, so graph-level SQL files containing only metadata plus METRIC/PARAMETER definitions still load. Graph metadata is preserved on the graph; model metadata is reattached when the frontmatter is a model.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d47659c49
ℹ️ 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".
| if dim.cortex_search_service_name: | ||
| dim_def["cortex_search_service_name"] = dim.cortex_search_service_name | ||
| snowflake_meta = (dim.metadata or {}).get("snowflake", {}) | ||
| for key, value in snowflake_meta.items(): | ||
| dim_def.setdefault(key, value) |
There was a problem hiding this comment.
When a dimension was imported from the newer nested cortex_search_service, _cortex_search_service_name() populates dim.cortex_search_service_name while _dimension_metadata() preserves the original nested object. This branch writes the deprecated flat cortex_search_service_name, and the metadata loop then also writes cortex_search_service, so round-tripped Snowflake YAML contains both; if a user edits the first-class native name, the stale nested service can conflict. Snowflake documents cortex_search_service as replacing deprecated cortex_search_service_name: https://docs.snowflake.com/en/user-guide/views-semantic/semantic-view-yaml-spec
Useful? React with 👍 / 👎.
1 participant
What changed
The Snowflake adapter only recognized the legacy
facts:table key, so a valid current Cortex Analyst file using the table-levelmeasures:key (for example the officialrevenue_timeseries.yamltutorial fixture) imported with zero metrics — silent data loss. This PR fixes that and brings the adapter up to date with the current Cortex Analyst semantic-model spec.Newly supported upstream features
measures:table key read as a legacy alias offacts:. Files that use either (or both) now import their measures.synonymson dimensions, facts/measures, and metrics.sample_valueson dimensions.cortex_search_service_namestring and the newer nestedcortex_search_service: { service, literal_column, database, schema }object are resolved to a service name.verified_queries,custom_instructions, andmodule_custom_instructionsare imported onto the graph and re-exported.metrics(semantic-model-scoped derived metrics) are attached to their referenced table or kept as graph-level metrics.metadata:access_modifier,is_enum,unique,labels,tags,non_additive_dimensions,using_relationships.Core model additions
Added optional
synonyms(Dimension + Metric),sample_values(Dimension), andcortex_search_service_name(Dimension) fields. These are broadly useful across formats and let the values be accessed directly rather than only through metadata.Tests
revenue_timeseries.yamltests intests/adapters/snowflake/test_fixtures.py(measures, synonyms, sample_values, cortex_search_service_name, verified_queries) and made them pass.tests/fixtures/snowflake/cortex_features.yamlandtests/adapters/snowflake/test_cortex_features.pycovering the nestedcortex_search_service, the per-field keys preserved in metadata, the top-level sections, and an export round-trip.Compatibility
Fully backward compatible: existing facts-based fixtures and the round-trip/conversion suites stay green. Export writes the canonical
facts:key plus the new enrichment fields. No package version bump.Known limitations
access_modifier,is_enum,unique,labels,tags,non_additive_dimensions,using_relationships) are preserved inmetadata["snowflake"]for fidelity and round-trip but are not yet mapped to first-class Sidemantic semantics.cortex_search_serviceobject is reduced to itsservicename on import (the rest of the object is preserved in dimension metadata).