Skip to content

Commit a96684f

Browse files
committed
fix(review): handle truncated GitCode file responses
1 parent 9549fc8 commit a96684f

1 file changed

Lines changed: 90 additions & 14 deletions

File tree

src/crates/services/services-integrations/src/review_platform.rs

Lines changed: 90 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ const MAX_REVIEW_TARGET_PAGES: usize = 10;
3838
const MAX_REVIEW_TARGET_LIST_ITEMS: usize = MAX_REVIEW_TARGET_PAGES * 100;
3939
const MAX_REVIEW_TARGET_RESPONSE_BYTES: usize = 4 * 1024 * 1024;
4040
const MAX_REVIEW_FILE_DIFF_CHARS: usize = 80_000;
41+
// GitCode truncates `GET /pulls/{number}/files` at 3,000 entries without a
42+
// total-count header. The line-count headers are truncated with the body.
43+
const GITCODE_PULL_REQUEST_FILES_RESPONSE_LIMIT: usize = 3_000;
4144
const DEFAULT_ISSUE_PAGE: u32 = 1;
4245
const DEFAULT_ISSUE_PAGE_SIZE: u32 = 100;
4346
const MAX_ISSUE_PAGE_SIZE: u32 = 100;
@@ -3553,12 +3556,20 @@ fn apply_gitcode_pull_request_change_stats(
35533556
let Some(values) = response.value.as_array() else {
35543557
return;
35553558
};
3559+
let file_count = i32::try_from(values.len()).unwrap_or(i32::MAX);
3560+
if values.len() >= GITCODE_PULL_REQUEST_FILES_RESPONSE_LIMIT {
3561+
if !pull_request.changed_file_count_known || pull_request.changed_files < file_count {
3562+
pull_request.changed_files = file_count;
3563+
pull_request.changed_file_count_known = false;
3564+
}
3565+
return;
3566+
}
35563567
let files = values
35573568
.iter()
35583569
.map(gitcode_file_from_value)
35593570
.collect::<Vec<_>>();
35603571
apply_files_stats(pull_request, &files);
3561-
pull_request.changed_files = i32::try_from(files.len()).unwrap_or(i32::MAX);
3572+
pull_request.changed_files = file_count;
35623573
pull_request.changed_file_count_known = true;
35633574
if let Some(total) = header_u64(&response.headers, "total_added_lines") {
35643575
pull_request.additions = i32::try_from(total).unwrap_or(i32::MAX);
@@ -6547,18 +6558,19 @@ fn gitcode_pull_request_from_value(value: &Value) -> ReviewPlatformPullRequest {
65476558
"closed" => ReviewItemState::Closed,
65486559
_ => ReviewItemState::Open,
65496560
};
6550-
let changed_files = value_string(value, "changes_count")
6551-
.trim_end_matches('+')
6552-
.parse::<i32>()
6553-
.ok()
6554-
.or_else(|| {
6555-
value.get("changed_files").and_then(|count| {
6556-
count
6557-
.as_i64()
6558-
.or_else(|| count.as_str()?.parse::<i64>().ok())
6559-
.and_then(|count| i32::try_from(count).ok())
6560-
})
6561-
});
6561+
let changes_count = value_string(value, "changes_count");
6562+
let changes_count = changes_count.trim();
6563+
let changes_count_is_approximate = changes_count.ends_with('+');
6564+
let changed_files_from_changes_count = changes_count.trim_end_matches('+').parse::<i32>().ok();
6565+
let changed_files_from_legacy_field = value.get("changed_files").and_then(|count| {
6566+
count
6567+
.as_i64()
6568+
.or_else(|| count.as_str()?.parse::<i64>().ok())
6569+
.and_then(|count| i32::try_from(count).ok())
6570+
});
6571+
let changed_files = changed_files_from_changes_count.or(changed_files_from_legacy_field);
6572+
let changed_file_count_known = changed_files.is_some()
6573+
&& !(changed_files_from_changes_count.is_some() && changes_count_is_approximate);
65626574
ReviewPlatformPullRequest {
65636575
id: number.to_string(),
65646576
provider_id: None,
@@ -6595,7 +6607,7 @@ fn gitcode_pull_request_from_value(value: &Value) -> ReviewPlatformPullRequest {
65956607
additions: gitcode_pull_request_line_count(value, "added_lines", "additions"),
65966608
deletions: gitcode_pull_request_line_count(value, "removed_lines", "deletions"),
65976609
changed_files: changed_files.unwrap_or(0),
6598-
changed_file_count_known: changed_files.is_some(),
6610+
changed_file_count_known,
65996611
comments: value_i64(value, "comments") as i32,
66006612
review_decision: ReviewDecision::Pending,
66016613
checks: empty_checks(),
@@ -8204,6 +8216,19 @@ mod tests {
82048216
assert!(pull_request.changed_file_count_known);
82058217
}
82068218

8219+
#[test]
8220+
fn gitcode_pull_request_marks_approximate_change_count_unknown() {
8221+
let pull_request = gitcode_pull_request_from_value(&json!({
8222+
"number": 5,
8223+
"title": "large change",
8224+
"state": "open",
8225+
"changes_count": "3000+"
8226+
}));
8227+
8228+
assert_eq!(pull_request.changed_files, 3_000);
8229+
assert!(!pull_request.changed_file_count_known);
8230+
}
8231+
82078232
#[test]
82088233
fn gitcode_pull_request_marks_missing_file_count_unknown() {
82098234
let pull_request = gitcode_pull_request_from_value(&json!({
@@ -8332,6 +8357,57 @@ mod tests {
83328357
assert_eq!(pull_request.deletions, 22);
83338358
}
83348359

8360+
#[test]
8361+
fn gitcode_capped_file_response_does_not_claim_truncated_stats() {
8362+
let response = JsonResponse {
8363+
value: Value::Array(
8364+
(0..GITCODE_PULL_REQUEST_FILES_RESPONSE_LIMIT)
8365+
.map(|index| {
8366+
json!({
8367+
"filename": format!("src/{index}.rs"),
8368+
"additions": "1",
8369+
"deletions": "1"
8370+
})
8371+
})
8372+
.collect(),
8373+
),
8374+
headers: ReviewHttpHeaders::from_pairs(&[
8375+
("total_added_lines", "1910"),
8376+
("total_removed_lines", "116799"),
8377+
]),
8378+
};
8379+
let mut unknown_count = gitcode_pull_request_from_value(&json!({
8380+
"number": 5,
8381+
"title": "large change",
8382+
"state": "open",
8383+
"added_lines": 133,
8384+
"removed_lines": 22
8385+
}));
8386+
8387+
apply_gitcode_pull_request_change_stats(&mut unknown_count, &response);
8388+
8389+
assert_eq!(unknown_count.changed_files, 3_000);
8390+
assert!(!unknown_count.changed_file_count_known);
8391+
assert_eq!(unknown_count.additions, 133);
8392+
assert_eq!(unknown_count.deletions, 22);
8393+
8394+
let mut provider_count = gitcode_pull_request_from_value(&json!({
8395+
"number": 5,
8396+
"title": "large change",
8397+
"state": "open",
8398+
"added_lines": 401011,
8399+
"removed_lines": 5219754,
8400+
"changes_count": "32202"
8401+
}));
8402+
8403+
apply_gitcode_pull_request_change_stats(&mut provider_count, &response);
8404+
8405+
assert_eq!(provider_count.changed_files, 32_202);
8406+
assert!(provider_count.changed_file_count_known);
8407+
assert_eq!(provider_count.additions, 401_011);
8408+
assert_eq!(provider_count.deletions, 5_219_754);
8409+
}
8410+
83358411
#[test]
83368412
fn github_cli_pull_request_maps_open_user_list_fields() {
83378413
let pull_request = github_pull_request_from_gh_cli_value(

0 commit comments

Comments
 (0)