Skip to content

MCP budget: get_query_store_top's default stays under 32 KB (#4198) - #4270

Closed
erikdarlingdata wants to merge 5 commits into
devfrom
fix/4198-qs-top-default
Closed

erikdarlingdata wants to merge 5 commits into
devfrom
fix/4198-qs-top-default

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 25, 2026 •

Copy link
Copy Markdown
Owner

Part of #4198.

Why

get_query_store_top (Darling and Lite) had no MCP response-size budget. query_text was truncated only at 2,000 characters, with no opt-in for the full statement and no flag saying a row was cut. At the default top=20, a busy production store measured 48 KB, over the shared 32 KB budget (McpResponseBudget.DefaultBytes).

What changes

  • Default preview cut to 400 characters. top (row count) stays 20. This row is narrow enough (17 fields) that only the wide field needed cutting. get_plan_corrections and get_query_store_regressions have wider rows that required a row-count cut too.
  • New full_text opt-in (default false) on both products, plus a per-row query_text_truncated flag. This mirrors get_plan_corrections's existing shape and the full_text name get_store_query_stats already uses.
  • Darling's MCP signature forwards to an internal previewLength overload (no async on the public wrapper, matching the shape #3897's trend tools use for TrendBudget.Chart). The web viewer's /api/read row calls that overload directly with previewLength: 2000 and full_text: QueryBool(c, "full_text", false). That is the exact number query_text was already capped at before this PR, so the viewer's page is unchanged. This differs from get_plan_corrections/get_deadlock_detail's web rows, which default to full_text/full_graph: true. Those fields had no cap before MCP read tools have no default response-size budget: at default arguments 12 tools return >50 KB and 3 return >100 KB for one server, more than an agent client's per-result cap #4198, so keeping the viewer unchanged meant keeping full text. This field already had a cap, so keeping the viewer unchanged means keeping the same 2,000-character limit. A no-rig source-pin test (QueryStoreTopWebDefaultTests, mirroring #4254's DeadlockDetailWebDefaultTests) checks the row's exact arguments.
  • Census and budget bookkeeping: FieldPreviewCutKeys in McpPayloadContractCensusTests now lists this tool's two files alongside DarlingMcpPlanCorrectionTools.cs/McpPlanCorrectionTools.cs under the same query_text_truncated key. tools/list budget ceilings raised by the measured growth: Darling +136 bytes, Lite +125 bytes (84 bytes of new-parameter description plus its JSON schema wrapper). DarlingCustomViewsTests' wire-key pin for this tool now includes full_text.

#4224 roster note

This tool is on PR #4224's (not yet merged) McpReadToolBudgetLiveTests ExemptOffenders roster. Per the coordinator's ruling, that wait is lifted since #4224 is stalled. McpReadToolBudgetLiveTests is not on dev yet, so nothing in this PR touches it. Whichever of #4224 or this PR merges second must remove get_query_store_top from that roster. It is now fixed and belongs under budget, not on the exempt list.

Touched only this tool's code

Lane TK is changing get_collection_health in the same DarlingMcpDataTools.cs file. This PR's diff in that file is scoped to get_query_store_top only: a new private const, the signature split, and the projection.

Test plan

Measured on the reused rig-tg rig (port 55982). darlingtest/probe dropped and recreated before use, per the lane brief.

  • New live test QueryStoreTopBudgetLiveTests (Darling): seeds 25 rows across 5 query-text lengths (120-2,600 chars). Default call measured 17,394 bytes (was 48 KB pre-fix). Well under the 32 KB budget, with margin for real-world field-width variance. full_text=true returns the whole statement. query_text_truncated is correct on both sides of the 400-char line.
  • New DuckDB test QueryStoreTopBudgetTests (Lite): same shape, no rig needed.
  • Darling.Tests full suite: 13,887 total, 0 failed (1 unrelated failure below, fixed and re-verified). 47 skipped, 1 not-run, both pre-existing categories unrelated to this change. ~11 min.
  • Lite.Tests full suite: 5,345 total, 0 failed, ~4 min.
  • Targeted re-runs green: McpToolsListBudgetTests (both products), McpPayloadContractCensusTests, QueryStoreTopWebDefaultTests, QueryStoreTopWindowTests (re-anchored to the new internal method), DarlingCustomViewsTests, DeadlockDetailWebDefaultTests.
  • Both projects build with 0 Warning(s), 0 Error(s).

One failure found and fixed: the first full run caught DarlingCustomViewsTests.Catalog_ParamsNameTheActualWireQueryKeys_NotCSharpParamNames failing. The Custom Views catalog reflects get_query_store_top's MCP parameters, and the test's hardcoded expected-keys list was stale. Fixed by adding full_text to that list. Re-ran the class alone (71/71 green) and then the full suite again.

One failure in the full run judged not mine: EventWindowedReadsAreBoundedLivePostgresTests.TheFlooredReads_PlanAtMostThreeChunksForA24HourWindow_AgainstDevPostgres. It is an EXPLAIN-plan chunk-count assertion in a file I never touched, about #4229/#4235 (event-windowed reads). It reproduces alone on this rig too.

The job_history hypertable accumulates many TimescaleDB chunks over a long-lived rig's full-suite runs. The EXPLAIN plan is sensitive to that count. This is not a regression from this diff. dev's last Build run is green. Flagging for the coordinator's census rather than re-running against a from-scratch rig, which did not fit this lane's remaining budget.

Open items for the coordinator

  • The A budget pin over every MCP read tool (part of #4198) #4224 roster removal above, whichever PR merges second.
  • The EventWindowedReadsAreBoundedLivePostgresTests failure: worth a look on a fresh rig if it recurs elsewhere tonight, since several lanes reused long-lived rigs.
  • That Lane TK's get_collection_health change in DarlingMcpDataTools.cs merges cleanly against this diff.

CHANGELOG entry

SECTION: Fixed
ENTRY:

Generated with Claude Code

https://claude.ai/code/session_01FVjn4PBJN71NQXdFo6ZxNQ

erikdarlingdata and others added 3 commits September 25, 2026 04:48
query_text was truncated only at 2,000 characters with no opt-in. At the
default top=20 that measured 48 KB on a busy production store's single
server, over McpResponseBudget.DefaultBytes (32 KB). Cuts the default
preview to 400 characters, adds a full_text opt-in and a per-row
query_text_truncated flag, on both Darling and Lite. Darling's MCP
signature forwards to an internal overload taking an explicit preview
length, so the web viewer can keep the old 2000-char cap unchanged
(wired in a follow-up commit).

New live test (Darling, rig) and DuckDB test (Lite) seed 25 rows across
five query-text lengths and assert the default call stays under budget
while full_text still returns the whole statement.

Part of #4198.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3
Web viewer /api/read keeps the exact old 2000-char query_text cap
(through the new previewLength overload) instead of inheriting the
MCP tool's new 400-char default or switching to full text, pinned by
a no-rig source test. Census FieldPreviewCutKeys grows to cover this
tool's query_text_truncated key. tools/list budget gets the new
full_text parameter on both products, ceilings raised by the measured
growth (Darling +136 bytes, Lite +125).

Part of #4198.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3
Catalog_ParamsNameTheActualWireQueryKeys_NotCSharpParamNames asserts
the exact wire-key list per tool; add full_text now that the tool has
one.

Part of #4198.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TszxYhJJbTEh4LrZ56NYo3
@erikdarlingdata erikdarlingdata changed the title DO NOT MERGE (MCP budget): get_query_store_top's default stays under 32 KB (#4198) MCP budget: get_query_store_top's default stays under 32 KB (#4198) Sep 25, 2026
@erikdarlingdata
erikdarlingdata marked this pull request as ready for review September 25, 2026 09:44
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FVjn4PBJN71NQXdFo6ZxNQ
@erikdarlingdata

Copy link
Copy Markdown
Owner Author

Closing to retrigger Build workflow CI (draft PR never triggered it on open)

GitHub did not fire pull_request events for this draft-opened PR.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FVjn4PBJN71NQXdFo6ZxNQ
@erikdarlingdata

Copy link
Copy Markdown
Owner Author

Re-creating as non-draft to fix missing Build CI (see PR history for context)

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