feat(tools): data_uri opt-in + records-mode pivot + envelope unwrap on create_* tools - #7
Merged
Conversation
…table, create_card
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://<conv-id>/<token>` 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.
4 tasks
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.
romer8
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Receiving side of chatbox-core's MCP result-by-reference protocol (plan
2026-05-18-002-feat-mcp-result-by-reference-protocol-plan.mdin the firoh workspace — Unit 5).The three create_* tools that take inline
dataarrays gain an optionaldata_uri: str | list[str]arg. After chatbox-core's substitution layer (engine/uri-substitution.js — chatbox-core PR pending) resolves the URI from its IndexedDB cache, the server sees the call as if the LLM had passeddatadirectly — no MCP wire-contract change for the mediated path.Why this matters
Production observation 2026-05-18: a 200-second response on the deployed tethysdash chatbox was traced to LLM transcription of a 240-row data array between two MCP servers. The LLM regenerated the data token-by-token for the chart call. The earlier description-bias fix (PR #6 — open) didn't move the needle. This is the actual architectural fix: cache the data client-side, pass a reference, skip the regeneration entirely.
Changes
tethysdash_mcp/_uri_field.py(new) — shared Pydantic Field-factory for the*_uriopt-in pattern:uri_field(inline_arg_name=...)— Annotated metadata for the URI arg with regex validation against^mcp\+cache://<conv-id>/<token>$andmax_length=128ensure_exactly_one_set(inline, inline_name, uri, uri_name)— server-side validator for the mutual-exclusion contractPer-tool changes in
tethysdash_mcp/mcp_server.py:create_plotly_chart(line 317),create_data_table(line 465),create_card(line 635) each gain an optionaldata_uriarg alongsidedata(now also Optional). Tool descriptions fordatarecommenddata_uriwhen the data came from a prior tool call in the conversation.dataanddata_uriset → exactly-one-of errordata_uriarrived unresolved at the server → "unresolved URI" error with afix_hintexplaining chatbox-core mediation is expected (catches unmediated clients like Claude Desktop)Backward compat
Mediated happy path (chatbox-core substitutes
data_uri→dataand dropsdata_uribefore dispatch) flows through the existing inline-data branch unchanged. Every existing call that passes onlydatacontinues to work — verified by 3 regression tests, one per create_* tool.Tests
13 initial tests in
test_data_uri_opt_in.pycovering:Companion changes
feat/indexeddb-result-cache(PR feat(engine): MCP result-by-reference protocol — IndexedDB cache + URI substitution (Units 1-4) chatbox-core#36) — Units 1-4 of the plan: IndexedDB cache module, tool-result instrumentation, URI substitution layer,enableResultCachehost prop. Ships as v0.7.0.enableResultCache={true} conversationId={dashboardUuid}on its<ChatSidebar>mount, shipped at tethysplatform/tethysapp-tethys_dash@05b7c08 onfeature/tethysdash-mcp-server.Independent value
This PR is independently safe to merge ahead of the chatbox-core companion: the schema change is additive (existing inline-data calls work unchanged), the unmediated-client rejection path is a clear error for any non-chatbox-core MCP client that might encounter the new arg in
tools/list.Follow-up commit: records-mode + envelope unwrap (
116ea40)Smoke-testing the cache+URI path mid-shipment on Ollama Cloud (nemotron-3-{nano,super}, qwen-3.5-397b, deepseek-pro-4) exposed a structural mismatch that this PR's original commit didn't fully handle, plus a closely-related LLM-as-ETL bypass on chart creation. Both fixed in
116ea40.The bug
create_plotly_chartandcreate_data_tablerejected calls with a recurring Pydantic 2-error signature:Translation:
dataarrived as neither a list nor a string — typically a dict. Causal chain: nrds_mcps emits{ok, rows, columns, data:[records], ...}(~25KB envelope) → chatbox-core caches it + mints_cache_uri→ truncation drops the data array but preserves_cache_uri→ LLM correctly passesdata_uri = "mcp+cache://..."to viz tool → chatbox-core's substitution layer writes the WHOLE cached envelope intodata(it's intentionally dumb — copies the payload verbatim) → Pydantic rejects.A third layer surfaces even after envelope unwrap: nrds emits records (
[{feature_id, time, flow}, ...]) but Plotly expects traces ([{x:[...], y:[...], type:"scatter"}, ...]). The records→traces pivot was implicitly done by the LLM before the URI protocol; bypassing the LLM bypasses the transform.The fix (server-side defenses on
mcp_server.py)BeforeValidator(_unwrap_data_envelope)oncreate_plotly_chart.dataandcreate_data_table.data— unwraps dict envelopes (extracts first list-valueddata/rows/recordskey) before Pydantic's Union check. Published schema is unchanged — the LLM-visible type staysUnion[List, str].create_plotly_chart— three new optional args (x_field,y_field,series_field). Server detects records (list of dicts without Plotly trace keys) and pivots into traces. LLM names columns instead of constructing trace arrays — removes the LLM-as-ETL step.layout/config(Optional[Dict] args). Models emit"None"/"null"/""for genuinely-empty optional dicts; server coerces to actual None. Models also emit nested dicts as JSON strings; server decodes withjson.loads._uri_field.pydescription — dropped"DO NOT" / "WRONG" / "wasting tokens"framing that was suspected of biasing weak models.Tests + verification
24 additional tests in
test_data_uri_opt_in.py:Combined suite: 821/821 passing.
End-to-end smoke test: 240-row time-series chart prompt against nemotron-3-super and nemotron-3-nano-30b on Ollama Cloud — chart renders successfully after server restart.
Documentation
Solution captured at
docs/solutions/integration-issues/mcp-data-envelope-unwrap-and-records-pivot-2026-05-19.md(firoh workspace). Lightweight cross-link refresh applied to two related best-practices docs.Plan + companion PR
docs/plans/2026-05-18-002-feat-mcp-result-by-reference-protocol-plan.md(firoh workspace)dataUnion) — orthogonal, may also merge.