From cbae27cffaf695658daf217fbdf04e64b87c3430 Mon Sep 17 00:00:00 2001 From: Shayanide Date: Thu, 28 May 2026 00:39:05 +0000 Subject: [PATCH 1/4] Trim unsupported geography keys from audience-targeting hints --- mcp_server/server.py | 3 --- tests/test_error_mapping.py | 4 ++-- tests/test_plan_formatters.py | 21 ++++++++++++++------- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/mcp_server/server.py b/mcp_server/server.py index 9594dd2..6809d43 100644 --- a/mcp_server/server.py +++ b/mcp_server/server.py @@ -207,9 +207,6 @@ def upload_media(file_paths: list[str]) -> str: "country_name", "region", "city", - "postal", - "timezone", - "is_eu", } diff --git a/tests/test_error_mapping.py b/tests/test_error_mapping.py index 90c2d2c..50faf88 100644 --- a/tests/test_error_mapping.py +++ b/tests/test_error_mapping.py @@ -231,12 +231,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() diff --git a/tests/test_plan_formatters.py b/tests/test_plan_formatters.py index 8f705ce..5b98e2e 100644 --- a/tests/test_plan_formatters.py +++ b/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:") From 5aa986f682e927938f9fe2e979f667a8e90ea545 Mon Sep 17 00:00:00 2001 From: Shayanide Date: Thu, 28 May 2026 00:39:41 +0000 Subject: [PATCH 2/4] Render response labels in raw survey responses --- mcp_server/server.py | 4 +++- tests/test_renderers.py | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/mcp_server/server.py b/mcp_server/server.py index 6809d43..01074f3 100644 --- a/mcp_server/server.py +++ b/mcp_server/server.py @@ -860,7 +860,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) diff --git a/tests/test_renderers.py b/tests/test_renderers.py index 825628d..bd73ff2 100644 --- a/tests/test_renderers.py +++ b/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) From ea1f44cb75169471f798fb026412b8f117962ec3 Mon Sep 17 00:00:00 2001 From: Shayanide Date: Thu, 28 May 2026 00:41:08 +0000 Subject: [PATCH 3/4] Surface clearer media upload errors --- mcp_server/client.py | 7 ++++++- mcp_server/server.py | 29 +++++++++++++++++++++++++---- tests/test_client.py | 20 +++++++++++++++++++- tests/test_error_mapping.py | 20 ++++++++++++++++++++ 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/mcp_server/client.py b/mcp_server/client.py index 05c0c08..949acb7 100644 --- a/mcp_server/client.py +++ b/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() diff --git a/mcp_server/server.py b/mcp_server/server.py index 01074f3..718f27c 100644 --- a/mcp_server/server.py +++ b/mcp_server/server.py @@ -109,13 +109,34 @@ 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") + """Render a user-friendly message for a media upload error. + + 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() diff --git a/tests/test_client.py b/tests/test_client.py index 83903f5..075d5c6 100644 --- a/tests/test_client.py +++ b/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() diff --git a/tests/test_error_mapping.py b/tests/test_error_mapping.py index 50faf88..1f0934b 100644 --- a/tests/test_error_mapping.py +++ b/tests/test_error_mapping.py @@ -42,6 +42,26 @@ def test_413_without_dict_detail_falls_through(self): err = DatapointAPIError(413, "Payload Too Large") self.assertEqual(_describe_upload_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_upload_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_upload_error(err)) + + def test_invalid_svg_includes_reason(self): + err = DatapointAPIError(400, {"code": "invalid_svg", "filename": "logo.svg", "reason": "embedded script"}) + self.assertEqual(_describe_upload_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_upload_error(err)) + class UploadMediaErrorTests(unittest.TestCase): def test_413_renders_friendly_message_in_failed_block(self): From 00c8acbb11cd10efc5bccbeb26e1b2f0ae869d37 Mon Sep 17 00:00:00 2001 From: Shayanide Date: Thu, 28 May 2026 01:18:47 +0000 Subject: [PATCH 4/4] Map create_survey media rejections to clear messages --- mcp_server/server.py | 11 ++++++++--- tests/test_error_mapping.py | 35 ++++++++++++++++++++++++++--------- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/mcp_server/server.py b/mcp_server/server.py index 718f27c..3f5c5d3 100644 --- a/mcp_server/server.py +++ b/mcp_server/server.py @@ -108,8 +108,8 @@ def setup() -> str: # --------------------------------------------------------------------------- -def _describe_upload_error(e: DatapointAPIError) -> str: - """Render a user-friendly message for a media upload error. +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. @@ -181,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) @@ -502,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 = [ diff --git a/tests/test_error_mapping.py b/tests/test_error_mapping.py index 1f0934b..3a39143 100644 --- a/tests/test_error_mapping.py +++ b/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,41 +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_upload_error(err), "unsupported file type (.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_upload_error(err)) + 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_upload_error(err), "invalid SVG: 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_upload_error(err)) + self.assertIn("content review rejected this file: Depicts violence.", _describe_media_error(err)) class UploadMediaErrorTests(unittest.TestCase): @@ -338,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: