fix(stats): flag group-filtered counts truncated at the search cap - #314
fix(stats): flag group-filtered counts truncated at the search cap#314seanGSISG wants to merge 2 commits into
Conversation
zammad_get_ticket_stats can silently under-report for large groups. _collect_ticket_stats_paginated pages through client.search_tickets(). With no group filter that falls through to ticket.all(), the list endpoint, which is uncapped. With a group filter it builds a query and uses ticket.search(), which is backed by Elasticsearch and bounded by index.max_result_window, 10,000 by default. Past that bound the endpoint returns empty pages rather than an error, so the loop exits through its `if not tickets: break` path as though it had reached the end of the data. The existing MAX_PAGES_FOR_TICKET_SCAN warning cannot catch this: at 100 per page the cap is reached on page 100, far below the 1000 page guard. A group with more than 10,000 tickets therefore reports exactly 10,000 as an exact total, with nothing in the response or the logs indicating the number is short. Detect the cap during group-filtered scans and surface it rather than guessing at a correction: add counts_truncated to TicketStats, defaulted False, so callers can tell a lower bound from an exact total. Also log a warning naming the group. This does not raise the ceiling. Counting beyond it means enumerating via the list endpoint and filtering client side, which is a much larger change and a real cost on big instances. Reporting the limit honestly seemed like the right first step; happy to follow up with the fuller approach if you would prefer it. Refs: https://community.zammad.org/t/api-tickets-search-endpoint-limited-to-10-000-results/10644
|
Warning Review limit reached
Next review available in: 56 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 8 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Problem
zammad_get_ticket_statscan silently under-report for large groups, returning a wrong number with no indication anything was missed.Cause
_collect_ticket_stats_paginatedpages throughclient.search_tickets(). Which endpoint that reaches depends on whether a group filter is set:search_partsis empty, sosearch_ticketsfalls through toticket.all(), the list endpoint. Uncapped, counts are correct.group.name:<x>and callsticket.search(), which is backed by Elasticsearch and bounded byindex.max_result_window, 10,000 by default.Past that bound the search endpoint returns empty pages rather than an error, so the loop exits through its
if not tickets: breakpath exactly as if it had reached the end of the data.The existing
MAX_PAGES_FOR_TICKET_SCANguard can't catch this. AtMAX_PER_PAGE = 100the cap is hit on page 100, well short of the 1000-page limit, so that warning never fires. A group with more than 10,000 tickets reports exactly10000as an exact total, with nothing in the response or the logs saying otherwise.Change
Detect the cap during group-filtered scans and surface it, rather than guessing at a correction:
counts_truncatedadded toTicketStats, defaulting toFalse, so callers can distinguish a lower bound from an exact totalSEARCH_RESULT_CAPdocuments where the 10,000 comes fromThe new field is optional with a default, so existing consumers are unaffected.
What this deliberately does not do
It doesn't raise the ceiling. Counting past it means enumerating via the list endpoint and filtering client-side, which is a substantially larger change and a real cost on big instances. Reporting the limit honestly seemed like the right first step — happy to follow up with the fuller approach if you'd prefer it, or to drop this in favour of that.
Tests
Five tests: truncation flagged at the cap, exact counts below it, unfiltered scans never flagged, and the flag surfacing on
TicketStatsin both states.ruff check,ruff format --check,mypyand the full suite (227 tests) all pass.