(superset) 01/01: fix(semantic-layers): separate grain and offset time-axis resolvers

[jl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sadpandajoe pushed a commit to branch claude/master-ci-failure-fix-225ef4
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 993ddc94ad7f4ff1a0303c017577f1bced333bf4
Author: Joe Li <jo...@preset.io>
AuthorDate: Wed Jul 15 14:41:32 2026 -0700

    fix(semantic-layers): separate grain and offset time-axis resolvers
    
    Two `_get_time_axis_column` functions coexisted in `mapper.py`: an
    offset-oriented resolver (first temporal column, falling back to a
    `TEMPORAL_RANGE` adhoc filter so aggregate-only charts still get
    offset-aware bounds) and a grain-oriented resolver (only claims an axis
    when exactly one temporal column is selected). The second definition
    shadowed the first, so:
    
    - mypy failed pre-commit with `Name "_get_time_axis_column" already
      defined [no-redef]`, turning the branch red.
    - The offset call sites (`_get_time_filter`, `_get_group_limit_filters`)
      silently ran the grain resolver, dropping the time offset / time
      comparison bounds for aggregate-only charts whose temporal column lives
      only in a `TEMPORAL_RANGE` filter. The unit test covering that path was
      already failing but stayed hidden because an unrelated Python 3.10 CI
      leg errored out first and cancelled the sibling jobs.
    
    Rename the grain resolver to `_get_grain_time_axis_column` and point its
    call sites (`map_query_object`, `_validate_granularity`) at it, leaving
    the offset resolver under the original name for the offset call sites.
    Reconcile the two duplicated test blocks so both resolvers are covered.
    
    Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
---
 superset/semantic_layers/mapper.py              |  6 +++---
 tests/unit_tests/semantic_layers/mapper_test.py | 27 +++++++++++++------------
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/superset/semantic_layers/mapper.py b/superset/semantic_layers/mapper.py
index de3e4258ddf..936a0d576dc 100644
--- a/superset/semantic_layers/mapper.py
+++ b/superset/semantic_layers/mapper.py
@@ -337,7 +337,7 @@ def map_query_object(query_object: ValidatedQueryObject) -> list[SemanticQuery]:
     metrics = [all_metrics[metric] for metric in (query_object.metrics or [])]
 
     grain = _convert_time_grain(query_object.extras.get("time_grain_sqla"))
-    time_axis_column = _get_time_axis_column(query_object, all_dimensions)
+    time_axis_column = _get_grain_time_axis_column(query_object, all_dimensions)
     # A semantic view can expose multiple Dimension variants per name (one per
     # supported time grain). Pick exactly one variant per selected column:
     # for the time-axis column we honor the user's grain selection, falling
@@ -1037,7 +1037,7 @@ def _get_group_limit_filters(
     return filters if filters else None
 
 
-def _get_time_axis_column(
+def _get_grain_time_axis_column(
     query_object: ValidatedQueryObject,
     all_dimensions: dict[str, Dimension],
 ) -> str | None:
@@ -1180,7 +1180,7 @@ def _validate_granularity(query_object: ValidatedQueryObject) -> None:
         )
 
     if time_grain := query_object.extras.get("time_grain_sqla"):
-        time_column = _get_time_axis_column(query_object, all_dimensions)
+        time_column = _get_grain_time_axis_column(query_object, all_dimensions)
         if not time_column:
             raise ValueError(
                 "A time column must be specified when a time grain is provided."
diff --git a/tests/unit_tests/semantic_layers/mapper_test.py b/tests/unit_tests/semantic_layers/mapper_test.py
index dd90a7c14ad..3821455aca0 100644
--- a/tests/unit_tests/semantic_layers/mapper_test.py
+++ b/tests/unit_tests/semantic_layers/mapper_test.py
@@ -48,6 +48,7 @@ from superset.semantic_layers.mapper import (
     _convert_time_grain,
     _get_filters_from_extras,
     _get_filters_from_query_object,
+    _get_grain_time_axis_column,
     _get_group_limit_filters,
     _get_group_limit_from_query_object,
     _get_order_from_query_object,
@@ -1280,19 +1281,19 @@ def test_map_query_object_picks_raw_variant_for_non_axis_time_dim(
     assert shipped_dims[0].grain is None
 
 
-def test_get_time_axis_column_returns_granularity_when_set(
+def test_get_grain_time_axis_column_returns_granularity_when_set(
     mocker: MockerFixture,
 ) -> None:
     """Legacy path: ``granularity`` short-circuits scanning ``columns``."""
     qo = mocker.Mock()
     qo.granularity = "order_date"
-    assert _get_time_axis_column(qo, {}) == "order_date"
+    assert _get_grain_time_axis_column(qo, {}) == "order_date"
 
 
-def test_get_time_axis_column_finds_temporal_in_columns(
+def test_get_grain_time_axis_column_finds_temporal_in_columns(
     mocker: MockerFixture,
 ) -> None:
-    """Modern x_axis path: pick the first temporal dim from ``columns``."""
+    """Modern x_axis path: the sole temporal dim in ``columns`` is the axis."""
     all_dims = {
         "category": Dimension(
             id="products.category",
@@ -1310,10 +1311,10 @@ def test_get_time_axis_column_finds_temporal_in_columns(
     qo = mocker.Mock()
     qo.granularity = None
     qo.columns = ["category", "order_date"]
-    assert _get_time_axis_column(qo, all_dims) == "order_date"
+    assert _get_grain_time_axis_column(qo, all_dims) == "order_date"
 
 
-def test_get_time_axis_column_skips_unknown_columns(
+def test_get_grain_time_axis_column_skips_unknown_columns(
     mocker: MockerFixture,
 ) -> None:
     """A column not present in the semantic view is silently skipped."""
@@ -1329,10 +1330,10 @@ def test_get_time_axis_column_skips_unknown_columns(
     qo.granularity = None
     qo.columns = ["does_not_exist", "category"]
     # No temporal dim in columns — returns None.
-    assert _get_time_axis_column(qo, all_dims) is None
+    assert _get_grain_time_axis_column(qo, all_dims) is None
 
 
-def test_get_time_axis_column_skips_unparseable_adhoc_columns(
+def test_get_grain_time_axis_column_skips_unparseable_adhoc_columns(
     mocker: MockerFixture,
 ) -> None:
     """An adhoc column that ``_normalize_column`` rejects is silently skipped."""
@@ -1349,10 +1350,10 @@ def test_get_time_axis_column_skips_unparseable_adhoc_columns(
     # First column is an adhoc dict without ``isColumnReference`` — raises in
     # ``_normalize_column`` and the loop should keep going.
     qo.columns = [{"label": "unsupported", "sqlExpression": "lower(x)"}, "order_date"]
-    assert _get_time_axis_column(qo, all_dims) == "order_date"
+    assert _get_grain_time_axis_column(qo, all_dims) == "order_date"
 
 
-def test_get_time_axis_column_returns_none_on_multiple_temporal_columns(
+def test_get_grain_time_axis_column_returns_none_on_multiple_temporal_columns(
     mocker: MockerFixture,
 ) -> None:
     """
@@ -1379,10 +1380,10 @@ def test_get_time_axis_column_returns_none_on_multiple_temporal_columns(
     qo = mocker.Mock()
     qo.granularity = None
     qo.columns = ["created_at", "shipped_at"]
-    assert _get_time_axis_column(qo, all_dims) is None
+    assert _get_grain_time_axis_column(qo, all_dims) is None
 
 
-def test_get_time_axis_column_returns_none_when_no_temporal_columns(
+def test_get_grain_time_axis_column_returns_none_when_no_temporal_columns(
     mocker: MockerFixture,
 ) -> None:
     """Without ``granularity`` and without a temporal column we return None."""
@@ -1397,7 +1398,7 @@ def test_get_time_axis_column_returns_none_when_no_temporal_columns(
     qo = mocker.Mock()
     qo.granularity = None
     qo.columns = ["category"]
-    assert _get_time_axis_column(qo, all_dims) is None
+    assert _get_grain_time_axis_column(qo, all_dims) is None
 
 
 def test_convert_query_object_filter_unknown_operator(