Skip to content

MCP: read_range returns long ranges in pages, and tool results are compact JSON - #1086

Open
CouchRiv wants to merge 3 commits into
storytold:mainfrom
CouchRiv:fix/read-range-paging
Open

CouchRiv wants to merge 3 commits into
storytold:mainfrom
CouchRiv:fix/read-range-paging

Conversation

@CouchRiv

Copy link
Copy Markdown
Contributor

Reading an export is the first thing an agent does with a workbook, and the engine is fast at it, but the MCP reply wasn't usable at size. read_range without range returned the whole used range in one reply (up to the 1,000,000-cell limit), and every tool result was pretty-printed, one cell value per line. On a synthetic 15,000 × 30 CSV (generated data), read_range {} was one text block of 6,582,075 characters on 480,038 lines, with nothing telling the agent how to ask for less. MCP clients cap tool output, so in practice the agent got a truncated or spilled-to-disk result.

What changed

Two independent commits: pagination first, compact tool-result JSON second. Pagination can land without compact JSON if indented output should be retained, but must include the follow-up commit that rejects invalid maxCells values.

1. Paging (MCP: read_range returns long ranges in pages)

  • sheet.read takes maxCells, a positive integer; zero, negative and non-integer values return an error, including on an empty sheet. It reads whole rows up to that many cells (at least one row, so a row wider than maxCells still comes back whole). When the range is longer it adds truncated: true, totalRows (rows of the whole range) and next, the remaining rows as a sheet-qualified range ('My Data'!A334:AD15001). Passing next back as range reads on from there whatever the active sheet is. Without maxCells the command behaves exactly as before, so the control channel and CLI are unchanged.
  • The MCP read_range tool exposes maxCells and passes 5,000 when the caller gives none. The 1,000,000-cell limit still applies to what is read.
  • params text, the tool description, docs/mcp.md and docs/control-protocol.md describe it.

2. Compact JSON (MCP: tool results as compact JSON)

  • ToolResult::json uses serde_json::to_string instead of to_string_pretty. Agents parse the JSON either way; indented, 100 rows × 30 columns took 44,368 characters on 3,238 lines, compact 22,132 on one. docs/mcp.md said "pretty JSON" and now says compact.

Not included: an ISO date option for read_range (dates come back as serials unless formatted, where numbers become text too), and documenting in write_range that "'0010" or a Text format keeps a code with leading zeros as text.

Verification (Linux x86_64, original baseline fc45b7f)

  • cargo test -p gridcraft-engine --lib tests::reading_a_range_in_pages: 10 rows × 3 columns on a sheet named My Data with another sheet active. maxCells: 7 gives A1:C2, totalRows 10 and next 'My Data'!A3:C10; reading next returns the 8 remaining rows with no truncated; a positive maxCells smaller than a row gives one row; no maxCells gives the whole range. On main: left: "A1:C10", right: "A1:C2". Passes here.
  • cargo test -p gridcraft-mcp tests::read_range_pages_long_ranges: 2,000 rows × 3 through write_range, then read_range {} gives A1:C1666, truncated, totalRows 2000, next Sheet1!A1667:C2000; the next call reads the rest; maxCells: 6000 reads everything. On main: left: ("A1:C2000", Null, Null). Passes here.
  • cargo test -p gridcraft-mcp tests::tool_results_are_compact_json: the text of a read_range result has no line break. Fails on main, passes here.
  • gridcraft-cli mcp on the synthetic 15,000 × 30 CSV:
call main (fc45b7f) this branch
read_range {} 6,582,075 chars, 480,038 lines, A1:AD15001 36,398 chars, 1 line, A1:AD166, truncated, totalRows 15001, next large!A167:AD15001
read_range {range: "A1:AD101"} 44,368 chars, 3,238 lines 22,132 chars, 1 line
  • cargo fmt --all -- --check, cargo clippy -p gridcraft-engine -p gridcraft-mcp --all-targets -- -D warnings, cargo test -p gridcraft-engine -p gridcraft-mcp -p gridcraft-cli

Limits and questions

  • The default of 5,000 cells is a guess at what fits a client's output limit with room to spare: about 36,000 characters on that export, more with long text. Happy to change it, or to count characters instead of cells.
  • An agent that relied on read_range {} returning the whole used range now gets the first page with truncated: true, and needs maxCells or next to read the rest.

Independent verification against updated main (84acf62, 2026-10-13)

Before the invalid-maxCells follow-up, the independent review reran cargo fmt --all -- --check, cargo clippy -p gridcraft-engine -p gridcraft-mcp --all-targets -- -D warnings and cargo test -p gridcraft-engine -p gridcraft-mcp on the branch and its clean merge with 84acf62; all exited 0. The added regression tests failed with production code from 84acf62 and passed on both branch and merged trees. These checks are separate from the historical fc45b7f measurements above.

Follow-up verification: invalid maxCells (2026-10-13)

  • sheet_read_rejects_invalid_max_cells fails without the fix, including on an empty sheet, and passes with it.
  • read_range_rejects_invalid_max_cells writes 2,000 rows by 3 columns and confirms that the default page contains 1,666 rows. Without the fix, maxCells: -1 returns all 2,000 rows instead of an error. With the fix, negative integers and zero fail; non-integer MCP arguments fail as JSON-RPC invalid params. maxCells: 1 still returns one complete row.
  • cargo fmt --all -- --check, cargo clippy -p gridcraft-engine -p gridcraft-mcp --all-targets -- -D warnings, and cargo test -p gridcraft-engine -p gridcraft-mcp pass: 245 engine tests and 22 MCP tests, no failures. These checks ran on the branch after the follow-up, separately from the earlier review of its merged tree.

Conflicts

Against upstream/main at 84acf62, git merge-tree is clean. #884 (my open conditional-format PR) conflicts only where the engine tests are inserted in tests.rs; retain both tests. #870, #480, #885 and #1064 merge cleanly. The review covered 100 listed open PRs plus #480, not every older PR.

CouchRiv and others added 3 commits October 12, 2026 23:26
read_range without `range` returned the whole used range in one reply, up
to the 1,000,000-cell limit: a 15,000-row export of 30 columns is 6.6 MB
of JSON, far more than an agent can take in one tool result, and there
was no way to ask for less but to compute ranges by hand.

sheet.read takes `maxCells`: it reads whole rows up to that many cells (at
least one row) and, when the range is longer, adds `truncated: true`,
`totalRows` and `next`, the sheet-qualified range of the remaining rows to
pass back as `range`. Without `maxCells` nothing changes. The MCP
read_range tool passes maxCells 5,000 unless the caller gives one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every tool result was pretty-printed, so read_range put each cell value on
a line of its own: 100 rows of 30 columns took 44,368 characters on 3,238
lines, against 22,132 on one line compact. Agents parse the JSON either
way; compact halves what a range read costs them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Validate the engine parameter before empty-sheet handling so invalid values cannot bypass paging. Cover engine and MCP callers and document the positive integer requirement.

Co-Authored-By: OpenAI Codex <noreply@openai.com>

This branch has not been deployed

No deployments
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