Skip to content

fix(mcp): reduce LLM-facing tool friction, token cost, and a scratch-table leak - #332

Open
StefanSteiner wants to merge 2 commits into
tableau:mainfrom
StefanSteiner:fix/mcp-tool-ergonomics-and-scratch-leak
Open

StefanSteiner wants to merge 2 commits into
tableau:mainfrom
StefanSteiner:fix/mcp-tool-ergonomics-and-scratch-leak

Conversation

@StefanSteiner

Copy link
Copy Markdown
Contributor

Summary

Makes the HyperDB MCP more reliable and cheaper for an LLM to drive. Came out of a systematic pass exercising all 33 tools live and tracing the friction points back to source. Two axes: fewer LLM mistakes, and lower always-in-context token cost.

Bugs fixed

  • Scratch-table leak: query_data / query_file dropped their _tmp_* staging table only after the query ran, so a failed query orphaned it — and it then showed up in describe as a mystery table. The DROP now runs whether the query succeeds or fails.
  • Substring-collision in SQL rewrite: the table-name substitution was a naive sql.replace(alias, temp) that corrupted a column like metadata when the alias was data. Replaced with a whole-word replace_identifier helper (unit-tested). Known limitation documented: it is identifier-boundary aware but not SQL-aware, so a string literal exactly equal to the alias is still rewritten — rare, since the alias is a table name.
  • describe clutter: table listings now hide transient scratch tables (_tmp_*, __hyperdb_merge_*), not just _hyperdb_* internals.

Ergonomics

  • query_file default table is now data (matching query_data and the documented FROM data example, which previously failed because the default was the unpredictable file stem).
  • kind is now optional on attach_database and copy_query's temp_attach, defaulting to local_file — no more required ceremony field with only one legal value.
  • Better unknown-table error: a 42P01 now suggests running describe and names the data alias, instead of the generic "check SQL syntax."

Token cost

  • Trimmed the essay-length tool descriptions (load_file, export, chart, load_files, set_table_metadata, kv_set + 3 updated): ~15.8k → ~11.7k chars (~26%) of always-in-context tool-catalog text. The format-selection and edge-case depth moved into get_readme (consolidated format-preference block), which stays under its 24k budget.

Type-name casing normalization (TEXT vs text) was deliberately deferred — it originates from two engine code paths asserted on by many tests, and type names are case-insensitive so it causes no actual LLM mistakes (high-risk / low-value).

Test plan

  • cargo test -p hyperdb-mcp --all-targets against pinned .hyperd/current — 669 tests, 0 failures (incl. new replace_identifier and is_internal_table unit tests)
  • cargo clippy --workspace --all-targets --all-features -- -D warnings — clean
  • cargo fmt --all — no diff
  • npx markdownlint-cli2 on the touched CHANGELOG — 0 issues
  • Independent code review — approved merge-ready
  • All 33 tools exercised live against a running server

…table leak

Improves how reliably an LLM can drive the MCP and cuts the always-in-context
token cost of the tool catalog, plus fixes two real scratch-table bugs.

Bugs:
- query_data / query_file dropped their `_tmp_*` scratch table only after the
  query, so a failed query orphaned it (and it then surfaced in `describe`).
  The DROP now runs whether the query succeeds or fails.
- The SQL table-name substitution was a naive substring replace that corrupted
  columns like `metadata` when the alias was `data`; it is now whole-word via
  `replace_identifier`.
- `describe` now hides transient scratch tables (`_tmp_*`, `__hyperdb_merge_*`),
  not just `_hyperdb_*` internals.

Ergonomics:
- query_file exposes rows as `data` by default (matching query_data and the
  documented example, which previously failed).
- `kind` is optional on attach_database and copy_query temp_attach, defaulting
  to `local_file`.
- Unknown-table (42P01) errors now suggest `describe` and name the `data`
  alias instead of the generic "check SQL syntax".

Tokens:
- Trimmed the essay-length tool descriptions (~15.8k -> ~11.7k chars, ~26%),
  moving format/edge-case depth into get_readme while keeping actionable rules
  inline. get_readme stays under its 24k budget.
`is_internal_table`'s doc linked to the private `INTERNAL_SCRATCH_PREFIXES`
const, which `cargo doc` rejects under `RUSTDOCFLAGS=-D warnings`
(rustdoc::private_intra_doc_links). Demote it to a plain code span.
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