fix(mcp): broaden tagged-union cleanup and drop chart-type-specific hint

Addresses review feedback on #39522:

1. ``_sanitize_validation_error`` only stripped the Pydantic
   ``tagged-union[...]`` header when the body began with ``Value error,``.
   Other common Pydantic failure bodies — ``Input should be ...`` from
   literal enums, ``String should match pattern ...``, etc. — still kept
   the long header and got truncated before the actionable part. Match the
   first two-space-indented body line instead, which is Pydantic's
   universal separator between the tagged-union header and the per-field
   message. Works for every body style without a keyword allowlist.

2. The ``validation_error`` template fires for every chart type, so the
   ``primary_kind`` / ``secondary_kind`` suggestion was misleading for
   non-mixed_timeseries failures. Replace it with chart-type-agnostic
   guidance that points callers at the valid-fields list already rendered
   in the error details.

Add regression tests for the non-``Value error`` body case (invalid
aggregate enum on a pie chart) and for the suggestion's chart-type
agnosticism.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: EnxDev

superset/mcp_service/chart/validation/pipeline.py

@@ -81,13 +81,19 @@ def _sanitize_validation_error(error: Exception) -> str:
 
     # Pydantic tagged-union errors prefix the message with a long
     # ``1 validation error for tagged-union[...]`` header before the
-    # actionable ``Value error, ...`` body — extract the body so the
-    # 200-char truncation below doesn't swallow the useful part.
-    if "tagged-union[" in error_str and "Value error," in error_str:
-        idx = error_str.find("Value error,")
-        footer = error_str.find("\n    For further information")
-        end = footer if footer != -1 else len(error_str)
-        error_str = error_str[idx:end].strip()
+    # per-field body (e.g. ``Value error, ...``, ``Field required``,
+    # ``Input should be ...``). The body always lives on a line indented
+    # by exactly two spaces — pull it out so the 200-char truncation
+    # below doesn't swallow the actionable part. The pydantic footer
+    # ``\n    For further information ...`` uses four-space indent and
+    # is dropped here.
+    if "tagged-union[" in error_str:
+        body_match = re.search(r"\n  (?! )", error_str)
+        if body_match:
+            idx = body_match.end()
+            footer_idx = error_str.find("\n    For further information", idx)
+            end = footer_idx if footer_idx != -1 else len(error_str)
+            error_str = error_str[idx:end].strip()
 
     # SECURITY FIX: Limit length FIRST to prevent ReDoS attacks
     if len(error_str) > 200:

superset/mcp_service/utils/error_builder.py

@@ -194,8 +194,8 @@ class ChartErrorBuilder:
                 "chart_type's schema",
                 "Call get_chart_type_schema or read the chart://configs resource "
                 "for valid fields and examples",
-                "For mixed_timeseries charts, use 'primary_kind' and "
-                "'secondary_kind' (not 'kind' / 'kind_secondary')",
+                "Replace any unknown field names with the ones listed in the "
+                "error details above",
             ],
         },
     }

tests/unit_tests/mcp_service/chart/validation/test_pipeline_error_surface.py

@@ -138,3 +138,58 @@ def test_valid_mixed_timeseries_config_passes() -> None:
 
     assert result.is_valid is True
     assert result.error is None
+
+
+def test_non_value_error_pydantic_body_is_surfaced() -> None:
+    """The tagged-union cleanup must also handle non-``Value error`` bodies
+    like ``Input should be ...`` (literal_error from Pydantic enums).
+
+    Regression for the case where the sanitizer only matched ``Value error,``
+    and left the long tagged-union header attached, causing the 200-char
+    truncation to swallow the actionable message.
+    """
+    # Invalid aggregate value — triggers a Pydantic literal_error whose body
+    # starts with ``Input should be 'SUM', 'COUNT', ...``, not ``Value error,``.
+    request_data = {
+        "dataset_id": 1,
+        "config": {
+            "chart_type": "pie",
+            "dimension": {"name": "product"},
+            "metric": {"name": "revenue", "aggregate": "BOGUS"},
+        },
+    }
+
+    with (
+        patch.object(ValidationPipeline, "_get_dataset_context", return_value=None),
+        patch.object(
+            ValidationPipeline, "_validate_dataset", return_value=(True, None)
+        ),
+        patch.object(
+            ValidationPipeline, "_validate_runtime", return_value=(True, None)
+        ),
+        patch.object(
+            ValidationPipeline,
+            "_normalize_column_names",
+            side_effect=_passthrough_normalize,
+        ),
+    ):
+        result = ValidationPipeline.validate_request_with_warnings(request_data)
+
+    assert result.is_valid is False
+    assert result.error is not None
+    dumped = result.error.model_dump()
+    assert dumped["error_code"] == "VALIDATION_PIPELINE_ERROR"
+    assert "tagged-union" not in dumped["message"]
+    assert "tagged-union" not in dumped["details"]
+    # The actionable body must survive the 200-char truncation.
+    assert "Input should be" in dumped["details"]
+
+
+def test_validation_error_template_suggestions_are_chart_type_agnostic() -> None:
+    """The ``validation_error`` template fires for every chart type, so its
+    suggestions must not mention specific chart-type field names."""
+    template = ChartErrorBuilder.TEMPLATES["validation_error"]
+    joined = " ".join(template["suggestions"]).lower()
+    assert "mixed_timeseries" not in joined
+    assert "primary_kind" not in joined
+    assert "secondary_kind" not in joined