[venkat-uk]

Fixes

Summary

Changes

_hit_output_limit() in src/cloudwatch-mcp-server/awslabs/cloudwatch_mcp_server/cloudwatch_logs/cwl_insights_batch.py decided that a query chunk had hit the 10,000-record output cap by reading statistics.recordsMatched. It now reads the number of returned rows.

recordsMatched is documented as "The number of log events that matched the query string". That is an input count. It cannot tell you whether the output was cut off. A stats ... by bin(1h) query can match millions of events and return ten rows.

The consequence is in _run_chunk(). When _hit_output_limit() returns True the chunk splits its time range in half and re-runs both halves, up to MAX_RECHUNK_DEPTH (4) levels. So any query matching 10,000 or more events was split, whether or not its output was ever truncated.

The returned row count is the only signal that the output was cut off, and it is the number the 10,000 cap applies to. GetQueryResults returns at most 10,000 rows per call, and this tool does not page past that, so len(results) >= MAX_OUTPUT_RECORDS is exactly the truncation test.

User experience

Before, a three-hour aggregation query over one log group that matched 50,000 events and returned 3 rows:

StartQuery calls : 31
rows returned    : 48
re_chunked       : 15
rows             : [{'bin': '2025-01-01T00:00:00Z', 'errors': '100', ...},
                    {'bin': '2025-01-01T01:00:00Z', 'errors': '100', ...},
                    {'bin': '2025-01-01T02:00:00Z', 'errors': '100', ...},
                    {'bin': '2025-01-01T00:00:00Z', 'errors': '100', ...},   <- bin repeated
                    ...]

Each bin came back 16 times, once per leaf sub-range, each holding a partial count. An agent reading that sees 16 buckets where there is one, and cannot add them up because the merge carries no sub-range boundaries.

After, the same call:

StartQuery calls : 1
rows returned    : 3
re_chunked       : 0
rows             : [{'bin': '2025-01-01T00:00:00Z', 'errors': '100', ...},
                    {'bin': '2025-01-01T01:00:00Z', 'errors': '100', ...},
                    {'bin': '2025-01-01T02:00:00Z', 'errors': '100', ...}]

Three effects for users:

1. Aggregation results are correct. One row per group, not one partial row per sub-range.
2. 31 StartQuery calls become 1. Insights is billed on data scanned, and each re-run rescans its slice of the range, so the old path multiplied both cost and query time. It also consumed the per-region concurrency budget that the tool sets aside at 25% of the account limit.
3. limit is honoured. limit is applied per StartQuery, so a split chunk returned up to 16 times the number of rows the caller asked for. The tool's own parameter help warns against overwhelming the agent context window.

Nothing changes for a query that genuinely fills its output. The existing test test_rechunk_on_output_limit, which drives splitting from 10,000 returned rows, is untouched and still passes. Time-range splitting on Timeout and PollingTimeout is untouched.

How this was tested

Run from src/cloudwatch-mcp-server:

uv run --frozen pytest tests/cloudwatch_logs/test_cwl_insights_batch.py -q
55 passed

uv run --frozen pytest --cov --cov-branch --cov-report=term-missing -q
480 passed, TOTAL coverage 97%, cwl_insights_batch.py 96% (unchanged)

uv run --frozen ruff check .            # All checks passed
uv run --frozen ruff format --check .   # 47 files already formatted
uv run --frozen --all-extras --dev pyright --stats   # 0 errors, 0 warnings

uvx pre-commit run --files <the two changed files>   # all hooks pass

All tests use unittest.mock. No AWS calls.

Four tests added or changed:

• test_no_rechunk_when_many_events_matched_but_few_rows_returned is the regression test. It asserts one StartQuery call, re_chunked == 0, and three distinct bin values in the merged output.
• test_high_records_matched_with_few_rows pins the unit behaviour: 50,000 matched, 3 rows, not a hit.
• test_at_limit_via_row_count_with_statistics_present pins that the row count decides even when statistics are present.
• test_at_limit_via_results_fallback is renamed to test_at_limit_via_row_count, since it is no longer a fallback. Its assertion is unchanged.

One existing test was removed: test_at_limit_via_stats asserted _hit_output_limit({'statistics': {'recordsMatched': 10_000}, 'results': []}) is True. Zero rows returned and "hit the output cap" cannot both be true, so it encoded the defect.

Tradeoffs

• This is a behaviour change, not an additive one. A query whose output really was truncated will still split, but only when the truncation is visible in the row count.
• If a caller passes a limit below 10,000 and the query fills it, the chunk no longer splits. That is deliberate: the caller bounded the output on purpose, and splitting returned more rows than they asked for. If splitting on an explicit limit is wanted, it should be a separate opt-in rather than an accident of the wrong statistic.
• The narrower detection means the tool will under-split rather than over-split. That asymmetry is on purpose. Under-splitting returns the same data a single query would have returned. Over-splitting returns wrong aggregates and costs many times more.
• Not addressed here: _run_chunk merges sub-range results by concatenation, so even a legitimate split of an aggregation query produces one partial row per sub-range. That is a pre-existing limitation of splitting aggregations at all, and worth a separate look. This change makes it much rarer by removing the case where splitting was never needed.

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.

[venkat-uk]

This is merged up to date with main and the cloudwatch-mcp-server tests pass. I've also added a CHANGELOG entry under Unreleased. @gcacace @shri-tambe @agiuliano @goranmod, could one of you review when you get a chance?

[github-actions]

This pull request is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon. If you wish to exclude this issue from being marked as stale, add the "backlog" label.

[codecov]

Codecov Report

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4524      +/-   ##
==========================================
- Coverage   93.79%   93.79%   -0.01%     
==========================================
  Files        1080     1080              
  Lines       94701    94697       -4     
  Branches    15434    15433       -1     
==========================================
- Hits        88828    88823       -5     
  Misses       3500     3500              
- Partials     2373     2374       +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.