fix(mcp): fix dashboard slug null and execute_sql encoding error#38710
Merged
Antonio-RiveroMartnez merged 1 commit intoapache:masterfrom Mar 20, 2026
Conversation
- serialize_dashboard_object now returns slug="" instead of null when dashboard has no slug, matching dashboard_serializer behavior - Add _sanitize_row_values to handle Decimal, bytes, memoryview, and other non-JSON-serializable types in execute_sql query results - Add unit tests for both fixes
Contributor
Code Review Agent Run #9f7417Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
dosubot
Bot
added
api
codeant-ai-for-open-source
Bot
added
the
size:L
Contributor
Sequence DiagramThis PR standardizes MCP output formats in two core paths: dashboard serialization and SQL execution responses. It ensures missing dashboard slugs are returned as empty strings and query row values are sanitized into JSON-safe types before returning to clients. sequenceDiagram
participant Client
participant MCPService
participant DashboardSerializer
participant SQLEngine
Client->>MCPService: Request dashboard list
MCPService->>DashboardSerializer: Serialize dashboard objects
DashboardSerializer->>DashboardSerializer: Replace missing slug with empty string
DashboardSerializer-->>MCPService: Return normalized dashboard data
MCPService-->>Client: Dashboard list with consistent slug values
Client->>MCPService: Request execute sql
MCPService->>SQLEngine: Execute query
SQLEngine-->>MCPService: Return statement rows
MCPService->>MCPService: Sanitize non serializable row values
MCPService-->>Client: SQL result with JSON safe rows
Generated by CodeAnt AI |
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (99.92%) 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 #38710 +/- ##
==========================================
- Coverage 65.12% 64.31% -0.81%
==========================================
Files 1819 2532 +713
Lines 72690 129745 +57055
Branches 23235 29999 +6764
==========================================
+ Hits 47339 83449 +36110
- Misses 25351 44844 +19493
- Partials 0 1452 +1452
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
Comment on lines
+165
to
+176
| for row in rows: | ||
| for key, value in row.items(): | ||
| if isinstance(value, (bytes, memoryview)): | ||
| raw = bytes(value) if isinstance(value, memoryview) else value | ||
| try: | ||
| row[key] = raw.decode("utf-8") | ||
| except (UnicodeDecodeError, AttributeError): | ||
| row[key] = raw.hex() | ||
| elif isinstance(value, Decimal): | ||
| row[key] = float(value) | ||
| elif not isinstance(value, (str, int, float, bool, type(None), list, dict)): | ||
| row[key] = str(value) |
Contributor
There was a problem hiding this comment.
Suggestion: The sanitizer only converts top-level cell values and skips nested values inside list/dict, so rows that contain structures like {"meta": {"amount": Decimal("1.2")}} or lists with bytes can still fail JSON encoding at runtime. Sanitize recursively so nested non-serializable values are converted too. [logic error]
Severity Level: Major ⚠️
- ❌ execute_sql crashes on nested non-JSON row values.
- ⚠️ JSON/array SQL columns may return unusable MCP responses.
- ⚠️ Current tests miss nested sanitization regression coverage.
Suggested change
| for row in rows: | |
| for key, value in row.items(): | |
| if isinstance(value, (bytes, memoryview)): | |
| raw = bytes(value) if isinstance(value, memoryview) else value | |
| try: | |
| row[key] = raw.decode("utf-8") | |
| except (UnicodeDecodeError, AttributeError): | |
| row[key] = raw.hex() | |
| elif isinstance(value, Decimal): | |
| row[key] = float(value) | |
| elif not isinstance(value, (str, int, float, bool, type(None), list, dict)): | |
| row[key] = str(value) | |
| def _sanitize_value(value: Any) -> Any: | |
| if isinstance(value, (bytes, memoryview)): | |
| raw = bytes(value) if isinstance(value, memoryview) else value | |
| try: | |
| return raw.decode("utf-8") | |
| except UnicodeDecodeError: | |
| return raw.hex() | |
| if isinstance(value, Decimal): | |
| return float(value) | |
| if isinstance(value, list): | |
| return [_sanitize_value(item) for item in value] | |
| if isinstance(value, tuple): | |
| return [_sanitize_value(item) for item in value] | |
| if isinstance(value, dict): | |
| return {k: _sanitize_value(v) for k, v in value.items()} | |
| if isinstance(value, (str, int, float, bool, type(None))): | |
| return value | |
| return str(value) | |
| for row in rows: | |
| for key, value in row.items(): | |
| row[key] = _sanitize_value(value) |
Steps of Reproduction ✅
1. Call MCP tool `execute_sql` through FastMCP (same entrypoint used in
`tests/unit_tests/mcp_service/sql_lab/tool/test_execute_sql.py:163`, `213`, `305`) after
server registration in `superset/mcp_service/app.py:417-418`.
2. Execution reaches `execute_sql()` at
`superset/mcp_service/sql_lab/tool/execute_sql.py:66`, then `_convert_to_response()` at
line `179` after `database.execute(...)` at line `128`.
3. `_convert_to_response()` converts DataFrame rows (`line 197`) and calls
`_sanitize_row_values(rows_data)` (`line 198`); inside sanitizer, `list`/`dict` values are
explicitly treated as already-safe by the allowlist check at `line 175`, so nested members
are not traversed.
4. If a row contains a container like `{"meta": {"raw": b"\x00\xff"}}` or `{"vals":
[Decimal("1.2")]}`, nested values remain non-JSON-safe and response serialization fails on
the MCP return path; this is the same failure class already documented in existing
regression comments
(`tests/unit_tests/mcp_service/sql_lab/tool/test_execute_sql.py:896-897`, `956-957`) but
still reachable for nested values.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/mcp_service/sql_lab/tool/execute_sql.py
**Line:** 165:176
**Comment:**
*Logic Error: The sanitizer only converts top-level cell values and skips nested values inside `list`/`dict`, so rows that contain structures like `{"meta": {"amount": Decimal("1.2")}}` or lists with `bytes` can still fail JSON encoding at runtime. Sanitize recursively so nested non-serializable values are converted too.
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.
Antonio-RiveroMartnez
approved these changes
Mar 20, 2026
michael-s-molina
pushed a commit
that referenced
this pull request
Mar 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Summary
list_dashboardsreturnedslug: nullfor dashboards without a slug, whileget_dashboard_inforeturned"". Fixedserialize_dashboard_object()to useslug or ""for consistency.execute_sqlfailed withencoding without a string argumentwhen query results contained bytes, memoryview, Decimal, or other non-JSON-serializable types. Added_sanitize_row_values()to handle these types gracefully.BEFORE/AFTER
Before:
list_dashboardsreturns"slug": nullfor dashboards without slugs;execute_sqlcrashes on non-serializable column types.After: Slugs return as
""consistently; all column types are safely serialized to JSON-compatible values.TESTING INSTRUCTIONS
list_dashboardsvia MCP - verify nonullslugsexecute_sqlwith a query that returns binary/Decimal columns - verify no encoding errorADDITIONAL INFORMATION
CodeAnt-AI Description
Fix dashboard slug null and prevent execute_sql JSON encoding errors
What Changed
Impact
✅ Consistent dashboard slugs in API responses (no nulls)✅ No execute_sql crashes on binary or Decimal columns✅ Stable JSON serialization for query results💡 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:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
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:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
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.