MCP budget: get_query_store_top's default stays under 32 KB (#4198) - #4270
Closed
erikdarlingdata wants to merge 5 commits into
Closed
erikdarlingdata wants to merge 5 commits into
erikdarlingdata wants to merge 5 commits into
Conversation
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
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
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
Owner
Author
|
Re-creating as non-draft to fix missing Build CI (see PR history for context) |
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
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.
Part of #4198.
Why
get_query_store_top(Darling and Lite) had no MCP response-size budget.query_textwas truncated only at 2,000 characters, with no opt-in for the full statement and no flag saying a row was cut. At the defaulttop=20, a busy production store measured 48 KB, over the shared 32 KB budget (McpResponseBudget.DefaultBytes).What changes
top(row count) stays 20. This row is narrow enough (17 fields) that only the wide field needed cutting.get_plan_correctionsandget_query_store_regressionshave wider rows that required a row-count cut too.full_textopt-in (defaultfalse) on both products, plus a per-rowquery_text_truncatedflag. This mirrorsget_plan_corrections's existing shape and thefull_textnameget_store_query_statsalready uses.previewLengthoverload (noasyncon the public wrapper, matching the shape#3897's trend tools use forTrendBudget.Chart). The web viewer's/api/readrow calls that overload directly withpreviewLength: 2000andfull_text: QueryBool(c, "full_text", false). That is the exact numberquery_textwas already capped at before this PR, so the viewer's page is unchanged. This differs fromget_plan_corrections/get_deadlock_detail's web rows, which default tofull_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'sDeadlockDetailWebDefaultTests) checks the row's exact arguments.FieldPreviewCutKeysinMcpPayloadContractCensusTestsnow lists this tool's two files alongsideDarlingMcpPlanCorrectionTools.cs/McpPlanCorrectionTools.csunder the samequery_text_truncatedkey.tools/listbudget 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 includesfull_text.#4224 roster note
This tool is on PR #4224's (not yet merged)
McpReadToolBudgetLiveTestsExemptOffendersroster. Per the coordinator's ruling, that wait is lifted since #4224 is stalled.McpReadToolBudgetLiveTestsis not ondevyet, so nothing in this PR touches it. Whichever of #4224 or this PR merges second must removeget_query_store_topfrom 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_healthin the sameDarlingMcpDataTools.csfile. This PR's diff in that file is scoped toget_query_store_toponly: a new private const, the signature split, and the projection.Test plan
Measured on the reused
rig-tgrig (port 55982).darlingtest/probedropped and recreated before use, per the lane brief.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=truereturns the whole statement.query_text_truncatedis correct on both sides of the 400-char line.QueryStoreTopBudgetTests(Lite): same shape, no rig needed.Darling.Testsfull 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.Testsfull suite: 5,345 total, 0 failed, ~4 min.McpToolsListBudgetTests(both products),McpPayloadContractCensusTests,QueryStoreTopWebDefaultTests,QueryStoreTopWindowTests(re-anchored to the new internal method),DarlingCustomViewsTests,DeadlockDetailWebDefaultTests.0 Warning(s),0 Error(s).One failure found and fixed: the first full run caught
DarlingCustomViewsTests.Catalog_ParamsNameTheActualWireQueryKeys_NotCSharpParamNamesfailing. The Custom Views catalog reflectsget_query_store_top's MCP parameters, and the test's hardcoded expected-keys list was stale. Fixed by addingfull_textto 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_historyhypertable 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
EventWindowedReadsAreBoundedLivePostgresTestsfailure: worth a look on a fresh rig if it recurs elsewhere tonight, since several lanes reused long-lived rigs.get_collection_healthchange inDarlingMcpDataTools.csmerges cleanly against this diff.CHANGELOG entry
SECTION: Fixed
ENTRY:
get_query_store_topstays under the MCP response-size budget ([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]): The default call returned about 48 KB, over the 32 KB limit some MCP clients enforce.query_textis now a 400-character preview by default. A newfull_textargument returns the whole statement. A newquery_text_truncatedflag on each row says whether the text was cut. The web viewer is unchanged and still shows up to 2,000 characters.REF:
[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]: 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
Generated with Claude Code
https://claude.ai/code/session_01FVjn4PBJN71NQXdFo6ZxNQ