Skip to content

Commit 2e65667

Browse files
fix(billing-cost-management-mcp-server)!: drop always-null ri-performance coverage_cost fields (#4835)
format_coverage_metrics read ReservedCost, TotalCost and CoverageCostPercentage from GetReservationCoverage's CoverageCost. That shape has only OnDemandCost (botocore, the API reference and live responses agree), so coverage_cost always carried reserved_cost, total_cost and coverage_cost_percentage as null, at every level (overall, per period, per group). An agent could read the null percentage as unknown or zero cost coverage. coverage_cost now holds only on_demand_cost. Coverage percentages come from coverage_hours (and coverage_normalized_units for EC2), as before. BREAKING CHANGE: ri-performance get_reservation_coverage no longer returns coverage_cost.reserved_cost, coverage_cost.total_cost or coverage_cost.coverage_cost_percentage. They were always null. Tests: the fixtures no longer invent CoverageCost members, and a new guard checks that the mocked GetReservationCoverage and GetReservationUtilization responses use only members of the botocore output shapes. Co-authored-by: Junaid Girkar <60307291+junaidgirkar@users.noreply.github.com>
1 parent 56775c6 commit 2e65667

3 files changed

Lines changed: 50 additions & 25 deletions

File tree

‎src/billing-cost-management-mcp-server/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3737
- Added a `get_preferences` operation to the `cost-optimization-hub` tool (`GetPreferences`), returning the calling account's savings estimation mode, member account discount visibility, and preferred commitment (term and payment option).
3838

3939
### Changed
40+
- **Breaking:** `ri-performance` `get_reservation_coverage` returns only `on_demand_cost` in `coverage_cost`. The `reserved_cost`, `total_cost` and `coverage_cost_percentage` fields are removed: Cost Explorer's `CoverageCost` has only `OnDemandCost`, so they were always `null`. Use `coverage_hours` (or `coverage_normalized_units` for EC2) for coverage percentages.
4041
- **Breaking:** `sp-performance` now labels the request window `requested_time_period` instead of `time_period` on all three operations. A MONTHLY utilization entry's `TimePeriod` is labelled with its calendar month (a 2026-08-17 start returns a first entry labelled 2026-08-01, whose figures still start on the 17th), and the description now says so. `get_savings_plans_utilization_details` now returns the API's `TimePeriod` as `time_period` (the requested start, and an end the API may move earlier to the first day without complete data), and returns `total` only when the whole query came back in one page: the API's `Total` covers only the Savings Plans on its own page, so a multi-page, partial (`max_items`) or `next_token` continuation result previously reported one page's figure as the query's (live: page 1 of 3 reported 100% utilization against 99.71% for the query). `get_savings_plans_coverage` no longer reads a `Total` (the GetSavingsPlansCoverage API does not return one) and reports `granularity` only when it was sent, not with `group_by`. The description lists the utilization details filter dimensions and says which period to report.
4142
- **Breaking:** `ri-performance` grouped `get_reservation_coverage` and `get_reservation_utilization` return a period's `total` only when the whole query came back in one page. With `group_by`, the API's period `Total` sums only the groups on its own page, and the tool kept page 1's as the period's: live, a 3-group page of an 877-group coverage query reported 638,311 running hours against 902,438,217 for the query, and a `next_token` continuation reported its own page's 1,311,271. `total` is now omitted from a multi-page, partial (`max_items`/`max_pages`) or `next_token` continuation result, and the description points to the ungrouped call for the overall figure. A `total` the API did not return is omitted rather than returned as `{}`. Ungrouped results, which are always one page, are unchanged.
4243
- **Breaking:** every paginated tool operation now takes `max_items` (the total number of items to return) and `next_token`. The `max_results`, `max_pages` and `page_size` parameters are removed (`ri-performance` keeps `max_pages` for `get_reservation_utilization`, below), and `next_page_token` (`sp-purchase-analyzer`, `sp-recommendation`) is renamed to `next_token`. Without `max_items` an operation fetches every page; with it, the operation stops before a page that could take the result past `max_items` (the first page is always returned) and returns the service's own continuation token unchanged. `ri-performance` `get_reservation_utilization` grouped by `SUBSCRIPTION_ID` takes `max_pages` (pages of 500 groups) instead of `max_items`, because its token only works with the same page size; `max_items` there returns `validation_error`. Billing Conductor `get-billing-group-cost-report` requests pages of 200 to 300 results, the range the service accepts, so a `max_items` below 200 can return up to 200. One call stops paging, on a page boundary, after `BCM_MCP_PAGINATION_MAX_SECONDS` (default 300) seconds. Paginated responses carry a lowercase `pagination` block (`complete_dataset`, `has_more`, `next_token`, `items_returned`) ahead of the data, inline and in the SQL-offload result; `has_more` is true only when there is a `next_token` to continue with, and `complete_dataset` reports whether the result is partial; a partial result also leads with top-level `complete_dataset: false` and a `warning`. The old `Pagination` block, top-level `next_token`, `has_more_results`, `total_count`, `pages_fetched`, `total_results` and `duration_ms` fields are removed. A rejected or expired `next_token` returns `error_type: invalid_next_token` (`retry_strategy: modify_request`). Operations that are not paginated take neither parameter. See "Pagination" in the README.

‎src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/ri_performance_tools.py‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -569,14 +569,13 @@ def format_coverage_metrics(coverage_data: Dict) -> Dict:
569569
'coverage_normalized_units_percentage': cnu.get('CoverageNormalizedUnitsPercentage'),
570570
}
571571

572-
# Format cost coverage
572+
# Format cost coverage. CoverageCost holds only OnDemandCost (the on-demand cost of
573+
# usage reservations did not cover); Cost Explorer reports no reserved cost or cost
574+
# coverage percentage, so use coverage_hours_percentage for coverage.
573575
if 'CoverageCost' in coverage_data:
574576
cc = coverage_data['CoverageCost']
575577
formatted_coverage['coverage_cost'] = {
576578
'on_demand_cost': cc.get('OnDemandCost'),
577-
'reserved_cost': cc.get('ReservedCost'),
578-
'total_cost': cc.get('TotalCost'),
579-
'coverage_cost_percentage': cc.get('CoverageCostPercentage'),
580579
}
581580

582581
return formatted_coverage

‎src/billing-cost-management-mcp-server/tests/tools/test_ri_performance_tools.py‎

Lines changed: 46 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
- Error handling for invalid date ranges and missing reservation data
2323
"""
2424

25+
import botocore.session
2526
import pytest
2627
from awslabs.billing_cost_management_mcp_server.tools.ri_performance_tools import (
2728
format_coverage_metrics,
@@ -63,9 +64,6 @@ async def ri_performance(ctx, operation, **kwargs):
6364
},
6465
'coverage_cost': {
6566
'on_demand_cost': '10.0',
66-
'reserved_cost': '30.0',
67-
'total_cost': '40.0',
68-
'coverage_cost_percentage': '75.0',
6967
},
7068
},
7169
},
@@ -118,9 +116,6 @@ def mock_ce_client():
118116
},
119117
'CoverageCost': {
120118
'OnDemandCost': '10.0',
121-
'ReservedCost': '30.0',
122-
'TotalCost': '40.0',
123-
'CoverageCostPercentage': '75.0',
124119
},
125120
},
126121
'Groups': [
@@ -138,9 +133,6 @@ def mock_ce_client():
138133
},
139134
'CoverageCost': {
140135
'OnDemandCost': '5.0',
141-
'ReservedCost': '15.0',
142-
'TotalCost': '20.0',
143-
'CoverageCostPercentage': '75.0',
144136
},
145137
},
146138
},
@@ -158,9 +150,6 @@ def mock_ce_client():
158150
},
159151
'CoverageCost': {
160152
'OnDemandCost': '5.0',
161-
'ReservedCost': '15.0',
162-
'TotalCost': '20.0',
163-
'CoverageCostPercentage': '75.0',
164153
},
165154
},
166155
},
@@ -176,9 +165,6 @@ def mock_ce_client():
176165
},
177166
'CoverageCost': {
178167
'OnDemandCost': '10.0',
179-
'ReservedCost': '30.0',
180-
'TotalCost': '40.0',
181-
'CoverageCostPercentage': '75.0',
182168
},
183169
},
184170
}
@@ -234,6 +220,49 @@ def mock_ce_client():
234220
return mock_client
235221

236222

223+
def _undeclared_members(value, shape, path='') -> list:
224+
"""Paths in a mocked response that the botocore output shape does not declare."""
225+
if shape.type_name == 'structure' and isinstance(value, dict):
226+
found = []
227+
for key, item in value.items():
228+
if key not in shape.members:
229+
found.append(f'{path}.{key}')
230+
else:
231+
found += _undeclared_members(item, shape.members[key], f'{path}.{key}')
232+
return found
233+
if shape.type_name == 'list' and isinstance(value, list):
234+
return [
235+
p
236+
for i, item in enumerate(value)
237+
for p in _undeclared_members(item, shape.member, f'{path}[{i}]')
238+
]
239+
if shape.type_name == 'map' and isinstance(value, dict):
240+
return [
241+
p
242+
for k, item in value.items()
243+
for p in _undeclared_members(item, shape.value, f'{path}[{k}]')
244+
]
245+
return []
246+
247+
248+
@pytest.mark.parametrize(
249+
'method,operation',
250+
[
251+
('get_reservation_coverage', 'GetReservationCoverage'),
252+
('get_reservation_utilization', 'GetReservationUtilization'),
253+
],
254+
)
255+
def test_mock_ce_responses_only_use_botocore_output_members(mock_ce_client, method, operation):
256+
"""Mocked Cost Explorer responses hold only members the real API can return."""
257+
output_shape = (
258+
botocore.session.get_session()
259+
.get_service_model('ce')
260+
.operation_model(operation)
261+
.output_shape
262+
)
263+
assert _undeclared_members(getattr(mock_ce_client, method).return_value, output_shape) == []
264+
265+
237266
def test_format_coverage_metrics():
238267
"""Test format_coverage_metrics function."""
239268
# Setup
@@ -246,9 +275,6 @@ def test_format_coverage_metrics():
246275
},
247276
'CoverageCost': {
248277
'OnDemandCost': '10.0',
249-
'ReservedCost': '30.0',
250-
'TotalCost': '40.0',
251-
'CoverageCostPercentage': '75.0',
252278
},
253279
'CoverageNormalizedUnits': {
254280
'OnDemandNormalizedUnits': '200.0',
@@ -270,9 +296,8 @@ def test_format_coverage_metrics():
270296
assert result['coverage_hours']['reserved_hours'] == '400.0'
271297
assert result['coverage_hours']['coverage_hours_percentage'] == '80.0'
272298

273-
assert result['coverage_cost']['on_demand_cost'] == '10.0'
274-
assert result['coverage_cost']['reserved_cost'] == '30.0'
275-
assert result['coverage_cost']['coverage_cost_percentage'] == '75.0'
299+
# CoverageCost has only OnDemandCost; no always-null reserved/total/percentage keys.
300+
assert result['coverage_cost'] == {'on_demand_cost': '10.0'}
276301

277302
assert result['coverage_normalized_units']['on_demand_normalized_units'] == '200.0'
278303
assert result['coverage_normalized_units']['reserved_normalized_units'] == '800.0'

0 commit comments

Comments
 (0)