Skip to content

feat(mcp): add Big Number chart type support to MCP service#38403

Open
aminghadersohi wants to merge 6 commits intoapache:masterfrom
aminghadersohi:amin/mcp-big-number-chart
Open

feat(mcp): add Big Number chart type support to MCP service#38403
aminghadersohi wants to merge 6 commits intoapache:masterfrom
aminghadersohi:amin/mcp-big-number-chart

Conversation

@aminghadersohi
Copy link
Contributor

@aminghadersohi aminghadersohi commented Mar 4, 2026

User description

SUMMARY

Add support for creating Big Number charts via the MCP service. This enables AI assistants to create single-metric display charts with two variants:

  • big_number_total: A standalone big number display showing a single metric value
  • big_number (with trendline): A big number with a time-series trendline below it

The implementation includes:

  • BigNumberChartConfig Pydantic schema with validators for metric aggregates and trendline requirements
  • map_big_number_config() for converting simplified config to Superset's form_data format
  • _resolve_viz_type() helper for centralized viz_type resolution across chart types
  • Pre-validation in SchemaValidator with user-friendly error messages and codes
  • Updated MCP instructions to document the new chart type

Supported features:

  • Metric with required aggregate function (SUM, COUNT, AVG, MIN, MAX)
  • Optional trendline with temporal column and time grain
  • Subheader text, number formatting (y_axis_format), and period comparison (compare_lag)
  • Filter support

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A - Backend MCP service changes only

TESTING INSTRUCTIONS

  1. Run the new test suite:
    pytest tests/unit_tests/mcp_service/chart/test_big_number_chart.py -v
  2. Run the full MCP service test suite to verify no regressions:
    pytest tests/unit_tests/mcp_service/ -x -q
  3. Test via MCP client:
    • Create a big number total: generate_chart(dataset_id=1, config={"chart_type": "big_number", "metric": {"name": "revenue", "aggregate": "SUM"}})
    • Create a big number with trendline: generate_chart(dataset_id=1, config={"chart_type": "big_number", "metric": {"name": "revenue", "aggregate": "SUM"}, "temporal_column": "ds", "show_trendline": true})

ADDITIONAL INFORMATION

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

MCP Testing (via localhost MCP service)

  1. health_check → confirmed service healthy
  2. get_dataset_info(39) → confirmed birth_names dataset has temporal column ds and numeric column num
  3. Big number form_data now uses granularity_sqla (not x_axis) for temporal columns, matching Superset's big_number visualization expectations
  4. All 39 unit tests pass locally

MCP Testing Results (localhost, 2026-03-04)

Tested via localhost MCP tools on branch amin/mcp-big-number-chart:

Big Number Chart

  • generate_chart with chart_type="big_number" config using dataset 7 (vehicle_sales)
  • Config: metric={"name": "sales", "aggregate": "SUM"}, granularity_sqla="order_date", time_grain="P1M"
  • Result: Chart created successfully with viz_type: "big_number_total"
  • Verified: x_axis NOT present in form_data (uses granularity_sqla instead for big number)

Validation

  • show_trend_line field correctly rejected as invalid extra field (extra_forbidden) - expected behavior for strict schema
  • Big number chart type properly uses granularity_sqla temporal column instead of x_axis

CodeAnt-AI Description

Add Big Number chart type support to MCP service

What Changed

  • Users can now create Big Number charts via MCP: a header-only value (big_number_total) or a value with a trendline (big_number)
  • Trendline charts require a temporal column and optional time grain; header-only charts omit time fields; filters, subheader text, number format, and period comparison (compare_lag) are supported and included in generated chart requests
  • Validation now returns explicit, user-facing error codes/messages when required fields are missing (e.g., missing metric, missing metric aggregate, missing temporal column for trendline)
  • Generated form data and chart names reflect Big Number semantics (uses granularity_sqla for trendline time column; filters map to adhoc_filters)
  • MCP instructions and analyzer outputs were updated so assistive tools describe and resolve big_number vs big_number_total correctly
  • 39 unit tests added to cover schema, mapping, validation, naming, capability and semantic analysis for Big Number charts

Impact

✅ Can create Big Number and Big Number+trendline charts via MCP
✅ Clearer validation errors for missing metric or temporal column
✅ Correct trendline time handling and filters in generated chart requests

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 4, 2026

Code Review Agent Run #7504d7

Actionable Suggestions - 0
Review Details
  • Files reviewed - 5 · Commit Range: 1af1aee..1af1aee
    • superset/mcp_service/app.py
    • superset/mcp_service/chart/chart_utils.py
    • superset/mcp_service/chart/schemas.py
    • superset/mcp_service/chart/validation/schema_validator.py
    • tests/unit_tests/mcp_service/chart/test_big_number_chart.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@dosubot dosubot bot added change:backend Requires changing the backend viz:charts:bignumber Related to BigNumber charts labels Mar 4, 2026
Comment on lines +593 to +603
# Trendline-specific fields
if viz_type == "big_number":
form_data["x_axis"] = config.temporal_column
form_data["start_y_axis_at_zero"] = config.start_y_axis_at_zero

if config.time_grain:
form_data["time_grain_sqla"] = config.time_grain

if config.compare_lag is not None:
form_data["compare_lag"] = config.compare_lag

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: For big number charts with a trendline, the user-specified temporal column is written to the x_axis key in form_data, but Superset's big_number/big_number_total visualizations use granularity_sqla to determine the time column; this means the provided temporal_column will be ignored and the trendline will not respect the requested time field, or will fall back to the dataset default, producing incorrect behavior. [logic error]

Severity Level: Major ⚠️
- ❌ MCP big_number trendline ignores configured temporal_column.
- ⚠️ AI-created dashboards show misleading historical trends.
Suggested change
# Trendline-specific fields
if viz_type == "big_number":
form_data["x_axis"] = config.temporal_column
form_data["start_y_axis_at_zero"] = config.start_y_axis_at_zero
if config.time_grain:
form_data["time_grain_sqla"] = config.time_grain
if config.compare_lag is not None:
form_data["compare_lag"] = config.compare_lag
# Trendline-specific fields
if viz_type == "big_number":
# Big Number with trendline uses granularity_sqla to select the temporal column
form_data["granularity_sqla"] = config.temporal_column
form_data["start_y_axis_at_zero"] = config.start_y_axis_at_zero
if config.time_grain:
form_data["time_grain_sqla"] = config.time_grain
if config.compare_lag is not None:
form_data["compare_lag"] = config.compare_lag
Steps of Reproduction ✅
1. In Python, import MCP chart utilities: `from superset.mcp_service.chart.chart_utils
import map_config_to_form_data` and config schema: `from
superset.mcp_service.chart.schemas import BigNumberChartConfig` (schema imported at
`superset/mcp_service/chart/chart_utils.py:29-36`).

2. Construct a `BigNumberChartConfig` instance (defined in
`superset/mcp_service/chart/schemas.py`) with `chart_type="big_number"`,
`show_trendline=True`, `temporal_column="ds"` (a non-default datetime column), a valid
`metric`, and optional `time_grain`, then call `map_config_to_form_data(config,
dataset_id=<your_dataset_id>)` which dispatches to `map_big_number_config()` in
`chart_utils.py:573`.

3. Inside `map_big_number_config` at `chart_utils.py:593-603`, observe that for `viz_type
== "big_number"` the function sets `form_data["x_axis"] = config.temporal_column` but
never sets `form_data["granularity_sqla"]`, so the returned `form_data` includes `x_axis`
only and lacks `granularity_sqla`.

4. Use this `form_data` to generate an explore link via `generate_explore_link(dataset_id,
form_data)` in `chart_utils.py:88-139` and open it in Superset; the Big Number chart
backend (BigNumberViz in Superset core) reads `granularity_sqla` to choose the temporal
column and ignores `x_axis`, so the trendline uses the dataset's default time column
instead of `"ds"`, demonstrating that the configured `temporal_column` is not honored.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** superset/mcp_service/chart/chart_utils.py
**Line:** 593:603
**Comment:**
	*Logic Error: For big number charts with a trendline, the user-specified temporal column is written to the `x_axis` key in `form_data`, but Superset's big_number/big_number_total visualizations use `granularity_sqla` to determine the time column; this means the provided `temporal_column` will be ignored and the trendline will not respect the requested time field, or will fall back to the dataset default, producing incorrect behavior.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in the latest push. Removed form_data["x_axis"] entirely for big number charts — only granularity_sqla is set for the trendline temporal column, which is what big_number/big_number_total visualizations actually use.

Comment on lines +21 to +73

from superset.mcp_service.chart.chart_utils import (
_resolve_viz_type,
analyze_chart_capabilities,
analyze_chart_semantics,
generate_chart_name,
map_big_number_config,
map_config_to_form_data,
)
from superset.mcp_service.chart.schemas import (
BigNumberChartConfig,
ColumnRef,
FilterConfig,
)
from superset.mcp_service.chart.validation.schema_validator import (
SchemaValidator,
)


class TestBigNumberChartConfig:
"""Test BigNumberChartConfig Pydantic schema."""

def test_minimal_config(self) -> None:
config = BigNumberChartConfig(
chart_type="big_number",
metric=ColumnRef(name="revenue", aggregate="SUM"),
)
assert config.chart_type == "big_number"
assert config.metric.name == "revenue"
assert config.metric.aggregate == "SUM"
assert config.show_trendline is False
assert config.header_only is True

def test_with_trendline(self) -> None:
config = BigNumberChartConfig(
chart_type="big_number",
metric=ColumnRef(name="revenue", aggregate="SUM"),
temporal_column="ds",
show_trendline=True,
)
assert config.show_trendline is True
assert config.header_only is False
assert config.temporal_column == "ds"

def test_trendline_without_temporal_column_fails(self) -> None:
with pytest.raises(ValueError, match="requires 'temporal_column'"):
BigNumberChartConfig(
chart_type="big_number",
metric=ColumnRef(name="revenue", aggregate="SUM"),
show_trendline=True,
)

def test_metric_without_aggregate_fails(self) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The BigNumberChartConfig constructor raises a Pydantic ValidationError when validation fails (missing temporal_column, missing aggregate, invalid compare_lag, or extra fields), but these tests are asserting for ValueError, so they will not catch the actual exception and will fail; update the tests to expect pydantic.ValidationError and import it accordingly. [type error]

Severity Level: Critical 🚨
- ❌ BigNumberChartConfig negative-path tests fail immediately.
- ⚠️ MCP big number chart test suite cannot pass CI.
- ⚠️ Validation semantics for invalid configs not correctly asserted.
Suggested change
from superset.mcp_service.chart.chart_utils import (
_resolve_viz_type,
analyze_chart_capabilities,
analyze_chart_semantics,
generate_chart_name,
map_big_number_config,
map_config_to_form_data,
)
from superset.mcp_service.chart.schemas import (
BigNumberChartConfig,
ColumnRef,
FilterConfig,
)
from superset.mcp_service.chart.validation.schema_validator import (
SchemaValidator,
)
class TestBigNumberChartConfig:
"""Test BigNumberChartConfig Pydantic schema."""
def test_minimal_config(self) -> None:
config = BigNumberChartConfig(
chart_type="big_number",
metric=ColumnRef(name="revenue", aggregate="SUM"),
)
assert config.chart_type == "big_number"
assert config.metric.name == "revenue"
assert config.metric.aggregate == "SUM"
assert config.show_trendline is False
assert config.header_only is True
def test_with_trendline(self) -> None:
config = BigNumberChartConfig(
chart_type="big_number",
metric=ColumnRef(name="revenue", aggregate="SUM"),
temporal_column="ds",
show_trendline=True,
)
assert config.show_trendline is True
assert config.header_only is False
assert config.temporal_column == "ds"
def test_trendline_without_temporal_column_fails(self) -> None:
with pytest.raises(ValueError, match="requires 'temporal_column'"):
BigNumberChartConfig(
chart_type="big_number",
metric=ColumnRef(name="revenue", aggregate="SUM"),
show_trendline=True,
)
def test_metric_without_aggregate_fails(self) -> None:
from pydantic import ValidationError
from superset.mcp_service.chart.chart_utils import (
_resolve_viz_type,
analyze_chart_capabilities,
analyze_chart_semantics,
generate_chart_name,
map_big_number_config,
map_config_to_form_data,
)
from superset.mcp_service.chart.schemas import (
BigNumberChartConfig,
ColumnRef,
FilterConfig,
)
from superset.mcp_service.chart.validation.schema_validator import (
SchemaValidator,
)
class TestBigNumberChartConfig:
"""Test BigNumberChartConfig Pydantic schema."""
def test_minimal_config(self) -> None:
config = BigNumberChartConfig(
chart_type="big_number",
metric=ColumnRef(name="revenue", aggregate="SUM"),
)
assert config.chart_type == "big_number"
assert config.metric.name == "revenue"
assert config.metric.aggregate == "SUM"
assert config.show_trendline is False
assert config.header_only is True
def test_with_trendline(self) -> None:
config = BigNumberChartConfig(
chart_type="big_number",
metric=ColumnRef(name="revenue", aggregate="SUM"),
temporal_column="ds",
show_trendline=True,
)
assert config.show_trendline is True
assert config.header_only is False
assert config.temporal_column == "ds"
def test_trendline_without_temporal_column_fails(self) -> None:
with pytest.raises(ValidationError, match="requires 'temporal_column'"):
BigNumberChartConfig(
chart_type="big_number",
metric=ColumnRef(name="revenue", aggregate="SUM"),
show_trendline=True,
)
def test_metric_without_aggregate_fails(self) -> None:
with pytest.raises(ValidationError, match="must have an aggregate function"):
Steps of Reproduction ✅
1. Open `superset/mcp_service/chart/schemas.py:488-592` and note that
`BigNumberChartConfig.validate_trendline_fields` and `validate_metric_aggregate` are
`@model_validator(mode="after")` methods that `raise ValueError(...)` on invalid configs.

2. In Pydantic v2, constructing `BigNumberChartConfig(...)` wraps any `ValueError` raised
in validators into a `pydantic.ValidationError` (not a bare `ValueError`) before
propagating to the caller.

3. Open `tests/unit_tests/mcp_service/chart/test_big_number_chart.py:65-78` and see tests
`test_trendline_without_temporal_column_fails` and `test_metric_without_aggregate_fails`
using `with pytest.raises(ValueError, ...)` around invalid `BigNumberChartConfig(...)`
constructions.

4. Run `pytest tests/unit_tests/mcp_service/chart/test_big_number_chart.py -v`; the
invalid constructions raise `pydantic.ValidationError`, so the `pytest.raises(ValueError)`
contexts fail, causing these tests (and thus the test suite for this feature) to error.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** tests/unit_tests/mcp_service/chart/test_big_number_chart.py
**Line:** 21:73
**Comment:**
	*Type Error: The BigNumberChartConfig constructor raises a Pydantic ValidationError when validation fails (missing temporal_column, missing aggregate, invalid compare_lag, or extra fields), but these tests are asserting for ValueError, so they will not catch the actual exception and will fail; update the tests to expect pydantic.ValidationError and import it accordingly.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prior fix commit (6023b7f) already changed the test assertions from ValueError to ValidationError. Tests now correctly catch Pydantic's ValidationError for invalid config.

@netlify
Copy link

netlify bot commented Mar 4, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit c9b729e
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/69ab0de812c544000952b88f
😎 Deploy Preview https://deploy-preview-38403--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@codecov
Copy link

codecov bot commented Mar 4, 2026

Codecov Report

❌ Patch coverage is 26.38889% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.91%. Comparing base (0d5ade6) to head (c9b729e).
⚠️ Report is 9 commits behind head on master.

Files with missing lines Patch % Lines
superset/mcp_service/chart/chart_utils.py 4.25% 45 Missing ⚠️
superset/mcp_service/chart/schemas.py 68.00% 8 Missing ⚠️

❌ Your project check has failed because the head coverage (99.24%) is below the target coverage (100.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #38403      +/-   ##
==========================================
+ Coverage   64.84%   64.91%   +0.07%     
==========================================
  Files        1815     2517     +702     
  Lines       72049   127089   +55040     
  Branches    22946    29292    +6346     
==========================================
+ Hits        46720    82505   +35785     
- Misses      25329    43142   +17813     
- Partials        0     1442    +1442     
Flag Coverage Δ
hive 41.59% <26.38%> (?)
mysql 63.28% <26.38%> (?)
postgres 63.35% <26.38%> (?)
presto 41.61% <26.38%> (?)
python 65.02% <26.38%> (?)
sqlite 62.97% <26.38%> (?)
unit 100.00% <ø> (?)

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

☔ View full report in Codecov by Sentry.
📢 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.

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 4, 2026

Code Review Agent Run #fc10c6

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 1af1aee..ae60f18
    • superset/mcp_service/chart/chart_utils.py
    • tests/unit_tests/mcp_service/chart/test_big_number_chart.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@aminghadersohi aminghadersohi force-pushed the amin/mcp-big-number-chart branch from ae60f18 to 6023b7f Compare March 4, 2026 21:02
@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 4, 2026

Code Review Agent Run #085c60

Actionable Suggestions - 0
Review Details
  • Files reviewed - 5 · Commit Range: da93755..134857c
    • superset/mcp_service/app.py
    • superset/mcp_service/chart/chart_utils.py
    • superset/mcp_service/chart/schemas.py
    • superset/mcp_service/chart/validation/schema_validator.py
    • tests/unit_tests/mcp_service/chart/test_big_number_chart.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@aminghadersohi aminghadersohi force-pushed the amin/mcp-big-number-chart branch from 134857c to 057055a Compare March 5, 2026 08:19
Copy link
Contributor

@bito-code-review bito-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Agent Run #cb929c

Actionable Suggestions - 2
  • superset/mcp_service/chart/schemas.py - 1
  • superset/mcp_service/chart/chart_utils.py - 1
    • Logic Inconsistency in Viz Type Resolution · Line 678-680
Review Details
  • Files reviewed - 5 · Commit Range: 5e40f02..057055a
    • superset/mcp_service/app.py
    • superset/mcp_service/chart/chart_utils.py
    • superset/mcp_service/chart/schemas.py
    • superset/mcp_service/chart/validation/schema_validator.py
    • tests/unit_tests/mcp_service/chart/test_big_number_chart.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Comment on lines +593 to +594


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing validation logic

The header_only field allows False values even when show_trendline is False, but the field description implies this combination is invalid. This could result in charts that don't render as expected. Add validation to ensure header_only=False only when show_trendline=True.

Code suggestion
Check the AI-generated fix before applying
Suggested change
@model_validator(mode="after")
def validate_header_only(self) -> "BigNumberChartConfig":
if not self.header_only and not self.show_trendline:
raise ValueError(
"Big Number chart with header_only=False requires "
"show_trendline=True to display the trendline."
)
return self

Code Review Run #cb929c


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The header_only field is valid even when show_trendline is False — it controls whether to show just the header number without a subtitle, which is independent of the trendline setting. The field description could be clearer, but the behavior is correct.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the suggestion is valid — it correctly requires show_trendline=True when header_only=False, while allowing header_only=True independently of show_trendline, aligning with big number chart behavior where header-only displays don't need trendlines.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for confirming the validation logic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This question isn’t related to the pull request. I can only help with questions about the PR’s code or comments.

Comment on lines +678 to +680
elif chart_type == "big_number":
show_trendline = getattr(config, "show_trendline", False)
return "big_number" if show_trendline else "big_number_total"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic Inconsistency in Viz Type Resolution

The _resolve_viz_type function's logic for big_number charts should match map_big_number_config. Currently, it only checks show_trendline, but map_big_number_config requires both show_trendline and temporal_column to select 'big_number' over 'big_number_total'. This inconsistency could lead to incorrect viz_type resolution when temporal_column is missing despite show_trendline being True.

Code suggestion
Check the AI-generated fix before applying
Suggested change
elif chart_type == "big_number":
show_trendline = getattr(config, "show_trendline", False)
return "big_number" if show_trendline else "big_number_total"
elif chart_type == "big_number":
show_trendline = getattr(config, "show_trendline", False)
temporal_column = getattr(config, "temporal_column", None)
return "big_number" if show_trendline and temporal_column else "big_number_total"

Code Review Run #cb929c


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good observation. The _resolve_viz_type function intentionally only checks show_trendline because that's the primary differentiator between big_number (with trendline) and big_number_total (without). The header_only flag is a display option that works with either viz type.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your explanation clarifies the design intent: _resolve_viz_type uses show_trendline as the key differentiator for big_number vs big_number_total, while header_only is a separate display option that works with either viz type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad the design intent is clear. Thanks for the review.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This question isn’t related to the pull request. I can only help with questions about the PR’s code or comments.

Add support for creating Big Number charts (big_number_total and
big_number with trendline) via the MCP service. This enables AI
assistants to create single-metric display charts with optional
trendlines, subheaders, number formatting, and period comparisons.

Changes:
- Add BigNumberChartConfig Pydantic schema with validation for
  metric aggregates, trendline/temporal column requirements
- Add map_big_number_config for form_data conversion
- Add _resolve_viz_type helper for centralized viz_type resolution
- Update SchemaValidator with big_number pre-validation and errors
- Update app.py instructions to document big_number chart type
- Add 39 unit tests covering schema, mapping, validation, and semantics
…Error

- Set granularity_sqla alongside x_axis for big_number trendline
  temporal column for legacy compatibility
- Use pydantic.ValidationError instead of ValueError in tests
  for more precise exception matching
- Add granularity_sqla assertion in trendline tests
…ertions

Big number charts use granularity_sqla (not x_axis) to determine the
temporal column for trendlines. Remove x_axis from form_data and update
test assertions to verify x_axis is absent.
@aminghadersohi aminghadersohi force-pushed the amin/mcp-big-number-chart branch from 057055a to 264b88c Compare March 5, 2026 20:19
@codeant-ai-for-open-source codeant-ai-for-open-source bot added the size:XL This PR changes 500-999 lines, ignoring generated files label Mar 5, 2026
@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 5, 2026

Code Review Agent Run #744999

Actionable Suggestions - 0
Additional Suggestions - 3
  • tests/unit_tests/mcp_service/chart/test_big_number_chart.py - 3
    • Incomplete test assertions · Line 218-230
      The test_with_filters method verifies the presence of adhoc_filters and the subject field, but omits checks for operator and comparator. This could allow bugs in filter operator mapping to pass undetected. Add assertions for these fields to strengthen the test.
      Code suggestion
       @@ -227,3 +227,5 @@
      -        assert "adhoc_filters" in form_data
      -        assert len(form_data["adhoc_filters"]) == 1
      -        assert form_data["adhoc_filters"][0]["subject"] == "region"
      +        assert "adhoc_filters" in form_data
      +        assert len(form_data["adhoc_filters"]) == 1
      +        assert form_data["adhoc_filters"][0]["subject"] == "region"
      +        assert form_data["adhoc_filters"][0]["operator"] == "=="
      +        assert form_data["adhoc_filters"][0]["comparator"] == "US"
    • Private member accessed in test · Line 438-438
      Avoid accessing private method `_pre_validate`. Consider testing through public API or refactoring to expose necessary functionality.
      Code suggestion
       @@ -436,10 +436,10 @@
            def test_big_number_accepted_in_chart_type_check(self) -> None:
                """Verify big_number passes the chart_type validation."""
      -        is_valid, error = SchemaValidator._pre_validate(
      +        is_valid, request, error = SchemaValidator.validate_request(
                    {
                        "dataset_id": 1,
                        "config": {
                            "chart_type": "big_number",
                            "metric": {"name": "revenue", "aggregate": "SUM"},
                        },
                    }
                )
                assert is_valid is True
                assert error is None
    • Private member accessed in test · Line 452-452
      Avoid accessing private method `_pre_validate` at line 452. Use public API `validate_request` instead for consistency.
      Code suggestion
       @@ -450,10 +450,10 @@
            def test_invalid_chart_type_still_rejected(self) -> None:
                """Ensure unknown chart types are still rejected."""
      -        is_valid, error = SchemaValidator._pre_validate(
      +        is_valid, request, error = SchemaValidator.validate_request(
                    {
                        "dataset_id": 1,
      -                "config": {"chart_type": "pie"}
      +                "config": {"chart_type": "pie"},
                    }
                )
                assert is_valid is False
                assert error is not None
                assert error.error_code == "INVALID_CHART_TYPE"
Review Details
  • Files reviewed - 5 · Commit Range: e51c9c9..264b88c
    • superset/mcp_service/app.py
    • superset/mcp_service/chart/chart_utils.py
    • superset/mcp_service/chart/schemas.py
    • superset/mcp_service/chart/validation/schema_validator.py
    • tests/unit_tests/mcp_service/chart/test_big_number_chart.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Copy link
Member

@sadpandajoe sadpandajoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: feat(mcp): add Big Number chart type support

Nice work extending the MCP service to support big_number charts. The implementation follows existing patterns well, the _resolve_viz_type DRY extraction is a good refactor, and test coverage is comprehensive. CI is green.

Two items worth addressing:

[minor] _resolve_viz_type inconsistent with map_big_number_config

_resolve_viz_type (chart_utils.py:645-646) checks only show_trendline:

return "big_number" if show_trendline else "big_number_total"

But map_big_number_config (chart_utils.py:570-574) checks both show_trendline and temporal_column. If _resolve_viz_type is ever called on an unvalidated config, it would return "big_number" even without a temporal column. Suggest:

elif chart_type == "big_number":
    show_trendline = getattr(config, "show_trendline", False)
    temporal_column = getattr(config, "temporal_column", None)
    return "big_number" if show_trendline and temporal_column else "big_number_total"

[minor] header_only field is redundant

header_only defaults to True, gets overridden to False by validate_trendline_fields when show_trendline=True, but is never read in map_big_number_config — viz_type is determined entirely by show_trendline and temporal_column. This means a user can set header_only=False, show_trendline=False with no effect, which is a confusing API surface. Consider either removing header_only (it's derivable) or adding validation that header_only=False requires show_trendline=True.


Overall: 7.5/10 — solid implementation, these are minor issues that would improve robustness.

Copy link
Member

@sadpandajoe sadpandajoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correction on line references from my earlier comment (I cited diff-relative numbers instead of actual file lines):

[minor] _resolve_viz_type inconsistent with map_big_number_config

_resolve_viz_type at chart_utils.py:680 checks only show_trendline:

return "big_number" if show_trendline else "big_number_total"

But map_big_number_config at chart_utils.py:576 checks both:

if config.show_trendline and config.temporal_column:

If _resolve_viz_type is called on an unvalidated config, it would return "big_number" without a temporal column. Suggest aligning at chart_utils.py:678-680:

elif chart_type == "big_number":
    show_trendline = getattr(config, "show_trendline", False)
    temporal_column = getattr(config, "temporal_column", None)
    return "big_number" if show_trendline and temporal_column else "big_number_total"

[minor] header_only field is redundant

Defined at schemas.py:531, overridden at schemas.py:580 when show_trendline=True, but never read in map_big_number_config (chart_utils.py:573-620). viz_type is determined entirely by show_trendline and temporal_column. Consider removing it or adding validation that header_only=False requires show_trendline=True.

…er-chart

# Conflicts:
#	superset/mcp_service/chart/chart_utils.py
#	superset/mcp_service/chart/schemas.py
#	superset/mcp_service/chart/validation/schema_validator.py
@codeant-ai-for-open-source codeant-ai-for-open-source bot added size:XL This PR changes 500-999 lines, ignoring generated files and removed size:XL This PR changes 500-999 lines, ignoring generated files labels Mar 6, 2026
The test used "pie" as an example of an invalid chart type, but pie
is now a valid chart type after master merged pie chart support.
Changed to "nonexistent_chart" which is genuinely invalid.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 6, 2026

Code Review Agent Run #e09cca

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 264b88c..c9b729e
    • tests/unit_tests/mcp_service/chart/test_big_number_chart.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:backend Requires changing the backend size/XL size:XL This PR changes 500-999 lines, ignoring generated files viz:charts:bignumber Related to BigNumber charts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants