diff --git a/test_mcp/test_tool_input_robustness.py b/test_mcp/test_tool_input_robustness.py index 97d56e7..91fd974 100644 --- a/test_mcp/test_tool_input_robustness.py +++ b/test_mcp/test_tool_input_robustness.py @@ -533,6 +533,54 @@ async def test_create_data_table_rejects_empty_data(client): ) +async def test_create_plotly_chart_malformed_json_string_carries_fix_hint(client): + """Regression for the 2026-05-18 200s incident: LLM emits `data` as a + malformed JSON-string-literal (e.g. stray comma mid-array), tool body + rejects on json.loads. The rejection envelope now carries a `fix_hint` + telling the LLM to retry with the STRUCTURED array form instead of + the stringified form — biasing the next attempt toward the schema's + primary type and away from the error-prone string arm. + """ + async with client: + result = await client.call_tool( + "create_plotly_chart", + # Malformed JSON string — stray comma after the closing brace. + {"data": '[{"x":[1,2,3],"y":[4,5,6]},,]'}, + ) + payload = _structured(result) + assert payload.get("error", "").startswith("invalid_args:"), payload + assert "is not valid JSON" in payload["error"], payload + fix_hint = payload.get("fix_hint", "") + # Hint must name the structured form as the recommended retry path. + assert "STRUCTURED" in fix_hint or "structured" in fix_hint, ( + f"fix_hint must recommend structured form; got {fix_hint!r}" + ) + # Hint must mention the error-prone nature of stringified arrays. + assert "string" in fix_hint.lower(), ( + f"fix_hint should reference the string form being error-prone; got {fix_hint!r}" + ) + + +async def test_create_data_table_malformed_json_string_carries_fix_hint(client): + """Companion to the create_plotly_chart test — same envelope shape + on create_data_table when the LLM emits a malformed JSON string for + `data`. Pins parity across both create_* tools that take a Union + list-or-string for `data`. + """ + async with client: + result = await client.call_tool( + "create_data_table", + {"data": '[{"col":1},{"col":2'}, # Unclosed bracket / missing closing. + ) + payload = _structured(result) + assert payload.get("error", "").startswith("invalid_args:"), payload + assert "is not valid JSON" in payload["error"], payload + fix_hint = payload.get("fix_hint", "") + assert "STRUCTURED" in fix_hint or "structured" in fix_hint, ( + f"fix_hint must recommend structured form; got {fix_hint!r}" + ) + + async def test_create_plotly_chart_rejects_tiny_h(client): """`h=5` is below the practical minimum (~10 grid units ≈ 50-100px tall, already squished). Pydantic ge=10 enforces the floor.""" diff --git a/tethysdash_mcp/mcp_server.py b/tethysdash_mcp/mcp_server.py index 172ccf0..2eb5e93 100644 --- a/tethysdash_mcp/mcp_server.py +++ b/tethysdash_mcp/mcp_server.py @@ -325,7 +325,12 @@ def create_plotly_chart( "'lines+markers'). MUST contain at least one trace — do NOT " "call this with `data=[]`. If a data-source tool failed or " "returned no rows, ABORT and report the data-fetch error to " - "the user; do NOT fall back to creating an empty chart." + "the user; do NOT fall back to creating an empty chart. " + "STRONGLY PREFER passing `data` as a structured array (the " + "primary type) — emitting valid JSON inside a stringified " + "array form is error-prone for long traces and silently " + "doubles latency on parse failure. Use the string form ONLY " + "if your runtime cannot emit nested arrays." ), min_length=1, ), @@ -371,7 +376,17 @@ def create_plotly_chart( try: data = json.loads(data) except json.JSONDecodeError as e: - return {"error": f"invalid_args: `data` is not valid JSON: {e}"} + return { + "error": f"invalid_args: `data` is not valid JSON: {e}", + "fix_hint": ( + "On retry, pass `data` as a STRUCTURED array directly " + "(e.g. data: [{...trace...}, ...]) rather than as a " + "JSON-stringified array. The string form is error-prone " + "for long traces — token-by-token generation of nested " + "JSON commonly drifts mid-array. The structured form is " + "what the tool's input schema accepts as the primary type." + ), + } # Server-side defense: Pydantic min_length=1 catches `data=[]` at the # input level, but `data="[]"` (JSON string of empty array) passes @@ -429,12 +444,16 @@ def create_data_table( Field( description=( "Array of row objects. Each dict maps column names to cell " - "values; all rows must share the same keys. May be passed " - "as a JSON-string array too. MUST contain at least one row " - "— do NOT call this with `data=[]`. If a data-source tool " - "failed or returned no rows, ABORT and report the data-fetch " - "error to the user; do NOT fall back to creating an empty " - "table." + "values; all rows must share the same keys. MUST contain at " + "least one row — do NOT call this with `data=[]`. If a " + "data-source tool failed or returned no rows, ABORT and " + "report the data-fetch error to the user; do NOT fall back " + "to creating an empty table. " + "STRONGLY PREFER passing `data` as a structured array (the " + "primary type) — emitting valid JSON inside a stringified " + "array form is error-prone for long row sets and silently " + "doubles latency on parse failure. Use the string form ONLY " + "if your runtime cannot emit nested arrays." ), min_length=1, ), @@ -474,7 +493,16 @@ def create_data_table( try: data = json.loads(data) except json.JSONDecodeError as e: - return {"error": f"invalid_args: `data` is not valid JSON: {e}"} + return { + "error": f"invalid_args: `data` is not valid JSON: {e}", + "fix_hint": ( + "On retry, pass `data` as a STRUCTURED array directly " + "(e.g. data: [{...row...}, ...]) rather than as a " + "JSON-stringified array. The string form is error-prone " + "for long row sets — token-by-token generation of " + "nested JSON commonly drifts mid-array." + ), + } # Server-side defense: Pydantic min_length catches data=[] but not # data="[]" (JSON string of empty array) — re-check after json.loads.