Uh oh!
There was an error while loading. Please reload this page.
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:
|
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| /// 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.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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. |
Uh oh!
There was an error while loading. Please reload this page.
## 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>
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