From bbb845e2b31ca5cd08c01a5b2cbc29d645494fce Mon Sep 17 00:00:00 2001 From: guantw Date: Mon, 24 Aug 2026 21:40:54 +0800 Subject: [PATCH 1/2] fix(review): load GitCode PR files as one bounded response GitCode returns pull request files as a single response truncated at 3,000 entries without pagination or a total header. Replace the per-page fetch for review targets, file diffs, and the detail Files section with one bounded request, slice pages client-side, and avoid claiming an exact total when the response is truncated. Also prefer old_path and new_file/deleted_file/renamed_file flags when mapping files, and accept a nested patch.diff payload. --- .../src/review_platform.rs | 255 +++++++++++++----- 1 file changed, 195 insertions(+), 60 deletions(-) diff --git a/src/crates/services/services-integrations/src/review_platform.rs b/src/crates/services/services-integrations/src/review_platform.rs index b9382476d3..d3915b698a 100644 --- a/src/crates/services/services-integrations/src/review_platform.rs +++ b/src/crates/services/services-integrations/src/review_platform.rs @@ -1844,62 +1844,63 @@ async fn gitcode_review_target_parts( ); let initial_detail = send_bounded_json(gitcode_request(client.clone(), &base, ctx.token.as_deref())).await?; - let token = ctx.token.clone(); let files_url = format!("{}/files", base); - let files = fetch_bounded_paginated_array( - |page| { - let page = page.to_string(); - gitcode_request(client.clone(), &files_url, token.as_deref()) - .query(&[("per_page", "100"), ("page", &page)]) - }, - github_next_page, - MAX_REVIEW_TARGET_LIST_ITEMS, - ) + let files_response = send_bounded_gitcode_files_response(gitcode_request( + client.clone(), + &files_url, + ctx.token.as_deref(), + )) .await?; + let files = files_response + .value + .as_array() + .ok_or_else(|| { + ReviewPlatformError::Parse( + "GitCode pull request files response was not an array".to_string(), + ) + })? + .iter() + .take(MAX_REVIEW_TARGET_LIST_ITEMS) + .map(gitcode_file_from_value) + .collect::>(); let confirmed_detail = send_bounded_json(gitcode_request(client, &base, ctx.token.as_deref())).await?; let initial_pull_request = gitcode_pull_request_from_value(&initial_detail); - let confirmed_pull_request = gitcode_pull_request_from_value(&confirmed_detail); + let mut confirmed_pull_request = gitcode_pull_request_from_value(&confirmed_detail); ensure_pull_request_revisions_stable(&initial_pull_request, &confirmed_pull_request)?; - Ok(( - confirmed_pull_request, - array_items(&files) - .iter() - .map(gitcode_file_from_value) - .collect(), - )) + apply_gitcode_review_target_file_stats(&mut confirmed_pull_request, &files); + Ok((confirmed_pull_request, files)) } async fn gitcode_review_file_parts( ctx: &ProviderContext, pull_request_id: &str, file_path: &str, - file_page_hint: Option, + _file_page_hint: Option, ) -> Result<(ReviewPlatformPullRequest, Vec), ReviewPlatformError> { let client = http_client()?; let base = format!( "{}/repos/{}/{}/pulls/{}", ctx.api_base_url, ctx.remote.owner, ctx.remote.repository_name, pull_request_id ); - let token = ctx.token.clone(); let files_url = format!("{}/files", base); - let file = fetch_bounded_paginated_file( - |page| { - let page = page.to_string(); - gitcode_request(client.clone(), &files_url, token.as_deref()) - .query(&[("per_page", "100"), ("page", &page)]) - }, - github_next_page, - file_page_hint.unwrap_or(1), - if file_page_hint.is_some() { - 100 - } else { - MAX_REVIEW_TARGET_LIST_ITEMS - }, - file_path, - gitcode_file_from_value, - ) - .await?; + let files = send_bounded_gitcode_files_response(gitcode_request( + client.clone(), + &files_url, + ctx.token.as_deref(), + )) + .await? + .value; + let file = files + .as_array() + .ok_or_else(|| { + ReviewPlatformError::Parse( + "GitCode pull request files response was not an array".to_string(), + ) + })? + .iter() + .map(gitcode_file_from_value) + .find(|file| file.path == file_path || file.old_path.as_deref() == Some(file_path)); let detail = send_bounded_json(gitcode_request(client, &base, ctx.token.as_deref())).await?; Ok(( gitcode_pull_request_from_value(&detail), @@ -2851,7 +2852,7 @@ async fn gitcode_pull_request_detail_page( match section { ReviewPlatformDetailSection::Overview => { - if let Ok(response) = send_json_response(gitcode_request( + if let Ok(response) = send_bounded_gitcode_files_response(gitcode_request( client.clone(), &format!("{}/files", base), ctx.token.as_deref(), @@ -2866,22 +2867,21 @@ async fn gitcode_pull_request_detail_page( ci = slice_page(ci, pagination); } ReviewPlatformDetailSection::Files => { - if let Ok(response) = fetch_array_page( - gitcode_request( - client.clone(), - &format!("{}/files", base), - ctx.token.as_deref(), - ), - pagination, - ) + if let Ok(response) = send_bounded_gitcode_files_response(gitcode_request( + client.clone(), + &format!("{}/files", base), + ctx.token.as_deref(), + )) .await { - apply_gitcode_pull_request_change_stats(&mut pull_request, &response); - section_pagination = pagination_from_response(&response, pagination); - files = array_items(&response.value) - .iter() - .map(gitcode_file_from_value) - .collect(); + if let Some(values) = response.value.as_array() { + apply_gitcode_pull_request_change_stats(&mut pull_request, &response); + section_pagination = gitcode_files_pagination(pagination, values.len()); + files = slice_page( + values.iter().map(gitcode_file_from_value).collect(), + pagination, + ); + } } } ReviewPlatformDetailSection::Commits => { @@ -3003,7 +3003,7 @@ impl ReviewProvider for GitcodeProvider { let detail = send_json(gitcode_request(client.clone(), &base, ctx.token.as_deref())).await?; let files_url = format!("{}/files", base); - let files_response = send_json_response(gitcode_request( + let files_response = send_bounded_gitcode_files_response(gitcode_request( client.clone(), &files_url, ctx.token.as_deref(), @@ -3242,6 +3242,15 @@ fn review_http_error(error: ReviewHttpError) -> ReviewPlatformError { } } +fn gitcode_files_http_error(error: ReviewHttpError) -> ReviewPlatformError { + match error { + ReviewHttpError::ResponseTooLarge { limit_bytes } => ReviewPlatformError::Api(format!( + "GitCode pull request files response exceeded the {limit_bytes}-byte limit" + )), + error => review_http_error(error), + } +} + async fn send_json(request: ReviewHttpRequest) -> Result { send_review_json(request).await.map_err(review_http_error) } @@ -3269,6 +3278,14 @@ async fn send_bounded_json_response( .map_err(review_http_error) } +async fn send_bounded_gitcode_files_response( + request: ReviewHttpRequest, +) -> Result { + send_review_json_response_bounded(request, MAX_REVIEW_TARGET_RESPONSE_BYTES) + .await + .map_err(gitcode_files_http_error) +} + async fn send_bounded_text( request: ReviewHttpRequest, max_bytes: usize, @@ -3432,6 +3449,17 @@ fn pagination_from_total( } } +fn gitcode_files_pagination( + pagination: PullRequestPagination, + available_files: usize, +) -> ReviewPlatformPagination { + let mut result = pagination_from_total(pagination, available_files); + if available_files >= GITCODE_PULL_REQUEST_FILES_RESPONSE_LIMIT { + result.total = None; + } + result +} + fn slice_page(items: Vec, pagination: PullRequestPagination) -> Vec { let start = pagination .page @@ -3536,7 +3564,8 @@ async fn enrich_gitcode_pull_request_change_stats( let token = ctx.token.clone(); async move { if let Ok(response) = - send_json_response(gitcode_request(client, &url, token.as_deref())).await + send_bounded_gitcode_files_response(gitcode_request(client, &url, token.as_deref())) + .await { apply_gitcode_pull_request_change_stats(&mut pull_request, &response); } @@ -6646,17 +6675,37 @@ fn gitcode_file_from_value(value: &Value) -> ReviewPlatformFile { value_string(value, "filename"), value_string(value, "new_path"), ]), - old_path: value - .get("previous_filename") - .and_then(Value::as_str) - .map(str::to_string), - status: file_status(&value_string(value, "status")), + old_path: optional_string(value, "old_path") + .or_else(|| optional_string(value, "previous_filename")), + status: gitcode_file_status(value), additions: value_i64(value, "additions") as i32, deletions: value_i64(value, "deletions") as i32, - patch: optional_string(value, "patch").or_else(|| optional_string(value, "diff")), + patch: gitcode_patch_from_value(value), + } +} + +fn gitcode_file_status(value: &Value) -> ReviewFileStatus { + if value_bool(value, "new_file") { + ReviewFileStatus::Added + } else if value_bool(value, "deleted_file") { + ReviewFileStatus::Deleted + } else if value_bool(value, "renamed_file") { + ReviewFileStatus::Renamed + } else { + file_status(&value_string(value, "status")) } } +fn gitcode_patch_from_value(value: &Value) -> Option { + optional_string(value, "patch") + .or_else(|| { + value + .get("patch") + .and_then(|patch| optional_string(patch, "diff")) + }) + .or_else(|| optional_string(value, "diff")) +} + fn gitlab_files(value: &Value) -> Vec { value .get("changes") @@ -7119,6 +7168,15 @@ 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, @@ -8245,6 +8303,83 @@ mod tests { assert_eq!(pull_request.deletions, 22); } + #[test] + fn gitcode_file_maps_nested_patch_and_change_flags() { + let file = gitcode_file_from_value(&json!({ + "filename": "src/new.rs", + "old_path": "src/old.rs", + "status": null, + "new_file": false, + "renamed_file": true, + "deleted_file": false, + "additions": 1, + "deletions": 1, + "patch": { "diff": "@@ -1 +1 @@\n-old\n+new" } + })); + + assert_eq!(file.path, "src/new.rs"); + assert_eq!(file.old_path.as_deref(), Some("src/old.rs")); + assert_eq!(file.status, ReviewFileStatus::Renamed); + assert_eq!(file.patch.as_deref(), Some("@@ -1 +1 @@\n-old\n+new")); + assert!(file_has_complete_patch(&file)); + } + + #[test] + fn gitcode_capped_files_pagination_does_not_claim_exact_total() { + let pagination = gitcode_files_pagination( + PullRequestPagination { + page: 60, + per_page: 50, + }, + GITCODE_PULL_REQUEST_FILES_RESPONSE_LIMIT, + ); + + assert_eq!(pagination.total, None); + assert!(!pagination.has_next); + } + + #[test] + fn gitcode_review_target_stats_use_the_same_thousand_file_budget() { + let mut pull_request = gitcode_pull_request_from_value(&json!({ + "number": 5, + "title": "large change", + "state": "open", + "added_lines": 9_999, + "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 + ]; + + apply_gitcode_review_target_file_stats(&mut 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); + } + + #[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, + }); + + assert_eq!( + error.to_string(), + "Provider API failed: GitCode pull request files response exceeded the 4194304-byte limit" + ); + } + #[test] fn gitcode_file_response_overrides_incomplete_list_stats() { let mut pull_request = gitcode_pull_request_from_value(&json!({ From 5808950390ee3cc1c775c2305930fa05cc42e476 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 24 Aug 2026 15:17:01 +0000 Subject: [PATCH 2/2] 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" ); }