(superset) 01/01: test(mcp): restore informational query-result envelope coverage

[mi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

aminghadersohi pushed a commit to branch sc-119162-mcp-sunburst
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 456177777b544d8b4176d6d8d575df11e8e2dda9
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Fri Sep 18 10:12:27 2026 +0000

    test(mcp): restore informational query-result envelope coverage
    
    The master merge resolved tests/unit_tests/mcp_service/chart/test_query_result.py
    as an add/add conflict by taking the branch side wholesale, which dropped
    test_query_result_failure_allows_valid_and_informational_envelopes and the
    expanded query_result_failure docstring.
    
    Restore both, reconciled with the stricter envelope contract. The claim the
    dropped test defended -- that a successful status carrying an informational
    message is not a failure -- still holds, so it comes back with a real query
    payload. The one payload that no longer passes, a success status with an
    empty queries list, now fails closed: query_result_failure is a strict
    envelope validator rather than only an embedded-failure detector, and every
    caller reads data from at least one query. That behaviour gets its own test
    so the contract is pinned rather than silently changed.
---
 superset/mcp_service/chart/query_result.py         | 13 +++++++-
 .../mcp_service/chart/test_query_result.py         | 39 ++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/superset/mcp_service/chart/query_result.py b/superset/mcp_service/chart/query_result.py
index ce8243143df..e997971be10 100644
--- a/superset/mcp_service/chart/query_result.py
+++ b/superset/mcp_service/chart/query_result.py
@@ -1428,7 +1428,18 @@ def validate_query_result_envelope(  # noqa: C901
 
 
 def query_result_failure(result: Any) -> ChartError | None:
-    """Return an embedded failure or malformed-envelope error."""
+    """Return a structured failure embedded in a ChartDataCommand payload.
+
+    ChartDataCommand can return an HTTP-successful envelope whose top level or
+    any query reports a failure. Every query is inspected before callers accept
+    data from the result. Successful statuses may carry informational messages,
+    so ``message`` alone is not treated as an error.
+
+    Envelope shape is validated just as strictly as embedded failures: an
+    envelope carrying no query payload is rejected as malformed rather than
+    reported as failure-free, because every caller reads data from at least one
+    query.
+    """
     return validate_query_result_envelope(result)
 
 
diff --git a/tests/unit_tests/mcp_service/chart/test_query_result.py b/tests/unit_tests/mcp_service/chart/test_query_result.py
index 78d91696ac8..c0e1c43fd2e 100644
--- a/tests/unit_tests/mcp_service/chart/test_query_result.py
+++ b/tests/unit_tests/mcp_service/chart/test_query_result.py
@@ -258,6 +258,45 @@ def test_query_result_failure_rejects_arbitrary_status_enum_without_hooks():
     assert failure.error_type == "InvalidQueryResult"
 
 
+@pytest.mark.parametrize(
+    "payload",
+    [
+        {
+            "status": "success",
+            "message": "served from cache",
+            "queries": [{"status": "success", "data": []}],
+        },
+        {
+            "queries": [
+                {"status": QueryStatus.SUCCESS, "message": "no rows", "data": []}
+            ]
+        },
+        {"queries": [{"status": "running", "message": "in progress", "data": []}]},
+        {"queries": [{"message": "informational", "data": []}]},
+        {"queries": [{"data": []}]},
+    ],
+)
+def test_query_result_failure_allows_valid_and_informational_envelopes(payload):
+    """Successful statuses may carry a message; ``message`` alone is no failure."""
+    assert query_result_failure(payload) is None
+
+
+def test_query_result_failure_rejects_envelope_without_any_query():
+    """Envelope shape is validated as strictly as embedded failures.
+
+    ``query_result_failure`` is a strict envelope validator, not only an
+    embedded-failure detector. Every caller reads data from at least one query,
+    so a success status carrying no query payload fails closed instead of being
+    reported as failure-free.
+    """
+    failure = query_result_failure(
+        {"status": "success", "message": "served from cache", "queries": []}
+    )
+
+    assert failure is not None
+    assert failure.error_type == "InvalidQueryResult"
+
+
 @pytest.mark.parametrize(
     "payload",
     [