aminghadersohi commented on code in PR #38403:
URL: https://github.com/apache/superset/pull/38403#discussion_r2931877176
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -690,6 +690,105 @@ class MixedTimeseriesChartConfig(BaseModel):
filters: List[FilterConfig] | None = Field(None, description="Filters to
apply")
+class BigNumberChartConfig(BaseModel):
+ model_config = ConfigDict(extra="forbid")
+
+ chart_type: Literal["big_number"] = Field(
+ ...,
+ description=(
+ "Chart type discriminator - MUST be 'big_number'. "
+ "Creates Big Number charts that display a single prominent "
+ "metric value. Set show_trendline=True with a temporal_column "
+ "for a number with trendline, or leave show_trendline=False "
+ "for a standalone number."
+ ),
+ )
+ metric: ColumnRef = Field(
+ ...,
+ description=(
+ "The metric to display as a big number. "
+ "Must include an aggregate function (e.g., SUM, COUNT)."
+ ),
+ )
+ temporal_column: str | None = Field(
+ None,
+ description=(
+ "Temporal column for the trendline x-axis. "
+ "Required when show_trendline is True."
+ ),
+ min_length=1,
+ max_length=255,
+ pattern=r"^[a-zA-Z0-9_][a-zA-Z0-9_\s\-\.]*$",
+ )
+ time_grain: TimeGrain | None = Field(
+ None,
+ description=(
+ "Time granularity for trendline data. "
+ "Common values: PT1H (hour), P1D (day), P1W (week), "
+ "P1M (month), P1Y (year)."
+ ),
+ )
+ show_trendline: bool = Field(
+ False,
+ description=(
+ "Show a trendline below the big number. "
+ "Requires 'temporal_column' to be set."
+ ),
+ )
+ subheader: str | None = Field(
+ None,
+ description="Subtitle text displayed below the big number",
+ max_length=500,
+ )
+ y_axis_format: str | None = Field(
+ None,
+ description=(
+ "Number format string for the metric value "
+ "(e.g., '$,.2f' for currency, ',.0f' for integers, "
+ "'.2%' for percentages)"
+ ),
+ max_length=50,
+ )
+ start_y_axis_at_zero: bool = Field(
+ True,
+ description="Anchor trendline y-axis at zero",
+ )
+ compare_lag: int | None = Field(
+ None,
+ description=(
+ "Number of time periods to compare against. "
+ "Displays a percentage change vs the prior period."
+ ),
+ ge=1,
+ )
+ filters: list[FilterConfig] | None = Field(
+ None,
+ description="Filters to apply",
+ )
+
+ @model_validator(mode="after")
+ def validate_trendline_fields(self) -> "BigNumberChartConfig":
+ """Validate trendline requires temporal column."""
+ if self.show_trendline and not self.temporal_column:
+ raise ValueError(
+ "Big Number chart with show_trendline=True requires "
+ "'temporal_column'. Specify a date/time column for "
+ "the trendline x-axis."
+ )
+ return self
Review Comment:
Valid. Added validation that `compare_lag` requires `show_trendline=True`.
The schema now raises a `ValueError` if `compare_lag` is set without trendline
mode. Fixed in the latest commit with a corresponding test.
##########
superset/mcp_service/chart/validation/schema_validator.py:
##########
@@ -282,6 +286,60 @@ def _pre_validate_pie_config(
return True, None
+ @staticmethod
+ def _pre_validate_big_number_config(
+ config: Dict[str, Any],
+ ) -> Tuple[bool, ChartGenerationError | None]:
+ """Pre-validate big number chart configuration."""
+ if "metric" not in config:
+ return False, ChartGenerationError(
+ error_type="missing_metric",
+ message="Big Number chart missing required field: metric",
+ details="Big Number charts require a 'metric' field "
+ "specifying the value to display",
+ suggestions=[
+ "Add 'metric' with name and aggregate: "
+ "{'name': 'revenue', 'aggregate': 'SUM'}",
+ "The aggregate function is required (SUM, COUNT, AVG, MIN,
MAX)",
+ "Example: {'chart_type': 'big_number', "
+ "'metric': {'name': 'sales', 'aggregate': 'SUM'}}",
+ ],
+ error_code="MISSING_BIG_NUMBER_METRIC",
+ )
+
+ metric = config.get("metric", {})
+ if isinstance(metric, dict) and not metric.get("aggregate"):
+ return False, ChartGenerationError(
+ error_type="missing_metric_aggregate",
+ message="Big Number metric must include an aggregate function",
+ details="The metric 'aggregate' field is required "
+ "for Big Number charts",
+ suggestions=[
+ "Add 'aggregate' to your metric: "
+ "{'name': 'col', 'aggregate': 'SUM'}",
+ "Valid aggregates: SUM, COUNT, AVG, MIN, MAX",
+ ],
+ error_code="MISSING_BIG_NUMBER_AGGREGATE",
+ )
Review Comment:
Valid. Added type validation for the `metric` field in
`_pre_validate_big_number_config`. Non-dict values now get a clear
`INVALID_BIG_NUMBER_METRIC_TYPE` error before reaching Pydantic. Fixed in the
latest commit with a test.
--
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]
