From 7e1d0eb432bc9129218a454e7e56b19040c5c144 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 24 Aug 2026 15:17:01 +0000 Subject: [PATCH] fix(review): keep GitCode provider change stats on capped review targets The review-target path replaced the provider change counts with sums over the first 1,000 mapped files, so a pull request with more changed files than that budget reported a file count equal to the budget and an omitted count of 1. Reuse the shared GitCode change-stats rule, which reads the whole files response, keeps the provider count when it is larger, and already refuses to claim truncated totals. Size the GitCode files request against the documented 3,000-entry single response instead of the pull request detail budget. The 4 MB detail limit turned file lists that previously loaded under the 16 MB default into a hard failure. Co-authored-by: Bob Lee --- .../src/review_platform.rs | 70 +++++++++++-------- 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/src/crates/services/services-integrations/src/review_platform.rs b/src/crates/services/services-integrations/src/review_platform.rs index d3915b698a..472f65c963 100644 --- a/src/crates/services/services-integrations/src/review_platform.rs +++ b/src/crates/services/services-integrations/src/review_platform.rs @@ -41,6 +41,9 @@ const MAX_REVIEW_FILE_DIFF_CHARS: usize = 80_000; // GitCode truncates `GET /pulls/{number}/files` at 3,000 entries without a // total-count header. The line-count headers are truncated with the body. const GITCODE_PULL_REQUEST_FILES_RESPONSE_LIMIT: usize = 3_000; +// The whole file list, diffs included, arrives in that one response, so it needs +// a larger budget than a single pull request detail payload. +const GITCODE_PULL_REQUEST_FILES_RESPONSE_BYTES: usize = 16 * 1024 * 1024; const DEFAULT_ISSUE_PAGE: u32 = 1; const DEFAULT_ISSUE_PAGE_SIZE: u32 = 100; const MAX_ISSUE_PAGE_SIZE: u32 = 100; @@ -1868,10 +1871,12 @@ async fn gitcode_review_target_parts( let initial_pull_request = gitcode_pull_request_from_value(&initial_detail); let mut confirmed_pull_request = gitcode_pull_request_from_value(&confirmed_detail); ensure_pull_request_revisions_stable(&initial_pull_request, &confirmed_pull_request)?; - apply_gitcode_review_target_file_stats(&mut confirmed_pull_request, &files); + apply_gitcode_pull_request_change_stats(&mut confirmed_pull_request, &files_response); Ok((confirmed_pull_request, files)) } +// The page hint is unusable here: GitCode answers `/files` with the whole list +// regardless of the page parameters, so the file is searched in that response. async fn gitcode_review_file_parts( ctx: &ProviderContext, pull_request_id: &str, @@ -3281,7 +3286,7 @@ async fn send_bounded_json_response( async fn send_bounded_gitcode_files_response( request: ReviewHttpRequest, ) -> Result { - send_review_json_response_bounded(request, MAX_REVIEW_TARGET_RESPONSE_BYTES) + send_review_json_response_bounded(request, GITCODE_PULL_REQUEST_FILES_RESPONSE_BYTES) .await .map_err(gitcode_files_http_error) } @@ -7168,15 +7173,6 @@ fn apply_files_stats(pull_request: &mut ReviewPlatformPullRequest, files: &[Revi pull_request.deletions = deletions; } -fn apply_gitcode_review_target_file_stats( - pull_request: &mut ReviewPlatformPullRequest, - files: &[ReviewPlatformFile], -) { - apply_files_stats(pull_request, files); - pull_request.changed_files = i32::try_from(files.len()).unwrap_or(i32::MAX); - pull_request.changed_file_count_known = files.len() < MAX_REVIEW_TARGET_LIST_ITEMS; -} - async fn fetch_bounded_paginated_array( mut build_request: F, next_page: fn(&ReviewHttpHeaders, u32) -> Option, @@ -8339,7 +8335,7 @@ mod tests { } #[test] - fn gitcode_review_target_stats_use_the_same_thousand_file_budget() { + fn gitcode_review_target_reports_the_files_the_budget_left_out() { let mut pull_request = gitcode_pull_request_from_value(&json!({ "number": 5, "title": "large change", @@ -8348,35 +8344,49 @@ mod tests { "removed_lines": 8_888, "changes_count": "2500" })); - let files = vec![ - ReviewPlatformFile { - path: "src/file.rs".to_string(), - old_path: None, - status: ReviewFileStatus::Modified, - additions: 1, - deletions: 2, - patch: Some("@@ -1 +1 @@\n-old\n+new".to_string()), - }; - MAX_REVIEW_TARGET_LIST_ITEMS - ]; + let response = JsonResponse { + value: Value::Array( + (0..2_500) + .map(|index| { + json!({ + "filename": format!("src/file-{index}.rs"), + "additions": "1", + "deletions": "2" + }) + }) + .collect(), + ), + headers: ReviewHttpHeaders::default(), + }; + let files = array_items(&response.value) + .iter() + .take(MAX_REVIEW_TARGET_LIST_ITEMS) + .map(gitcode_file_from_value) + .collect::>(); - apply_gitcode_review_target_file_stats(&mut pull_request, &files); + apply_gitcode_pull_request_change_stats(&mut pull_request, &response); + let target = review_target_from_parts(pull_request, files); - assert_eq!(pull_request.changed_files, 1_000); - assert!(!pull_request.changed_file_count_known); - assert_eq!(pull_request.additions, 1_000); - assert_eq!(pull_request.deletions, 2_000); + assert_eq!(target.pull_request.changed_files, 2_500); + assert!(target.pull_request.changed_file_count_known); + assert_eq!(target.pull_request.additions, 2_500); + assert_eq!(target.pull_request.deletions, 5_000); + assert_eq!(target.files.len(), MAX_REVIEW_TARGET_LIST_ITEMS); + assert_eq!(target.omitted_file_count, 1_500); + assert!(target + .limitations + .contains(&"provider_file_list_incomplete".to_string())); } #[test] fn gitcode_files_response_too_large_reports_explicit_reason() { let error = gitcode_files_http_error(ReviewHttpError::ResponseTooLarge { - limit_bytes: MAX_REVIEW_TARGET_RESPONSE_BYTES, + limit_bytes: GITCODE_PULL_REQUEST_FILES_RESPONSE_BYTES, }); assert_eq!( error.to_string(), - "Provider API failed: GitCode pull request files response exceeded the 4194304-byte limit" + "Provider API failed: GitCode pull request files response exceeded the 16777216-byte limit" ); }