Skip to content

Conversation

@aminghadersohi
Copy link
Contributor

@aminghadersohi aminghadersohi commented Dec 8, 2025

User description

SUMMARY

Consolidates three separate schema discovery tools (get_chart_available_filters, get_dataset_available_filters, get_dashboard_available_filters) into a single unified get_schema tool that returns comprehensive metadata for any model type.

Token Usage Improvement

Tool Definition Overhead (loaded into LLM context each session):

Metric Before After Savings
Number of tools 3 1 67% fewer tools
Schema tokens ~750 tokens ~400 tokens ~47% reduction

Response Value (per API call):

Data Returned Before After
Filter columns Yes Yes
Select columns with metadata No Yes
Sortable columns No Yes
Default columns No Yes
Search columns No Yes
Column descriptions & types No Yes

Net Benefits:

  • ~350 fewer tokens loaded per MCP session
  • 5x richer metadata per response
  • LLMs can now discover valid select_columns, order_column, and search parameters
  • Eliminates trial-and-error for column names

Changes

Added:

  • get_schema tool in superset/mcp_service/system/tool/get_schema.py
  • ModelGetSchemaCore in mcp_core.py for reusable schema discovery logic
  • schema_discovery.py with column metadata definitions for all model types
  • Comprehensive unit tests (6 new tests) with edge case coverage

Removed:

  • get_chart_available_filters.py (deprecated)
  • get_dataset_available_filters.py (deprecated)
  • get_dashboard_available_filters.py (deprecated)
  • Associated filter-only schemas from each module

Updated:

  • app.py instructions to reference new get_schema tool
  • MCP service CLAUDE.md documentation

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before (3 separate tools, limited info):

// get_chart_available_filters response
{"filter_columns": {"slice_name": ["eq", "sw"], "viz_type": ["eq", "in"]}}

After (1 unified tool, comprehensive info):

// get_schema response for model_type="chart"
{
  "schema_info": {
    "model_type": "chart",
    "select_columns": [
      {"name": "id", "description": "Chart ID", "type": "int", "is_default": true},
      {"name": "slice_name", "description": "Chart name", "type": "str", "is_default": true}
    ],
    "filter_columns": {"slice_name": ["eq", "sw"], "viz_type": ["eq", "in"]},
    "sortable_columns": ["id", "slice_name", "viz_type", "changed_on", "created_on"],
    "default_select": ["id", "slice_name", "viz_type", "uuid"],
    "default_sort": "changed_on",
    "default_sort_direction": "desc",
    "search_columns": ["slice_name", "description"]
  }
}

TESTING INSTRUCTIONS

  1. Start MCP server: python -m superset.mcp_service.server
  2. Test via MCP client:
    # Get chart schema
    result = await client.call_tool("get_schema", {"request": {"model_type": "chart"}})
    
    # Get dataset schema
    result = await client.call_tool("get_schema", {"request": {"model_type": "dataset"}})
    
    # Get dashboard schema  
    result = await client.call_tool("get_schema", {"request": {"model_type": "dashboard"}})
  3. Run tests: pytest tests/unit_tests/mcp_service/ -v

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

CodeAnt-AI Description

Add unified get_schema tool and return minimal default columns for list APIs

What Changed

  • Added a single get_schema tool that returns comprehensive schema metadata for charts, datasets, and dashboards (selectable columns with descriptions/types, filterable columns and operators, sortable columns, default select, default sort, and search columns).
  • Removed the three legacy get_*_available_filters tools; callers should use get_schema instead.
  • List endpoints (list_charts, list_datasets, list_dashboards) now apply field filtering by default: when select_columns is empty, only a minimal set of default columns (4 per model) is returned and heavy fields (e.g., columns, metrics, form_data, json_metadata) are excluded unless explicitly requested.
  • List responses now include columns_available and sortable_columns metadata so clients can discover which columns can be requested or used for ordering.
  • Added static schema resources (superset://schema/chart, /dataset, /dashboard, and /all) and updated default MCP instructions to recommend get_schema; tests updated/added to cover schema discovery and default-column behavior.

Impact

✅ ~47% fewer tool tokens per MCP session
✅ Smaller default list responses
✅ Fewer trial-and-error column 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

This comment was marked as outdated.

@dosubot dosubot bot added api Related to the REST API change:backend Requires changing the backend risk:breaking-change Issues or PRs that will introduce breaking changes labels Dec 8, 2025
@github-actions github-actions bot removed the api Related to the REST API label Dec 8, 2025
@codeant-ai-for-open-source codeant-ai-for-open-source bot added the size:XXL This PR changes 1000+ lines, ignoring generated files label Dec 8, 2025
@codeant-ai-for-open-source
Copy link
Contributor

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Eager init
    ModelGetSchemaCore instances are created at import time. If construction becomes heavier (or DAOs have side effects), this can increase startup cost and complicate testing/mocking. Consider lazy initialization or a factory to create cores on first use.

  • Parsing Issue
    request.select_columns is used directly when building the serialization context. If the request
    field can be provided as a JSON string, CSV, or other serialized form, using it directly may lead
    to incorrect behavior. Use the same parsing utility used by ModelListCore to normalize the value
    (e.g., parse JSON/CSV -> list) before applying it.

  • Columns vs. serialization mismatch
    The response returned by ModelListCore includes columns_loaded (the actual columns loaded). The code uses request.select_columns (or default) to filter fields for output; this can cause the reported columns_loaded to diverge from the fields actually serialized, producing inconsistent API responses.

  • Unparsed select_columns (Possible Bug)
    The code now uses request.select_columns directly for the serialization context when calling result.model_dump(...). However, request.select_columns may be a JSON string or CSV (the same inputs that ModelListCore.run_tool parses internally). Passing an unparsed string into the context can break field filtering logic which expects a list of column names. Use the parsed column list that the DAO used (or coerce/parse the request value) to ensure consistent behavior.

  • Request parsing mismatch
    request.select_columns is used directly to build the serialization context. However, ModelListCore internally parses select_columns (accepting CSV / JSON / list). If request.select_columns is a CSV or raw string, the serialization context may receive a string instead of a list which can break filtering logic in DatasetInfo serialization.

  • Identifier mutation
    The ColumnRef.sanitize_name validator returns html.escape(...) (escaped value) for name, which can change column identifiers used for queries/joins (e.g., converting spaces/quotes to HTML entities). That may make the name no longer match database/metric identifiers and break query generation or equality checks. Consider preserving a canonical identifier for internals and only escaping for display.

  • Serialization mismatch
    The simple serializer serialize_dashboard_object populates keys like created_by_name and changed_by_name when constructing DashboardInfo, but DashboardInfo defines created_by and changed_by (no _name fields). This can lead to ignored fields or validation errors / unexpected missing data in responses.

  • Schema field name mismatch
    The run_tool constructs the output schema using the keyword default_select. Ensure the target Pydantic output_schema actually expects default_select (not default_columns or similar). A mismatch will raise validation errors at runtime.

@codeant-ai-for-open-source
Copy link
Contributor

CodeAnt AI finished reviewing your PR.

@codecov
Copy link

codecov bot commented Dec 8, 2025

Codecov Report

❌ Patch coverage is 0% with 186 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.93%. Comparing base (5909e90) to head (308ab20).
⚠️ Report is 18 commits behind head on master.

Files with missing lines Patch % Lines
superset/mcp_service/common/schema_discovery.py 0.00% 81 Missing ⚠️
superset/mcp_service/mcp_core.py 0.00% 38 Missing ⚠️
...t/mcp_service/system/resources/schema_discovery.py 0.00% 35 Missing ⚠️
superset/mcp_service/system/tool/get_schema.py 0.00% 30 Missing ⚠️
superset/mcp_service/system/resources/__init__.py 0.00% 1 Missing ⚠️
superset/mcp_service/system/tool/__init__.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #36458       +/-   ##
===========================================
+ Coverage        0   67.93%   +67.93%     
===========================================
  Files           0      642      +642     
  Lines           0    47878    +47878     
  Branches        0     5226     +5226     
===========================================
+ Hits            0    32527    +32527     
- Misses          0    14070    +14070     
- Partials        0     1281     +1281     
Flag Coverage Δ
hive 42.92% <0.00%> (?)
mysql 65.95% <0.00%> (?)
postgres 66.00% <0.00%> (?)
presto 46.50% <0.00%> (?)
python 67.90% <0.00%> (?)
sqlite 65.71% <0.00%> (?)
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.

aminghadersohi added a commit to aminghadersohi/superset that referenced this pull request Dec 8, 2025
Addresses code review feedback:

1. mcp_core.py - Fix constructor parameter order
   - Move logger parameter before new optional all_columns/sortable_columns
   - Add list copy protection for mutable state (default_columns, search_columns)
   - Add property accessors for all_columns/sortable_columns that return copies

2. get_schema.py - Add defensive key lookup
   - Use .get() instead of direct dictionary access for model_type
   - Add error logging and helpful ValueError message for invalid types

3. list_* tools - Fix return type consistency
   - Use result.columns_requested instead of request.select_columns
   - Return validated Pydantic model via model_validate() instead of dict

4. test_dashboard_tools.py - Update test response parsing
   - Use json.loads(result.content[0].text) pattern for consistency
   - Match pattern used in test_get_schema.py and test_dataset_tools.py
aminghadersohi added a commit to aminghadersohi/superset that referenced this pull request Dec 8, 2025
- Fix fragile DAO filter handling in mcp_core.py with defensive checks
- Fix JSON serialization risks in schema_discovery.py with safe helper
- Fix brittle test assertions using issubset() pattern
- Ensure Field(default_factory=list) for mutable defaults in schemas

Co-Authored-By: Claude <noreply@anthropic.com>
@rusackas rusackas requested review from dpgaspar and villebro December 9, 2025 18:23
aminghadersohi added a commit to aminghadersohi/superset that referenced this pull request Jan 5, 2026
Addresses code review feedback:

1. mcp_core.py - Fix constructor parameter order
   - Move logger parameter before new optional all_columns/sortable_columns
   - Add list copy protection for mutable state (default_columns, search_columns)
   - Add property accessors for all_columns/sortable_columns that return copies

2. get_schema.py - Add defensive key lookup
   - Use .get() instead of direct dictionary access for model_type
   - Add error logging and helpful ValueError message for invalid types

3. list_* tools - Fix return type consistency
   - Use result.columns_requested instead of request.select_columns
   - Return validated Pydantic model via model_validate() instead of dict

4. test_dashboard_tools.py - Update test response parsing
   - Use json.loads(result.content[0].text) pattern for consistency
   - Match pattern used in test_get_schema.py and test_dataset_tools.py
aminghadersohi added a commit to aminghadersohi/superset that referenced this pull request Jan 5, 2026
- Fix fragile DAO filter handling in mcp_core.py with defensive checks
- Fix JSON serialization risks in schema_discovery.py with safe helper
- Fix brittle test assertions using issubset() pattern
- Ensure Field(default_factory=list) for mutable defaults in schemas

Co-Authored-By: Claude <noreply@anthropic.com>
@aminghadersohi aminghadersohi force-pushed the feat/mcp-schema-discovery branch from dabee71 to 386c0a6 Compare January 5, 2026 20:54
@bito-code-review
Copy link
Contributor

bito-code-review bot commented Jan 5, 2026

Code Review Agent Run #b893a7

Actionable Suggestions - 0
Additional Suggestions - 7
  • superset/mcp_service/chart/tool/list_charts.py - 1
    • Error Handling Behavior Change · Line 147-149
      The error handling for field filtering has changed from returning full results on failure to raising exceptions, which may break clients expecting graceful degradation when filtering encounters issues.
  • superset/mcp_service/chart/tool/__init__.py - 2
    • Update schema docs for removed function · Line 19-19
      The removal of get_chart_available_filters import requires updating the Field descriptions in ChartFilter schema (lines 272-273 and 277-278) to point users to get_schema for allowed filter values, as get_schema consolidates this functionality.
    • Update docs for removed tool · Line 30-30
      The docs list get_chart_available_filters in the discovery tools (line 35), but since it's removed, update the table to reflect current tools or note the consolidation into get_schema.
  • superset/mcp_service/dashboard/tool/list_dashboards.py - 1
    • Documentation mismatch · Line 68-71
      The docstring states that the function returns dashboard metadata 'including title, slug, and charts', but the code now returns only minimal fields by default. As noted in the test comment, charts and other fields are excluded unless explicitly requested. This could mislead users expecting charts in the response. Please update the docstring to accurately describe the default behavior.
  • tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py - 3
    • Test Update for API Change · Line 195-202
      Test assertions removed as columns/metrics are no longer included by default in list_datasets responses, matching the DEFAULT_DATASET_COLUMNS = ['id', 'table_name', 'schema', 'uuid'] implementation.
    • Test Data Enhancement · Line 1020-1035
      Added uuid to mock dataset and updated docstring to clarify that columns/metrics require explicit selection.
    • Explicit Selection Test · Line 1074-1085
      Modified request to explicitly include columns and metrics in select_columns, and test now verifies they are returned when requested.
Review Details
  • Files reviewed - 24 · Commit Range: 64e0ead..386c0a6
    • superset/mcp_service/app.py
    • superset/mcp_service/chart/schemas.py
    • superset/mcp_service/chart/tool/__init__.py
    • superset/mcp_service/chart/tool/get_chart_available_filters.py
    • superset/mcp_service/chart/tool/list_charts.py
    • superset/mcp_service/common/schema_discovery.py
    • superset/mcp_service/dashboard/schemas.py
    • superset/mcp_service/dashboard/tool/__init__.py
    • superset/mcp_service/dashboard/tool/get_dashboard_available_filters.py
    • superset/mcp_service/dashboard/tool/list_dashboards.py
    • superset/mcp_service/dataset/schemas.py
    • superset/mcp_service/dataset/tool/__init__.py
    • superset/mcp_service/dataset/tool/get_dataset_available_filters.py
    • superset/mcp_service/dataset/tool/list_datasets.py
    • superset/mcp_service/mcp_core.py
    • superset/mcp_service/system/resources/__init__.py
    • superset/mcp_service/system/resources/schema_discovery.py
    • superset/mcp_service/system/tool/__init__.py
    • superset/mcp_service/system/tool/get_schema.py
    • tests/unit_tests/mcp_service/chart/tool/test_list_charts.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py
    • tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py
    • tests/unit_tests/mcp_service/system/tool/test_get_schema.py
    • tests/unit_tests/mcp_service/system/tool/test_mcp_core.py
  • Files skipped - 1
    • superset/mcp_service/CLAUDE.md - Reason: Filter setting
  • 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 feat/mcp-schema-discovery branch from b478e81 to e45fec1 Compare January 5, 2026 22:10
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 #17856c

Actionable Suggestions - 1
  • superset/mcp_service/mcp_core.py - 1
Additional Suggestions - 5
  • superset/mcp_service/system/tool/get_schema.py - 1
    • Async/Sync Mismatch in DB Calls · Line 140-140
      The async function calls synchronous database operations via core.run_tool(), which can block the event loop and hurt concurrency in an async MCP service. Consider refactoring ModelGetSchemaCore.run_tool to be async or wrapping the call in run_in_executor.
  • superset/mcp_service/dashboard/schemas.py - 1
    • Schema Validation Risk · Line 350-357
      Changing columns_requested and columns_loaded from Optional[List[str]] with default None to List[str] with default_factory=list alters the schema contract. If the underlying tool sets these fields to None, Pydantic will raise a ValidationError during response serialization, potentially breaking dashboard listing.
  • superset/mcp_service/chart/tool/__init__.py - 1
    • Incomplete cleanup · Line 19-19
      Removal of get_chart_available_filters is correct, but leaves stale references in schemas.py and docs. Update them to reference get_schema for consistency.
  • tests/unit_tests/mcp_service/system/tool/test_get_schema.py - 2
    • Incomplete assertion coverage · Line 201-225
      In test_get_schema_dashboard, the test checks 'dashboard_title' and 'changed_on' in sortable_columns but misses 'created_on'. Add an assertion for 'created_on' to match the full expected sortable columns list.
    • Incomplete test coverage · Line 339-344
      The test for dashboard sortable columns checks only some expected columns ('id', 'dashboard_title', 'slug', 'published') but misses 'changed_on' and 'created_on' which are defined in DASHBOARD_SORTABLE_COLUMNS. To ensure completeness, add assertions for the missing columns.
Review Details
  • Files reviewed - 24 · Commit Range: e45fec1..e45fec1
    • superset/mcp_service/app.py
    • superset/mcp_service/chart/schemas.py
    • superset/mcp_service/chart/tool/__init__.py
    • superset/mcp_service/chart/tool/get_chart_available_filters.py
    • superset/mcp_service/chart/tool/list_charts.py
    • superset/mcp_service/common/schema_discovery.py
    • superset/mcp_service/dashboard/schemas.py
    • superset/mcp_service/dashboard/tool/__init__.py
    • superset/mcp_service/dashboard/tool/get_dashboard_available_filters.py
    • superset/mcp_service/dashboard/tool/list_dashboards.py
    • superset/mcp_service/dataset/schemas.py
    • superset/mcp_service/dataset/tool/__init__.py
    • superset/mcp_service/dataset/tool/get_dataset_available_filters.py
    • superset/mcp_service/dataset/tool/list_datasets.py
    • superset/mcp_service/mcp_core.py
    • superset/mcp_service/system/resources/__init__.py
    • superset/mcp_service/system/resources/schema_discovery.py
    • superset/mcp_service/system/tool/__init__.py
    • superset/mcp_service/system/tool/get_schema.py
    • tests/unit_tests/mcp_service/chart/tool/test_list_charts.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py
    • tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py
    • tests/unit_tests/mcp_service/system/tool/test_get_schema.py
    • tests/unit_tests/mcp_service/system/tool/test_mcp_core.py
  • Files skipped - 1
    • superset/mcp_service/CLAUDE.md - Reason: Filter setting
  • 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 feat/mcp-schema-discovery branch from e45fec1 to 379fa24 Compare January 6, 2026 16:14
@netlify
Copy link

netlify bot commented Jan 6, 2026

Deploy Preview for superset-docs-preview canceled.

Name Link
🔨 Latest commit 24d9069
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/695d8c2d1c1ad70009735eb0

@aminghadersohi aminghadersohi force-pushed the feat/mcp-schema-discovery branch 2 times, most recently from 90ca98e to 9ee83b3 Compare January 6, 2026 17:28
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 #8d0365

Actionable Suggestions - 2
  • superset/mcp_service/mcp_core.py - 2
Additional Suggestions - 2
  • superset/mcp_service/dataset/tool/__init__.py - 1
    • Incomplete Function Removal · Line 18-18
      The import removal here indicates get_dataset_available_filters was deleted, but schema descriptions still reference it for allowed values, which could mislead users. It looks like the cleanup is incomplete.
  • superset/mcp_service/app.py - 1
    • Inconsistent schema documentation · Line 128-129
      The usage tips have been updated to reference get_schema for discovering filterable columns, but the schema descriptions in the Pydantic models still reference the removed get_chart_available_filters, get_dashboard_available_filters, and get_dataset_available_filters tools. This creates inconsistent documentation where users see outdated references in API schemas.
Review Details
  • Files reviewed - 24 · Commit Range: 90ca98e..90ca98e
    • superset/mcp_service/app.py
    • superset/mcp_service/chart/schemas.py
    • superset/mcp_service/chart/tool/__init__.py
    • superset/mcp_service/chart/tool/get_chart_available_filters.py
    • superset/mcp_service/chart/tool/list_charts.py
    • superset/mcp_service/common/schema_discovery.py
    • superset/mcp_service/dashboard/schemas.py
    • superset/mcp_service/dashboard/tool/__init__.py
    • superset/mcp_service/dashboard/tool/get_dashboard_available_filters.py
    • superset/mcp_service/dashboard/tool/list_dashboards.py
    • superset/mcp_service/dataset/schemas.py
    • superset/mcp_service/dataset/tool/__init__.py
    • superset/mcp_service/dataset/tool/get_dataset_available_filters.py
    • superset/mcp_service/dataset/tool/list_datasets.py
    • superset/mcp_service/mcp_core.py
    • superset/mcp_service/system/resources/__init__.py
    • superset/mcp_service/system/resources/schema_discovery.py
    • superset/mcp_service/system/tool/__init__.py
    • superset/mcp_service/system/tool/get_schema.py
    • tests/unit_tests/mcp_service/chart/tool/test_list_charts.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py
    • tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py
    • tests/unit_tests/mcp_service/system/tool/test_get_schema.py
    • tests/unit_tests/mcp_service/system/tool/test_mcp_core.py
  • Files skipped - 1
    • superset/mcp_service/CLAUDE.md - Reason: Filter setting
  • 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

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Jan 6, 2026

Code Review Agent Run #cd2cc0

Actionable Suggestions - 0
Additional Suggestions - 3
  • tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py - 3
    • Test Update for API Change · Line 195-202
      These test changes remove assertions expecting columns and metrics in default responses and add comments noting they are not in minimal defaults. This matches the implementation where DEFAULT_DATASET_COLUMNS = ['id', 'table_name', 'schema', 'uuid'], and columns/metrics require explicit select_columns. The added assertion for 'uuid' in columns_requested is correct since uuid is in defaults.
    • Test Update for API Change · Line 1020-1035
      Updates test docstring and adds uuid to mock dataset, preparing for explicit columns/metrics request.
    • Test Update for API Change · Line 1074-1085
      This test explicitly uses select_columns to request columns and metrics, then asserts they are present - correct behavior for when they are requested.
Review Details
  • Files reviewed - 24 · Commit Range: 9ee83b3..9ee83b3
    • superset/mcp_service/app.py
    • superset/mcp_service/chart/schemas.py
    • superset/mcp_service/chart/tool/__init__.py
    • superset/mcp_service/chart/tool/get_chart_available_filters.py
    • superset/mcp_service/chart/tool/list_charts.py
    • superset/mcp_service/common/schema_discovery.py
    • superset/mcp_service/dashboard/schemas.py
    • superset/mcp_service/dashboard/tool/__init__.py
    • superset/mcp_service/dashboard/tool/get_dashboard_available_filters.py
    • superset/mcp_service/dashboard/tool/list_dashboards.py
    • superset/mcp_service/dataset/schemas.py
    • superset/mcp_service/dataset/tool/__init__.py
    • superset/mcp_service/dataset/tool/get_dataset_available_filters.py
    • superset/mcp_service/dataset/tool/list_datasets.py
    • superset/mcp_service/mcp_core.py
    • superset/mcp_service/system/resources/__init__.py
    • superset/mcp_service/system/resources/schema_discovery.py
    • superset/mcp_service/system/tool/__init__.py
    • superset/mcp_service/system/tool/get_schema.py
    • tests/unit_tests/mcp_service/chart/tool/test_list_charts.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py
    • tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py
    • tests/unit_tests/mcp_service/system/tool/test_get_schema.py
    • tests/unit_tests/mcp_service/system/tool/test_mcp_core.py
  • Files skipped - 1
    • superset/mcp_service/CLAUDE.md - Reason: Filter setting
  • 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 feat/mcp-schema-discovery branch from 9ee83b3 to e918806 Compare January 6, 2026 19:01
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 #939d4a

Actionable Suggestions - 1
  • superset/mcp_service/mcp_core.py - 1
Additional Suggestions - 1
  • superset/mcp_service/dataset/schemas.py - 1
    • Incorrect field description · Line 217-223
      The description mentions "if not specified" but the field has default_factory, so it's always specified. This misleads users about when defaults apply.
      Code suggestion
       diff --git a/superset/mcp_service/dataset/schemas.py b/superset/mcp_service/dataset/schemas.py
       index abc123..def456 100644
      --- a/superset/mcp_service/dataset/schemas.py
      +++ b/superset/mcp_service/dataset/schemas.py
       @@ -220,7 +220,7 @@ class ListDatasetsRequest(MetadataCacheControl):
                    Field(
                        default_factory=list,
      -                description="List of columns to select. Defaults to common columns if not specified.",
      +                description="List of columns to select. If empty, defaults to common columns.",
                    ),
            ]
Review Details
  • Files reviewed - 24 · Commit Range: e918806..e918806
    • superset/mcp_service/app.py
    • superset/mcp_service/chart/schemas.py
    • superset/mcp_service/chart/tool/__init__.py
    • superset/mcp_service/chart/tool/get_chart_available_filters.py
    • superset/mcp_service/chart/tool/list_charts.py
    • superset/mcp_service/common/schema_discovery.py
    • superset/mcp_service/dashboard/schemas.py
    • superset/mcp_service/dashboard/tool/__init__.py
    • superset/mcp_service/dashboard/tool/get_dashboard_available_filters.py
    • superset/mcp_service/dashboard/tool/list_dashboards.py
    • superset/mcp_service/dataset/schemas.py
    • superset/mcp_service/dataset/tool/__init__.py
    • superset/mcp_service/dataset/tool/get_dataset_available_filters.py
    • superset/mcp_service/dataset/tool/list_datasets.py
    • superset/mcp_service/mcp_core.py
    • superset/mcp_service/system/resources/__init__.py
    • superset/mcp_service/system/resources/schema_discovery.py
    • superset/mcp_service/system/tool/__init__.py
    • superset/mcp_service/system/tool/get_schema.py
    • tests/unit_tests/mcp_service/chart/tool/test_list_charts.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py
    • tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py
    • tests/unit_tests/mcp_service/system/tool/test_get_schema.py
    • tests/unit_tests/mcp_service/system/tool/test_mcp_core.py
  • Files skipped - 2
    • superset/mcp_service/CLAUDE.md - Reason: Filter setting
    • superset/mcp_service/docs/tool-search-optimization.md - Reason: Filter setting
  • 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 feat/mcp-schema-discovery branch from e918806 to fd6e792 Compare January 6, 2026 21:01
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 #ac5719

Actionable Suggestions - 1
  • superset/mcp_service/common/schema_discovery.py - 1
    • Incorrect SQLAlchemy type mapping for date/time · Line 106-111
Additional Suggestions - 2
  • superset/mcp_service/dataset/schemas.py - 1
    • Schema default mismatch · Line 217-223
      The default_factory changed to list (empty), but the description still says 'Defaults to common columns if not specified.' This creates confusion, as the default is now empty, and the tool likely needs to handle empty as a signal to use common columns.
  • tests/unit_tests/mcp_service/system/tool/test_get_schema.py - 1
    • Incomplete test assertion · Line 334-340
      The test for chart sortable columns checks several expected values but misses 'description', which is present in the CHART_SORTABLE_COLUMNS constant. Adding this check improves test completeness.
Review Details
  • Files reviewed - 24 · Commit Range: fd6e792..fd6e792
    • superset/mcp_service/app.py
    • superset/mcp_service/chart/schemas.py
    • superset/mcp_service/chart/tool/__init__.py
    • superset/mcp_service/chart/tool/get_chart_available_filters.py
    • superset/mcp_service/chart/tool/list_charts.py
    • superset/mcp_service/common/schema_discovery.py
    • superset/mcp_service/dashboard/schemas.py
    • superset/mcp_service/dashboard/tool/__init__.py
    • superset/mcp_service/dashboard/tool/get_dashboard_available_filters.py
    • superset/mcp_service/dashboard/tool/list_dashboards.py
    • superset/mcp_service/dataset/schemas.py
    • superset/mcp_service/dataset/tool/__init__.py
    • superset/mcp_service/dataset/tool/get_dataset_available_filters.py
    • superset/mcp_service/dataset/tool/list_datasets.py
    • superset/mcp_service/mcp_core.py
    • superset/mcp_service/system/resources/__init__.py
    • superset/mcp_service/system/resources/schema_discovery.py
    • superset/mcp_service/system/tool/__init__.py
    • superset/mcp_service/system/tool/get_schema.py
    • tests/unit_tests/mcp_service/chart/tool/test_list_charts.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py
    • tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py
    • tests/unit_tests/mcp_service/system/tool/test_get_schema.py
    • tests/unit_tests/mcp_service/system/tool/test_mcp_core.py
  • Files skipped - 2
    • superset/mcp_service/CLAUDE.md - Reason: Filter setting
    • superset/mcp_service/docs/tool-search-optimization.md - Reason: Filter setting
  • 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

- Consolidate 3 separate schema discovery tools into unified get_schema tool
- Extract column metadata dynamically from SQLAlchemy models using inspect()
- Add accurate type mapping for SQLAlchemy types (datetime, date, time separately)
- Reduce hardcoded column definitions to minimal config (default/sortable/search)
- Add extra columns for computed fields with explicit is_default=False
- Update list tools to use dynamic column getters
- Add MCP resources for schema discovery via resource URIs
- Add comprehensive tests for schema validation and tool behavior
- Update schema descriptions to reference get_schema instead of removed tools
- Update documentation to reflect consolidated schema discovery
- Add columns_available and sortable_columns to test schema

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@aminghadersohi aminghadersohi force-pushed the feat/mcp-schema-discovery branch from fd6e792 to 24d9069 Compare January 6, 2026 22:26
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 #1b43c3

Actionable Suggestions - 1
  • superset/mcp_service/mcp_core.py - 1
Additional Suggestions - 11
  • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py - 4
    • API Migration · Line 559-662
      Similar API migration in another test method, replacing result.data with json.loads(result.content[0].text).
    • Schema Validation · Line 547-562
      Test confirms ListDashboardsRequest defaults select_columns to empty list, triggering minimal defaults in the tool.
    • Feature Test · Line 553-562
      Test ensures explicit select_columns can include non-default fields like 'published' and 'charts', verifying tool flexibility.
    • Test Coverage · Line 523-546
      New test validates DASHBOARD_DEFAULT_COLUMNS constant matches expected minimal set, ensuring consistency between tool defaults and test expectations.
  • superset/mcp_service/common/schema_discovery.py - 1
    • Direct Attribute Access · Line 145-145
      For SQLAlchemy columns, directly access col.comment instead of using getattr, as comment is a standard attribute on Column objects.
  • tests/unit_tests/mcp_service/system/tool/test_mcp_core.py - 3
    • no-issue (tool-verified) · Line 20-20
      Adding Dict to typing imports is appropriate as it's used later in the file for filter_columns type hint.
    • no-issue (tool-verified) · Line 25-29
      Importing ModelGetSchemaCore aligns with existing test functions that use it, as seen in the file's test definitions.
    • no-issue (tool-verified) · Line 50-51
      Adding columns_available and sortable_columns fields matches the ModelListCore output schema, ensuring tests reflect actual implementation.
  • superset/mcp_service/chart/tool/__init__.py - 2
    • Verified Import Change · Line 19-19
      The import change removes the deprecated get_chart_available_filters and adds the new get_chart_data function. Both the removal and addition are correct, as the old function no longer exists and the new one is properly defined in its module.
    • Verified __all__ Update · Line 30-31
      The __all__ list update removes the deprecated get_chart_available_filters and adds the new update_chart and update_chart_preview functions. This maintains consistency with the imports and ensures proper module exports.
  • superset/mcp_service/system/tool/get_schema.py - 1
    • Docstring Parameter Mismatch · Line 134-134
      The docstring incorrectly documents 'model_type' as a direct argument, but the function takes a 'request' parameter of type GetSchemaRequest.
Review Details
  • Files reviewed - 24 · Commit Range: 24d9069..24d9069
    • superset/mcp_service/app.py
    • superset/mcp_service/chart/schemas.py
    • superset/mcp_service/chart/tool/__init__.py
    • superset/mcp_service/chart/tool/get_chart_available_filters.py
    • superset/mcp_service/chart/tool/list_charts.py
    • superset/mcp_service/common/schema_discovery.py
    • superset/mcp_service/dashboard/schemas.py
    • superset/mcp_service/dashboard/tool/__init__.py
    • superset/mcp_service/dashboard/tool/get_dashboard_available_filters.py
    • superset/mcp_service/dashboard/tool/list_dashboards.py
    • superset/mcp_service/dataset/schemas.py
    • superset/mcp_service/dataset/tool/__init__.py
    • superset/mcp_service/dataset/tool/get_dataset_available_filters.py
    • superset/mcp_service/dataset/tool/list_datasets.py
    • superset/mcp_service/mcp_core.py
    • superset/mcp_service/system/resources/__init__.py
    • superset/mcp_service/system/resources/schema_discovery.py
    • superset/mcp_service/system/tool/__init__.py
    • superset/mcp_service/system/tool/get_schema.py
    • tests/unit_tests/mcp_service/chart/tool/test_list_charts.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py
    • tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py
    • tests/unit_tests/mcp_service/system/tool/test_get_schema.py
    • tests/unit_tests/mcp_service/system/tool/test_mcp_core.py
  • Files skipped - 2
    • superset/mcp_service/CLAUDE.md - Reason: Filter setting
    • superset/mcp_service/docs/tool-search-optimization.md - Reason: Filter setting
  • 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

@villebro villebro left a comment

Choose a reason for hiding this comment

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

Great improvement! Minor nit, but other than that LGTM.

@apache apache deleted a comment from codeant-ai-for-open-source bot Jan 7, 2026
@apache apache deleted a comment from codeant-ai-for-open-source bot Jan 7, 2026
@apache apache deleted a comment from codeant-ai-for-open-source bot Jan 7, 2026
- Use modern Python type hints (list/dict) instead of typing.List/Dict
- Replace broad Exception catch with specific exception types

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@codeant-ai-for-open-source
Copy link
Contributor

CodeAnt AI is running Incremental review


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@apache apache deleted a comment from codeant-ai-for-open-source bot Jan 7, 2026
@rusackas rusackas merged commit 84279ac into apache:master Jan 7, 2026
65 checks passed
@bito-code-review
Copy link
Contributor

Bito Automatic Review Skipped – PR Already Merged

Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.
No action is needed if you didn't intend to review it. To get a review, you can type /review in a comment and save it

@sadpandajoe
Copy link
Member

@dosubot what's the reason for the breaking change label?

@dosubot
Copy link

dosubot bot commented Jan 8, 2026

The breaking change label is applied because this PR removes the three legacy schema discovery tools (get_chart_available_filters, get_dataset_available_filters, get_dashboard_available_filters) and replaces them with a new unified get_schema tool. This is not a drop-in replacement—any integrations or code relying on the old tools will need to update to use get_schema instead. There are also changes to default API behavior: list endpoints now return only a minimal set of default columns unless select_columns is explicitly provided, which may affect downstream consumers relying on previous defaults. These removals and behavioral changes require updates from users and integrations, which is why the breaking change label is warranted.
Source: PR #36458
Superset Semantic Versioning Wiki

To reply, just mention @dosu.


How did I do? Good | Irrelevant | Incorrect | Verbose | Hallucination | Report 🐛 | Other

Chat with Dosu Open in Cursor Join Discord Share on X

JCelento pushed a commit to JCelento/superset that referenced this pull request Jan 8, 2026
…6458)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
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 risk:breaking-change Issues or PRs that will introduce breaking changes size/XXL size:XXL This PR changes 1000+ lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants