Skip to content

fix(billing-cost-management-mcp-server): send filter for getTagsOrValues and getCostCategories - #4970

Merged
finklen-amazon merged 1 commit into
awslabs:mainfrom
finklen-amazon:fix/bcm-ce-tags-categories-filter
Nov 2, 2026
Merged

finklen-amazon merged 1 commit into
awslabs:mainfrom
finklen-amazon:fix/bcm-ce-tags-categories-filter

Conversation

@finklen-amazon

@finklen-amazon finklen-amazon commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Fixes

Summary

Changes

cost-explorer now sends filter for getTagsOrValues and getCostCategories.

Both operations accepted filter but never sent it, so a filtered call returned the whole, unfiltered list as status: success. GetTags and GetCostCategories both take Filter and apply it: the lists keep only the tag keys, tag values, cost category names or category values found on costs that match it.

  • filter is now forwarded as Filter for tag keys, a tag key's values (tag_key), category names, and a category's values (cost_category_name). An empty filter is rejected before any AWS call, as for the other operations.
  • The description lists filter as optional for both operations, says what it does there, and adds both to the "Limits by operation" line: an And of leaves only (an And inside it is rejected); Or and Not are rejected. The getDimensionValues line gets the same nested-And correction.
  • The "Tags leaves" section of the description, and the response hint for a Tags key with a user: prefix or a TAG group_by label (from fix(billing-cost-management-mcp-server): document the cost-explorer Tags filter format and hint on tag keys that cannot match #4941), now also cover getTagsOrValues and getCostCategories. Now that they apply the filter, a user: key with Values silently empties their lists too (with ABSENT it matches every cost).

Small fixes in the same filter and group_by validation:

  • filter='""' (an empty JSON string), or a blank one such as '" "', is now "filter is empty", like '', instead of a botocore type error. Other JSON scalars are still left to the SDK.
  • cost-comparison: a whitespace-only filter now says "filter is empty" instead of "Invalid JSON format", and an empty filter is rejected before the Cost Explorer client is created.
  • group_by='[]' (or 'null') on an operation without GroupBy is now "no grouping", as getCostAndUsage reads it, instead of a validation_error. A non-empty group_by is still rejected, and so is one that is not JSON (e.g. 'SERVICE'), with the same "group_by is not supported" error as on main.

User experience

Before: {"operation": "getCostCategories", "filter": "{\"Dimensions\": {\"Key\": \"REGION\", \"Values\": [\"no-such-region-1\"]}}"} returned status: success with every category name.

After: the same call returns an empty CostCategoryNames, and a real filter such as SERVICE = Amazon Elastic Kubernetes Service returns only the names (or tag keys) on that service's costs. {"filter": "{\"Or\": [...]}"} now returns the service's ValidationException ("Or expression is not allowed ...") instead of a silent unfiltered success.

Merge conflict

Rebased on main after #4941, which documented the Tags filter format and added a hint for Tags keys that cannot match, in the same function and description as this change. The only textual conflict was the import block of tools/cost_explorer_tools.py (#4941 added import json; this PR splits the aws_service_base import to add parse_json); both were kept. Because this PR makes getTagsOrValues and getCostCategories apply filter, #4941's Tags section and hint were extended to them (see Changes), after checking each of that section's claims live on both operations.

A second rebase, after #4987, conflicted in the same places: #4987 extended that section, the hint's docstring and the hint tests to getCostAndUsageWithResources, and added an empty-key line. Both are kept. The section heading and docstring now list all five operations, the hint tests keep both sides' cases, and getTagsOrValues and getCostCategories were added to the operations that reject an empty Tags key (checked live: an empty key with or without a value returns ValidationException; #4987's 6,000-value limit also holds for both).

Testing

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

  • uv run --frozen pytest: 4427 passed. ruff format --check, ruff check, pyright and pre-commit are clean. Every changed source line is covered.
  • New and changed tests fail on main, except 2 regression guards (no Filter is sent when filter is omitted). The tests added with the rebase were shown failing before their fix: group_by that is not JSON or is blank on each of the six operations, the blank quoted filter, the Tags hint on getTagsOrValues and getCostCategories (keys and values), and the extended description.
  • Each claim was checked three ways: the service code, the GetTags / GetCostCategories / Expression API reference (which do not say which operators these two APIs reject), and live calls.
  • Live, read-only, through the server over stdio, with matched controls (the same call with and without the filter; a no-match vs a matching value; each operator; a bare vs a user: tag key):
    • Filter and validation claims: 28/28 on this branch; 5/28 on main (negative control: Or, Not and {} return success there because the filter never reaches AWS).
    • Tags-leaf claims on getTagsOrValues and getCostCategories: 29/29 on this branch; 8/29 on main, where the filter is ignored (and 16/23 of the first 23 before the hint and section were extended: the data claims held, the hint and description claims failed). A user: key with a value returned an empty list on both operations where the bare key returned a non-empty one; ABSENT on a user: key returned the whole list; another case of key or value matched nothing; Values: [""] returned the same list as ABSENT; an aws: key with a value matched; an empty key and 6,001 values were rejected.
  • Independent reviews of the rebased diff re-checked these claims with their own calls (including Not/And around a user: key and the values modes) and found no contradiction.

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. A filter that was silently ignored is now applied; 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.

bhatia-di
bhatia-di previously approved these changes Oct 30, 2026
@codecov

codecov Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.79%. Comparing base (c25c287) to head (3dea6e0).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4970   +/-   ##
=======================================
  Coverage   93.79%   93.79%           
=======================================
  Files        1080     1080           
  Lines       94692    94703   +11     
  Branches    15438    15440    +2     
=======================================
+ Hits        88812    88823   +11     
  Misses       3505     3505           
  Partials     2375     2375           

☔ 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.

anniewn
anniewn previously approved these changes Oct 30, 2026
@finklen-amazon
finklen-amazon added this pull request to the merge queue Oct 30, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 30, 2026
@Comusus
Comusus added this pull request to the merge queue Oct 30, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 30, 2026
@finklen-amazon
finklen-amazon added this pull request to the merge queue Oct 30, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 30, 2026
@finklen-amazon
finklen-amazon added this pull request to the merge queue Oct 30, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 30, 2026
Dinkie
Dinkie previously approved these changes Nov 2, 2026
…ues and getCostCategories

cost-explorer accepted `filter` for getTagsOrValues and getCostCategories but
never sent it, so a filtered call returned the unfiltered list as a success.
Both APIs take Filter and apply it: the lists keep only the keys, names or
values found on matching costs.

- Forward filter to GetTags and GetCostCategories (keys and values).
- Describe filter for both operations and their limit: an And of leaves
  only; Or, Not and a nested And are rejected by the service.
- Extend the Tags leaves section and the response hint for a user: key or a
  group_by label to both operations: they now apply the filter, and there
  too a user: key with Values matches nothing (with ABSENT, every cost).
  Both also reject an empty Tags key, like getDimensionValues.
- Treat filter '""' (or a blank JSON string) as empty, like ''.
- cost-comparison: call a blank filter empty, before creating the client.
- group_by '[]' means no grouping on operations without GroupBy, as it does
  in getCostAndUsage. A group_by that is not JSON is still rejected there as
  not supported.
@finklen-amazon
finklen-amazon added this pull request to the merge queue Nov 2, 2026
Merged via the queue into awslabs:main with commit d5872b3 Nov 2, 2026
272 checks passed
@finklen-amazon
finklen-amazon deleted the fix/bcm-ce-tags-categories-filter branch November 2, 2026 17:23
@github-project-automation github-project-automation Bot moved this from To triage to Done in awslabs/mcp Project Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants