fix(semantic-layer): honor provider preferred time dimension

[mikebridge]

SUMMARY

Semantic views had no way to say which temporal dimension Explore should default to. The host always set main_dttm_col=None, so Explore picked the first temporal column in whatever order the provider returned. For MetricFlow that is often an entity date such as customer__first_ordered_at rather than metric_time, and the choice changes when the dimension order changes.

This adds an optional, typed SemanticView.preferred_time_dimension to the core SDK. The host projects it to main_dttm_col only when the name matches an exposed temporal dimension. When it is absent, unknown or not temporal, the existing fallback is unchanged. Saved charts keep their chosen temporal subject; nothing is migrated or rewritten.

No end-to-end behaviour changes until a provider opts in. The MetricFlow opt-in (declaring metric_time when it is exposed) is a separate change.

TESTING INSTRUCTIONS

• Red first: host tests with entity-first dimensions plus a declared metric_time failed before the change (missing SDK field; main_dttm_col=None). New cases also cover absent, unknown and non-temporal declarations.
• pytest tests/unit_tests/semantic_layers superset-core/tests (semantic suites): 703 passed. Existing getTemporalColumns Jest suite: 5 passed. Changed-file pre-commit hooks including MyPy pass.
• Whole service-free unit suite at this commit: 21,575 passed; the 15 failures match a known local machine-specific baseline and none are in files this branch touches.
• Not yet verified live: with a provider that declares metric_time, a fresh Table chart should default to it, and a dashboard native time range should filter on it.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration
• Introduces new feature or API
• Removes existing feature or API

🤖 Generated with Claude Code

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	65cc66e
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6ac87c5cb548aa0008da29e1
😎 Deploy Preview	https://deploy-preview-44997--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.82%. Comparing base (4ca736d) to head (65cc66e).

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #44997      +/-   ##
==========================================
+ Coverage   82.79%   82.82%   +0.02%     
==========================================
  Files        3016     3021       +5     
  Lines      192466   193104     +638     
  Branches    44835    44837       +2     
==========================================
+ Hits       159354   159937     +583     
- Misses      30079    30134      +55     
  Partials     3033     3033              

Flag	Coverage Δ
hive	35.38% <20.00%> (-0.01%)	⬇️
mysql	53.79% <20.00%> (-0.01%)	⬇️
postgres	53.79% <20.00%> (-0.01%)	⬇️
presto	37.16% <20.00%> (-0.01%)	⬇️
python	86.46% <100.00%> (+<0.01%)	⬆️
sqlite	53.54% <20.00%> (-0.01%)	⬇️
superset-extensions-cli	91.24% <ø> (?)
unit	80.05% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[codeant-ai-for-open-source]

CodeAnt PR Risk: Low Risk

• The PR appears safe to merge; the preferred temporal dimension is validated and exposed as a time-granularity choice.
• Tests cover valid and invalid preferences, fallback behavior, and applying the preferred dimension to bounded queries.

Assessed commit: f443f6090bef

[EnxDev]

The logic and tests hold up. I ran tests/unit_tests/semantic_layers on b198137 and all 706 pass. The metric_time case also fails against master's models.py, so the new test really does guard the change.

About the CodeAnt thread on granularity: the modern control gets its options from the is_dttm columns through getTemporalColumns, not from granularity_sqla. So the default isn't cleared, and it becomes the TEMPORAL_RANGE subject on new charts.

I left two notes inline. The one that matters: the preference only reaches the data payload, so backend code that reads main_dttm_col doesn't see it.

[EnxDev]

Supersedes my earlier pass on b198137.

Both earlier notes are fixed in 2260340. main_dttm_col is a model property now, so data, _resolve_time_column and _apply_granularity all read the same value, and the warning fires when the declared name doesn't match. I ran tests/unit_tests/semantic_layers and tests/unit_tests/common at this head and all 1,057 pass.

I left one note inline about a test for the dashboard time range path. Otherwise this looks good to me.

[EnxDev]

Once a view declares metric_time, this also turns on the filter granularity inference in query_context_factory.py:268. That's the dashboard native time range case the description lists as not yet verified live.

Could we add a test that runs _apply_granularity against a semantic view declaring metric_time and checks that granularity lands on it? Right now the coverage only goes through the payload and _resolve_time_column.

[mikebridge]

Good catch, this was a coverage gap. 9168006 adds a test that gives a semantic view entity_time first and the preferred metric_time, then runs it through _apply_granularity with a bounded ad-hoc axis. It checks that metric_time becomes the filter granularity without changing the axis. The test passed against the existing production path, so this follow-up adds coverage without changing behavior.

[aminghadersohi]

No findings at 9168006. The fixes for the three resolved threads and the new _apply_granularity test all fail when main_dttm_col is reverted. Holding approval until CI runs on this head and EnxDev's open thread is resolved.

[EnxDev]

Supersedes my earlier pass on 2260340.

The new _apply_granularity test covers what I asked for. I stubbed main_dttm_col to return None and it fails, along with the three payload cases, so it guards the dashboard time range path. tests/unit_tests/semantic_layers and tests/unit_tests/common pass at this head (1,058).

No new notes from me. CI hasn't run on 9168006 yet, so that's the only thing left on my side.

[fitzee]

Verdict: approve. I found nothing blocking. Reviewed at e22594d. Every commit after the approved 9168006 is a merge from master, and the PR's own diff against master is the same as at 9168006. CI is green. tests/unit_tests/semantic_layers and tests/unit_tests/common pass locally at this head (1,163 tests).

Checked and correct

• Preference missing, not temporal, or absent: main_dttm_col (superset/semantic_layers/models.py:601-626) returns None, so the old fallbacks still apply: getTemporalColumns picks the first is_dttm column, and _resolve_time_column still raises on multiple datetime columns. The temporal check uses get_column_type(...) == TEMPORAL. That matches the is_dttm predicate at models.py:584 and :674, so the default can never be a column the UI treats as non-temporal.
• User choice wins everywhere:
  • _resolve_time_column only consults main_dttm_col when time_column is None (superset/common/tabular_query.py:148-149).
  • _apply_granularity only infers when query_object.granularity is None (superset/common/query_context_factory.py:265-275).
  • Explore uses it only as a control default / initialValue (mixins.tsx:61-69, :105-118). Saved form_data and saved TEMPORAL_RANGE subjects are left alone.
• Regular SqlaTable datasets: unaffected. The only change in tabular_query.py is the docstring. SqlaTable.main_dttm_col and .data are untouched.
• Frontend and backend agree: a semantic-view payload passes isDataset (types.ts:565-571, since it has no sqlEditorId). So getTemporalColumns.ts:54-55 reads the same main_dttm_col that the backend paths read through the model property.

Non-blocking

1. Warning spam and repeated provider calls (models.py:601-626). This is a plain @property, so every access calls implementation.get_dimensions() again. With a misconfigured preference it also logs the warning again. One chart-data request reads it in data (models.py:730, plus _unique_dimensions again at :630) and in _apply_granularity (query_context_factory.py:268 and :307). That can mean several warnings and dimension listings per query object, on every refresh. A cached_property, like implementation already uses (models.py:369), would keep this to one per instance.

2. granularity_sqla is populated whether or not a provider opts in (models.py:737-741). This conflicts with the PR body's "no end-to-end behaviour changes until a provider opts in". In practice it's inert. The only reader of datasource.granularity_sqla is the legacy SelectControl in src/explore/controls.tsx:313-338, and in Explore the shared DnD granularity_sqla control (sharedControls.tsx:484, resolved first by expandControlConfig.tsx:55) overrides it. It takes its options from columns. Fine to keep for shape parity with SqlaTable.granularity_sqla, but the PR body could say so.

3. "Saved charts keep their chosen temporal subject" is true, but some saved charts still change once a provider opts in:

   • A saved chart with an ad-hoc x-axis (needs ADHOC_COLUMN_EXPRESSIONS) and a dashboard time range, but no time column. Today _get_time_axis_column (superset/semantic_layers/mapper.py:569-595) finds nothing and the range is silently not applied. After opt-in, _apply_granularity sets granularity = metric_time and the range filters on it. That is the intended fix, but it is a visible data change on existing dashboards.
   • A saved chart with no TEMPORAL_RANGE filter, reopened in Explore. It gets the injected default filter (mixins.tsx:105-118) on metric_time instead of the first temporal column. This only matters when DEFAULT_TIME_FILTER is set to something other than "No filter".

   Worth a line in the follow-up MetricFlow PR or its release note.

4. Tests (tests/unit_tests/semantic_layers/models_test.py:806-870) do fail if the property is reverted. What they don't pin is precedence: no assertion that an explicit time_column beats the preference (_resolve_time_column(view, name, "entity_time", True) == "entity_time"), or that a set granularity survives _apply_granularity. The current code handles both correctly, so this only guards against regressions.

5. Docs (docs/developer_docs/extensions/contribution-types.md:343-346) only mention the Explore default. Provider authors should also know the declaration:

   • sets the dashboard time-range subject for ad-hoc-axis charts;
   • stops MCP/tabular time_range queries from failing with "multiple datetime dimensions" (tabular_query.py:150-157).

[villebro]

nit: should we call this preferred_temporal_dimension?

[mikebridge]

Good call, renamed to preferred_temporal_dimension in f443f60: the core SemanticView declaration, host code, tests and the extension docs, plus a short UPDATING note.

It's a plain rename with no compatibility alias, since nothing outside this PR uses the field yet. The declaration stays optional, and the temporal-column fallback and saved-chart behavior are unchanged.