-
Notifications
You must be signed in to change notification settings - Fork 48
feat: Add pagination support to SearchIndexTool and GetSegmentsTool to prevent token overflow #114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…ent token overflow Signed-off-by: Bruno Selva <bselva@cj.com>
|
Thanks for raising this PR @brunoselvacj! Can you add the changes to the changelog? I also have a high level question regarding the |
rithin-pullela-aws
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks like a positive step to improve reliability of the tools, but added few concerns with the change
| effective_size = min(args.size, 100) if args.size else 10 | ||
| query_body['size'] = effective_size | ||
| query_body['from'] = args.from_ if args.from_ is not None else 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make the max limit variable configurable? Either via CLI parameter or config file? We can default to a maximum of 100 if the variable is not provided
| # Apply limit to prevent token overflow | ||
| if args.limit and isinstance(response, list): | ||
| return response[:args.limit] | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we apply the max limit, perhaps adding the information that the size has been truncated in the response message sent back to the MCP client would help?
The same thing can be applied for the Search Index tool response as well
| 'SearchIndexTool': { | ||
| 'display_name': 'SearchIndexTool', | ||
| 'description': 'Searches an index using a query written in query domain-specific language (DSL) in OpenSearch', | ||
| 'description': 'Searches an index using a query written in query domain-specific language (DSL) in OpenSearch. Supports pagination with size (default: 10, max: 100) and from parameters to limit response size and prevent token overflow.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new information about the parameters is sent to the LLM as parameter description. Maybe we can limit the tool description to just it's function and not describe its parameters?
| 'GetSegmentsTool': { | ||
| 'display_name': 'GetSegmentsTool', | ||
| 'description': 'Gets information about Lucene segments in indices, including memory usage, document counts, and segment sizes. Can be filtered by specific indices.', | ||
| 'description': 'Gets information about Lucene segments in indices, including memory usage, document counts, and segment sizes. Can be filtered by specific indices. Supports limit parameter (default: 1000) to prevent token overflow.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same concern as above
Description
This change adds pagination support to SearchIndexTool and GetSegmentsTool to prevent token overflow errors when MCP tool responses exceed the 25,000 token limit.
SearchIndexTool changes:
sizeparameter (default: 10, max: 100) to limit number of search results returnedfromparameter (default: 0) to support offset-based paginationGetSegmentsTool changes:
limitparameter (default: 1000) to cap number of segments returnedAdditional improvements:
Issues Resolved
Fixes token overflow issues where SearchIndexTool and GetSegmentsTool responses exceed the 25,000 token MCP limit, causing errors like:
MCP tool 'SearchIndexTool' response (35969 tokens) exceeds maximum allowed tokens (25000).By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check
here.