sadpandajoe commented on code in PR #44148:
URL: https://github.com/apache/superset/pull/44148#discussion_r4178203829
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -3738,6 +3738,178 @@ def validate_gantt_roles(self) -> "GanttChartConfig":
# Discriminated union for runtime validation (not exposed in JSON Schema)
+def _omit_inherited_descriptions(schema: dict[str, Any]) -> None:
+ """Drop field descriptions the published parent schema already carries.
+
+ The geographic references and filters only tighten bounds on the shared
+ ``ColumnRef``/``FilterConfig`` fields, whose descriptions appear in the
+ same tool schema.
+ """
+ for prop in schema.get("properties", {}).values():
+ prop.pop("description", None)
+
+
+class GeographicColumnRef(ColumnRef):
+ """Closed ColumnRef for geographic roles."""
+
+ model_config = ConfigDict(
+ extra="forbid",
+ populate_by_name=True,
+ strict=True,
+ json_schema_extra=_omit_inherited_descriptions,
+ )
+ dtype: str | None = Field(None, max_length=128)
+
+
+GeographicFilterValue = Annotated[str, Field(max_length=1000)] | int | float |
bool
+
+
+class GeographicFilterConfig(FilterConfig):
+ """Closed FilterConfig with bounded values."""
+
+ model_config = ConfigDict(
+ extra="forbid",
+ populate_by_name=True,
+ strict=True,
+ allow_inf_nan=False,
+ json_schema_extra=_omit_inherited_descriptions,
+ )
+ value: (
+ GeographicFilterValue
+ | Annotated[list[GeographicFilterValue], Field(max_length=1000)]
+ | None
+ ) = Field(None, validation_alias=AliasChoices("value", "val"))
+
+
+class GeographicChartConfig(BaseChartConfig):
+ """Bounded controls shared by geographic visualizations."""
+
+ model_config = ConfigDict(extra="forbid", strict=True)
+ filters: list[GeographicFilterConfig] | None = Field(None, max_length=100)
+ row_limit: int = Field(10000, ge=1, le=10000)
+ time_range: str | None = Field(None, max_length=1000)
+
+ @field_validator("time_range")
+ @classmethod
+ def validate_geographic_time_range(cls, value: str | None) -> str | None:
+ """Reject time expressions the query parser would silently ignore."""
+ return validate_time_range(value)
+
+ @model_validator(mode="after")
+ def validate_geographic_roles(self) -> "GeographicChartConfig":
+ """Reject metric dimensions and unaggregated metric roles."""
+ for field in ("entity", "latitude", "longitude", "dimension"):
+ ref = getattr(self, field, None)
+ if ref is not None and (not ref.name or ref.is_metric):
+ raise ValueError(
+ f"{field} requires a named dataset column, not a metric"
+ )
+ for field in ("metric", "secondary_metric", "radius_metric"):
+ ref = getattr(self, field, None)
+ if ref is not None and not ref.is_metric:
+ raise ValueError(
+ f"{field} requires aggregate, saved_metric, or
sql_expression"
+ )
+ # These helpers import chart schemas, so defer to avoid a cycle.
+ from superset.mcp_service.chart.chart_utils import create_metric_object
+ from superset.mcp_service.chart.query_result import metric_result_label
+
+ dimensions = {
+ ref.name
+ for field in ("entity", "latitude", "longitude", "dimension")
+ if (ref := getattr(self, field, None)) is not None
+ }
+ seen_metrics: dict[str | None, ColumnRef] = {}
+ for field in ("metric", "secondary_metric", "radius_metric"):
+ ref = getattr(self, field, None)
+ if ref is None:
+ continue
+ label = metric_result_label(create_metric_object(ref))
+ if self.chart_type == "deck_scatter" and label in {
Review Comment:
A fixed-radius `deck_scatter` with `dimension: {"name": "position"}` still
passes validation, but the native spatial transform copies that category over
the computed coordinate tuple, breaking point rendering and autozoom. Could the
reserved-field protection cover dimension columns as well, with a transform
regression checking that the emitted position remains a coordinate pair?
##########
superset/mcp_service/chart/plugins/geographic.py:
##########
@@ -0,0 +1,650 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Native geographic chart mapping shared by the three public map types."""
+
+from __future__ import annotations
+
+import logging
+import math
+import re
+from collections.abc import Mapping
+from decimal import Decimal
+from functools import lru_cache
+from numbers import Real
+from typing import Any, ClassVar, TypeGuard
+
+from superset.mcp_service.chart.chart_utils import (
+ _add_adhoc_filters,
+ create_metric_object,
+ merge_geographic_update_form_data,
+)
+from superset.mcp_service.chart.plugin import BaseChartPlugin
+from superset.mcp_service.chart.query_result import (
+ metric_result_label,
+ query_result_failure,
+)
+from superset.mcp_service.chart.schemas import (
+ ChartError,
+ ColumnRef,
+ CountryMapChartConfig,
+ DeckScatterChartConfig,
+ VegaLitePreview,
+ WorldMapChartConfig,
+)
+from superset.mcp_service.chart.validation.dataset_validator import
DatasetValidator
+
+logger = logging.getLogger(__name__)
+
+GeographicConfig = CountryMapChartConfig | WorldMapChartConfig |
DeckScatterChartConfig
+ROLE_FIELDS = (
+ "entity",
+ "metric",
+ "secondary_metric",
+ "latitude",
+ "longitude",
+ "dimension",
+ "radius_metric",
+)
+MAX_GEOGRAPHIC_ROWS = 10000
+ASCII_BANNER = "Geographic source data (geometry not reproduced)"
+
+
+def _is_finite_geographic_number(value: object) -> TypeGuard[Real | Decimal]:
+ """Accept database NUMERIC/real scalars that remain finite in JSON.
+
+ Validation precedes JSON conversion; retain the original Decimal values for
+ data/export while rejecting booleans, complex numbers, and numeric strings.
+ """
+ if isinstance(value, bool) or not isinstance(value, (Real, Decimal)):
+ return False
+ if isinstance(value, Decimal) and not value.is_finite():
+ return False
+ try:
+ return math.isfinite(value)
+ except (OverflowError, ValueError):
+ return False
+
+
+def _decode_geographic_coordinates(value: object, spatial_type: str) ->
list[float]:
+ """Decode native encoded coordinates for validation, not result
replacement."""
+ if not isinstance(value, str) or not value:
+ raise ValueError("Spatial coordinates require a nonempty string")
+ if spatial_type == "geohash":
+ import pygeohash
+
+ try:
+ latitude, longitude = pygeohash.decode(value.lower())
+ except (ValueError, KeyError, TypeError) as ex:
+ raise ValueError("Invalid geographic geohash") from ex
+ return [longitude, latitude]
+ # Match the numeric prefix consumed by JavaScript parseFloat in the native
+ # Deck.gl spatial transform, including incomplete exponents and suffixes.
+ coordinates: list[float] = []
+ for part in value.split(","):
+ prefix = re.match(
+ r"[+-]?(?:Infinity|(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]+)"
+ r"(?:[eE][+-]?[0-9]+)?)",
+ part.lstrip(
+ "\t\n\v\f\r \u00a0\u1680\u2000\u2001\u2002\u2003"
+ "\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u2028"
+ "\u2029\u202f\u205f\u3000\ufeff"
+ ),
+ )
+ if prefix is None:
+ raise ValueError("Invalid delimited geographic coordinates")
+ coordinates.append(float(prefix[0]))
+ if len(coordinates) != 2:
+ raise ValueError("Delimited coordinates require two values")
+ return coordinates
+
+
+@lru_cache(maxsize=4)
+def _world_country_entries(field: str) -> tuple[tuple[str, str], ...]:
+ """Reuse immutable country aliases for the four supported world formats."""
+ from superset.examples.countries import countries
+
+ return tuple(
+ (country[field], country["cca3"]) for country in countries if
country[field]
+ )
+
+
+def _typed_row_limit(form_data: Mapping[str, Any]) -> int:
+ """Bound the typed map row limit, matching the frontend's full-map
query."""
+ try:
+ limit = int(form_data.get("row_limit") or MAX_GEOGRAPHIC_ROWS)
+ except (TypeError, ValueError, OverflowError):
+ return MAX_GEOGRAPHIC_ROWS
+ return min(MAX_GEOGRAPHIC_ROWS, max(1, limit))
+
+
+class GeographicChartPlugin(BaseChartPlugin):
+ """Translate explicit geographic roles into native plugin controls.
+
+ Typed MCP charts carry ``mcp_geographic`` in their saved form_data. Only
+ those charts get the full-map row limits and strict result contract;
+ charts built in Explore keep their native behavior.
+ """
+
+ native_viz_types: ClassVar[Mapping[str, str]] = {}
+ requires_compile_check = True
+ requires_config_for_dataset_rebind = True
+ dataset_rebind_roles = "geographic/metric roles"
+ strict_dataset_rebind = True
+ normalize_data_results = True
+ supports_vega_lite_preview = False
+ invalid_result_error_code = "INVALID_GEOGRAPHIC_RESULT"
+ invalid_result_message = "Geographic query returned invalid values"
+ invalid_result_suggestions: ClassVar[tuple[str, ...]] = (
+ "Match country and value format to the source identifiers",
+ "Correct source values or filter other geographies",
+ "Use finite numeric metrics and valid latitude/longitude",
+ )
+
+ # ------------------------------------------------------------------
+ # Result contract
+ # ------------------------------------------------------------------
+
+ def result_metrics(self, form_data: Mapping[str, Any]) -> list[Any]:
+ """Return the metrics whose values every result row must carry."""
+ raise NotImplementedError
+
+ def size_metric_labels(
+ self, form_data: Mapping[str, Any], labels: list[str]
+ ) -> set[str]:
+ """Return the metric labels that size marks and must be nonnegative."""
+ return set()
+
+ def row_identifier(
+ self, row: Mapping[str, Any], form_data: Mapping[str, Any]
+ ) -> str | None:
+ """Resolve the row's geography, or validate it and return None."""
+ raise NotImplementedError
+
+ def _metric_labels(self, form_data: Mapping[str, Any]) -> list[str]:
+ labels = [metric_result_label(m) for m in
self.result_metrics(form_data)]
+ if any(label is None for label in labels):
+ raise ValueError("Geographic metric has no resolvable result
label")
+ return [label for label in labels if label is not None]
+
+ def _validate_rows(self, result: Any, form_data: Mapping[str, Any]) ->
None:
+ if (
+ not isinstance(result, Mapping)
+ or not isinstance(result.get("queries"), list)
+ or len(result["queries"]) != 1
+ ):
+ raise ValueError("Expected exactly one geographic query result")
+ query = result["queries"][0]
+ if not isinstance(query, Mapping) or not isinstance(query.get("data"),
list):
+ raise ValueError("Expected geographic query data to be a list of
records")
+ labels = self._metric_labels(form_data)
+ size_labels = self.size_metric_labels(form_data, labels)
+ seen: set[str] = set()
+ for row in query["data"]:
+ if not isinstance(row, Mapping):
+ raise ValueError("Expected geographic rows to be records")
+ for label in labels:
+ value = row.get(label)
+ if not _is_finite_geographic_number(value):
+ raise ValueError(
+ f"Geographic metric {label!r} must be a finite number"
+ )
+ if value < 0 and label in size_labels:
+ raise ValueError("Geographic size metrics must be
nonnegative")
+ if identifier := self.row_identifier(row, form_data):
+ if identifier in seen:
+ raise ValueError(
+ f"Multiple result rows resolve to {identifier}; "
+ "normalize source values before aggregation"
+ )
+ seen.add(identifier)
+
+ def normalize_query_result(self, result: Any, form_data: Mapping[str,
Any]) -> Any:
+ """Reject unresolved regions and malformed or nonfinite results.
+
+ Source values are preserved for exports and filtering; the native
+ transform owns display-only ISO mapping using the same bundled
+ boundary identifiers. Charts without the typed MCP marker keep their
+ native behavior.
+ """
+ if failure := query_result_failure(result):
+ return failure
+ if not form_data.get("mcp_geographic"):
+ return result
+ try:
+ self._validate_rows(result, form_data)
+ except (ValueError, TypeError, KeyError) as exc:
+ return ChartError(error=str(exc),
error_type="InvalidGeographicResult")
+ return result
+
+ # ------------------------------------------------------------------
+ # Query limits and previews
+ # ------------------------------------------------------------------
+
+ def compile_row_limit(self, form_data: Mapping[str, Any]) -> int:
+ """Validate the full bounded map, not only its first rows."""
+ if form_data.get("mcp_geographic"):
+ return _typed_row_limit(form_data)
+ return super().compile_row_limit(form_data)
+
+ def preview_row_limit(self, form_data: Mapping[str, Any], fallback: int)
-> int:
+ """Preview the same bounded rows the native map renders."""
+ if form_data.get("mcp_geographic"):
+ return _typed_row_limit(form_data)
+ return fallback
+
+ def ascii_preview(
+ self, data: list[Any], form_data: dict[str, Any], width: int
+ ) -> str | ChartError | None:
+ """Render the source rows; map geometry has no ASCII form."""
+ from superset.mcp_service.chart.ascii_charts import
generate_ascii_table
+
+ try:
+ return f"{ASCII_BANNER}\n" + generate_ascii_table(data, max(width,
21))
+ except (TypeError, ValueError, KeyError, IndexError) as exc:
+ logger.error("ASCII chart generation failed: %s", exc,
exc_info=True)
+ return "ASCII chart generation failed"
+
+ def vega_lite_preview(
+ self, data: list[Any], form_data: dict[str, Any]
+ ) -> VegaLitePreview | ChartError | None:
+ """Never fabricate a non-geographic Vega-Lite chart for a map."""
+ return ChartError(
+ error=(
+ "Geographic Vega previews are not supported. Use table/ascii
for "
+ "source data, or open Explore for native geography."
+ ),
+ error_type="UnsupportedGeographicPreview",
+ )
+
+ # ------------------------------------------------------------------
+ # Updates
+ # ------------------------------------------------------------------
+
+ def merge_update_form_data(
+ self,
+ existing_form_data: dict[str, Any],
+ new_form_data: dict[str, Any],
+ config: Any,
+ *,
+ dataset_rebind: bool,
+ ) -> dict[str, Any] | None:
+ """Preserve omitted native controls; a rebind keeps presentation
only."""
+ merged = merge_geographic_update_form_data(
+ existing_form_data, new_form_data, config,
dataset_rebind=dataset_rebind
+ )
+ # Native query and compile validation must cover the same bounded rows.
+ if merged.get("mcp_geographic"):
+ merged["row_limit"] = _typed_row_limit(merged)
+ return merged
+
+ def extract_column_refs(self, config: GeographicConfig) -> list[ColumnRef]:
+ """Include all spatial, metric, and filter references."""
+ refs = [
+ ref
+ for field in ROLE_FIELDS
+ if (ref := getattr(config, field, None)) is not None
+ ]
+ refs.extend(ColumnRef(name=f.column) for f in config.filters or [])
+ return refs
+
+ def normalize_column_refs(
+ self, config: GeographicConfig, dataset_context: Any
+ ) -> GeographicConfig:
+ """Canonicalize dataset identifiers without losing explicit field
sets."""
+ patch = config.model_dump(exclude_unset=True)
+ for field in ROLE_FIELDS:
+ ref = patch.get(field)
+ if ref and ref.get("name"):
+ canonical = (
+ DatasetValidator.get_canonical_metric_name
+ if ref.get("saved_metric")
+ else DatasetValidator.get_canonical_column_name
+ )
+ ref["name"] = canonical(ref["name"], dataset_context)
+ DatasetValidator.normalize_filters(patch, dataset_context)
+ return type(config).model_validate(patch)
+
+ def resolve_viz_type(self, config: GeographicConfig) -> str:
+ """Public names match frontend registration keys."""
+ return config.chart_type
+
+ def generate_name(
+ self, config: GeographicConfig, dataset_name: str | None = None
+ ) -> str:
+ """Name geographic charts without guessing the source geography."""
+ return self._with_context(self.display_name, dataset_name)
+
+ def to_form_data(
+ self, config: GeographicConfig, dataset_id: int | str | None = None
+ ) -> dict[str, Any]:
+ """Mirror native country/world/deck Scatter controls and defaults."""
+ result: dict[str, Any] = {
+ "viz_type": config.chart_type,
+ "row_limit": config.row_limit,
+ "mcp_geographic": True,
+ }
+ if config.time_range is not None:
+ result["time_range"] = config.time_range
+ _add_adhoc_filters(result, config.filters)
+ if isinstance(config, CountryMapChartConfig):
+ result.update(
+ entity=config.entity.name,
+ metric=create_metric_object(config.metric),
+ select_country=config.country,
+ region_format=config.region_format,
+ linear_color_scheme=config.linear_color_scheme,
+ number_format=config.number_format,
+ )
+ elif isinstance(config, WorldMapChartConfig):
+ result.update(
+ entity=config.entity.name,
+ metric=create_metric_object(config.metric),
+ country_fieldtype=config.country_format,
+ secondary_metric=create_metric_object(config.secondary_metric)
+ if config.secondary_metric
+ else None,
+ show_bubbles=config.show_bubbles,
+ max_bubble_size=config.max_bubble_size,
+ sort_by_metric=config.sort_by_metric,
+ linear_color_scheme=config.linear_color_scheme,
+ color_by="metric",
+ color_picker={"r": 0, "g": 122, "b": 135, "a": 1},
+ color_scheme="supersetColors",
+ y_axis_format="SMART_NUMBER",
+ )
+ else:
+ radius: dict[str, Any] = {"type": "fix", "value": config.radius}
+ if config.radius_metric:
+ radius = {
+ "type": "metric",
+ "value": create_metric_object(config.radius_metric),
+ }
+ result.update(
+ spatial={
+ "type": "latlong",
+ "latCol": config.latitude.name,
+ "lonCol": config.longitude.name,
+ },
+ dimension=config.dimension.name if config.dimension else None,
+ point_radius_fixed=radius,
+ point_unit=config.point_unit,
+ multiplier=1,
+ min_radius=2,
+ max_radius=250,
+ color_picker={"r": 0, "g": 122, "b": 135, "a": 1},
+ color_scheme="supersetColors",
+ map_renderer="maplibre",
+
maplibre_style="https://basemaps.cartocdn.com/gl/positron-gl-style/style.json";,
+ viewport={
+ "longitude": 0,
+ "latitude": 0,
+ "zoom": 1,
+ "bearing": 0,
+ "pitch": 0,
+ },
+ autozoom=True,
+ filter_nulls=True,
+ )
+ return result
+
+
+class CountryMapChartPlugin(GeographicChartPlugin):
+ """Register the Country Map visualization."""
+
+ chart_type = "country_map"
+ display_name = "Country Map"
+ native_viz_types: ClassVar[Mapping[str, str]] = {"country_map": "Country
Map"}
+
+ def resolve_query_fields(
+ self, form_data: Mapping[str, Any], viz_type: str
+ ) -> tuple[list[Any], list[Any]] | None:
+ """Query the region entity and its single metric."""
+ metric = form_data.get("metric")
+ entity = form_data.get("entity")
+ return [metric] if metric else [], [entity] if entity else []
+
+ def result_metrics(self, form_data: Mapping[str, Any]) -> list[Any]:
+ return [form_data.get("metric")]
+
+ def row_identifier(
+ self, row: Mapping[str, Any], form_data: Mapping[str, Any]
+ ) -> str | None:
+ """Resolve the row to one bundled region boundary."""
+ from superset.utils.geographic import resolve_region
+
+ entity = form_data.get("entity")
+ if not isinstance(entity, str):
Review Comment:
Changing a generated map’s Entity to Custom SQL in Explore (for example,
`UPPER(state)` labeled `region`) saves an adhoc-column object that renders
normally, but this string-only check makes subsequent MCP data reads and
JSON/CSV/Excel exports fail with `InvalidGeographicResult`. Could both country
and world maps resolve the entity’s result label like the native transforms do?
##########
superset/mcp_service/chart/plugins/geographic.py:
##########
@@ -0,0 +1,650 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Native geographic chart mapping shared by the three public map types."""
+
+from __future__ import annotations
+
+import logging
+import math
+import re
+from collections.abc import Mapping
+from decimal import Decimal
+from functools import lru_cache
+from numbers import Real
+from typing import Any, ClassVar, TypeGuard
+
+from superset.mcp_service.chart.chart_utils import (
+ _add_adhoc_filters,
+ create_metric_object,
+ merge_geographic_update_form_data,
+)
+from superset.mcp_service.chart.plugin import BaseChartPlugin
+from superset.mcp_service.chart.query_result import (
+ metric_result_label,
+ query_result_failure,
+)
+from superset.mcp_service.chart.schemas import (
+ ChartError,
+ ColumnRef,
+ CountryMapChartConfig,
+ DeckScatterChartConfig,
+ VegaLitePreview,
+ WorldMapChartConfig,
+)
+from superset.mcp_service.chart.validation.dataset_validator import
DatasetValidator
+
+logger = logging.getLogger(__name__)
+
+GeographicConfig = CountryMapChartConfig | WorldMapChartConfig |
DeckScatterChartConfig
+ROLE_FIELDS = (
+ "entity",
+ "metric",
+ "secondary_metric",
+ "latitude",
+ "longitude",
+ "dimension",
+ "radius_metric",
+)
+MAX_GEOGRAPHIC_ROWS = 10000
+ASCII_BANNER = "Geographic source data (geometry not reproduced)"
+
+
+def _is_finite_geographic_number(value: object) -> TypeGuard[Real | Decimal]:
+ """Accept database NUMERIC/real scalars that remain finite in JSON.
+
+ Validation precedes JSON conversion; retain the original Decimal values for
+ data/export while rejecting booleans, complex numbers, and numeric strings.
+ """
+ if isinstance(value, bool) or not isinstance(value, (Real, Decimal)):
+ return False
+ if isinstance(value, Decimal) and not value.is_finite():
+ return False
+ try:
+ return math.isfinite(value)
+ except (OverflowError, ValueError):
+ return False
+
+
+def _decode_geographic_coordinates(value: object, spatial_type: str) ->
list[float]:
+ """Decode native encoded coordinates for validation, not result
replacement."""
+ if not isinstance(value, str) or not value:
+ raise ValueError("Spatial coordinates require a nonempty string")
+ if spatial_type == "geohash":
+ import pygeohash
+
+ try:
+ latitude, longitude = pygeohash.decode(value.lower())
+ except (ValueError, KeyError, TypeError) as ex:
+ raise ValueError("Invalid geographic geohash") from ex
+ return [longitude, latitude]
+ # Match the numeric prefix consumed by JavaScript parseFloat in the native
+ # Deck.gl spatial transform, including incomplete exponents and suffixes.
+ coordinates: list[float] = []
+ for part in value.split(","):
+ prefix = re.match(
+ r"[+-]?(?:Infinity|(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]+)"
+ r"(?:[eE][+-]?[0-9]+)?)",
+ part.lstrip(
+ "\t\n\v\f\r \u00a0\u1680\u2000\u2001\u2002\u2003"
+ "\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u2028"
+ "\u2029\u202f\u205f\u3000\ufeff"
+ ),
+ )
+ if prefix is None:
+ raise ValueError("Invalid delimited geographic coordinates")
+ coordinates.append(float(prefix[0]))
+ if len(coordinates) != 2:
+ raise ValueError("Delimited coordinates require two values")
+ return coordinates
+
+
+@lru_cache(maxsize=4)
+def _world_country_entries(field: str) -> tuple[tuple[str, str], ...]:
+ """Reuse immutable country aliases for the four supported world formats."""
+ from superset.examples.countries import countries
+
+ return tuple(
+ (country[field], country["cca3"]) for country in countries if
country[field]
+ )
Review Comment:
A `world_map` result containing `SG` is accepted here, but the imported
Datamaps world topology has no `SGP` boundary, so the default no-bubble chart
silently draws no mark for that row. Could choropleth validation check actual
geometry coverage rather than the broader country dictionary, with a regression
using the real topology instead of a synthetic click target?
##########
superset/mcp_service/chart/plugins/geographic.py:
##########
@@ -0,0 +1,650 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Native geographic chart mapping shared by the three public map types."""
+
+from __future__ import annotations
+
+import logging
+import math
+import re
+from collections.abc import Mapping
+from decimal import Decimal
+from functools import lru_cache
+from numbers import Real
+from typing import Any, ClassVar, TypeGuard
+
+from superset.mcp_service.chart.chart_utils import (
+ _add_adhoc_filters,
+ create_metric_object,
+ merge_geographic_update_form_data,
+)
+from superset.mcp_service.chart.plugin import BaseChartPlugin
+from superset.mcp_service.chart.query_result import (
+ metric_result_label,
+ query_result_failure,
+)
+from superset.mcp_service.chart.schemas import (
+ ChartError,
+ ColumnRef,
+ CountryMapChartConfig,
+ DeckScatterChartConfig,
+ VegaLitePreview,
+ WorldMapChartConfig,
+)
+from superset.mcp_service.chart.validation.dataset_validator import
DatasetValidator
+
+logger = logging.getLogger(__name__)
+
+GeographicConfig = CountryMapChartConfig | WorldMapChartConfig |
DeckScatterChartConfig
+ROLE_FIELDS = (
+ "entity",
+ "metric",
+ "secondary_metric",
+ "latitude",
+ "longitude",
+ "dimension",
+ "radius_metric",
+)
+MAX_GEOGRAPHIC_ROWS = 10000
+ASCII_BANNER = "Geographic source data (geometry not reproduced)"
+
+
+def _is_finite_geographic_number(value: object) -> TypeGuard[Real | Decimal]:
+ """Accept database NUMERIC/real scalars that remain finite in JSON.
+
+ Validation precedes JSON conversion; retain the original Decimal values for
+ data/export while rejecting booleans, complex numbers, and numeric strings.
+ """
+ if isinstance(value, bool) or not isinstance(value, (Real, Decimal)):
+ return False
+ if isinstance(value, Decimal) and not value.is_finite():
+ return False
+ try:
+ return math.isfinite(value)
+ except (OverflowError, ValueError):
+ return False
+
+
+def _decode_geographic_coordinates(value: object, spatial_type: str) ->
list[float]:
+ """Decode native encoded coordinates for validation, not result
replacement."""
+ if not isinstance(value, str) or not value:
+ raise ValueError("Spatial coordinates require a nonempty string")
+ if spatial_type == "geohash":
+ import pygeohash
+
+ try:
+ latitude, longitude = pygeohash.decode(value.lower())
+ except (ValueError, KeyError, TypeError) as ex:
+ raise ValueError("Invalid geographic geohash") from ex
+ return [longitude, latitude]
+ # Match the numeric prefix consumed by JavaScript parseFloat in the native
+ # Deck.gl spatial transform, including incomplete exponents and suffixes.
+ coordinates: list[float] = []
+ for part in value.split(","):
+ prefix = re.match(
+ r"[+-]?(?:Infinity|(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]+)"
+ r"(?:[eE][+-]?[0-9]+)?)",
+ part.lstrip(
+ "\t\n\v\f\r \u00a0\u1680\u2000\u2001\u2002\u2003"
+ "\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u2028"
+ "\u2029\u202f\u205f\u3000\ufeff"
+ ),
+ )
+ if prefix is None:
+ raise ValueError("Invalid delimited geographic coordinates")
+ coordinates.append(float(prefix[0]))
+ if len(coordinates) != 2:
+ raise ValueError("Delimited coordinates require two values")
+ return coordinates
+
+
+@lru_cache(maxsize=4)
+def _world_country_entries(field: str) -> tuple[tuple[str, str], ...]:
+ """Reuse immutable country aliases for the four supported world formats."""
+ from superset.examples.countries import countries
+
+ return tuple(
+ (country[field], country["cca3"]) for country in countries if
country[field]
+ )
+
+
+def _typed_row_limit(form_data: Mapping[str, Any]) -> int:
+ """Bound the typed map row limit, matching the frontend's full-map
query."""
+ try:
+ limit = int(form_data.get("row_limit") or MAX_GEOGRAPHIC_ROWS)
+ except (TypeError, ValueError, OverflowError):
+ return MAX_GEOGRAPHIC_ROWS
+ return min(MAX_GEOGRAPHIC_ROWS, max(1, limit))
+
+
+class GeographicChartPlugin(BaseChartPlugin):
+ """Translate explicit geographic roles into native plugin controls.
+
+ Typed MCP charts carry ``mcp_geographic`` in their saved form_data. Only
+ those charts get the full-map row limits and strict result contract;
+ charts built in Explore keep their native behavior.
+ """
+
+ native_viz_types: ClassVar[Mapping[str, str]] = {}
+ requires_compile_check = True
+ requires_config_for_dataset_rebind = True
+ dataset_rebind_roles = "geographic/metric roles"
+ strict_dataset_rebind = True
+ normalize_data_results = True
+ supports_vega_lite_preview = False
+ invalid_result_error_code = "INVALID_GEOGRAPHIC_RESULT"
+ invalid_result_message = "Geographic query returned invalid values"
+ invalid_result_suggestions: ClassVar[tuple[str, ...]] = (
+ "Match country and value format to the source identifiers",
+ "Correct source values or filter other geographies",
+ "Use finite numeric metrics and valid latitude/longitude",
+ )
+
+ # ------------------------------------------------------------------
+ # Result contract
+ # ------------------------------------------------------------------
+
+ def result_metrics(self, form_data: Mapping[str, Any]) -> list[Any]:
+ """Return the metrics whose values every result row must carry."""
+ raise NotImplementedError
+
+ def size_metric_labels(
+ self, form_data: Mapping[str, Any], labels: list[str]
+ ) -> set[str]:
+ """Return the metric labels that size marks and must be nonnegative."""
+ return set()
+
+ def row_identifier(
+ self, row: Mapping[str, Any], form_data: Mapping[str, Any]
+ ) -> str | None:
+ """Resolve the row's geography, or validate it and return None."""
+ raise NotImplementedError
+
+ def _metric_labels(self, form_data: Mapping[str, Any]) -> list[str]:
+ labels = [metric_result_label(m) for m in
self.result_metrics(form_data)]
+ if any(label is None for label in labels):
+ raise ValueError("Geographic metric has no resolvable result
label")
+ return [label for label in labels if label is not None]
+
+ def _validate_rows(self, result: Any, form_data: Mapping[str, Any]) ->
None:
+ if (
+ not isinstance(result, Mapping)
+ or not isinstance(result.get("queries"), list)
+ or len(result["queries"]) != 1
+ ):
+ raise ValueError("Expected exactly one geographic query result")
+ query = result["queries"][0]
+ if not isinstance(query, Mapping) or not isinstance(query.get("data"),
list):
+ raise ValueError("Expected geographic query data to be a list of
records")
+ labels = self._metric_labels(form_data)
+ size_labels = self.size_metric_labels(form_data, labels)
+ seen: set[str] = set()
+ for row in query["data"]:
+ if not isinstance(row, Mapping):
+ raise ValueError("Expected geographic rows to be records")
+ for label in labels:
+ value = row.get(label)
+ if not _is_finite_geographic_number(value):
+ raise ValueError(
+ f"Geographic metric {label!r} must be a finite number"
+ )
+ if value < 0 and label in size_labels:
+ raise ValueError("Geographic size metrics must be
nonnegative")
+ if identifier := self.row_identifier(row, form_data):
+ if identifier in seen:
+ raise ValueError(
+ f"Multiple result rows resolve to {identifier}; "
+ "normalize source values before aggregation"
+ )
+ seen.add(identifier)
+
+ def normalize_query_result(self, result: Any, form_data: Mapping[str,
Any]) -> Any:
+ """Reject unresolved regions and malformed or nonfinite results.
+
+ Source values are preserved for exports and filtering; the native
+ transform owns display-only ISO mapping using the same bundled
+ boundary identifiers. Charts without the typed MCP marker keep their
+ native behavior.
+ """
+ if failure := query_result_failure(result):
+ return failure
+ if not form_data.get("mcp_geographic"):
+ return result
+ try:
+ self._validate_rows(result, form_data)
+ except (ValueError, TypeError, KeyError) as exc:
+ return ChartError(error=str(exc),
error_type="InvalidGeographicResult")
+ return result
+
+ # ------------------------------------------------------------------
+ # Query limits and previews
+ # ------------------------------------------------------------------
+
+ def compile_row_limit(self, form_data: Mapping[str, Any]) -> int:
+ """Validate the full bounded map, not only its first rows."""
+ if form_data.get("mcp_geographic"):
+ return _typed_row_limit(form_data)
+ return super().compile_row_limit(form_data)
+
+ def preview_row_limit(self, form_data: Mapping[str, Any], fallback: int)
-> int:
+ """Preview the same bounded rows the native map renders."""
+ if form_data.get("mcp_geographic"):
+ return _typed_row_limit(form_data)
+ return fallback
+
+ def ascii_preview(
+ self, data: list[Any], form_data: dict[str, Any], width: int
+ ) -> str | ChartError | None:
+ """Render the source rows; map geometry has no ASCII form."""
+ from superset.mcp_service.chart.ascii_charts import
generate_ascii_table
+
+ try:
+ return f"{ASCII_BANNER}\n" + generate_ascii_table(data, max(width,
21))
+ except (TypeError, ValueError, KeyError, IndexError) as exc:
+ logger.error("ASCII chart generation failed: %s", exc,
exc_info=True)
+ return "ASCII chart generation failed"
+
+ def vega_lite_preview(
+ self, data: list[Any], form_data: dict[str, Any]
+ ) -> VegaLitePreview | ChartError | None:
+ """Never fabricate a non-geographic Vega-Lite chart for a map."""
+ return ChartError(
+ error=(
+ "Geographic Vega previews are not supported. Use table/ascii
for "
+ "source data, or open Explore for native geography."
+ ),
+ error_type="UnsupportedGeographicPreview",
+ )
+
+ # ------------------------------------------------------------------
+ # Updates
+ # ------------------------------------------------------------------
+
+ def merge_update_form_data(
+ self,
+ existing_form_data: dict[str, Any],
+ new_form_data: dict[str, Any],
+ config: Any,
+ *,
+ dataset_rebind: bool,
+ ) -> dict[str, Any] | None:
+ """Preserve omitted native controls; a rebind keeps presentation
only."""
+ merged = merge_geographic_update_form_data(
+ existing_form_data, new_form_data, config,
dataset_rebind=dataset_rebind
+ )
+ # Native query and compile validation must cover the same bounded rows.
+ if merged.get("mcp_geographic"):
+ merged["row_limit"] = _typed_row_limit(merged)
+ return merged
+
+ def extract_column_refs(self, config: GeographicConfig) -> list[ColumnRef]:
+ """Include all spatial, metric, and filter references."""
+ refs = [
+ ref
+ for field in ROLE_FIELDS
+ if (ref := getattr(config, field, None)) is not None
+ ]
+ refs.extend(ColumnRef(name=f.column) for f in config.filters or [])
+ return refs
+
+ def normalize_column_refs(
+ self, config: GeographicConfig, dataset_context: Any
+ ) -> GeographicConfig:
+ """Canonicalize dataset identifiers without losing explicit field
sets."""
+ patch = config.model_dump(exclude_unset=True)
+ for field in ROLE_FIELDS:
+ ref = patch.get(field)
+ if ref and ref.get("name"):
+ canonical = (
+ DatasetValidator.get_canonical_metric_name
+ if ref.get("saved_metric")
+ else DatasetValidator.get_canonical_column_name
+ )
+ ref["name"] = canonical(ref["name"], dataset_context)
+ DatasetValidator.normalize_filters(patch, dataset_context)
+ return type(config).model_validate(patch)
+
+ def resolve_viz_type(self, config: GeographicConfig) -> str:
+ """Public names match frontend registration keys."""
+ return config.chart_type
+
+ def generate_name(
+ self, config: GeographicConfig, dataset_name: str | None = None
+ ) -> str:
+ """Name geographic charts without guessing the source geography."""
+ return self._with_context(self.display_name, dataset_name)
+
+ def to_form_data(
+ self, config: GeographicConfig, dataset_id: int | str | None = None
+ ) -> dict[str, Any]:
+ """Mirror native country/world/deck Scatter controls and defaults."""
+ result: dict[str, Any] = {
+ "viz_type": config.chart_type,
+ "row_limit": config.row_limit,
+ "mcp_geographic": True,
+ }
+ if config.time_range is not None:
+ result["time_range"] = config.time_range
+ _add_adhoc_filters(result, config.filters)
+ if isinstance(config, CountryMapChartConfig):
+ result.update(
+ entity=config.entity.name,
+ metric=create_metric_object(config.metric),
+ select_country=config.country,
+ region_format=config.region_format,
+ linear_color_scheme=config.linear_color_scheme,
+ number_format=config.number_format,
+ )
+ elif isinstance(config, WorldMapChartConfig):
+ result.update(
+ entity=config.entity.name,
+ metric=create_metric_object(config.metric),
+ country_fieldtype=config.country_format,
+ secondary_metric=create_metric_object(config.secondary_metric)
+ if config.secondary_metric
+ else None,
+ show_bubbles=config.show_bubbles,
+ max_bubble_size=config.max_bubble_size,
+ sort_by_metric=config.sort_by_metric,
+ linear_color_scheme=config.linear_color_scheme,
+ color_by="metric",
+ color_picker={"r": 0, "g": 122, "b": 135, "a": 1},
+ color_scheme="supersetColors",
+ y_axis_format="SMART_NUMBER",
+ )
+ else:
+ radius: dict[str, Any] = {"type": "fix", "value": config.radius}
+ if config.radius_metric:
+ radius = {
+ "type": "metric",
+ "value": create_metric_object(config.radius_metric),
+ }
+ result.update(
+ spatial={
+ "type": "latlong",
+ "latCol": config.latitude.name,
+ "lonCol": config.longitude.name,
+ },
+ dimension=config.dimension.name if config.dimension else None,
+ point_radius_fixed=radius,
+ point_unit=config.point_unit,
+ multiplier=1,
+ min_radius=2,
+ max_radius=250,
+ color_picker={"r": 0, "g": 122, "b": 135, "a": 1},
+ color_scheme="supersetColors",
+ map_renderer="maplibre",
+
maplibre_style="https://basemaps.cartocdn.com/gl/positron-gl-style/style.json";,
+ viewport={
+ "longitude": 0,
+ "latitude": 0,
+ "zoom": 1,
+ "bearing": 0,
+ "pitch": 0,
+ },
+ autozoom=True,
+ filter_nulls=True,
+ )
+ return result
+
+
+class CountryMapChartPlugin(GeographicChartPlugin):
+ """Register the Country Map visualization."""
+
+ chart_type = "country_map"
+ display_name = "Country Map"
+ native_viz_types: ClassVar[Mapping[str, str]] = {"country_map": "Country
Map"}
+
+ def resolve_query_fields(
+ self, form_data: Mapping[str, Any], viz_type: str
+ ) -> tuple[list[Any], list[Any]] | None:
+ """Query the region entity and its single metric."""
+ metric = form_data.get("metric")
+ entity = form_data.get("entity")
+ return [metric] if metric else [], [entity] if entity else []
+
+ def result_metrics(self, form_data: Mapping[str, Any]) -> list[Any]:
+ return [form_data.get("metric")]
+
+ def row_identifier(
+ self, row: Mapping[str, Any], form_data: Mapping[str, Any]
+ ) -> str | None:
+ """Resolve the row to one bundled region boundary."""
+ from superset.utils.geographic import resolve_region
+
+ entity = form_data.get("entity")
+ if not isinstance(entity, str):
+ raise ValueError("Geographic maps require an entity column")
+ # Explore's legacy format uses full boundary ISO codes without
normalization.
+ return resolve_region(
+ row.get(entity),
+ form_data.get("select_country", ""),
+ form_data.get("region_format") or "iso_3166_2",
+ )
+
+
+def _bubbles_shown(form_data: Mapping[str, Any]) -> bool:
+ """Whether the world map renders its secondary metric as bubble sizes."""
+ return bool(form_data.get("show_bubbles"))
+
+
+class WorldMapChartPlugin(GeographicChartPlugin):
+ """Register the World Map visualization."""
+
+ chart_type = "world_map"
+ display_name = "World Map"
+ native_viz_types: ClassVar[Mapping[str, str]] = {"world_map": "World Map"}
+
+ def resolve_query_fields(
+ self, form_data: Mapping[str, Any], viz_type: str
+ ) -> tuple[list[Any], list[Any]] | None:
+ """Query the country entity, its metric and the bubble-size metric.
+
+ The secondary metric is queried unless its result label is shared
+ with the primary metric.
+ """
+ metrics: list[Any] = []
+ labels: set[str | None] = set()
+ for field in ("metric", "secondary_metric"):
+ if metric := form_data.get(field):
+ label = metric_result_label(metric)
+ if label not in labels:
+ metrics.append(metric)
+ labels.add(label)
+ entity = form_data.get("entity")
+ return metrics, [entity] if entity else []
+
+ def validate_merged_form_data(
+ self,
+ form_data: Mapping[str, Any],
+ dataset_id: int | str | None,
+ dataset_context: Any = None,
+ ) -> Any | None:
+ """Reject merged color and bubble metrics that share a label.
+
+ An update can keep a saved ``secondary_metric`` while replacing
+ ``metric`` with a different expression under the same label. Query
+ deduplication would then size bubbles from the color metric.
+ """
+ metric = form_data.get("metric")
+ secondary = form_data.get("secondary_metric")
+ if (
+ _bubbles_shown(form_data)
+ and metric is not None
+ and secondary is not None
+ and metric != secondary
+ and metric_result_label(metric) == metric_result_label(secondary)
Review Comment:
A row-limit update to a saved world map using the same SUM metric for color
and bubbles can fail here because the remapped primary metric and preserved
secondary metric differ only in `optionName` or column metadata. Could this
compare expression semantics rather than whole dictionaries so unchanged native
metrics remain updateable?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
