fix: correct multi-status pagination in bugs list#301
Conversation
fetch_page_multi_status previously applied the same shared offset to each status query and then truncated the concatenated results. This caused bugs from later statuses to be permanently unreachable on page 2+ because the per-status offset skipped items that were truncated away on page 1. Now each status is fetched from offset 0 with a limit of offset+limit, the results are combined, the first offset items are drained, and the remainder is truncated to limit. This ensures every bug is reachable through pagination regardless of status ordering. Closes #300 Co-Authored-By: Sachin Iyer <siyer@detail.dev>
Original prompt from Sachin
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| /// Fetch a single page of bugs across multiple `statuses`, concatenated in | ||
| /// order and then truncated to `limit` items. | ||
| /// order and then paginated from the combined result. | ||
| /// | ||
| /// Each status is queried with the full `limit` and a shared `offset` | ||
| /// derived from `page` so that every status gets a fair chance to | ||
| /// contribute results regardless of how many bugs it contains. The merged | ||
| /// list is then truncated to at most `limit` items, preserving the | ||
| /// page-size contract. | ||
| /// Each status is queried starting at offset 0 with a fetch limit of | ||
| /// `offset + limit` so that the full combined window is available. The | ||
| /// merged list is then advanced past `offset` items and truncated to at | ||
| /// most `limit` items, ensuring every bug is reachable across pages. | ||
| /// | ||
| /// Unlike `fetch_all_bugs_multi_status` this does NOT exhaust every page — | ||
| /// it issues one bounded request per status and merges the results, keeping |
There was a problem hiding this comment.
📝 Info: Performance regression on later pages for multi-status queries
The new fetch_page_multi_status computes fetch_limit = offset.saturating_add(limit) and fetches that many items per status starting from offset 0 (src/commands/bugs.rs:466). For page N with limit L, this means fetching up to N×L items per status instead of just L. For example, page 10 with limit 50 fetches up to 500 items per status (5 API calls each at BUG_PAGE_SIZE=100), compared to 1 API call per status with the old approach. This is a necessary tradeoff for correctness — the old approach silently dropped bugs — but could be noticeable on repos with many thousands of bugs across multiple statuses at high page numbers. The CLI's --limit cap of 100 and typical usage patterns (most users view page 1–3) mitigate this in practice.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Agreed on the performance concern for deep pages. In practice, the CLI's --limit is capped at 100 and most users paginate through the first few pages, so fetch_limit stays small. For truly deep pages (e.g. page 100), the number of round-trips approaches what fetch_all_bugs_multi_status would do anyway. A fallback threshold could be added in a follow-up if needed, but keeping it simple for now.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The initial fix passed offset+limit as the API limit parameter, which can exceed the server maximum of 100 on deeper pages. Introduce fetch_bugs_up_to which paginates each status in BUG_PAGE_SIZE (100) chunks, collecting up to offset+limit items before applying the combined drain+truncate. Also add simulation tests that reproduce the original issue (#300) and verify the new pagination returns all bugs across pages. Co-Authored-By: Sachin Iyer <siyer@detail.dev>
| let mut total: usize = 0; | ||
| for status in dedupe_statuses(statuses) { | ||
| let response = client | ||
| .list_bugs(repo_id, status, limit, offset, scan_id) | ||
| .await | ||
| .context("Failed to fetch bugs from repository")?; | ||
| total += usize::try_from(response.total.max(0)).unwrap_or(0); | ||
| combined.extend(response.bugs); | ||
| let (bugs, status_total) = | ||
| fetch_bugs_up_to(client, repo_id, status, fetch_limit, scan_id).await?; | ||
| total += status_total; | ||
| combined.extend(bugs); |
There was a problem hiding this comment.
📝 Info: total count remains a sum of per-status server totals, unaffected by combined pagination window
The total returned by fetch_page_multi_status (src/commands/bugs.rs:472) is the sum of each status's API-reported total, which is correct for computing total_pages in output_list. However, this total represents the sum of all bugs across queried statuses, while the page window is applied to the concatenated list. If a user pages through all results, they'll see exactly total bugs — the math is consistent. Worth noting: fetch_bugs_up_to overwrites its local total on each API page (src/commands/bugs.rs:432) rather than accumulating, which is correct because the API's total field is the server-side count for that status (constant across pages), not a per-page count.
Was this helpful? React with 👍 or 👎 to provide feedback.
Test ResultsTested the built CLI binary from PR branch against the live Detail API and ran all unit/integration tests. Devin session Live E2E: Multi-status pagination (primary test)Ran Page 5 correctly returned 0 items. All 36 bugs reachable across pages with zero duplicates. This is the exact scenario that would have failed with the old code (bugs from later statuses dropped on page 2+). Unit test simulations (6 tests)
Regression: Full test suite295 unit tests + 18 integration tests — 0 failures. |
## Summary Patch release v0.2.4. Bumps version in `Cargo.toml` from 0.2.3 → 0.2.4. Changes since v0.2.3: - fix: correct multi-status pagination in `bugs list` (#301) - style: ban inline imports and hoist all use statements to module level (#299) ## Review & Testing Checklist for Human - [ ] Verify `Cargo.toml` and `Cargo.lock` version is `0.2.4` - [ ] Confirm CI passes (fmt, clippy, tests, vendored artifact checks) ### Notes Q/A testing was performed before cutting this release — all features verified working: - `detail version`, `detail auth status`, `detail repos list` - `detail bugs list` with multi-status, time filters, author filters, scan-id, JSON output - `detail bugs show` (table and JSON formats) - `detail bugs close` / `detail bugs reopen` lifecycle - `detail scans list` with status/time filters - `detail rules list`, `detail rules requests list` - `detail completions bash` - Error handling (invalid bug IDs)  Link to Devin session: https://app.devin.ai/sessions/b5a6272c900f4fc58425f4c555bec1e6 Requested by: @sachiniyer <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/usedetail/cli/pull/302" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open in Devin Review"> </picture> </a> <!-- devin-review-badge-end --> <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Bump `detail-cli` to v0.2.4 for a patch release. Includes a fix for multi-status pagination in `bugs list` and a style cleanup that hoists inline imports to module scope. <sup>Written for commit 76dbbb9. Summary will update on new commits. <a href="https://cubic.dev/pr/usedetail/cli/pull/302?utm_source=github">Review in cubic</a></sup> <!-- End of auto-generated description by cubic. --> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Sachin Iyer <siyer@detail.dev>
1 participant
Summary
Fixes #300.
fetch_page_multi_statusapplied a sharedoffsetto every per-status API call, then truncated the concatenated results tolimit. This made bugs from later statuses permanently unreachable on page 2+ because their per-status offset skipped items that were only truncated away (never shown) on page 1.Before: each status queried with
(limit, offset)→ concatenated → truncated tolimit.After: each status queried with
(offset + limit, 0)→ concatenated → drain firstoffsetitems → truncate tolimit.This ensures the pagination window is applied over the combined result set rather than per-status, so every bug is reachable regardless of status ordering.
Review & Testing Checklist for Human
detail bugs list --status pending --status resolved --limit 50 --page 1and--page 2and confirm all bugs appear across pages with no gapsbugs listpagination is unaffected (this code path is only used for multi-status queries)--page 1still returns the correct first page of resultsNotes
The fix uses
saturating_addforoffset + limitto avoid overflow on extreme page/limit values.Link to Devin session: https://app.devin.ai/sessions/7a885031946844ed914b5740feff94fa
Requested by: @sachiniyer
Summary by cubic
Fixes multi-status pagination in
bugs listso pages are built over the combined results, making bugs from later statuses reachable on page 2+.offset + limit, paginating in chunks to respect the API limit; merge, drop the firstoffset, then takelimit.saturating_addto prevent overflow on large page/limit values.Written for commit 1002b2e. Summary will update on new commits. Review in cubic