From 7fd90fddda419efe0bb160268c22506778c285c8 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 20 May 2026 06:51:44 +0000 Subject: [PATCH 1/2] fix: correct multi-status pagination to fetch from offset 0 per status 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 --- src/commands/bugs.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/commands/bugs.rs b/src/commands/bugs.rs index 7d739ca..149b714 100644 --- a/src/commands/bugs.rs +++ b/src/commands/bugs.rs @@ -404,13 +404,12 @@ async fn fetch_all_bugs_multi_status( } /// 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 @@ -424,17 +423,20 @@ async fn fetch_page_multi_status( scan_id: Option<&ListPublicBugsWorkflowRequestId>, ) -> Result<(Vec, usize)> { let offset = page_to_offset(page, limit); + let fetch_limit = offset.saturating_add(limit); let mut combined = Vec::new(); let mut total: usize = 0; for status in dedupe_statuses(statuses) { let response = client - .list_bugs(repo_id, status, limit, offset, scan_id) + .list_bugs(repo_id, status, fetch_limit, 0, 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 offset_usize = usize::try_from(offset).unwrap_or(usize::MAX); let limit_usize = usize::try_from(limit).unwrap_or(usize::MAX); + combined.drain(..offset_usize.min(combined.len())); combined.truncate(limit_usize); Ok((combined, total)) } From 1002b2ef362ca3ca513f03ef99b81e6a6cd82328 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 20 May 2026 07:03:20 +0000 Subject: [PATCH 2/2] fix: paginate each status internally to respect API limit of 100 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 --- src/commands/bugs.rs | 245 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 233 insertions(+), 12 deletions(-) diff --git a/src/commands/bugs.rs b/src/commands/bugs.rs index 149b714..e12de8b 100644 --- a/src/commands/bugs.rs +++ b/src/commands/bugs.rs @@ -403,17 +403,57 @@ async fn fetch_all_bugs_multi_status( Ok(combined) } +/// Fetch up to `max_items` bugs for a single `status`, paginating through +/// the API in `BUG_PAGE_SIZE` chunks so we never exceed the server limit. +/// +/// Returns the collected bugs and the total count reported by the API. +async fn fetch_bugs_up_to( + client: &ApiClient, + repo_id: &RepoId, + status: BugReviewState, + max_items: u32, + scan_id: Option<&ListPublicBugsWorkflowRequestId>, +) -> Result<(Vec, usize)> { + let max_usize = usize::try_from(max_items).unwrap_or(usize::MAX); + let mut bugs = Vec::new(); + let mut offset: u32 = 0; + let mut total: usize = 0; + + loop { + let remaining = max_items.saturating_sub(offset); + if remaining == 0 { + break; + } + let page_size = BUG_PAGE_SIZE.min(remaining); + let response = client + .list_bugs(repo_id, status, page_size, offset, scan_id) + .await + .context("Failed to fetch bugs from repository")?; + total = usize::try_from(response.total.max(0)).unwrap_or(0); + let page_len = response.bugs.len(); + bugs.extend(response.bugs); + + if page_len == 0 || bugs.len() >= max_usize || bugs.len() >= total { + break; + } + offset = offset.saturating_add(u32::try_from(page_len).unwrap_or(u32::MAX)); + } + + bugs.truncate(max_usize); + Ok((bugs, total)) +} + /// Fetch a single page of bugs across multiple `statuses`, concatenated in /// order and then paginated from the combined result. /// -/// 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. +/// Each status is fetched from offset 0 up to `offset + limit` items +/// (paginating internally in `BUG_PAGE_SIZE` chunks to respect the API +/// limit). 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 -/// multi-status queries fast even on repos with thousands of bugs. +/// it fetches only enough items per status to fill the requested window. async fn fetch_page_multi_status( client: &ApiClient, repo_id: &RepoId, @@ -427,12 +467,10 @@ async fn fetch_page_multi_status( let mut combined = Vec::new(); let mut total: usize = 0; for status in dedupe_statuses(statuses) { - let response = client - .list_bugs(repo_id, status, fetch_limit, 0, 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); } let offset_usize = usize::try_from(offset).unwrap_or(usize::MAX); let limit_usize = usize::try_from(limit).unwrap_or(usize::MAX); @@ -1107,6 +1145,189 @@ mod tests { assert!(page.is_empty()); } + // ── multi-status pagination ──────────────────────────────────── + + fn make_bugs(prefix: &str, count: usize) -> Vec { + (0..count) + .map(|i| { + serde_json::from_value(serde_json::json!({ + "id": format!("bug_{prefix}_{i}"), + "title": format!("{prefix} bug {i}"), + "summary": "...", + "createdAt": i, + "repoId": "repo_1", + "linkedIssues": [] + })) + .unwrap() + }) + .collect() + } + + /// Simulate the API: given the full list of bugs for a status, return + /// at most `limit` items starting at `offset`. + fn simulate_api(all: &[Bug], limit: u32, offset: u32) -> Vec { + let off = usize::try_from(offset).unwrap_or(usize::MAX); + let lim = usize::try_from(limit).unwrap_or(usize::MAX); + all.iter().skip(off).take(lim).cloned().collect() + } + + /// Simulate the *old* `fetch_page_multi_status` approach: shared offset + /// per status, then concatenate and truncate. + fn simulate_old_pagination(per_status: &[&[Bug]], page: u32, limit: u32) -> Vec { + let offset = page_to_offset(page, limit); + let mut combined = Vec::new(); + for bugs in per_status { + combined.extend(simulate_api(bugs, limit, offset)); + } + let lim = usize::try_from(limit).unwrap_or(usize::MAX); + combined.truncate(lim); + combined + } + + /// Simulate the *new* `fetch_page_multi_status` approach: each status + /// fetched from offset 0 up to `offset + limit`, then drain + truncate + /// on the combined list. + fn simulate_new_pagination(per_status: &[&[Bug]], page: u32, limit: u32) -> Vec { + let offset = page_to_offset(page, limit); + let fetch_limit = offset.saturating_add(limit); + let mut combined = Vec::new(); + for bugs in per_status { + combined.extend(simulate_api(bugs, fetch_limit, 0)); + } + let off = usize::try_from(offset).unwrap_or(usize::MAX); + let lim = usize::try_from(limit).unwrap_or(usize::MAX); + combined.drain(..off.min(combined.len())); + combined.truncate(lim); + combined + } + + /// Collect all bug IDs across every page until empty. + fn collect_all_ids( + per_status: &[&[Bug]], + limit: u32, + paginate: fn(&[&[Bug]], u32, u32) -> Vec, + ) -> Vec { + let mut ids = Vec::new(); + for page in 1_u32.. { + let bugs = paginate(per_status, page, limit); + if bugs.is_empty() { + break; + } + ids.extend(bugs.iter().map(|b| b.id.to_string())); + } + ids + } + + #[test] + fn old_pagination_drops_resolved_bugs_issue_300() { + // Issue scenario: 50 pending + 40 resolved, limit=50. + // The old approach drops all 40 resolved bugs. + let pending = make_bugs("pending", 50); + let resolved = make_bugs("resolved", 40); + let per_status: Vec<&[Bug]> = vec![&pending, &resolved]; + + let all_ids = collect_all_ids(&per_status, 50, simulate_old_pagination); + // Old approach only finds the 50 pending bugs; the 40 resolved are lost. + assert_eq!(all_ids.len(), 50, "old approach should only find 50 of 90"); + assert!( + !all_ids.iter().any(|id| id.starts_with("bug_resolved")), + "old approach should miss all resolved bugs" + ); + } + + #[test] + fn new_pagination_returns_all_bugs_issue_300() { + // Same scenario: 50 pending + 40 resolved, limit=50. + // The new approach returns all 90 bugs across 2 pages. + let pending = make_bugs("pending", 50); + let resolved = make_bugs("resolved", 40); + let per_status: Vec<&[Bug]> = vec![&pending, &resolved]; + + let all_ids = collect_all_ids(&per_status, 50, simulate_new_pagination); + assert_eq!(all_ids.len(), 90, "new approach should find all 90 bugs"); + assert_eq!( + all_ids + .iter() + .filter(|id| id.starts_with("bug_pending")) + .count(), + 50 + ); + assert_eq!( + all_ids + .iter() + .filter(|id| id.starts_with("bug_resolved")) + .count(), + 40 + ); + } + + #[test] + fn new_pagination_page1_returns_first_limit_bugs() { + let pending = make_bugs("pending", 50); + let resolved = make_bugs("resolved", 40); + let per_status: Vec<&[Bug]> = vec![&pending, &resolved]; + + let page1 = simulate_new_pagination(&per_status, 1, 50); + assert_eq!(page1.len(), 50); + assert!(page1 + .iter() + .all(|b| b.id.to_string().starts_with("bug_pending"))); + } + + #[test] + fn new_pagination_page2_returns_remaining_bugs() { + let pending = make_bugs("pending", 50); + let resolved = make_bugs("resolved", 40); + let per_status: Vec<&[Bug]> = vec![&pending, &resolved]; + + let page2 = simulate_new_pagination(&per_status, 2, 50); + assert_eq!(page2.len(), 40); + assert!(page2 + .iter() + .all(|b| b.id.to_string().starts_with("bug_resolved"))); + } + + #[test] + fn new_pagination_even_split_across_pages() { + // 30 pending + 30 resolved, limit=50 → page 1 has 50, page 2 has 10. + let pending = make_bugs("pending", 30); + let resolved = make_bugs("resolved", 30); + let per_status: Vec<&[Bug]> = vec![&pending, &resolved]; + + let page1 = simulate_new_pagination(&per_status, 1, 50); + assert_eq!(page1.len(), 50); + // First 30 are pending, next 20 are resolved. + assert_eq!( + page1 + .iter() + .filter(|b| b.id.to_string().starts_with("bug_pending")) + .count(), + 30 + ); + assert_eq!( + page1 + .iter() + .filter(|b| b.id.to_string().starts_with("bug_resolved")) + .count(), + 20 + ); + + let page2 = simulate_new_pagination(&per_status, 2, 50); + assert_eq!(page2.len(), 10); + assert!(page2 + .iter() + .all(|b| b.id.to_string().starts_with("bug_resolved"))); + } + + #[test] + fn new_pagination_past_last_page_is_empty() { + let pending = make_bugs("pending", 10); + let per_status: Vec<&[Bug]> = vec![&pending]; + + let page2 = simulate_new_pagination(&per_status, 2, 50); + assert!(page2.is_empty()); + } + // `format_introduced_in` moved to `crate::api::types`; tests now live // alongside the function in `src/api/types.rs`. }