Improve response, audience-hint, and upload-error rendering

mcp_server/client.py

@@ -156,7 +156,12 @@ def upload_media(self, file_path: str) -> dict:
             )
 
         if resp.status_code >= 400:
-            raise DatapointAPIError(resp.status_code, resp.text)
+            try:
+                body = resp.json()
+                detail = body.get("detail", body.get("message", resp.text))
+            except Exception:
+                detail = resp.text
+            raise DatapointAPIError(resp.status_code, detail)
 
         return resp.json()
 

mcp_server/server.py

@@ -108,14 +108,35 @@ def setup() -> str:
 # ---------------------------------------------------------------------------
 
 
-def _describe_upload_error(e: DatapointAPIError) -> str:
-    """Render a user-friendly message for a media upload error."""
-    if e.status_code == 413 and isinstance(e.detail, dict) and e.detail.get("code") == "media_too_large":
-        max_bytes = e.detail.get("max_bytes")
+def _describe_media_error(e: DatapointAPIError) -> str:
+    """Render a user-friendly message for a media rejection (upload or create).
+
+    The server returns a structured ``{"code": ...}`` body for media rejections;
+    map the known codes to short messages and fall back to the raw detail.
+    """
+    detail = e.detail
+    if not isinstance(detail, dict):
+        return str(detail)
+
+    code = detail.get("code")
+    if code == "media_too_large":
+        max_bytes = detail.get("max_bytes")
         if max_bytes:
             return f"file exceeds the upload cap ({max_bytes / 1_048_576:.0f} MB max)"
         return "file exceeds the upload cap"
-    return str(e.detail)
+    if code == "unsupported_media_extension":
+        ext = detail.get("extension")
+        return f"unsupported file type ({ext})" if ext else "unsupported file type"
+    if code == "media_type_mismatch":
+        return "file contents do not match its extension"
+    if code == "invalid_svg":
+        reason = detail.get("reason")
+        return f"invalid SVG: {reason}" if reason else "invalid SVG"
+    if code == "content_blocked":
+        reason = detail.get("reason") or "content violates platform policy"
+        return f"content review rejected this file: {reason}"
+    # Unknown structured error — prefer a human field over dumping the raw dict.
+    return str(detail.get("message") or detail.get("reason") or detail)
 
 
 @mcp.tool()
@@ -160,7 +181,7 @@ def upload_media(file_paths: list[str]) -> str:
         except FileNotFoundError as e:
             errors.append(f"{path}: {e}")
         except DatapointAPIError as e:
-            errors.append(f"{path}: {_describe_upload_error(e)}")
+            errors.append(f"{path}: {_describe_media_error(e)}")
 
     total = len(file_paths)
     files_failed = len(errors)
@@ -207,9 +228,6 @@ def upload_media(file_paths: list[str]) -> str:
     "country_name",
     "region",
     "city",
-    "postal",
-    "timezone",
-    "is_eu",
 }
 
 
@@ -484,6 +502,11 @@ def create_survey(plan: dict) -> str:
             return msg
         if e.status_code == 503:
             return f"Service temporarily unavailable: {e.detail}"
+        if isinstance(e.detail, dict):
+            # Structured rejection (e.g. unsupported_media_extension,
+            # media_type_mismatch): render it like an upload error instead of
+            # dumping the raw dict, which would leak the rejected media URL.
+            return f"Couldn't create the survey: {_describe_media_error(e)}"
         return f"Error creating survey: {e.detail}"
 
     lines = [
@@ -863,7 +886,9 @@ def _format_response_row(r: dict) -> str:
     """Render one raw-response row as a single chat-display string."""
     annotator = (r.get("annotator_id") or "?")[:8]
     timestamp = r.get("timestamp") or "?"
-    response_text = r.get("response")
+    # `response_label` is the backend's display form (e.g. a multiple-choice
+    # opt-id resolved to its option text); fall back to the raw `response`.
+    response_text = r.get("response_label") or r.get("response")
     rt_ms = r.get("response_time_ms")
     rt_str = f" ({rt_ms / 1000:.1f}s)" if rt_ms is not None else ""
     location = _format_annotator_location(r)

tests/test_client.py

@@ -6,10 +6,11 @@
 
 from __future__ import annotations
 
+import tempfile
 import unittest
 from unittest import mock
 
-from mcp_server.client import DatapointClient
+from mcp_server.client import DatapointAPIError, DatapointClient
 
 
 def _make_client() -> DatapointClient:
@@ -67,5 +68,22 @@ def test_cancel_job_returns_request_payload(self):
         self.assertEqual(result, payload)
 
 
+class UploadMediaErrorParsingTests(unittest.TestCase):
+    def test_structured_error_body_becomes_dict_detail(self):
+        """upload_media must parse a JSON error body into a dict detail so the
+        renderer's structured-error branches fire (it used to pass raw text)."""
+        client = _make_client()
+        resp = mock.Mock(status_code=413)
+        resp.json.return_value = {"detail": {"code": "media_too_large", "max_bytes": 20971520}}
+        with tempfile.NamedTemporaryFile(suffix=".mp4") as tf:
+            tf.write(b"data")
+            tf.flush()
+            with mock.patch.object(client._http, "post", return_value=resp):
+                with self.assertRaises(DatapointAPIError) as ctx:
+                    client.upload_media(tf.name)
+        self.assertEqual(ctx.exception.status_code, 413)
+        self.assertEqual(ctx.exception.detail, {"code": "media_too_large", "max_bytes": 20971520})
+
+
 if __name__ == "__main__":
     unittest.main()

tests/test_error_mapping.py

@@ -12,7 +12,7 @@
 
 from mcp_server.client import DatapointAPIError
 from mcp_server.server import (
-    _describe_upload_error,
+    _describe_media_error,
     cancel_survey,
     check_balance,
     check_survey,
@@ -26,21 +26,41 @@ class DescribeUploadErrorTests(unittest.TestCase):
     def test_413_media_too_large_renders_human_cap(self):
         err = DatapointAPIError(413, {"code": "media_too_large", "max_bytes": 20 * 1024 * 1024})
         self.assertEqual(
-            _describe_upload_error(err),
+            _describe_media_error(err),
             "file exceeds the upload cap (20 MB max)",
         )
 
     def test_413_without_max_bytes_falls_back(self):
         err = DatapointAPIError(413, {"code": "media_too_large"})
-        self.assertEqual(_describe_upload_error(err), "file exceeds the upload cap")
+        self.assertEqual(_describe_media_error(err), "file exceeds the upload cap")
 
     def test_other_error_falls_through_to_detail_string(self):
         err = DatapointAPIError(500, "internal error")
-        self.assertEqual(_describe_upload_error(err), "internal error")
+        self.assertEqual(_describe_media_error(err), "internal error")
 
     def test_413_without_dict_detail_falls_through(self):
         err = DatapointAPIError(413, "Payload Too Large")
-        self.assertEqual(_describe_upload_error(err), "Payload Too Large")
+        self.assertEqual(_describe_media_error(err), "Payload Too Large")
+
+    def test_unsupported_extension_names_the_type(self):
+        err = DatapointAPIError(
+            400, {"code": "unsupported_media_extension", "filename": "a.heic", "extension": ".heic"}
+        )
+        self.assertEqual(_describe_media_error(err), "unsupported file type (.heic)")
+
+    def test_media_type_mismatch_is_explained(self):
+        err = DatapointAPIError(
+            400, {"code": "media_type_mismatch", "extension": ".png", "declared": "image/gif"}
+        )
+        self.assertIn("do not match", _describe_media_error(err))
+
+    def test_invalid_svg_includes_reason(self):
+        err = DatapointAPIError(400, {"code": "invalid_svg", "filename": "logo.svg", "reason": "embedded script"})
+        self.assertEqual(_describe_media_error(err), "invalid SVG: embedded script")
+
+    def test_content_blocked_includes_reason(self):
+        err = DatapointAPIError(422, {"code": "content_blocked", "reason": "Depicts violence."})
+        self.assertIn("content review rejected this file: Depicts violence.", _describe_media_error(err))
 
 
 class UploadMediaErrorTests(unittest.TestCase):
@@ -231,12 +251,12 @@ def test_renders_targeting_when_filter_present(self):
     def test_renders_distribution_when_present(self):
         client = mock.Mock()
         client.get_job_status.return_value = self._status(
-            annotator_distribution=["country", "is_eu"],
+            annotator_distribution=["country", "region"],
         )
         client.get_job_results.return_value = {"results": [], "task_type": "comparison"}
         with mock.patch("mcp_server.server._get_client", return_value=client):
             out = check_survey("job_x")
-        self.assertIn("Balanced by: country, is_eu", out)
+        self.assertIn("Balanced by: country, region", out)
 
     def test_renders_response_options_when_present(self):
         client = mock.Mock()
@@ -318,6 +338,23 @@ def test_402_insufficient_balance_renders_credit_amounts(self):
         self.assertIn("Need 500 credits", out)
         self.assertIn("have 150 credits", out)
 
+    def test_400_media_rejection_is_friendly_and_hides_url(self):
+        err = DatapointAPIError(
+            400,
+            {
+                "code": "unsupported_media_extension",
+                "location": "datapoints[0].media",
+                "url": "https://example.com/secret-asset.heic",
+                "extension": ".heic",
+            },
+        )
+        client = self._client_raising(err)
+        with mock.patch("mcp_server.server._get_client", return_value=client):
+            out = create_survey({"datapoints": [], "task_type": "comparison"})
+        self.assertIn("unsupported file type (.heic)", out)
+        self.assertNotIn("secret-asset", out)
+        self.assertNotIn("https://", out)
+
 
 class CheckBalanceTests(unittest.TestCase):
     def _balance_dict(self) -> dict:

tests/test_plan_formatters.py

@@ -150,13 +150,13 @@ def test_filter_with_multiple_columns_joined_with_semicolons(self):
         self.assertIn(";", out[0])
 
     def test_distribution_only(self):
-        out = _format_audience_targeting({"annotator_distribution": ["country", "is_eu"]})
-        self.assertEqual(out, ["Balanced by: country, is_eu"])
+        out = _format_audience_targeting({"annotator_distribution": ["country", "region"]})
+        self.assertEqual(out, ["Balanced by: country, region"])
 
     def test_filter_and_distribution_both_rendered(self):
         out = _format_audience_targeting(
             {
-                "annotator_filter": {"is_eu": [True]},
+                "annotator_filter": {"region": ["CA"]},
                 "annotator_distribution": ["country"],
             }
         )
@@ -178,7 +178,7 @@ def test_privacy_columns_marked_limited_support(self):
 
     def test_supported_filter_columns_have_no_annotation(self):
         out = _format_audience_targeting(
-            {"annotator_filter": {"country": ["US"], "is_eu": [True]}}
+            {"annotator_filter": {"country": ["US"], "region": ["CA"]}}
         )
         self.assertNotIn("(limited support)", out[0])
 
@@ -197,8 +197,15 @@ def test_distribution_marks_unsupported_columns_only(self):
         self.assertEqual(out, ["Balanced by: country, asn (limited support)"])
 
     def test_supported_distribution_columns_have_no_annotation(self):
-        out = _format_audience_targeting({"annotator_distribution": ["country", "is_eu"]})
-        self.assertEqual(out, ["Balanced by: country, is_eu"])
+        out = _format_audience_targeting({"annotator_distribution": ["country", "region"]})
+        self.assertEqual(out, ["Balanced by: country, region"])
+
+    def test_dropped_geo_keys_marked_limited_support(self):
+        """is_eu / postal / timezone were removed from the client-supported set,
+        so a hand-edited plan using them must surface the limited-support hint."""
+        for key, val in (("postal", ["94107"]), ("timezone", ["PST"]), ("is_eu", [True])):
+            out = _format_audience_targeting({"annotator_filter": {key: val}})
+            self.assertIn("(limited support)", out[0], key)
 
 
 class StandalonePlanWithTargetingTests(unittest.TestCase):
@@ -223,7 +230,7 @@ def test_standalone_omits_targeting_when_absent(self):
 
 class ChainPlanWithTargetingTests(unittest.TestCase):
     def test_chain_renders_targeting_above_chain_structure(self):
-        plan = _make_chain_plan(annotator_filter={"is_eu": [True]})
+        plan = _make_chain_plan(annotator_filter={"region": ["CA"]})
         out = "\n".join(_format_chain_plan_output(plan, "s", 3, []))
         targeting_pos = out.index("Targeting:")
         structure_pos = out.index("Chain structure:")

tests/test_renderers.py

@@ -269,6 +269,21 @@ def test_includes_annotator_timestamp_response(self):
         self.assertIn("'A'", out)
         self.assertIn("(4.8s)", out)
 
+    def test_prefers_response_label_over_raw_response(self):
+        row = {
+            "annotator_id": "anon_x",
+            "timestamp": "t",
+            "response": "opt_2",
+            "response_label": "Strongly agree",
+        }
+        out = _format_response_row(row)
+        self.assertIn("Strongly agree", out)
+        self.assertNotIn("opt_2", out)
+
+    def test_falls_back_to_raw_response_when_label_absent(self):
+        out = _format_response_row({"annotator_id": "anon_x", "timestamp": "t", "response": "A"})
+        self.assertIn("'A'", out)
+
     def test_response_time_converted_to_seconds(self):
         out = _format_response_row({"response_time_ms": 12345, "response": "x"})
         self.assertIn("(12.3s)", out)