Skip to content

DEV-1810: MCP query row cap — default 20 rows, LIMIT push-down, truncation notice - #369

Merged
ZmeiGorynych merged 6 commits into
mainfrom
egor/dev-1810-mcp-query-tool-add-a-response-row-cap-with-a-truncation
Sep 4, 2026
Merged

DEV-1810: MCP query row cap — default 20 rows, LIMIT push-down, truncation notice#369
ZmeiGorynych merged 6 commits into
mainfrom
egor/dev-1810-mcp-query-tool-add-a-response-row-cap-with-a-truncation

Conversation

@ZmeiGorynych

Copy link
Copy Markdown
Member

Fixes DEV-1810.

What

  • The MCP query tool caps returned rows at 20 when the caller passes no limit; an explicit limit is trusted verbatim — no ceiling, no response-side truncation, even for explain plan rows.
  • On the structured no-limit path the generated SQL carries LIMIT 21 (cap + 1), so truncation is detectable without fetching the full result; a post-execution slice guards the paths push-down can't reach (run-by-name stored queries, explain plans). Run-by-name stored SQL is never modified.
  • query_nested applies the same rule keyed on the ROOT (last) stage's limit only; the pushed-down limit goes into a copy of the root dict, so caller input is never mutated. Non-root limits neither lift nor lower the cap.
  • A truncated response carries a new ResponseTruncationWarning (kind "truncated", returned_rows, hint) added to the AnySlayerWarning union in slayer/core/warnings.py. It rides the existing warnings machinery, so all three formats stay in sync by construction: markdown trailing Warnings: block, csv leading # comment line, json {"data", "warnings"} shape. It coexists with engine warnings and is appended last. The engine never emits it — additive schema only from the REST API's perspective.
  • Docs: docs/reference/mcp.md (cap, root-stage rule, per-format warnings rendering incl. the json shape change) and .claude/skills/slayer-query.md (one-line cap mention); limit docstrings updated on both tools, so the rendered tool schema mentions the cap.
  • Engine, REST API, Flight, PG facade, stored-query semantics: unchanged.

Tests

  • New tests/test_mcp_row_cap.py (25 tests, one class per spec requirement): default cap boundary cases (20 / 21 rows) against a live sqlite datasource; explicit limit trusted verbatim (incl. mocked engine returning more rows than the limit — no slice, no notice); LIMIT 21 push-down visible in dry_run / show_sql and absent from run-by-name stored SQL; run-by-name capping at/above the cap; query_nested root-stage keying (non-root limit ignored, root limit trusted, caller dicts not mutated); notice rendering in all three formats incl. coexistence ordering with an engine warning; union round-trip of ResponseTruncationWarning; explain-plan capping and dry_run never truncating.
  • Full non-integration suite: 15555 passed, 0 failed. Ruff clean. Conventions gate CLEAR.

Spec surface (openspec show dev-1810-mcp-query-tool-add-a-response-row-cap-with-a-truncation --diff)

mcp/response-row-cap — seven added requirements
mcp/response-row-cap

  ADDED: Default row cap on MCP query responses
    ### Requirement: Default row cap on MCP query responses
    
    When the MCP `query` tool is called without a `limit`, the response SHALL contain at most 20 data rows. When the underlying result has more rows than the cap, the response SHALL be truncated to exactly 20 rows and carry a truncation notice.
    
    #### Scenario: Uncapped query over a large result
    
    - WHEN `query` runs without `limit` against a model whose result has more than 20 rows
    - THEN the response contains exactly 20 data rows and a truncation notice
    
    #### Scenario: Result exactly at the cap
    
    - WHEN `query` runs without `limit` and the result has exactly 20 rows
    - THEN all 20 rows are returned and no truncation notice appears
    
    #### Scenario: Result one past the cap
    
    - WHEN `query` runs without `limit` and the result has exactly 21 rows
    - THEN the response contains exactly 20 rows and a truncation notice

  ADDED: Explicit limit is trusted verbatim
    ### Requirement: Explicit limit is trusted verbatim
    
    When the caller passes an explicit `limit`, the MCP layer SHALL return the rows as executed, with no response-side truncation and no truncation notice — regardless of how many rows come back, including `explain` plan rows.
    
    #### Scenario: Explicit limit honored
    
    - WHEN `query` runs with `limit=25` against a result with 30 available rows
    - THEN 25 rows are returned and no truncation notice appears
    
    #### Scenario: Explicit small limit
    
    - WHEN `query` runs with `limit=5`
    - THEN 5 rows are returned and no truncation notice appears
    
    #### Scenario: Rows exceeding an explicit limit are not sliced by the MCP layer
    
    - WHEN the engine returns more rows than an explicit `limit` (e.g. a mocked execution)
    - THEN the MCP layer neither slices the rows nor adds a truncation notice
    
    #### Scenario: Explain with explicit limit untouched
    
    - WHEN `query` runs with `explain=True` and an explicit `limit`, and the plan has more rows than the limit
    - THEN all plan rows are returned and no truncation notice appears

  ADDED: Cap push-down into the generated query
    ### Requirement: Cap push-down into the generated query
    
    When no explicit `limit` is given on the structured query path, the generated SQL SHALL carry `LIMIT 21` (cap + 1) so truncation is detectable without fetching the full result. Run-by-name execution of a stored query SHALL leave the stored query's SQL untouched.
    
    #### Scenario: Pushed-down limit visible in SQL
    
    - WHEN `query` runs without `limit` on the structured path with `show_sql=True` or `dry_run=True`
    - THEN the generated SQL contains `LIMIT 21`, not `LIMIT 20`
    
    #### Scenario: Stored query SQL untouched
    
    - WHEN a stored query runs by bare name (run-by-name path) without `limit`
    - THEN the SQL executed is the stored query's own, with no injected LIMIT

  ADDED: Run-by-name responses are capped response-side
    ### Requirement: Run-by-name responses are capped response-side
    
    A stored query executed by bare name without a `limit` SHALL have its response sliced to 20 rows with a truncation notice when it returns more, and returned whole with no notice when it returns 20 or fewer.
    
    #### Scenario: Stored query above the cap
    
    - WHEN a run-by-name stored query returns more than 20 rows
    - THEN the response contains exactly 20 rows and a truncation notice
    
    #### Scenario: Stored query at the cap
    
    - WHEN a run-by-name stored query returns exactly 20 rows
    - THEN all 20 rows are returned and no truncation notice appears

  ADDED: query_nested capped by the root stage's limit only
    ### Requirement: query_nested capped by the root stage's limit only
    
    The `query_nested` tool SHALL apply the same rule keyed on the ROOT stage (last entry of `queries`): an explicit root `limit` is trusted verbatim; without one, the final response is capped at 20 with a truncation notice whose hint points at the root query's `limit`. Non-root stages' limits SHALL NOT affect the cap. The tool SHALL NOT mutate the caller's `queries` dicts.
    
    #### Scenario: Root without limit is capped
    
    - WHEN `query_nested` runs with a root stage that has no `limit` and the final result has more than 20 rows
    - THEN the response contains exactly 20 rows and a truncation notice telling the caller to set a higher `limit` on the root query
    
    #### Scenario: Non-root limit does not lift the cap
    
    - WHEN a non-root stage has an explicit `limit` but the root stage has none
    - THEN the default cap of 20 still applies to the final response
    
    #### Scenario: Root limit trusted
    
    - WHEN the root stage has an explicit `limit`
    - THEN no response-side truncation occurs and no notice appears
    
    #### Scenario: Caller dicts unchanged
    
    - WHEN `query_nested` pushes the cap into the root stage
    - THEN the caller's submitted `queries` dicts are structurally unchanged afterwards (no `limit` key added)

  ADDED: Truncation notice content and rendering
    ### Requirement: Truncation notice content and rendering
    
    The truncation notice SHALL state the returned row count, say that more rows exist, and tell the caller how to get more rows. It SHALL appear in every output format through the warnings channel: the markdown `Warnings:` block, a leading `#` comment line in csv, and a warning entry with kind `"truncated"` in the json `{"data", "warnings"}` payload. It SHALL coexist with other warnings, appended last.
    
    #### Scenario: Markdown notice
    
    - WHEN a truncated result is formatted as markdown
    - THEN the output ends with a `Warnings:` block containing "showing first 20 rows — more rows exist" and a hint to pass a higher `limit`
    
    #### Scenario: CSV notice
    
    - WHEN a truncated result is formatted as csv
    - THEN a leading `#` comment line carries the notice and the data rows below keep a uniform column count
    
    #### Scenario: JSON notice
    
    - WHEN a truncated result is formatted as json
    - THEN the payload has the `{"data", "warnings"}` shape and `warnings` contains an entry with `kind == "truncated"` and the returned row count
    
    #### Scenario: Coexists with other warnings
    
    - WHEN a truncated result already carries an engine warning
    - THEN both warnings render in every format and the truncation notice comes last
    
    #### Scenario: Warning round-trips through the union
    
    - WHEN a response carrying the truncation warning is serialized and re-validated
    - THEN the warning deserializes back to the truncation kind with its fields intact

  ADDED: Explain plan rows capped without a limit
    ### Requirement: Explain plan rows capped without a limit
    
    When `query` runs with `explain=True` and no `limit`, the returned plan rows SHALL be subject to the same 20-row cap and notice. `dry_run` output (SQL only) SHALL never carry a truncation notice.
    
    #### Scenario: Large explain plan capped
    
    - WHEN `explain=True` without `limit` yields a plan of more than 20 rows
    - THEN 20 plan rows are returned with a truncation notice
    
    #### Scenario: Dry run unaffected
    
    - WHEN `dry_run=True`
    - THEN the response contains only SQL and never a truncation notice

🤖 Generated with Claude Code

https://claude.ai/code/session_01JJT5gJBS5A4akHsisRomVC

@linear

linear Bot commented Sep 4, 2026

Copy link
Copy Markdown

DEV-1810

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 15 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available. Your 53 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Essentials

Run ID: a9702ff4-5e6b-4c2d-8893-bbc37ecddfac

📥 Commits

Reviewing files that changed from the base of the PR and between 26fbd1c and 9bf0fda.

📒 Files selected for processing (11)
  • .claude/skills/slayer-query.md
  • docs/reference/mcp.md
  • openspec/changes/archive/2026-09-04-dev-1810-mcp-query-tool-add-a-response-row-cap-with-a-truncation/.openspec.yaml
  • openspec/changes/archive/2026-09-04-dev-1810-mcp-query-tool-add-a-response-row-cap-with-a-truncation/design.md
  • openspec/changes/archive/2026-09-04-dev-1810-mcp-query-tool-add-a-response-row-cap-with-a-truncation/proposal.md
  • openspec/changes/archive/2026-09-04-dev-1810-mcp-query-tool-add-a-response-row-cap-with-a-truncation/specs/mcp/response-row-cap/spec.md
  • openspec/changes/archive/2026-09-04-dev-1810-mcp-query-tool-add-a-response-row-cap-with-a-truncation/tasks.md
  • openspec/specs/mcp/response-row-cap/spec.md
  • slayer/core/warnings.py
  • slayer/mcp/server.py
  • tests/test_mcp_row_cap.py

Comment @coderabbitai help to get the list of available commands.

Field attributes were appended after _format_output, which for markdown
already carries the trailing Warnings block, so a truncated response with
labeled/formatted fields no longer ended with the notice. Render the
attributes footer before the markdown warnings via a new _format_output
footer param; add a regression test.
@ZmeiGorynych

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@sonarqubecloud

sonarqubecloud Bot commented Sep 4, 2026

Copy link
Copy Markdown

@ZmeiGorynych
ZmeiGorynych merged commit 6576784 into main Sep 4, 2026
4 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