Skip to content

chore(mcp): Simplify chart preview response#40020

Open
alexandrusoare wants to merge 1 commit into
masterfrom
alexandrusoare/chore/simplify-chart-preview
Open

chore(mcp): Simplify chart preview response#40020
alexandrusoare wants to merge 1 commit into
masterfrom
alexandrusoare/chore/simplify-chart-preview

Conversation

@alexandrusoare
Copy link
Copy Markdown
Contributor

SUMMARY

The get_chart_preview response duplicates data — the same ASCII/table content appears in both content.ascii_content (or content.table_data) AND in flat top-level fields (ascii_chart, table_data). Both copies also get sanitized independently, wasting work.

Solution:
Remove the redundant flat fields (format, ascii_chart, table_data, width, height) from ChartPreview. Clients should read from the content discriminated union, which already has all of this data via ASCIIPreview, TablePreview, etc.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

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

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 11, 2026

Code Review Agent Run #ca749c

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: 46167ac..46167ac
    • superset/mcp_service/chart/schemas.py
    • superset/mcp_service/chart/tool/get_chart_preview.py
    • tests/unit_tests/mcp_service/chart/tool/test_get_chart_preview.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

@netlify
Copy link
Copy Markdown

netlify Bot commented May 11, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 46167ac
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a01c8fd4192d1000767bdc2
😎 Deploy Preview https://deploy-preview-40020--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

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

@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 63.84%. Comparing base (f67dd4a) to head (46167ac).
⚠️ Report is 67 commits behind head on master.

Files with missing lines Patch % Lines
...perset/mcp_service/chart/tool/get_chart_preview.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master   #40020   +/-   ##
=======================================
  Coverage   63.83%   63.84%           
=======================================
  Files        2589     2589           
  Lines      137821   137802   -19     
  Branches    31928    31924    -4     
=======================================
- Hits        87978    87973    -5     
+ Misses      48327    48313   -14     
  Partials     1516     1516           
Flag Coverage Δ
hive 39.37% <0.00%> (+<0.01%) ⬆️
mysql 59.02% <0.00%> (+<0.01%) ⬆️
postgres 59.10% <0.00%> (+0.01%) ⬆️
presto 41.06% <0.00%> (+<0.01%) ⬆️
python 60.54% <0.00%> (+0.01%) ⬆️
sqlite 58.74% <0.00%> (+<0.01%) ⬆️
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.

@msyavuz msyavuz added the hold:testing! On hold for testing label May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hold:testing! On hold for testing size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants