Skip to content

fix: inject available cache_uri hints into MCP tool error envelopes - #46

Merged
romer8 merged 1 commit into
mainfrom
fix/error-envelope-cache-uri-hint
May 29, 2026
Merged

fix: inject available cache_uri hints into MCP tool error envelopes#46
romer8 merged 1 commit into
mainfrom
fix/error-envelope-cache-uri-hint

Conversation

@romer8

@romer8 romer8 commented May 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

Closes a real underperformance pattern observed against the workshop stack: NRDS query → create_plotly_chart fail → re-query NRDS → retry chart. The wasted upstream round-trip costs 5–30s on local Ollama models.

Root cause (from /ce:debug):

  • The error envelope on a failed create_plotly_chart (engine-side cap, MCP-server Pydantic validation, or any {error: "..."} shape) tells the LLM abstractly to use data_uri, but does NOT include the URI value.
  • The LLM has to scroll back through prior tool messages to find a _cache_uri.
  • Small/quantized models (qwen3-coder, glm-flash, gemma3) reliably drop the URI from attention by the time they see the chart failure, so they defend by re-running the upstream query to get a fresh URI in immediate context.

Fix: Engine-side enrichment. When a tool returns {error: "..."}, scan the last 10 role: "tool" messages backward, harvest _cache_uri per upstream tool name (deduped, most-recent-first), and append available_cache_uris: [{tool_name, cache_uri}, ...] to the LLM-visible envelope. The URI now lives in immediate error-envelope context — no scrollback required.

Wire-up

Site Line Enriched
Inline-list cap engine/index.js:934
Dispatch path (MCP-server error) :1270 ✓ (BEFORE truncation block)
Cache-miss :957 ✗ deliberately — _missing_uris already names the failing URIs verbatim

Critical ordering: the dispatch-site enrichment runs BEFORE the truncation block at :1179. The truncation summary at :1257-1264 then explicitly copies available_cache_uris from resultForLlm alongside _cache_uri. Without this ordering, oversized error envelopes would silently drop the field — exactly the case where the LLM most needs the hint (HIGH-severity feasibility finding from document-review on the plan).

Short-circuits:

  • cacheOptions.enabled !== true → URI cannot exist; zero JSON.parse work.
  • typeof envelope.error !== "string" → only enrich error envelopes.
  • Empty messages array → no scan.

Test plan

  • 11 helper-level cases in engine/cacheUriHint.test.js (happy paths, malformed JSON, scan window, dedup, short-circuits, defensive overwrite, mutation invariant).
  • 7 integration cases through processToolCalls (inline-cap wire-up, dispatch error wire-up, truncation + error for HIGH feasibility finding, success path unchanged, first-turn empty history, enableResultCache=false host).
  • Cache-miss non-enrichment enforced by wire-up location (documented in test file; integration would require fake-indexeddb harness, covered in cache.test.js).
  • Full chatbox-core suite: 698/698 across 43 files, no regressions.
  • tethysdash visual smoke after npm publish + bundle rebuild — pending image rebake.

Release

Version bump: 0.16.0-beta.00.16.1-beta.0 (patch bump on beta line, additive engine behavior, zero API surface change for hosts).

After merge: npm publish --tag beta from a fresh checkout.

Plan

docs/plans/2026-05-28-003-fix-error-envelope-cache-uri-hint-plan.md — reviewed via headless document-review (1 HIGH feasibility, 1 HIGH coherence, 4 MODERATE + 2 LOW auto-fixed before code).

When an MCP tool returns an `{error: "..."}` envelope, the engine now
scans the last 10 role=tool messages backward, harvests `_cache_uri` per
upstream tool name (deduped, most-recent-first), and appends
`available_cache_uris: [{tool_name, cache_uri}, ...]` to the LLM-visible
envelope before pushing into history.

Fixes the observed underperformance pattern: NRDS query → create_plotly_chart
fail → re-query NRDS → retry chart. Small/quantized models reliably drop
`_cache_uri` values from attention by the time they see a chart-tool
failure, so they defend by re-querying instead of reusing the cached
pointer. With the new field, the URI lives in immediate error-envelope
context — no scrollback required.

Wire-up:
- Inline-list-cap site (engine/index.js:934): enriched.
- Dispatch path (:1270): enriched BEFORE the truncation block so
  oversized error envelopes preserve the field through summary build.
- Cache-miss site (:957): NOT enriched (`_missing_uris` already names
  the failing URIs verbatim; peer-row enrichment would add no signal).
- Short-circuits when cacheOptions.enabled !== true (URI cannot exist).

Plan: docs/plans/2026-05-28-003-fix-error-envelope-cache-uri-hint-plan.md

Tests: 19 new in engine/cacheUriHint.test.js (11 helper + 7 integration +
1 invariant). Full suite: 698/698 across 43 files.

Bumps to 0.16.1-beta.0 on the beta dist-tag.
@romer8
romer8 merged commit 5e57071 into main May 29, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant