Skip to content

fix(billing-cost-management-mcp-server): correct paging limit, metrics default, effort and filter texts - #4994

Open
finklen-amazon wants to merge 1 commit into
mainfrom
fix/bcm-desc-accuracy
Open

finklen-amazon wants to merge 1 commit into
mainfrom
fix/bcm-desc-accuracy

Conversation

@finklen-amazon

Copy link
Copy Markdown
Contributor

Fixes

Summary

Changes

Several served texts told the model something the server or the AWS APIs do not do. This PR corrects them. It changes text only; request and response handling are unchanged, apart from the wording of one empty-result hint.

  • Paging limit. business-support said that omitting max_items fetches every page "(no limit)", and cost-anomaly-monitors left the limit out. Both page through the same time-bounded pager as every other tool, which stops at BCM_MCP_PAGINATION_MAX_SECONDS. Both now splice in PAGINATION_LIMIT_TEXT. ri-performance hardcoded "300-second" in its max_pages line, so with the limit set to 2 the same description said both 2 and 300; it now says "the same time limit". The max_items / max_pages arg docs in these tools, and in cost-anomaly, now point at the limit. The business-support has_more note names the limit as a cause too.

  • Metrics default (server instructions). "If metrics is omitted, the tool returns UnblendedCost" holds only for cost-explorer. sp-performance coverage sends SpendCoveredBySavingsPlans, and ri-performance coverage sends no metrics, so the API returns hours only. Both coverage operations reject UnblendedCost with a ValidationException. The sentence now says so.

  • Graviton prompt. The Compute Optimizer fallback grouped results by implementation_effort, which Compute Optimizer does not return. Each Compute Optimizer recommendation option carries its own migration_effort (VeryLow, Low, Medium, High) and savings. That scale differs from Cost Optimization Hub's implementation_effort (VeryLow to VeryHigh): for the same instance and the same Graviton target, read-only calls returned Low or Medium from Compute Optimizer and VeryHigh from Cost Optimization Hub. The prompt now names migration_effort, says to use the rank 1 option, and says never to mix the two scales.

  • sp-explorer filters. The text said any describe_savings_plans filter value that matches no plan returns an empty list. That holds only for well-formed values. The service rejects these with a ValidationException:

    • a term that is not a whole number of seconds ("1yr", "31536000.0");
    • an upfront or commitment that is not a number ("$4.38");
    • a start or end that does not begin with a millisecond timestamp (a date only, or no milliseconds).

    upfront and commitment compare as numbers ("4.380" matches 4.38). Rate tenancy also takes host; capitalized spellings such as Shared match nothing. The empty-result hint now limits "matched exactly" to text values.

Tests:

  • New test_time_bounded_tools_state_the_configured_limit (business-support, cost-anomaly-monitors, ri-performance; description and served arg docs, with the limit overridden).
  • New test_metrics_default_is_scoped_to_cost_explorer.
  • Extended Graviton prompt and sp-explorer description/hint tests.
  • 8 of the new or changed tests fail on main and pass here. The rest are regression guards.

Not changed: cost-comparison rejects filters nested 97 or more levels deep. A probe showed some 97-level shapes succeed in the service, but the exact failing shapes are not pinned down yet, so the limit is left as is. Many other tools' max_items arg docs also say "None fetches every page" without the limit (their tool descriptions do state it). That sweep is left for a follow-up.

User experience

Before:

  • A model could read "(no limit)" and treat a time-limited partial result as complete.
  • It could pass UnblendedCost to a coverage operation and get a ValidationException.
  • On the Compute Optimizer path, it found no implementation_effort to group by, or mixed two different effort scales.
  • It could report "no matching plans" for a filter the service had in fact rejected as malformed.

After: the descriptions, arg docs, instructions and prompt match what the server and the APIs do. Every claim was checked against a live call (read-only), and each text claim was shown to fail on main and pass on this branch.

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? N

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.

…s default, effort and filter texts

- business-support and cost-anomaly-monitors: omitting max_items fetches pages only up
  to the configured BCM_MCP_PAGINATION_MAX_SECONDS limit, not "no limit". Their paging
  arg docs, and ri-performance's, point at that limit.
- ri-performance: the max_pages line no longer hardcodes 300 seconds.
- Server instructions: only cost-explorer returns UnblendedCost when metrics is omitted;
  sp-performance and ri-performance coverage take their own metrics.
- Graviton prompt: the Compute Optimizer fallback reports migration_effort (VeryLow to
  High), not implementation_effort, on a different scale from Cost Optimization Hub's
  (VeryLow to VeryHigh); use the rank 1 option and never mix the two scales.
- sp-explorer filters: a term that is not a whole number, a non-numeric upfront or
  commitment, or a start or end not starting with a millisecond timestamp is a
  ValidationException, not an empty list; upfront and commitment compare as numbers;
  rate tenancy also takes host. The empty-result hint scopes exact matching to text
  values.
@codecov

codecov Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.79%. Comparing base (69e2666) to head (a6c0cfc).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4994      +/-   ##
==========================================
- Coverage   93.79%   93.79%   -0.01%     
==========================================
  Files        1080     1080              
  Lines       94712    94713       +1     
  Branches    15442    15442              
==========================================
  Hits        88833    88833              
  Misses       3505     3505              
- Partials     2374     2375       +1     

☔ 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