From 924a17021e23b5bc19aac82b48d5f8c9fb42dce2 Mon Sep 17 00:00:00 2001 From: romer8 Date: Mon, 18 May 2026 20:20:50 -0600 Subject: [PATCH 1/2] feat(tools): add data_uri opt-in to create_plotly_chart, create_data_table, create_card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plan: docs/plans/2026-05-18-002-feat-mcp-result-by-reference-protocol-plan.md (in the firoh workspace) — Unit 5. Receiving side of chatbox-core's MCP result-by-reference protocol. The three create_* tools that take inline `data` arrays gain an optional `data_uri: str | list[str]` arg. After chatbox-core's substitution layer (engine/uri-substitution.js, plan Unit 3) resolves the URI from its IndexedDB cache, the server sees the call as if the LLM had passed `data` directly — no MCP wire-contract change for the mediated path. Two new pieces: 1. `tethysdash_mcp/_uri_field.py` — shared Pydantic Field-factory for the `*_uri` opt-in pattern. Provides: - `uri_field(inline_arg_name=...)` builds the standard Annotated metadata: scalar OR list of strings, regex-validated against the `mcp+cache:///` shape, max_length=128. - `ensure_exactly_one_set(inline_value, inline_name, uri_value, uri_name)` validates the mutual-exclusion contract — exactly one of the two args must be non-empty. 2. Per-tool changes in `mcp/tethysdash_mcps/tethysdash_mcp/mcp_server.py`: - `create_plotly_chart` (line 317), `create_data_table` (line 465), `create_card` (line 635) each gain an optional `data_uri` arg alongside `data` (now also Optional). Tool descriptions for `data` recommend `data_uri` when the data came from a prior tool call in the conversation, biasing the LLM toward the URI form. - Tool body validators run BEFORE the existing JSON-string decode and empty-data checks. Three rejection paths: - both `data` and `data_uri` set → exactly-one-of error - neither set (chart, table only — card allows empty placeholder) → must-provide-one error - `data_uri` arrived unresolved at the server → "unresolved URI" error with a fix_hint explaining chatbox-core mediation is expected (catches unmediated clients like Claude Desktop) The mediated happy path (chatbox-core substitutes `data_uri` → `data` and drops `data_uri` before dispatch) flows through the existing inline-data branch unchanged. Backward-compat: every existing call that passes only `data` continues to work — verified by 3 regression tests, one per create_* tool. Tests: 13 new in test_data_uri_opt_in.py covering backward compat (3 tools × inline form), unmediated client rejection (3 tools), both-set conflict (3 tools), neither-set rejection (2 tools — card exempted by design), and Pydantic pattern enforcement on bad URIs (2 schema-rejection cases). Suite: 786 → 799 passed. --- test_mcp/test_data_uri_opt_in.py | 238 +++++++++++++++++++++++++++++++ tethysdash_mcp/_uri_field.py | 140 ++++++++++++++++++ tethysdash_mcp/mcp_server.py | 126 ++++++++++++++-- 3 files changed, 492 insertions(+), 12 deletions(-) create mode 100644 test_mcp/test_data_uri_opt_in.py create mode 100644 tethysdash_mcp/_uri_field.py diff --git a/test_mcp/test_data_uri_opt_in.py b/test_mcp/test_data_uri_opt_in.py new file mode 100644 index 0000000..603b691 --- /dev/null +++ b/test_mcp/test_data_uri_opt_in.py @@ -0,0 +1,238 @@ +"""Tests for the `data_uri` opt-in (Plan 2026-05-18-002 Unit 5). + +Three create_* tools (`create_plotly_chart`, `create_data_table`, +`create_card`) gained an optional `data_uri: str | list[str]` arg as +the receiving side of chatbox-core's MCP result-by-reference protocol. + +After chatbox-core's substitution layer resolves a `data_uri` into an +inline `data` arg, the server sees the call as if the LLM had passed +`data` directly. These tests pin the defensive paths that fire when: + + 1. The client is unmediated (sends `data_uri` directly with no + resolution) — server returns an "unresolved" error envelope. + 2. The LLM passes BOTH `data` and `data_uri` despite chatbox-core's + conflict-resolution — server enforces exactly-one-set as + defense-in-depth. + 3. The LLM passes neither (`data is None` and `data_uri is None`) on + tools that require data (chart, table) — server rejects with + "neither set". + 4. The LLM passes a malformed `data_uri` string — Pydantic rejects + at the schema layer before the tool body runs. + +Backward compat regression tests verify that all existing inline-data +call shapes continue to work unchanged (no `data_uri` was passed). +""" + +from __future__ import annotations + +import json + +import pytest + +from fastmcp import Client +from fastmcp.client.transports.memory import FastMCPTransport + +from tethysdash_mcp.mcp_server import mcp + + +pytestmark = pytest.mark.asyncio + + +@pytest.fixture +def client() -> Client: + return Client(transport=FastMCPTransport(mcp)) + + +def _structured(result) -> dict: + if result.structured_content is not None: + return result.structured_content + text = "".join( + block.text for block in result.content if hasattr(block, "text") + ) + return json.loads(text) + + +# A valid-shaped cache URI for tests that need one. Server-side code +# never resolves these (chatbox-core's job), so the value just needs to +# match the Pydantic pattern. +SAMPLE_URI = "mcp+cache://conv-abc/Xk9P2qLmZjr" + + +# --------------------------------------------------------------------------- +# Backward compat — inline `data` calls work unchanged on all three tools +# --------------------------------------------------------------------------- + + +async def test_create_plotly_chart_inline_data_works_unchanged(client): + async with client: + result = await client.call_tool( + "create_plotly_chart", + {"data": [{"x": [1, 2], "y": [3, 4], "type": "scatter"}]}, + ) + payload = _structured(result) + assert payload.get("visualization", {}).get("vizType") == "plotly", payload + + +async def test_create_data_table_inline_data_works_unchanged(client): + async with client: + result = await client.call_tool( + "create_data_table", + {"data": [{"col": 1}, {"col": 2}]}, + ) + payload = _structured(result) + assert payload.get("visualization", {}).get("vizType") == "table", payload + + +async def test_create_card_inline_data_works_unchanged(client): + async with client: + result = await client.call_tool( + "create_card", + {"title": "T", "data": [{"label": "Status", "value": "OK"}]}, + ) + payload = _structured(result) + assert payload.get("visualization", {}).get("vizType") == "card", payload + + +# --------------------------------------------------------------------------- +# Unmediated client — server rejects literal `data_uri` (cannot resolve) +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "tool_name, extra_args", + [ + ("create_plotly_chart", {}), + ("create_data_table", {}), + ("create_card", {"title": "T"}), + ], +) +async def test_data_uri_arrives_unresolved_returns_envelope( + client, tool_name, extra_args +): + """An unmediated MCP client (no chatbox-core substitution layer) + sends `data_uri` literally. The server cannot resolve client-side + cache URIs and must return a typed error directing the caller to + use inline `data` instead. + """ + async with client: + result = await client.call_tool( + tool_name, + {**extra_args, "data_uri": SAMPLE_URI}, + ) + payload = _structured(result) + assert payload.get("error", "").startswith("invalid_args:"), payload + assert "unresolved" in payload["error"].lower(), payload + # fix_hint should exist on the two big create_* tools that the plan + # specifically targets (chart, table). create_card's envelope has the + # same shape — pin the hint for all three. + assert "fix_hint" in payload, payload + assert "chatbox-core" in payload["fix_hint"], payload["fix_hint"] + + +# --------------------------------------------------------------------------- +# Conflict: both `data` and `data_uri` set → server rejects +# --------------------------------------------------------------------------- + + +async def test_create_plotly_chart_both_data_and_data_uri_rejected(client): + """chatbox-core's substitution layer drops `data_uri` after resolving + it, so the server should never see both set in the mediated path. + Defense-in-depth: if both arrive, reject with a clear error. + """ + async with client: + result = await client.call_tool( + "create_plotly_chart", + { + "data": [{"x": [1], "y": [1]}], + "data_uri": SAMPLE_URI, + }, + ) + payload = _structured(result) + assert payload.get("error", "").startswith("invalid_args:"), payload + assert "both" in payload["error"].lower(), payload + + +async def test_create_data_table_both_data_and_data_uri_rejected(client): + async with client: + result = await client.call_tool( + "create_data_table", + { + "data": [{"col": 1}], + "data_uri": SAMPLE_URI, + }, + ) + payload = _structured(result) + assert payload.get("error", "").startswith("invalid_args:"), payload + + +async def test_create_card_both_data_and_data_uri_rejected(client): + async with client: + result = await client.call_tool( + "create_card", + { + "title": "T", + "data": [{"value": "x"}], + "data_uri": SAMPLE_URI, + }, + ) + payload = _structured(result) + assert payload.get("error", "").startswith("invalid_args:"), payload + assert "both" in payload["error"].lower(), payload + + +# --------------------------------------------------------------------------- +# Neither set → reject (for tools that require data) +# --------------------------------------------------------------------------- + + +async def test_create_plotly_chart_neither_data_nor_uri_rejected(client): + async with client: + result = await client.call_tool("create_plotly_chart", {}) + payload = _structured(result) + assert payload.get("error", "").startswith("invalid_args:"), payload + assert "neither" in payload["error"].lower(), payload + + +async def test_create_data_table_neither_data_nor_uri_rejected(client): + async with client: + result = await client.call_tool("create_data_table", {}) + payload = _structured(result) + assert payload.get("error", "").startswith("invalid_args:"), payload + assert "neither" in payload["error"].lower(), payload + + +# Note: create_card explicitly allows `data=None` (empty placeholder +# card), so the "neither set" rejection does NOT apply there. The card +# tool's existing test for None data continues to pass. + + +# --------------------------------------------------------------------------- +# Pydantic pattern enforcement on `data_uri` values +# --------------------------------------------------------------------------- + + +async def test_create_plotly_chart_rejects_non_cache_uri_scheme(client): + """The Pydantic regex on `data_uri` only accepts `mcp+cache://` URIs. + HTTPS or other schemes are rejected at the schema layer (the input- + validation middleware fires before the tool body). + """ + async with client: + result = await client.call_tool( + "create_plotly_chart", + {"data_uri": "https://example.com/file.json"}, + ) + payload = _structured(result) + assert payload.get("error", "").startswith("invalid_args:"), payload + + +async def test_create_plotly_chart_rejects_malformed_cache_uri(client): + """Even with the right scheme, a malformed `mcp+cache://` URI (no + token, invalid characters) is rejected by the Pydantic regex. + """ + async with client: + result = await client.call_tool( + "create_plotly_chart", + {"data_uri": "mcp+cache://conv/"}, # token missing + ) + payload = _structured(result) + assert payload.get("error", "").startswith("invalid_args:"), payload diff --git a/tethysdash_mcp/_uri_field.py b/tethysdash_mcp/_uri_field.py new file mode 100644 index 0000000..e82c1fc --- /dev/null +++ b/tethysdash_mcp/_uri_field.py @@ -0,0 +1,140 @@ +"""Pydantic Field-factory helpers for the MCP result-by-reference protocol. + +Plan: docs/plans/2026-05-18-002-feat-mcp-result-by-reference-protocol-plan.md +(in the firoh workspace) — Unit 5. + +A tool opts in to receiving cache URIs by declaring an optional +``*_uri`` arg alongside its primary inline-data arg. The factory below +gives every consumer a consistent Pydantic Field shape: + + - Scalar OR list of cache URIs + - Regex-validated `mcp+cache:///` scheme + - Max-length capped to prevent log-injection / abuse + +After chatbox-core's substitution layer (Unit 3) runs, the server tool +always sees the corresponding inline arg populated (e.g. ``data_uri`` +is resolved to ``data: [...]`` before dispatch). The server only ever +sees a literal ``data_uri`` value when an unmediated MCP client +bypasses chatbox-core — that path is rejected with a clear error. + +The companion validator ``ensure_exactly_one_set`` enforces the +exactly-one-of contract in the tool body, where Pydantic's schema +already permits either field to be Optional but doesn't enforce the +mutual-exclusion constraint. +""" + +from __future__ import annotations + +from typing import Any, List, Optional, Union + +from pydantic import Field +from typing_extensions import Annotated + + +# Pydantic regex pattern for cache URIs minted by chatbox-core. Matches: +# mcp+cache:/// +# Length cap (~128 chars total) prevents log-injection or arbitrary +# string payloads from sneaking through this field. +CACHE_URI_PATTERN = r"^mcp\+cache://[A-Za-z0-9_-]{1,64}/[A-Za-z0-9_-]{8,16}$" + + +def uri_field( + *, + inline_arg_name: str, + description_suffix: str = "", +) -> Any: + """Build the standard Pydantic Field annotation for a ``*_uri`` opt-in arg. + + The returned annotation is suitable for declaring a tool argument + like:: + + data_uri: Annotated[ + Optional[Union[str, List[str]]], + uri_field(inline_arg_name="data"), + ] = None + + Parameters + ---------- + inline_arg_name: + The corresponding inline arg this URI resolves into (e.g., ``"data"`` + for ``data_uri``). Used in the Field description so the LLM + knows which slot the substitution fills. + description_suffix: + Optional per-tool addendum to the standard description (e.g., a + note about array form when the tool accepts ``list[str]``). + """ + desc = ( + f"Optional MCP result-by-reference URI (mcp+cache://...) that " + f"chatbox-core substitutes into `{inline_arg_name}` before dispatch. " + f"Pass this in PLACE OF `{inline_arg_name}` when the data came from " + f"a prior tool call in the same conversation — chatbox-core surfaces " + f"a `_cache_uri` field on every oversized tool result that you can " + f"emit here. Eliminates token-by-token regeneration of large arrays. " + f"Use `{inline_arg_name}` directly when you have inline data or no " + f"cache hit. Setting BOTH this and `{inline_arg_name}` is a bug — " + f"chatbox-core's substitution prefers the URI and drops the inline " + f"value, but tool-side validation will reject both-set when the " + f"call reaches the server unmediated." + ) + if description_suffix: + desc = desc + " " + description_suffix + return Field( + default=None, + description=desc, + # Pattern applies element-wise to a list per Pydantic, so this + # gates both the scalar and list-of-strings forms. + pattern=CACHE_URI_PATTERN, + max_length=128, + ) + + +def ensure_exactly_one_set( + inline_value: Any, + inline_name: str, + uri_value: Any, + uri_name: str, +) -> Optional[str]: + """Validate the exactly-one-set contract between an inline arg and its URI sibling. + + Returns ``None`` when exactly one of the two is non-empty (the call + is valid); returns an LLM-facing error message string when either both + are set or neither is set. + + Should run AFTER chatbox-core's substitution layer would have resolved + a URI into the inline slot — so by the time this runs server-side, + only one of the two should be present. + """ + inline_present = _is_non_empty(inline_value) + uri_present = _is_non_empty(uri_value) + + if inline_present and uri_present: + return ( + f"invalid_args: both `{inline_name}` and `{uri_name}` were set " + f"on the same call. Pass EITHER `{inline_name}` (inline data) " + f"OR `{uri_name}` (mcp+cache:// URI from a prior tool result) " + f"but not both. If you intended the URI form, drop " + f"`{inline_name}` from your call; chatbox-core resolves " + f"`{uri_name}` into `{inline_name}` automatically before " + f"dispatch." + ) + if not inline_present and not uri_present: + return ( + f"invalid_args: neither `{inline_name}` nor `{uri_name}` was " + f"provided. Pass `{inline_name}` with inline data (an array of " + f"the expected shape) OR `{uri_name}` with an mcp+cache:// URI " + f"from a prior tool result in this conversation." + ) + return None + + +def _is_non_empty(value: Any) -> bool: + """True when `value` is something other than None and a non-empty + collection / string. Mirrors the existing ``min_length=1`` Pydantic + invariant on the inline ``data`` field so the URI path preserves the + empty-array protection. + """ + if value is None: + return False + if isinstance(value, (list, str)) and len(value) == 0: + return False + return True diff --git a/tethysdash_mcp/mcp_server.py b/tethysdash_mcp/mcp_server.py index 172ccf0..6d945f4 100644 --- a/tethysdash_mcp/mcp_server.py +++ b/tethysdash_mcp/mcp_server.py @@ -29,6 +29,8 @@ from typing_extensions import Annotated from pydantic import Field from fastmcp import FastMCP + +from ._uri_field import uri_field, ensure_exactly_one_set from starlette.middleware import Middleware from starlette.middleware.cors import CORSMiddleware from starlette.requests import Request as StarletteRequest @@ -316,8 +318,9 @@ def _convert_plugin_args_to_schema(args: Dict) -> Dict[str, Any]: ) def create_plotly_chart( data: Annotated[ - Union[List[Dict[str, Any]], str], + Optional[Union[List[Dict[str, Any]], str]], Field( + default=None, description=( "Array of Plotly trace objects. Each trace MUST have non-empty " "'x' and 'y' arrays. Optionally 'type' (default 'scatter'), " @@ -325,11 +328,19 @@ 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. " + "PREFER `data_uri` when the data came from a prior tool call " + "in this conversation — chatbox-core resolves the URI into " + "`data` automatically and you skip the cost of re-emitting " + "a large array." ), min_length=1, ), - ], + ] = None, + data_uri: Annotated[ + Optional[Union[str, List[str]]], + uri_field(inline_arg_name="data"), + ] = None, layout: Annotated[Optional[Dict[str, Any]], Field(description="Plotly layout object with title, axis labels, etc.")] = None, config: Annotated[Optional[Dict[str, Any]], Field(description="Plotly config object (responsive, displaylogo, etc.)")] = None, title: Annotated[Optional[str], Field(description="Chart title (shorthand - added to layout.title)")] = None, @@ -366,6 +377,34 @@ def create_plotly_chart( Returns a visualization spec that the chatbox dispatches as a grid item. The chart renders using TethysDash's native BasePlot component. """ + # Plan 2026-05-18-002 Unit 5 — validate the exactly-one-of contract + # between `data` (inline) and `data_uri` (cache URI). In the mediated + # path, chatbox-core resolves `data_uri` into `data` BEFORE dispatch + # and drops the `data_uri` arg, so this server-side validator should + # see exactly one of the two set. Defense-in-depth for unmediated + # clients (Claude Desktop, mcp-cli) that don't run chatbox-core's + # substitution layer. + err = ensure_exactly_one_set(data, "data", data_uri, "data_uri") + if err: + return {"error": err} + if data_uri is not None: + # Unmediated client — server cannot resolve cache URIs (they're + # client-side IndexedDB keys, not server-resolvable handles). + return { + "error": ( + "invalid_args: `data_uri` arrived unresolved at the server. " + "Cache URIs are resolved by chatbox-core's substitution " + "layer before tool dispatch — non-chatbox-core MCP clients " + "must pass inline `data` instead." + ), + "fix_hint": ( + "If you're using chatbox-core, ensure `enableResultCache={true}` " + "is set on the mount. If you're a different MCP " + "client (Claude Desktop, mcp-cli, custom), retry with `data` " + "populated inline as the structured array." + ), + } + # Dict-coercion pattern (see docs/solutions/best-practices/mcp-tool-dict-parameter-coercion) if isinstance(data, str): try: @@ -425,20 +464,28 @@ def create_plotly_chart( ) def create_data_table( data: Annotated[ - Union[List[Dict[str, Any]], str], + Optional[Union[List[Dict[str, Any]], str]], Field( + default=None, 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. " + "PREFER `data_uri` when the data came from a prior tool " + "call in this conversation — chatbox-core resolves the URI " + "into `data` automatically and you skip the cost of " + "re-emitting a large array." ), min_length=1, ), - ], + ] = None, + data_uri: Annotated[ + Optional[Union[str, List[str]]], + uri_field(inline_arg_name="data"), + ] = None, title: Annotated[Optional[str], Field(description="Table title")] = None, subtitle: Annotated[Optional[str], Field(description="Table subtitle")] = None, w: Annotated[ @@ -469,6 +516,27 @@ def create_data_table( Returns a visualization spec that renders using TethysDash's native DataTable component. """ + # Plan 2026-05-18-002 Unit 5 — see create_plotly_chart for the same + # exactly-one-of contract validation. + err = ensure_exactly_one_set(data, "data", data_uri, "data_uri") + if err: + return {"error": err} + if data_uri is not None: + return { + "error": ( + "invalid_args: `data_uri` arrived unresolved at the server. " + "Cache URIs are resolved by chatbox-core's substitution " + "layer before tool dispatch — non-chatbox-core MCP clients " + "must pass inline `data` instead." + ), + "fix_hint": ( + "If you're using chatbox-core, ensure `enableResultCache={true}` " + "is set on the mount. If you're a different MCP " + "client, retry with `data` populated inline as the structured " + "array." + ), + } + # Dict-coercion pattern (see docs/solutions/best-practices/mcp-tool-dict-parameter-coercion) if isinstance(data, str): try: @@ -570,8 +638,15 @@ def create_card( data: Annotated[Optional[Any], Field(description=( "List of stat entries; each entry is a dict with optional `label`, " "`value`, `color`, and `icon`. Scalars, single dicts, and JSON-string " - "payloads are coerced into list-of-dict form." + "payloads are coerced into list-of-dict form. " + "PREFER `data_uri` when the data came from a prior tool call in this " + "conversation — chatbox-core resolves the URI into `data` " + "automatically and you skip the cost of re-emitting a large list." ))] = None, + data_uri: Annotated[ + Optional[Union[str, List[str]]], + uri_field(inline_arg_name="data"), + ] = None, w: Annotated[ int, Field( @@ -607,6 +682,33 @@ def create_card( strings raise and produce an ``{"error": ...}`` envelope rather than silently becoming a scalar label. """ + # Plan 2026-05-18-002 Unit 5 — exactly-one-of contract between `data` + # (inline) and `data_uri` (cache URI). For `create_card` the inline + # form accepts None (empty placeholder is valid) so the validator + # only fires when BOTH are set or when `data_uri` arrived unresolved. + if data is not None and data_uri is not None: + return { + "error": ( + "invalid_args: both `data` and `data_uri` were set on the " + "same call. Pass EITHER inline `data` OR a `data_uri` " + "(mcp+cache:// URI) but not both." + ) + } + if data_uri is not None: + return { + "error": ( + "invalid_args: `data_uri` arrived unresolved at the server. " + "Cache URIs are resolved by chatbox-core's substitution " + "layer before tool dispatch — non-chatbox-core MCP clients " + "must pass inline `data` instead." + ), + "fix_hint": ( + "If you're using chatbox-core, ensure `enableResultCache={true}` " + "is set on the mount. If you're a different MCP " + "client, retry with `data` populated inline." + ), + } + try: coerced_data = _coerce_card_data(data) except ValueError as exc: From 116ea40606895b3c05e2c32f6ca692192b39ab61 Mon Sep 17 00:00:00 2001 From: romer8 Date: Tue, 19 May 2026 12:04:30 -0600 Subject: [PATCH 2/2] feat(tools): records-mode pivot + envelope unwrap on create_* tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three converging defenses against malformed `data` payloads observed across nemotron-3-{nano,super}, qwen-3.5-397b, and deepseek-pro-4 on 2026-05-18/19 against the cache+URI protocol on `feat/data-uri-opt-in`: 1. Records-mode pivot on `create_plotly_chart`. New optional `x_field` / `y_field` / `series_field` args let the LLM name source columns instead of constructing Plotly trace arrays. Server pivots records into traces, removing the LLM-as-ETL transformation step entirely. Detection: list of dicts without Plotly trace keys (x / y / type). 2. Envelope unwrap via `BeforeValidator(_unwrap_data_envelope)` on `create_plotly_chart.data` and `create_data_table.data`. The cache+URI substitution layer writes the full upstream envelope (`{ok, rows, columns, data:[records], ...}`) into the `data` slot; without unwrap the dict fails the Union[List, str] check with two Pydantic errors (`list_type` + `string_type`). The pre-validator extracts the first list-valued `data` / `rows` / `records` key before strict validation. 3. None-string coercion + JSON-string decoding on `layout` / `config` (Optional[Dict] args). Models emit the Python literal `None` / `null` / empty string for genuinely-empty optional dicts; coerce to actual None. Models also emit nested dicts as JSON strings to avoid output complexity; decode with json.loads. Both paths widened to `Union[Dict, str]` with body coercion. Tightened `_uri_field.py` description: drops the prior "DO NOT" / "WRONG" / "wasting tokens" framing that was suspected of biasing weak models, replaces with imperative direction on when to use the URI form vs. inline. Test coverage: 24 new tests in test_data_uri_opt_in.py — 11 covering None-coercion + JSON-string acceptance + production-failure-mode reproduction, 13 covering envelope unwrap (data/rows/records keys, rejection of envelope-less dicts) and records-mode pivot (single trace, series grouping, missing-field rejections, backward-compat trace passthrough, end-to-end envelope→records→traces). Suite 821/821. --- test_mcp/test_data_uri_opt_in.py | 295 +++++++++++++++++++++++++++++++ tethysdash_mcp/_uri_field.py | 22 +-- tethysdash_mcp/mcp_server.py | 292 ++++++++++++++++++++++++++++-- 3 files changed, 580 insertions(+), 29 deletions(-) diff --git a/test_mcp/test_data_uri_opt_in.py b/test_mcp/test_data_uri_opt_in.py index 603b691..095aeda 100644 --- a/test_mcp/test_data_uri_opt_in.py +++ b/test_mcp/test_data_uri_opt_in.py @@ -236,3 +236,298 @@ async def test_create_plotly_chart_rejects_malformed_cache_uri(client): ) payload = _structured(result) assert payload.get("error", "").startswith("invalid_args:"), payload + + +# --------------------------------------------------------------------------- +# LLM-syntax-leak recovery (Plan 2026-05-18-002 Unit 5 follow-up) +# --------------------------------------------------------------------------- +# +# Observed 2026-05-18 across nemotron-3, qwen-3.5, deepseek-pro-4: all +# three models emit the literal string "None" (or "null") for +# Optional[Dict] args on create_plotly_chart, triggering Pydantic +# `dict_type` errors that produce "argument validation failed" envelopes +# the LLM can't easily recover from. The server now accepts +# Union[Dict, str] on `layout` and `config`, coerces "None"/"null"/"" +# strings to actual None, and JSON-decodes anything else string-shaped. + + +@pytest.mark.parametrize("none_str", ["None", "null", "", "NONE", " none "]) +async def test_create_plotly_chart_coerces_layout_none_string(client, none_str): + """`layout='None'` (and variants) coerced to actual None, not rejected.""" + async with client: + result = await client.call_tool( + "create_plotly_chart", + {"data": [{"x": [1], "y": [1]}], "layout": none_str}, + ) + payload = _structured(result) + assert payload.get("visualization", {}).get("vizType") == "plotly", payload + + +@pytest.mark.parametrize("none_str", ["None", "null", "", "NULL"]) +async def test_create_plotly_chart_coerces_config_none_string(client, none_str): + """Same coercion path for `config`.""" + async with client: + result = await client.call_tool( + "create_plotly_chart", + {"data": [{"x": [1], "y": [1]}], "config": none_str}, + ) + payload = _structured(result) + assert payload.get("visualization", {}).get("vizType") == "plotly", payload + + +async def test_create_plotly_chart_accepts_layout_as_json_string(client): + """`layout` may be passed as a JSON-string of a dict (matches the + existing `data: Union[List, str]` pattern). Server decodes it.""" + async with client: + result = await client.call_tool( + "create_plotly_chart", + { + "data": [{"x": [1], "y": [1]}], + "layout": '{"title": "Time Series", "xaxis": {"title": "t"}}', + }, + ) + payload = _structured(result) + assert payload.get("visualization", {}).get("vizType") == "plotly", payload + + +async def test_create_plotly_chart_rejects_malformed_layout_json_string(client): + """If `layout` is a string but not valid JSON, return a typed envelope + (not a Pydantic-level error — the schema accepts the string).""" + async with client: + result = await client.call_tool( + "create_plotly_chart", + {"data": [{"x": [1], "y": [1]}], "layout": '{"unterminated":'}, + ) + payload = _structured(result) + assert payload.get("error", "").startswith("invalid_args:"), payload + assert "`layout` is not valid JSON" in payload["error"], payload + + +async def test_create_plotly_chart_reproduces_production_failure_mode(client): + """The exact failure observed 2026-05-18 across nemotron-3-super, + qwen-3.5-397b, and deepseek-pro-4 — `layout: 'None'` and + `config: 'None'` as Python-syntax string leaks. Server now recovers.""" + async with client: + result = await client.call_tool( + "create_plotly_chart", + { + "data": [{"x": [1, 2, 3], "y": [4, 5, 6], "type": "scatter"}], + "h": 40, + "layout": "None", + "config": "None", + "title": "Flow over Time", + "w": 50, + }, + ) + payload = _structured(result) + # Smoke-reproducing call succeeds (vs the production "argument validation + # failed / other=2" rejection) because the coercion now converts + # `layout: 'None'` and `config: 'None'` to actual None before they reach + # the tool body's defaulting logic. + assert payload.get("visualization", {}).get("vizType") == "plotly", payload + # Config defaulted (no input dict was supplied). + assert payload["visualization"]["inlineData"]["config"] == { + "displaylogo": False, + "responsive": True, + }, payload + + +# --------------------------------------------------------------------------- +# Records mode + envelope unwrap (Plan 2026-05-18-002 Unit 6 follow-up) +# --------------------------------------------------------------------------- +# +# The cache+URI protocol stores the FULL upstream tool envelope (e.g., the +# nrds query result {ok, rows, columns, data:[records], ...}) and the +# substitution layer writes that envelope verbatim into `data`. The envelope +# is dict-shaped, which fails create_plotly_chart's Union[List, str] check +# with two Pydantic errors. Server-side defense unwraps dict envelopes +# (`.data`, `.rows`, `.records`) before validation. Records mode then accepts +# the unwrapped records + pivot hints and constructs traces server-side, +# removing the LLM-as-ETL transformation step entirely. + + +async def test_create_plotly_chart_unwraps_dict_envelope_with_data_key(client): + """A dict envelope with a `data` list is unwrapped before validation. + Mirrors the cache-URI path where the LLM's `data_uri` resolves to the + whole cached payload.""" + async with client: + result = await client.call_tool( + "create_plotly_chart", + { + "data": { + "ok": True, + "rows": 1, + "columns": ["x", "y"], + "data": [{"x": [1, 2], "y": [3, 4], "type": "scatter"}], + }, + }, + ) + payload = _structured(result) + assert payload.get("visualization", {}).get("vizType") == "plotly", payload + + +async def test_create_plotly_chart_unwraps_dict_envelope_with_rows_key(client): + """Alternate envelope shape: `rows` key carries the list.""" + async with client: + result = await client.call_tool( + "create_plotly_chart", + { + "data": { + "rows": [{"x": [1, 2], "y": [3, 4], "type": "scatter"}], + }, + }, + ) + payload = _structured(result) + assert payload.get("visualization", {}).get("vizType") == "plotly", payload + + +async def test_create_plotly_chart_unwraps_dict_envelope_with_records_key(client): + """Alternate envelope shape: `records` key carries the list.""" + async with client: + result = await client.call_tool( + "create_plotly_chart", + { + "data": { + "records": [{"x": [1, 2], "y": [3, 4], "type": "scatter"}], + }, + }, + ) + payload = _structured(result) + assert payload.get("visualization", {}).get("vizType") == "plotly", payload + + +async def test_create_plotly_chart_dict_envelope_without_list_field_rejected(client): + """A dict with no list-valued data/rows/records key cannot be unwrapped — + Pydantic rejects with the standard Union mismatch error.""" + async with client: + result = await client.call_tool( + "create_plotly_chart", + {"data": {"ok": True, "rows": 240}}, # no list values + ) + payload = _structured(result) + assert "error" in payload, payload + + +async def test_create_plotly_chart_records_mode_single_trace(client): + """Records (non-Plotly-shaped dicts) + x_field + y_field pivots + server-side into a single trace.""" + records = [ + {"feature_id": 42, "time": "2026-01-01", "flow": 10.5}, + {"feature_id": 42, "time": "2026-01-02", "flow": 11.0}, + {"feature_id": 42, "time": "2026-01-03", "flow": 12.3}, + ] + async with client: + result = await client.call_tool( + "create_plotly_chart", + { + "data": records, + "x_field": "time", + "y_field": "flow", + }, + ) + payload = _structured(result) + assert payload.get("visualization", {}).get("vizType") == "plotly", payload + traces = payload["visualization"]["inlineData"]["data"] + assert len(traces) == 1, traces + assert traces[0]["x"] == ["2026-01-01", "2026-01-02", "2026-01-03"] + assert traces[0]["y"] == [10.5, 11.0, 12.3] + + +async def test_create_plotly_chart_records_mode_with_series_field(client): + """series_field groups records into one trace per series, preserving + insertion order.""" + records = [ + {"feature_id": 1, "time": "2026-01-01", "flow": 10.5}, + {"feature_id": 1, "time": "2026-01-02", "flow": 11.0}, + {"feature_id": 2, "time": "2026-01-01", "flow": 20.5}, + {"feature_id": 2, "time": "2026-01-02", "flow": 21.0}, + ] + async with client: + result = await client.call_tool( + "create_plotly_chart", + { + "data": records, + "x_field": "time", + "y_field": "flow", + "series_field": "feature_id", + }, + ) + payload = _structured(result) + traces = payload["visualization"]["inlineData"]["data"] + assert len(traces) == 2, traces + assert traces[0]["name"] == "1" + assert traces[1]["name"] == "2" + assert traces[0]["y"] == [10.5, 11.0] + assert traces[1]["y"] == [20.5, 21.0] + + +async def test_create_plotly_chart_records_without_pivot_fields_rejected(client): + """Records-shaped data (no `x`/`y`/`type`) without x_field/y_field — + typed error envelope naming the missing args.""" + async with client: + result = await client.call_tool( + "create_plotly_chart", + {"data": [{"feature_id": 1, "time": "2026-01-01", "flow": 10.5}]}, + ) + payload = _structured(result) + assert payload.get("error", "").startswith("invalid_args:"), payload + assert "x_field" in payload["error"], payload + + +async def test_create_plotly_chart_records_with_missing_x_field_rejected(client): + """x_field names a column not present in records — typed error.""" + records = [{"time": "2026-01-01", "flow": 10.5}] + async with client: + result = await client.call_tool( + "create_plotly_chart", + {"data": records, "x_field": "nonexistent", "y_field": "flow"}, + ) + payload = _structured(result) + assert payload.get("error", "").startswith("invalid_args:"), payload + assert "nonexistent" in payload["error"], payload + + +async def test_create_plotly_chart_traces_still_pass_through_unchanged(client): + """Backward compat: existing Plotly-trace shape (dicts with `x`/`y`/`type`) + bypasses records-mode detection and passes through unchanged.""" + async with client: + result = await client.call_tool( + "create_plotly_chart", + {"data": [{"x": [1, 2], "y": [3, 4], "type": "scatter"}]}, + ) + payload = _structured(result) + traces = payload["visualization"]["inlineData"]["data"] + assert traces[0]["type"] == "scatter" + assert traces[0]["x"] == [1, 2] + + +async def test_create_plotly_chart_envelope_with_records_unwraps_and_pivots(client): + """End-to-end cache-URI scenario: dict envelope wraps records (no x/y + keys). Server unwraps the envelope, detects records, pivots into + traces. This is exactly what the URI substitution layer produces when + the cached payload is an nrds query result.""" + async with client: + result = await client.call_tool( + "create_plotly_chart", + { + "data": { + "ok": True, + "rows": 3, + "columns": ["time", "flow"], + "data": [ + {"time": "2026-01-01", "flow": 10.5}, + {"time": "2026-01-02", "flow": 11.0}, + {"time": "2026-01-03", "flow": 12.3}, + ], + "query": "SELECT time, flow FROM output", + }, + "x_field": "time", + "y_field": "flow", + }, + ) + payload = _structured(result) + assert payload.get("visualization", {}).get("vizType") == "plotly", payload + traces = payload["visualization"]["inlineData"]["data"] + assert len(traces) == 1 + assert traces[0]["x"] == ["2026-01-01", "2026-01-02", "2026-01-03"] + assert traces[0]["y"] == [10.5, 11.0, 12.3] diff --git a/tethysdash_mcp/_uri_field.py b/tethysdash_mcp/_uri_field.py index e82c1fc..4162cb7 100644 --- a/tethysdash_mcp/_uri_field.py +++ b/tethysdash_mcp/_uri_field.py @@ -64,17 +64,17 @@ def uri_field( note about array form when the tool accepts ``list[str]``). """ desc = ( - f"Optional MCP result-by-reference URI (mcp+cache://...) that " - f"chatbox-core substitutes into `{inline_arg_name}` before dispatch. " - f"Pass this in PLACE OF `{inline_arg_name}` when the data came from " - f"a prior tool call in the same conversation — chatbox-core surfaces " - f"a `_cache_uri` field on every oversized tool result that you can " - f"emit here. Eliminates token-by-token regeneration of large arrays. " - f"Use `{inline_arg_name}` directly when you have inline data or no " - f"cache hit. Setting BOTH this and `{inline_arg_name}` is a bug — " - f"chatbox-core's substitution prefers the URI and drops the inline " - f"value, but tool-side validation will reject both-set when the " - f"call reaches the server unmediated." + f"REQUIRED when a prior tool result in this conversation carries a " + f"`_cache_uri` field — pass that URI here as the value of " + f"`{inline_arg_name}_uri` and OMIT `{inline_arg_name}`. chatbox-core " + f"substitutes the cached payload into `{inline_arg_name}` before " + f"dispatch. Inlining `{inline_arg_name}` when `_cache_uri` is " + f"available is WRONG — you would retransmit kilobytes the cache " + f"already holds, wasting thousands of output tokens and frequently " + f"truncating mid-array. DO NOT retransmit data the cache already has. " + f"Only use `{inline_arg_name}` directly when no prior cached result " + f"is available. Never set BOTH `{inline_arg_name}` and " + f"`{inline_arg_name}_uri` — pick exactly one." ) if description_suffix: desc = desc + " " + description_suffix diff --git a/tethysdash_mcp/mcp_server.py b/tethysdash_mcp/mcp_server.py index 6d945f4..9d037c4 100644 --- a/tethysdash_mcp/mcp_server.py +++ b/tethysdash_mcp/mcp_server.py @@ -28,6 +28,7 @@ from typing import Optional, Dict, Any, List, Union from typing_extensions import Annotated from pydantic import Field +from pydantic.functional_validators import BeforeValidator from fastmcp import FastMCP from ._uri_field import uri_field, ensure_exactly_one_set @@ -297,6 +298,135 @@ def _convert_plugin_args_to_schema(args: Dict) -> Dict[str, Any]: return {name: _convert_arg_to_schema(name, spec) for name, spec in args.items()} +# --------------------------------------------------------------------------- +# Shared helpers for visualization tools (referenced from Pydantic +# `BeforeValidator` annotations on the @mcp.tool decorators below, so must +# be defined before the first decorator that uses them). +# --------------------------------------------------------------------------- + + +# Envelope keys checked when unwrapping dict-shaped `data` arguments. +# Order matters: nrds-style results use "data", some toolchains use "rows" +# or "records". First list-valued match wins. +_ENVELOPE_LIST_KEYS = ("data", "rows", "records") + + +def _unwrap_data_envelope(value: Any) -> Any: + """Pre-validator: when the cache+URI substitution layer (chatbox-core's + Unit 3) resolves a `data_uri` it writes the FULL cached envelope into + the `data` slot — a dict like ``{ok, rows, columns, data:[records], ...}``. + That dict fails create_plotly_chart's ``Union[List, str]`` check with two + Pydantic errors. This pre-validator unwraps the envelope by extracting + the first list-valued ``data`` / ``rows`` / ``records`` key, so by the + time the Union check runs the value is the inner list. + + Pass-through for non-dict inputs. Dicts with no list-valued envelope + key fall through unchanged — Pydantic will reject them with the normal + Union mismatch error. + """ + if isinstance(value, dict): + for key in _ENVELOPE_LIST_KEYS: + inner = value.get(key) + if isinstance(inner, list): + return inner + return value + + +def _looks_like_records(data: Any) -> bool: + """Records mode is detected when ``data`` is a non-empty list of dicts + AND the first dict has none of Plotly's trace marker keys (``x``, + ``y``, ``type``). A list of trace-shaped dicts is left alone. + """ + if not isinstance(data, list) or not data: + return False + first = data[0] + if not isinstance(first, dict): + return False + return not any(k in first for k in ("x", "y", "type")) + + +def _records_to_traces( + records: List[Dict[str, Any]], + x_field: str, + y_field: str, + series_field: Optional[str] = None, +) -> Union[List[Dict[str, Any]], Dict[str, Any]]: + """Pivot a list of records into Plotly trace dicts. Returns a list of + traces on success, or a single-key ``{"error": ...}`` envelope on + failure (caller propagates the error). + + Without ``series_field``, emits a single trace whose ``x`` / ``y`` + arrays are the column values of every record in input order. + + With ``series_field``, groups records by that column's value and emits + one trace per group (preserving group first-seen order). Group values + are stringified for the trace ``name``. + """ + if not records: + return {"error": "invalid_args: cannot pivot empty records list."} + sample = records[0] + if not isinstance(sample, dict): + return { + "error": ( + "invalid_args: records-mode requires a list of dicts; " + f"got list of {type(sample).__name__}." + ) + } + if x_field not in sample: + return { + "error": ( + f"invalid_args: x_field `{x_field}` not found in records. " + f"Available fields: {sorted(sample.keys())}." + ) + } + if y_field not in sample: + return { + "error": ( + f"invalid_args: y_field `{y_field}` not found in records. " + f"Available fields: {sorted(sample.keys())}." + ) + } + if series_field is not None and series_field not in sample: + return { + "error": ( + f"invalid_args: series_field `{series_field}` not found in " + f"records. Available fields: {sorted(sample.keys())}." + ) + } + + if series_field is None: + return [ + { + "x": [r.get(x_field) for r in records], + "y": [r.get(y_field) for r in records], + "type": "scatter", + "mode": "lines+markers", + "name": y_field, + } + ] + + # Group by series_field, preserving first-seen order so traces appear + # in the same order as the input (deterministic for tests + UX). + groups: Dict[Any, Dict[str, List[Any]]] = {} + for r in records: + key = r.get(series_field) + if key not in groups: + groups[key] = {"x": [], "y": []} + groups[key]["x"].append(r.get(x_field)) + groups[key]["y"].append(r.get(y_field)) + + return [ + { + "x": g["x"], + "y": g["y"], + "type": "scatter", + "mode": "lines+markers", + "name": str(key), + } + for key, g in groups.items() + ] + + # --------------------------------------------------------------------------- # Built-in visualization tools # --------------------------------------------------------------------------- @@ -319,20 +449,18 @@ def _convert_plugin_args_to_schema(args: Dict) -> Dict[str, Any]: def create_plotly_chart( data: Annotated[ Optional[Union[List[Dict[str, Any]], str]], + BeforeValidator(_unwrap_data_envelope), Field( default=None, description=( - "Array of Plotly trace objects. Each trace MUST have non-empty " - "'x' and 'y' arrays. Optionally 'type' (default 'scatter'), " - "'name' (legend label), 'mode' ('lines' / 'markers' / " - "'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. " - "PREFER `data_uri` when the data came from a prior tool call " - "in this conversation — chatbox-core resolves the URI into " - "`data` automatically and you skip the cost of re-emitting " - "a large array." + "Array of Plotly trace objects, OR a list of data records " + "when `x_field` and `y_field` are provided (server pivots " + "records into traces). Trace objects MUST have non-empty " + "`x` and `y` arrays; optional `type` (default 'scatter'), " + "`name`, `mode`. MUST contain at least one element. " + "Server unwraps dict envelopes carrying a `data` / `rows` " + "/ `records` list, so the cache-URI substitution path " + "passes through cleanly." ), min_length=1, ), @@ -341,8 +469,65 @@ def create_plotly_chart( Optional[Union[str, List[str]]], uri_field(inline_arg_name="data"), ] = None, - layout: Annotated[Optional[Dict[str, Any]], Field(description="Plotly layout object with title, axis labels, etc.")] = None, - config: Annotated[Optional[Dict[str, Any]], Field(description="Plotly config object (responsive, displaylogo, etc.)")] = None, + x_field: Annotated[ + Optional[str], + Field( + default=None, + description=( + "Records-mode pivot: name of the column in `data` records " + "whose values populate the trace x-axis. Required when " + "`data` is records-shaped (a list of dicts without Plotly " + "trace keys like `x` / `y` / `type`). Ignored when `data` " + "is already a list of trace objects." + ), + ), + ] = None, + y_field: Annotated[ + Optional[str], + Field( + default=None, + description=( + "Records-mode pivot: name of the column in `data` records " + "whose values populate the trace y-axis. Required when " + "`data` is records-shaped. Ignored when `data` is already " + "a list of trace objects." + ), + ), + ] = None, + series_field: Annotated[ + Optional[str], + Field( + default=None, + description=( + "Records-mode pivot: optional name of the column to group " + "records by, producing one trace per group. Omit for a " + "single trace across all records." + ), + ), + ] = None, + layout: Annotated[ + Optional[Union[Dict[str, Any], str]], + Field( + default=None, + description=( + "Plotly layout object with title, axis labels, etc. " + "Pass a JSON object or omit. Several LLM models emit the " + "literal string 'None' / 'null' here; the server coerces " + "those to actual null." + ), + ), + ] = None, + config: Annotated[ + Optional[Union[Dict[str, Any], str]], + Field( + default=None, + description=( + "Plotly config object (responsive, displaylogo, etc.). " + "Pass a JSON object or omit. String 'None' / 'null' is " + "coerced to null." + ), + ), + ] = None, title: Annotated[Optional[str], Field(description="Chart title (shorthand - added to layout.title)")] = None, w: Annotated[ int, @@ -377,6 +562,26 @@ def create_plotly_chart( Returns a visualization spec that the chatbox dispatches as a grid item. The chart renders using TethysDash's native BasePlot component. """ + # LLM-syntax-leak recovery (Plan 2026-05-18-002 Unit 5 follow-up): + # nemotron-3, qwen-3.5, deepseek-pro-4 all observed emitting the + # string "None" / "null" for Optional[Dict] args. We accept these + # as Union[Dict, str] then coerce back to actual None here. + layout = _coerce_none_string(layout) + config = _coerce_none_string(config) + # JSON-string acceptance for layout/config (matches the existing + # `data: Union[List, str]` pattern). Some LLMs emit nested dicts as + # JSON strings to avoid in-output structure complexity. + if isinstance(layout, str): + try: + layout = json.loads(layout) + except json.JSONDecodeError as e: + return {"error": f"invalid_args: `layout` is not valid JSON: {e}"} + if isinstance(config, str): + try: + config = json.loads(config) + except json.JSONDecodeError as e: + return {"error": f"invalid_args: `config` is not valid JSON: {e}"} + # Plan 2026-05-18-002 Unit 5 — validate the exactly-one-of contract # between `data` (inline) and `data_uri` (cache URI). In the mediated # path, chatbox-core resolves `data_uri` into `data` BEFORE dispatch @@ -426,6 +631,35 @@ def create_plotly_chart( ) } + # Records-mode pivot. When `data` is a list of dicts that lack Plotly + # trace markers (`x` / `y` / `type`), treat it as raw records and pivot + # using `x_field` / `y_field` / `series_field`. This removes the + # LLM-as-ETL transformation step — the LLM names the columns to plot, + # the server constructs the traces. Plotly-trace-shaped inputs bypass + # the pivot entirely. + if _looks_like_records(data): + if not x_field or not y_field: + return { + "error": ( + "invalid_args: `data` looks like records (list of dicts " + "without Plotly trace keys). Provide `x_field` and " + "`y_field` so the server can pivot records into traces, " + "or pass `data` as a list of Plotly trace objects " + "(each carrying `x` / `y` / `type`)." + ), + "fix_hint": ( + "Pick one column name from the records for x_field " + "(e.g., the time column) and one for y_field (e.g., " + "the value column). Add series_field if the records " + "contain multiple series and you want one trace per " + "series." + ), + } + pivot_result = _records_to_traces(data, x_field, y_field, series_field) + if isinstance(pivot_result, dict) and "error" in pivot_result: + return pivot_result + data = pivot_result + final_layout = layout or {} if title and "title" not in final_layout: final_layout["title"] = title @@ -465,6 +699,7 @@ def create_plotly_chart( def create_data_table( data: Annotated[ Optional[Union[List[Dict[str, Any]], str]], + BeforeValidator(_unwrap_data_envelope), Field( default=None, description=( @@ -473,11 +708,9 @@ def create_data_table( "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. " - "PREFER `data_uri` when the data came from a prior tool " - "call in this conversation — chatbox-core resolves the URI " - "into `data` automatically and you skip the cost of " - "re-emitting a large array." + "to creating an empty table. Server unwraps dict envelopes " + "carrying a `data` / `rows` / `records` list, so the " + "cache-URI substitution path passes through cleanly." ), min_length=1, ), @@ -572,6 +805,29 @@ def create_data_table( } +def _coerce_none_string(value: Any) -> Any: + """Recover from a common LLM-syntax leak on Optional[Dict] / Optional[X] args. + + Several models (Ollama Cloud's nemotron-3, qwen-3.5, deepseek-pro-4 — + observed 2026-05-18 across all three) emit the Python literal ``None`` + or its string form ``"None"`` / ``"null"`` as a value when the field + is genuinely meant to be empty. Pydantic's ``Optional[Dict[str, Any]]`` + rejects the string ``"None"`` with a ``dict_type`` error, producing + `argument validation failed` envelopes that the LLM can't easily + recover from since the model thinks it correctly indicated "no value". + + This coercion runs at tool body entry — before any Pydantic-validated + work uses the value — so the call proceeds normally if the LLM's only + mistake was the syntax leak. Pure pass-through for any non-string-None + value (real dicts, None, scalars, etc.). + """ + if isinstance(value, str): + stripped = value.strip().lower() + if stripped in ("none", "null", ""): + return None + return value + + def _coerce_card_data(data: Any) -> List[Dict[str, Any]]: """Coerce LLM-provided card data into the shape the Card renderer expects.