Skip to content

fix(mcp): changed_on_humanized null in write tool responses (generate_dashboard, generate_chart)#39488

Merged
alexandrusoare merged 3 commits into
masterfrom
alexandrusoare/fix/humanized_response_for_mcp
May 20, 2026
Merged

fix(mcp): changed_on_humanized null in write tool responses (generate_dashboard, generate_chart)#39488
alexandrusoare merged 3 commits into
masterfrom
alexandrusoare/fix/humanized_response_for_mcp

Conversation

@alexandrusoare
Copy link
Copy Markdown
Contributor

SUMMARY

generate_dashboard and generate_chart responses return changed_on_humanized=null and created_on_humanized=null for newly created resources, while list tools (list_dashboards, list_charts) show correct humanized values for the same resources.

Root Cause:
Two issues:

  1. generate_dashboard: The DashboardInfo constructor was setting created_on and changed_on but never computing the created_on_humanized / changed_on_humanized fields, so they defaulted to None. In contrast, serialize_dashboard_object() (used by list_dashboards) calls _humanize_timestamp() to compute these values.
  2. Both tools: After db.session.commit(), SQLAlchemy expires the ORM object's attributes. Server-generated timestamps (created_on, changed_on) were not reloaded before serialization, so they could be None or stale.

Fix:

  • generate_dashboard.py: Added db.session.refresh(dashboard) after commit to reload timestamps. Added created_on_humanized and changed_on_humanized fields to the DashboardInfo constructor using _humanize_timestamp().
  • generate_chart.py: Added db.session.refresh(chart) after chart creation to ensure timestamps are populated before serialize_chart_object() runs (which already computes humanized values).
  • Test fix: Updated mock dashboard timestamps from strings to datetime objects to match real ORM behavior.

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 Apr 20, 2026

Code Review Agent Run #1d2478

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: fbaaab0..fbaaab0
    • superset/mcp_service/chart/tool/generate_chart.py
    • superset/mcp_service/dashboard/tool/generate_dashboard.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.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 dashboard Namespace | Anything related to the Dashboard labels Apr 20, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

❌ Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.71%. Comparing base (a06e6ea) to head (35024fa).

Files with missing lines Patch % Lines
superset/mcp_service/chart/tool/generate_chart.py 0.00% 5 Missing ⚠️
...t/mcp_service/dashboard/tool/generate_dashboard.py 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #39488      +/-   ##
==========================================
- Coverage   64.16%   63.71%   -0.46%     
==========================================
  Files        2591     2581      -10     
  Lines      138127   136283    -1844     
  Branches    32046    31309     -737     
==========================================
- Hits        88635    86827    -1808     
+ Misses      47964    47928      -36     
  Partials     1528     1528              
Flag Coverage Δ
hive 39.46% <0.00%> (-0.01%) ⬇️
mysql 59.16% <0.00%> (-0.01%) ⬇️
postgres 59.24% <0.00%> (-0.01%) ⬇️
presto 41.15% <0.00%> (-0.01%) ⬇️
python 60.67% <0.00%> (-0.01%) ⬇️
sqlite 58.88% <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.

Comment thread superset/mcp_service/chart/tool/generate_chart.py Outdated
Copy link
Copy Markdown
Member

@msyavuz msyavuz left a comment

Choose a reason for hiding this comment

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

Small nit

Comment thread superset/mcp_service/chart/tool/generate_chart.py Outdated
@msyavuz msyavuz added the hold:testing! On hold for testing label Apr 21, 2026
@pull-request-size pull-request-size Bot added size/M and removed size/S labels Apr 21, 2026
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 21, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 1fec6d5
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/69e73b87ea072a0008857e42
😎 Deploy Preview https://deploy-preview-39488--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.

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Apr 21, 2026

Code Review Agent Run #514b80

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: fbaaab0..1fec6d5
    • superset/mcp_service/chart/tool/generate_chart.py
    • superset/mcp_service/dashboard/tool/generate_dashboard.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

…andrusoare/fix/humanized_response_for_mcp
@alexandrusoare alexandrusoare merged commit 0a3a350 into master May 20, 2026
65 of 66 checks passed
@alexandrusoare alexandrusoare deleted the alexandrusoare/fix/humanized_response_for_mcp branch May 20, 2026 11:08
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 dashboard Namespace | Anything related to the Dashboard hold:testing! On hold for testing size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants