Repository navigation
feat(billing-cost-management-mcp-server)!: move remaining tools to max_items and drop old helper - #4788
feat(billing-cost-management-mcp-server)!: move remaining tools to max_items and drop old helper#4788finklen-amazon wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
Bandit found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4788 +/- ##
==========================================
+ Coverage 93.61% 93.65% +0.04%
==========================================
Files 1076 1059 -17
Lines 93116 92083 -1033
Branches 15124 14996 -128
==========================================
- Hits 87168 86240 -928
+ Misses 3549 3469 -80
+ Partials 2399 2374 -25 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
9dd2016 to
22c5e44
Compare
| - max_items is the total number of groups returned. Omit it to fetch every page (no limit). Pass | ||
| it, or narrow the filter, for queries that can be large. get_reservation_utilization pages in | ||
| fixed pages of 500 groups, and its first page is returned whole even when max_items is smaller. | ||
| - Top-N: sort_by + max_items=N. |
There was a problem hiding this comment.
This will always return the first page of 500 groups when max_items is < 500, right?
I forgot why we don't limit on the first page?
There was a problem hiding this comment.
Right — it always asks for 500 at a time, so a smaller max_items still gets 500 back.
The reason: for this API (and Billing Conductor's cost report), next_token only works if every request asks for the same page size. So we can't shrink the last page to fit max_items. For max_items=700 with 500-item pages, the options are:
| Option | Returns | Can continue? | Downside |
|---|---|---|---|
| Stop after one page (today) | 500 | yes | less than asked |
| Fetch 2 pages, return only 700, drop the token | 700 | no — ask again with a bigger max_items |
throws away 300 fetched items; can't continue |
| Fetch 2 pages, return 700 with the token | 700 | yes, but skips 701–1000 | silently loses data |
| Same page size of 350 for both pages | 700 | only with the same max_items |
asking for more on a continuation fails |
| Fetch 2 pages, return all 1,000 | 1,000 | yes | returns more than asked |
I'd go with the last one for these two APIs: round max_items up to whole pages and return them, with the token. Nothing is dropped or skipped, continuing always works, and the response says how many items it holds, so the caller knows. The cost is a bigger response than requested — up to one extra page, so max_items=10 gets 500 and max_items=700 gets 1,000. Every other API keeps returning exactly max_items.
There was a problem hiding this comment.
Update after checking every paginated API in this server: only GetReservationUtilization ties its next_token to MaxResults — a continuation with a different (or omitted) MaxResults is rejected. Billing Conductor's cost report turned out not to (any 200–300 works on each page), and every other API accepts a changed page size.
So, per the team discussion, instead of rounding up:
- Grouped
get_reservation_utilizationnow takesmax_pages(pages of 500 groups) instead ofmax_items. Each response hasitems_returnedandnext_token, so nothing over- or under-shoots and the agent doesn't have to count. Passingmax_itemsthere returns a validation error pointing tomax_pages. get-billing-group-cost-reportuses normal sizing within the API's 200–300 range, so it returns exactlymax_itemsexcept that a request below 200 returns up to 200.- Every other API keeps
max_itemsand returns exactly that many.
Pushed in the latest commits on this PR.
22c5e44 to
1bc4ce6
Compare
Cost Explorer, cost-comparison, Reserved Instance and Savings Plans performance, Savings Plans recommendations, credits, Cost Optimization Hub, Compute Optimizer Automation, budgets and AWS Pricing now page through paginate_aws_items. These are the tools that need more than a pass-through: Cost Explorer counts the groups nested in each period against max_items and takes GroupDefinitions and TotalSize from the first page, coverage and utilization take Total from the first page, the Savings Plans recommendation and Cost Optimization Hub keep their first-page summaries, Cost Optimization Hub ranks efficiency metrics over a fixed fetch, Compute Optimizer Automation applies max_items per region in its multi-region calls, and budgets looks a budget_name up with DescribeBudget. The remaining paginate_aws_response calls, hand-written loops and the credits botocore paginator are replaced. With no caller left, the old paginate_aws_response is removed, and the SQL offload no longer reads the old Pagination wrapper or raw single-page tokens. Also fixes: Cost Explorer and Cost Optimization Hub list operations returning only the first page unless max_pages or next_token was passed, cost-comparison requesting 10 rows per page, a NextToken sent to getSavingsPlansUtilization, the getCostCategories result key, an extra Total request reusing the last page's token in Savings Plans and Reserved Instance coverage and utilization, AWS Pricing MaxResults above 100, and the budgets budget_name filter running after the cut. BREAKING CHANGE: max_results, max_pages and page_size are removed from these tools and sp-recommendation's next_page_token is renamed to next_token. Their paginated responses use the lowercase pagination block; Pagination, top-level next_token, has_more_results, total_count, pages_fetched, total_results and duration_ms are removed. utilities.aws_service_base.paginate_aws_response is removed in favor of paginate_aws_items, and convert_response_if_needed no longer takes pagination_token_key.
get_service_codes and get_attribute_values sorted the values in each response. With max_items and next_token a list is read in slices, so sorting each slice does not give one order across pages, and Python's sort differs from the service's order when one value is a prefix of another (the service returns us-east-1-x before us-east-1). Keep the order the service returns, which is the same across pages in practice; the API reference does not document an order. get_service_attributes is not paginated and still sorts its attribute names.
GetReservationUtilization and GetBillingGroupCostReport bind their continuation token to the page size, so every request sends the same MaxResults and a page cannot shrink to fit max_items. Previously the call stopped after the first page whenever the next full page could pass max_items, so max_items=700 with pages of 500 returned only 500 items. For PageSpec.fixed_page_size specs, paging now continues until at least max_items items are collected (or the data, time limit or a failure ends it) and returns every fetched item with the service's token: max_items is rounded up to whole pages. Other specs keep exact page sizing.
This reverts commit 1bc4ce6. Rounding max_items up to whole pages made get_reservation_utilization and get-billing-group-cost-report return more items than asked for. Fixed-page specs go back to the general rule: stop before a page that could take the result past max_items, and always return the first page. The following commits replace the rounding with max_pages for Reserved Instance utilization and a 200-300 page size for the Billing Conductor cost report.
GetReservationUtilization grouped by SUBSCRIPTION_ID rejects a continuation whose MaxResults differs from the first call's, so every page is requested at 500 groups and a page cannot shrink to fit max_items. A count-based limit therefore either undershoots (stop before a page that could exceed it) or overshoots (round up to whole pages), and the agent has to reason about page sizes to use it. paginate_aws_items gains a keyword max_pages: it fetches at most that many pages at the spec's page size and returns every item with the service's token. It cannot be combined with max_items, must be at least 1, and a result it cuts short warns to call again with a larger max_pages. The time limit, repeated-token guard and failure handling are unchanged. ri-performance takes max_pages and passes it to grouped get_reservation_utilization, which now returns validation_error for max_items. get_reservation_coverage keeps max_items and returns validation_error for max_pages. Ungrouped utilization returns every period in one response and still ignores both.
GetBillingGroupCostReport's token is bound to Arn, GroupBy and BillingPeriodRange, not to MaxResults: a continuation may ask for any size the service accepts, 200 to 300. Requesting a fixed 300 on every page made max_items=500 stop at 300 results. PageSpec gains page_min, the smallest page size the service accepts; a smaller requested size is raised to it, and the stop rule counts the raised size as the worst case. The cost report drops fixed_page_size and pages between 200 and 300, so max_items=500 returns 300 then 200, and a max_items below 200 requests, and can return, up to 200.
GetCreditAllocationHistory accepts maxResults up to 1,000 but clamps a page to 500 ledger rows. Set page_max to 500 so the page size the server requests, and the bound it checks pages against, match what the service returns. The service ends a page only at a bill boundary, so a page can still hold more rows than maxResults; the helper keeps such a page.
The second line of partial_data_warning had an f prefix but no placeholders.
dfde5a6 to
64cb0b1
Compare
Fixes
Summary
Changes
Moves the remaining tools, those that need merging, first-page, multi-region, ranking or nested-count logic on top of
paginate_aws_itemsfrom #4786, then removes the oldpaginate_aws_responseand the SQL offload's support for the old pagination shapes. The contract, envelope and helper are described in #4786. After this PR the tree matches #4785, with the helper namedpaginate_aws_items, except that AWS Pricing keeps the service's order and the page-size changes below.Tools changed:
cost-explorercost_explorer_operations.py,cost_explorer_tools.pygetCostAndUsage,getCostAndUsageWithResources,getDimensionValues,getTagsOrValuesandgetCostCategoriesfetched only the first page unlessmax_pagesornext_tokenwas passed; they now fetch every page unlessmax_itemsis passed.max_itemscounts the groups nested in each period.GroupDefinitions/TotalSizecome from page 1 andDimensionValueAttributesis concatenated. PaginatedgetCostCategorieswithoutcost_category_nameread names fromCostCategoriesinstead ofCostCategoryNames. Three false "automatic pagination" claims in the description are removed.cost-comparisoncost_comparison_tools.pyri-performanceri_performance_tools.pyTotalthat reused the last page's token (wrong window, extra cost).Totalnow comes from the first page. Groupedget_reservation_utilizationtakesmax_pages(pages of 500 groups) instead ofmax_items, because its token only works with the sameMaxResultson every page;max_itemsthere, andmax_pageson coverage, returnvalidation_error.sp-performancesp_performance_tools.pyTotalnow comes from the first page, as above.getSavingsPlansUtilizationsent aNextTokenparameter the API does not have; it is not paginated and no longer takes paging parameters.sp-recommendationsp_recommendation_tools.pynext_page_tokenis renamed tonext_token.creditscredits_operations.py,credits_tools.pypaginate_aws_items.cost-optimization(Cost Optimization Hub)cost_optimization_hub_helpers.py,cost_optimization_hub_tools.pymax_pagesornext_tokenwas passed. Summary aggregates come from page 1. The performance ranking readscomplete_datasetto tell a partial result.compute-optimizer-automationcompute_optimizer_automation_operations.py,compute_optimizer_automation_tools.pymax_itemsper region; a failed region leaves the result partial withhas_more: false.budgets,budget-actions,budget-notificationsbudget_tools.pyDescribeBudgetNotificationsForAccountcounts budgets per page, each with up to 10 notifications, so itsPageSpecusesitems_per_page_unitandcount_items. Thebudget_namefilter ran after the result was cut tomax_results, so a budget beyond the cut was reported as missing; a name now callsDescribeBudget.pricingaws_pricing_operations.py,aws_pricing_tools.pyget_service_codessentmax_resultsunchecked asMaxResults, so values above 100 failed withValidationException.total_count(len()of the returned list, misleading on partial results) is removed; usepagination.items_returned.get_service_codesandget_attribute_valuesno longer sort each response: sorting a slice does not give one order across pages, and Python's sort differs from the service's order when one value is a prefix of another (the service returnsus-east-1-xbeforeus-east-1). They return values in the order the service returns them, which is the same across pages in practice, though the AWS documentation does not guarantee an order.get_service_attributesis not paginated and still sorts its attribute names.Page sizes:
utilities/aws_service_base.py:paginate_aws_itemstakes a keywordmax_pages, mutually exclusive withmax_items(ValueErrornaming both;max_pages < 1is also aValueError). It fetches at most that many pages at the spec's page size and returns every item with the service's token; a result it cuts short warns to call again with a largermax_pages. The time limit, repeated-token guard and failure handling are unchanged.ri-performance(ri_performance_tools.py): groupedget_reservation_utilizationis the only paginated API here whose token is bound toMaxResults(a continuation with a different, or omitted vs explicit,MaxResultsfails withInvalidNextTokenException). It is requested at 500 groups per page (the API allows up to 7,000) and takesmax_pages, so a result neither over- nor undershoots and the agent does not count items. Ungrouped utilization returns every period in one response and ignores both.get-billing-group-cost-report(migrated in feat(billing-cost-management-mcp-server)!: move list tools to max_items #4787;billing_conductor_operations.py,billing_conductor_tools.py): its token is bound toArn,GroupByandBillingPeriodRange, not toMaxResults, so it no longer sends a fixed 300. A newPageSpec.page_min(smallest page size the service accepts) raises a smaller request to 200, and the stop rule counts that size:max_items=500requests 300 then 200,max_items=450stops after 300, and amax_itemsbelow 200 requests, and can return, up to 200. The description and docstrings say so.credits(credits_operations.py):GetCreditAllocationHistory'spage_maxis 500, the page size the service clamps to. A page ends only at a bill boundary, so it can hold more rows thanmaxResults; the helper keeps such a page.Cleanup:
utilities/aws_service_base.py:paginate_aws_responseis removed (and its exports fromutilitiesand the package root); its tests intests/utilities/test_aws_service_base.pygo with it.utilities/sql_utils.py:_derive_pagination_envelopereads only the lowercasepaginationblock, andconvert_response_if_neededno longer takespagination_token_key. The tests for the old shapes are replaced by one asserting they are ignored.Breaking changes
max_results,max_pagesandpage_sizeare removed from these tools (groupedri-performanceget_reservation_utilizationtakesmax_pagesinstead ofmax_items), andsp-recommendation'snext_page_tokenis renamed tonext_token. MCP clients reject unknown parameters, so callers that pass the old names must switch tomax_items/next_token.max_itemsnow fetches every page (bounded by the 300 s time limit), where several of these tools previously returned one page.paginationblock. Removed: the uppercasePaginationblock, top-levelnext_token,has_more_results,total_count,pages_fetched,total_resultsandduration_ms.utilities.aws_service_base.paginate_aws_responseis removed; usepaginate_aws_items(ctx, operation, api_call, request_params, spec, *, max_items=None, next_token=None, max_pages=None), which takes aPageSpecand returns(items, pages, pagination).convert_response_if_neededno longer takespagination_token_key.Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change? (Y/N) Y
RFC issue number:
Checklist:
Testing:
uv run --frozen pytest --cov --cov-branch(2233 passed, 98% branch coverage, every added line covered),ruff check,ruff format --checkandpyrightclean insrc/billing-cost-management-mcp-serveron this branch alone. Each migrated operation has tests for fetching every page withoutmax_items, stopping atmax_items(max_pagesfor grouped RI utilization), and continuing with the caller's token and identical parameters.Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.