Repository navigation
fix(mcp): default list-tool pagesize and advertise it in schemas - #149
Merged
Merged
Conversation
An omitted pagesize reached the upstream as no limit: a single list_txs_for on an exchange hot wallet returned ~2.7 MB (~677k LLM tokens). _params_from now defaults pagesize to 25 and caps it at 100 for every list tool; callers page onward via next_page. Auto-generated tools inherited their OpenAPI response model as an MCP outputSchema (graph_summary ~3.4k tokens, list_block_txs ~3.2k) that no known client shows the model; component_fn drops it. tools/list payload shrinks from ~18k to ~9k tokens.
- Add PagesizeCapMiddleware so auto-generated tools (list_tx_flows) get the default/ceiling instead of reaching the route with no limit - Share the capped() helper in mcp/pagesize.py with the consolidated tools, keeping one policy - Verify paging onward (next_page cursors) still reaches the tail of fan-out txs
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| ) | ||
|
|
||
| logger.info(f"Redis lock {key} acquired.") | ||
| logger.info(f"Redis lock {key} acquired by {token}.") |
Member
There was a problem hiding this comment.
I think github mistook "token" as an access token. this token here is no secret
| try: | ||
| lock.release() | ||
| client.delete(_heartbeat_key(key), _alert_key(key)) | ||
| logger.info(f"Redis lock {key} released.") |
| logger.info(f"Redis lock {key} released.") | ||
| except Exception as e: | ||
| # Never let a release problem mask what the body did or raised. | ||
| logger.warning(f"Redis lock {key} could not be released: {e}") |
| f"needs this lock. Check that host, then force-release with: " | ||
| f"redis-cli DEL {key}" | ||
| ) | ||
| logger.warning(msg) |
| send_msg_to_topic(_ALERT_TOPIC, msg) | ||
| except Exception as e: | ||
| # Reporting must never turn lock contention into a crash. | ||
| logger.warning(f"Could not report stale holder of lock {key}: {e}") |
frederik-raphael
changed the title
An explicit page size is the caller's decision and the route already bounds it: PagesizeQuery declares ge=1, le=5000, and the Cassandra layer clamps again to BIG_PAGE_SIZE / SMALL_PAGE_SIZE. Clamping a second time at 100 in the MCP layer only stopped a model from asking for what the API is willing to serve. Omission is the part that needed fixing, and it still is. `None` reaches the route as the internal ceiling (5000 rows for list_txs_for) or, for list_tx_flows, as no pagination at all: the slice needs both a page and a page size, so every flow event comes back with next_page null. So MAX_PAGESIZE is gone and only the default remains. capped() becomes resolve_pagesize() and PagesizeCapMiddleware becomes PagesizeDefaultMiddleware, since neither clamps anything now. The list_neighbors docstring names both defaults explicitly: 25 unfiltered, 50 as a match target when tag_filter is set, which are different quantities that happened to read as one number. The round-trip test sizes its fixture from DEFAULT_PAGESIZE so it keeps exercising three pages if that default ever moves again.
Member
|
One thought on the middleware: as far as I can tell the model never actually sees the 25. It's in the docstrings of the hand-written tools, but for list_tx_flows (the one the middleware is for) it's not documented anywhere, since the description comes from the curation YAML. So the only hint that truncation happened is next_page. Have you looked at fastmcp's ToolTransform with ArgTransformConfig(default=25)? If I understand it right, that would put default: 25 directly into the inputSchema so the model sees it natively. |
frederik-raphael
changed the title
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
3 participants
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.
Problem
When an MCP list-tool call omitted
pagesize, the REST route receivedNone. That selected the internal ceiling forlist_txs_forandlist_neighbors. Forlist_tx_flows, it disabled pagination entirely because the service slices only when both a page and a page size are present.One measured
list_txs_forcall on an exchange hot wallet returned about 2.7 MB, or roughly 677k model tokens. Account-model transactions can likewise contain hundreds of flow events.Fix
The MCP layer now uses a default page size of 25 when the caller omits
pagesize:_params_from.ToolTransformwithArgTransformConfig(default=25). This both supplies the omitted value and advertisesdefault: 25in the MCPinputSchema.nullto 25 because a present null bypasses schema-default insertion.There is no second MCP-side maximum. Explicit valid page sizes pass through, and the REST API retains its existing upper bound of 5000.
The generated-tool set is collected while
FastMCP.from_fastapiwalks curated routes. It currently containslist_tx_flows. Consolidated tools remain outside this set because they apply their own defaults, and filteredlist_neighborsusespagesizeas a target match count with a default of 50.Generated OpenAPI output schemas remain present. Any output-schema size reduction belongs in a separate change with client-compatibility testing.
Pagination behavior
For
list_tx_flows, omission and explicit null both produce a first page of 25 rows. Pass the returnednext_pagevalue back aspageuntil the tail is reached. An explicit validpagesize, including 5000, passes through unchanged.Tests
Coverage includes:
default: 25in the reallist_tx_flowsinput schema.Verification run: