Skip to content

fix: add maximum_records parameter to paginate calls#63

Closed
rdgifford wants to merge 2 commits into
PagerDuty:mainfrom
rdgifford:fix/pagination-maximum-records
Closed

fix: add maximum_records parameter to paginate calls#63
rdgifford wants to merge 2 commits into
PagerDuty:mainfrom
rdgifford:fix/pagination-maximum-records

Conversation

@rdgifford

Copy link
Copy Markdown

Description

Fixes pagination bug where user's limit parameter was ignored, causing excessive token usage in MCP responses.

The paginate() function has a maximum_records parameter that defaults to 1000, but most list operations weren't passing it. This meant user-specified limits were ignored.

Issue number: #62

Type of change

  • New feature (non-breaking change which adds functionality)
  • Fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation only

Checklist

  • I have performed a self-review of this change
  • Changes have been tested
  • Changes are documented
  • Changes generate no new warnings
  • PR title follows conventional commit semantics

Changes Made

  • Added maximum_records parameter to paginate() calls in 7 tool files
  • Updated test assertions to expect the new parameter
  • Fixed incidents.py fallback from 100 to 1000 for consistency

Affected Tools

  • schedules.py
  • oncalls.py
  • escalation_policies.py
  • teams.py (3 calls)
  • incidents.py
  • services.py
  • event_orchestrations.py

Test Results

All tests pass with same baseline as main: 2 failed, 194 passed
(The 2 failures are pre-existing event_orchestrations tests)

Impact

Before: User sets limit=5 but receives up to 1000 records
After: User sets limit=5 and receives exactly 5 records

Note: Evals not run as this is a pure bug fix that doesn't change tool descriptions or behavior.

Add missing maximum_records parameter to paginate() calls across
all list operations. Without this parameter, the paginate function
ignores the user's limit parameter and defaults to fetching up to
1000 records, causing excessive token usage.

Affected tools:
- schedules.py
- oncalls.py
- escalation_policies.py
- teams.py (3 calls)
- incidents.py (fixed fallback value)
- services.py
- event_orchestrations.py

Updated test assertions in test_escalation_policies.py to expect
the maximum_records parameter.
Update test assertions across all affected test files to expect
the maximum_records parameter in paginate() calls. Tests now pass
with same baseline as main (2 pre-existing failures in event_orchestrations).
@pdt-svillanelo

Copy link
Copy Markdown
Contributor

Closing — the maximum_records fix has been incorporated directly into all affected tool PRs:

incidents.py (#144) already had maximum_records=limit or MAX_RESULTS (MAX_RESULTS=1000) from the refactor.

All paginate() calls now correctly pass maximum_records=limit or 1000 so user-specified limits are respected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants