Skip to content

fix(billing-cost-management-mcp-server): return null for an estimate's IssuedDate and document filter limits - #4987

Open
finklen-amazon wants to merge 9 commits into
awslabs:mainfrom
finklen-amazon:fix/bcm-doc-fix-bundle
Open

finklen-amazon wants to merge 9 commits into
awslabs:mainfrom
finklen-amazon:fix/bcm-doc-fix-bundle

Conversation

@finklen-amazon

Copy link
Copy Markdown
Contributor

Fixes

Summary

Stacked on #4906: until it merges, this PR's diff also shows #4906's commits. Only the last commit, fix(billing-cost-management-mcp-server): return null for an estimate's IssuedDate and document filter limits, is new here.

Changes

invoicing returns IssuedDate: null for an estimated invoice instead of 1970-01-01T00:00:00, and every summary carries is_estimate.

An estimate (returned only with receiver_role) has an IssuedDate of the Unix epoch, meaning unset. The tool turned that into 1970-01-01T00:00:00, and the description had to warn callers never to report it. The estimate's DueDate is a real date and is kept.

  • IssuedDate equal to the epoch (as a datetime in any time zone, or as epoch seconds) is now null.
  • is_estimate is true when InvoiceId starts with Estimate-, the only field that tells an estimate from an issued invoice, and false otherwise.
  • The description uses both instead of the 1970 date.

cost-explorer attaches the user: tag-key hint to getCostAndUsageWithResources too. A user:-prefixed tag key matches nothing there, with no error, as in getCostAndUsage. The forecasts accept either form, so they still get no hint.

Description changes, each checked against the service:

  • cost-explorer filter grammar:
    • If MatchOptions is passed it must include EQUALS (or ABSENT): ["CASE_SENSITIVE"] alone is rejected, and ABSENT takes no case option.
    • Every operation allows at most 6,000 filter values in total, counted across all leaves, including under Not.
    • getCostAndUsage only: {"Tags": {"Key": "", "Values": []}} matches costs that carry no activated cost allocation tag, as does ABSENT with no Key. Other forms of an empty key are rejected, and getDimensionValues and getCostAndUsageWithResources reject an empty key.
    • The Tags section now names getCostAndUsageWithResources and says the forecasts accept either tag-key form.
  • cost-explorer DIMENSION REFERENCE: adds PAYER_ACCOUNT (the management account's ID). The service enables it per account, so the entry says to use LINKED_ACCOUNT if it is rejected.
  • cost-comparison: a filter holds at most 200 values in total, counted across all its leaves.
  • session-sql: drops the reference to a cost_explorer_sql tool that does not exist, and says how to recognize an offloaded result (data.data_stored, data.table_name).
  • compute-optimizer:
    • RDS idle and instance_finding are computed separately, so a DB instance can be Optimized and idle. idle is null for Aurora cluster recommendations.
    • effective_recommendation_preferences.savingsEstimationMode.source names the source of the savings figures (every type except idle).
  • aws-pricing:
    • EC2 Windows needs licenseModel: No License required is the standard Windows On-Demand price, and the other values are separate, cheaper products.
    • RDS needs databaseEngine and deploymentOption. Even then an UnusedBox twin at the same price (with no hardware attributes) can match, and Aurora adds a higher-priced InstanceUsageIOOptimized product. SQL Server and Oracle also need licenseModel and databaseEdition; deploymentModel Custom rows are RDS Custom.
  • describe-cost-category-definition: split charge rules apply only on the console's cost category details page. A cost-explorer result grouped by the category keeps the source value's full cost.
  • get-billing-view: a cost-category value of "" in a view filter matches the costs no rule of that category assigns a value to.
  • storage-lens: report_date is a string in CSV and Parquet exports.

User experience

Before, an estimate in list_invoice_summaries with receiver_role:

{"InvoiceId": "Estimate-...", "InvoiceType": "INVOICE", "IssuedDate": "1970-01-01T00:00:00", "DueDate": "2026-07-29T08:35:04.858000", ...}

After:

{"InvoiceId": "Estimate-...", "InvoiceType": "INVOICE", "IssuedDate": null, "DueDate": "2026-07-29T08:35:04.858000", "is_estimate": true, ...}

Issued invoices keep their IssuedDate and get "is_estimate": false.

Testing

All commands run from src/billing-cost-management-mcp-server.

  • uv run --frozen pytest: 4286 passed. ruff format, ruff check and pyright are clean.
  • 19 new or changed tests fail on the base (fix(billing-cost-management-mcp-server)!: request COH savings percentage, coerce boolean filters, fold summaries group_by #4906's head): the estimate IssuedDate/is_estimate cases (epoch as UTC datetime, int, float and a non-UTC aware datetime; issued invoice; prefix only at the start), the getCostAndUsageWithResources hint, the per-account PAYER_ACCOUNT contract, and the description contracts for each tool above. The other changed tests are regression guards that already pass there (for example, the forecasts still get no hint, and the existing epoch-to-ISO conversions are unchanged).
  • Live, read-only, through the server over stdio, with matched controls. Examples: ["CASE_SENSITIVE"] alone vs ["EQUALS", "CASE_SENSITIVE"]; 6,000 vs 6,001 values split over two leaves; untagged plus Not untagged equals the unfiltered total; a user: key vs the bare key in getCostAndUsageWithResources; 200 vs 201 values in both comparison operations; a billing view with cost-category values ["", "<value>"] equals the uncategorized plus that value's cost.
    • On this branch: 35/35 pass.
    • On the base (the negative control): 18/35. Every description check fails, as do the getCostAndUsageWithResources hint and both invoice checks; the AWS-only behavior checks pass on both, since the request sent is the same.
  • An independent re-check from service code, the API reference and its own calls found five draft claims wrong, all fixed here before pushing: the empty-key rule holds only for getCostAndUsage, ABSENT rejects a case option, PAYER_ACCOUNT is listed in the API reference, idle is null rather than absent for Aurora clusters, and the RDS pricing products vary by engine and deployment option.

Checklist

If your change doesn't seem to apply, please leave them unchecked.

  • I have reviewed the contributing guidelines
  • I have performed a self-review of this change
  • Changes have been tested
  • Changes are documented

Is this a breaking change? (Y/N) N. An estimate's IssuedDate was documented as the epoch meaning unset; it is now null, and is_estimate is a new field. No parameter or tool is renamed or removed.

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

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.

…age, coerce boolean filters, fold summaries group_by

- list_recommendation_summaries now sends metrics=['SavingsPercentage'].
  Without it the service returns the placeholder "Percentage not
  requested." in metrics.savingsPercentage, which the tool passed through
  as a metric. A percentage value ('12%', '>=80%', '<=2%') is returned as
  metrics.savings_percentage; any other text the service puts in that
  field (before-discounts mode, unsupported filter, not calculable) is
  returned as metrics.savings_percentage_unavailable_reason. A metrics
  block without a string value gives no metrics key.
- restartNeeded / rollbackPossible filters given as "true"/"false"
  strings are coerced to booleans instead of failing botocore parameter
  validation.
- Summaries group_by fixed dimensions match case- and
  underscore-insensitively (resource_type -> ResourceType), like filter
  keys. The missing-group_by error lists the same values as the
  invalid-group_by error, including TagKey:<tag key>. The unreachable
  'RESOURCE_TYPE' fallback is removed.
- The description documents the savings percentage and its limits, the
  resourceIds / resourceArns / recommendationIds filters (list only;
  summaries reject them), that there is no
  per-service resource type filter, and that summary fields are the same
  on every page, including continuation calls (it said they were not
  repeated).

No tool parameter is added, renamed, retyped or given a new default.

BREAKING CHANGE: cost-optimization list_recommendation_summaries no
longer returns the service's explanation text in
metrics.savings_percentage. When no percentage is computed,
metrics.savings_percentage is absent and the text is in
metrics.savings_percentage_unavailable_reason. Before this change the
field always held the "Percentage not requested." placeholder.
… excludes credits and refunds

The service computes metrics.savingsPercentage against the last 30 days'
net amortized cost with the Credit and Refund record types filtered out.
Say so in the cost-optimization description and the CHANGELOG entry, so a
caller comparing it to Cost Explorer's net amortized cost with no
record-type filter knows why the figures differ.

Also say that total_recommendation_count is not one of the summary fields
repeated on every page, and drop the CHANGELOG clause about documenting
the ID filters, which main already covers.
… case-insensitively only

Fold only letter case in list_recommendation_summaries group_by, not
underscores, so the legacy upper-snake spelling RESOURCE_TYPE (and
resource_type) is rejected locally as an invalid dimension instead of
being sent as ResourceType. resourcetype and ACCOUNTID still resolve.
The description, the CHANGELOG entry and the tests say the same.
# Conflicts:
#	src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/cost_optimization_hub_helpers.py
#	src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/cost_optimization_hub_tools.py
#	src/billing-cost-management-mcp-server/tests/tools/test_cost_optimization_hub_tools.py
…s IssuedDate and document filter limits

invoicing: an estimated invoice's IssuedDate (the epoch, meaning unset) is
returned as null instead of 1970-01-01, and every summary carries
is_estimate (InvoiceId prefixed "Estimate-"). The description now uses
both instead of the 1970 date.

cost-explorer: getCostAndUsageWithResources responses carry the user:
tag-key hint, since a user:-prefixed key matches nothing there too.
Documented: MatchOptions must include EQUALS (or ABSENT); at most 6,000
filter values across all leaves; PAYER_ACCOUNT; an empty tag key with an
empty Values list matches untagged costs; the forecasts accept either
tag-key form.

Description fixes, each checked against the service:
- cost-comparison: at most 200 filter values in total
- session-sql: drop the reference to a cost_explorer_sql tool that does
  not exist
- compute-optimizer: RDS idle is separate from the instance finding;
  where savingsEstimationMode.source is
- aws-pricing: RDS filters (and the UnusedBox twin product), Windows
  licenseModel
- describe-cost-category-definition: split charge rules apply only on
  the console's cost category details page
- get-billing-view: a cost-category value of "" matches uncategorized
  costs
- storage-lens: report_date is a string in CSV and Parquet exports
# Conflicts:
#	src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/aws_pricing_tools.py
@codecov

codecov Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.79%. Comparing base (dc8221e) to head (c895d21).

Files with missing lines Patch % Lines
...anagement_mcp_server/tools/invoicing_operations.py 83.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4987      +/-   ##
==========================================
- Coverage   93.79%   93.79%   -0.01%     
==========================================
  Files        1080     1080              
  Lines       94701    94725      +24     
  Branches    15434    15439       +5     
==========================================
+ Hits        88828    88849      +21     
- Misses       3500     3501       +1     
- Partials     2373     2375       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Status: To triage

Development

Successfully merging this pull request may close these issues.

1 participant