Skip to main content

aptu_core/github/
pulls.rs

1// SPDX-License-Identifier: Apache-2.0
2
3//! Pull request fetching via Octocrab.
4//!
5//! Provides functions to parse PR references and fetch PR details
6//! including file diffs for AI review.
7
8use anyhow::{Context, Result};
9use octocrab::Octocrab;
10use tracing::{debug, instrument};
11
12use super::{ReferenceKind, parse_github_reference};
13use crate::ai::types::{PrDetails, PrFile, PrReviewComment, ReviewEvent};
14use crate::error::{AptuError, ResourceType};
15use crate::triage::render_pr_review_comment_body;
16
17/// Result from creating a pull request.
18#[derive(Debug, serde::Serialize)]
19pub struct PrCreateResult {
20    /// PR number.
21    pub pr_number: u64,
22    /// PR URL.
23    pub url: String,
24    /// Head branch.
25    pub branch: String,
26    /// Base branch.
27    pub base: String,
28    /// PR title.
29    pub title: String,
30    /// Whether the PR is a draft.
31    pub draft: bool,
32    /// Number of files changed.
33    pub files_changed: u32,
34    /// Number of additions.
35    pub additions: u64,
36    /// Number of deletions.
37    pub deletions: u64,
38}
39
40/// Parses a PR reference into (owner, repo, number).
41///
42/// Supports multiple formats:
43/// - Full URL: `https://github.com/owner/repo/pull/123`
44/// - Short form: `owner/repo#123`
45/// - Bare number: `123` (requires `repo_context`)
46///
47/// # Arguments
48///
49/// * `reference` - PR reference string
50/// * `repo_context` - Optional repository context for bare numbers (e.g., "owner/repo")
51///
52/// # Returns
53///
54/// Tuple of (owner, repo, number)
55///
56/// # Errors
57///
58/// Returns an error if the reference format is invalid or `repo_context` is missing for bare numbers.
59pub fn parse_pr_reference(
60    reference: &str,
61    repo_context: Option<&str>,
62) -> Result<(String, String, u64)> {
63    parse_github_reference(ReferenceKind::Pull, reference, repo_context)
64}
65
66/// Fetches PR details including file diffs from GitHub.
67///
68/// Uses Octocrab to fetch PR metadata and file changes.
69///
70/// # Arguments
71///
72/// * `client` - Authenticated Octocrab client
73/// * `owner` - Repository owner
74/// * `repo` - Repository name
75/// * `number` - PR number
76///
77/// # Returns
78///
79/// `PrDetails` struct with PR metadata and file diffs.
80///
81/// # Errors
82///
83/// Returns an error if the API call fails or PR is not found.
84#[instrument(skip(client), fields(owner = %owner, repo = %repo, number = number))]
85#[allow(clippy::too_many_lines)]
86pub async fn fetch_pr_details(
87    client: &Octocrab,
88    owner: &str,
89    repo: &str,
90    number: u64,
91    review_config: &crate::config::ReviewConfig,
92) -> Result<PrDetails> {
93    debug!("Fetching PR details");
94
95    // Fetch PR metadata
96    let pr = match client.pulls(owner, repo).get(number).await {
97        Ok(pr) => pr,
98        Err(e) => {
99            // Check if this is a 404 error and if an issue exists instead
100            if let octocrab::Error::GitHub { source, .. } = &e
101                && source.status_code == 404
102            {
103                // Try to fetch as an issue to provide a better error message
104                if (client.issues(owner, repo).get(number).await).is_ok() {
105                    return Err(AptuError::TypeMismatch {
106                        number,
107                        expected: ResourceType::PullRequest,
108                        actual: ResourceType::Issue,
109                    }
110                    .into());
111                }
112                // Issue check failed, fall back to original error
113            }
114            return Err(e)
115                .with_context(|| format!("Failed to fetch PR #{number} from {owner}/{repo}"));
116        }
117    };
118
119    // Fetch PR files (diffs) with pagination (per_page=100, max 300 files)
120    let mut pr_files: Vec<PrFile> = Vec::new();
121    let mut page = client
122        .pulls(owner, repo)
123        .list_files(number)
124        .await
125        .with_context(|| format!("Failed to fetch files for PR #{number}"))?;
126
127    loop {
128        pr_files.extend(page.items.into_iter().map(|f| PrFile {
129            filename: f.filename,
130            status: format!("{:?}", f.status),
131            additions: f.additions,
132            deletions: f.deletions,
133            patch: f.patch,
134            patch_truncated: false,
135            full_content: None,
136        }));
137
138        if pr_files.len() >= 300 {
139            tracing::warn!(
140                "PR #{} has reached 300-file cap; stopping pagination",
141                number
142            );
143            pr_files.truncate(300);
144            break;
145        }
146
147        match client
148            .get_page::<octocrab::models::repos::DiffEntry>(&page.next)
149            .await
150        {
151            Ok(Some(next_page)) => page = next_page,
152            Ok(None) => break,
153            Err(e) => {
154                tracing::warn!("Error fetching next page of files: {}", e);
155                break;
156            }
157        }
158    }
159
160    // Detect truncated patches and attempt Contents API fallback
161    for file in &mut pr_files {
162        #[allow(clippy::collapsible_if)]
163        if let Some(patch) = &file.patch {
164            if is_patch_truncated(patch) {
165                file.patch_truncated = true;
166                // Attempt Contents API fallback
167                if let Ok(Some(content)) = fetch_file_contents_single(
168                    client,
169                    owner,
170                    repo,
171                    &file.filename,
172                    &pr.head.sha,
173                    review_config.max_chars_per_file,
174                )
175                .await
176                {
177                    file.patch = Some(content);
178                }
179            }
180        }
181    }
182
183    // Fetch full file contents for eligible files (default: up to 10 files, max 4000 chars each)
184    let file_contents = fetch_file_contents(
185        client,
186        owner,
187        repo,
188        &pr_files,
189        &pr.head.sha,
190        review_config.max_full_content_files,
191        review_config.max_chars_per_file,
192    )
193    .await;
194
195    // Merge file contents back into pr_files
196    debug_assert_eq!(
197        pr_files.len(),
198        file_contents.len(),
199        "fetch_file_contents must return one entry per file"
200    );
201    let pr_files: Vec<PrFile> = pr_files
202        .into_iter()
203        .zip(file_contents)
204        .map(|(mut file, content)| {
205            file.full_content = content;
206            file
207        })
208        .collect();
209
210    let labels: Vec<String> = pr.labels.iter().map(|l| l.name.clone()).collect();
211
212    let details = PrDetails {
213        owner: owner.to_string(),
214        repo: repo.to_string(),
215        number,
216        title: pr.title.clone(),
217        body: pr.body.clone().unwrap_or_default(),
218        base_branch: pr.base.ref_field,
219        head_branch: pr.head.ref_field,
220        head_sha: pr.head.sha,
221        files: pr_files,
222        url: pr.html_url.to_string(),
223        labels,
224        review_comments: Vec::new(),
225        instructions: None,
226    };
227
228    debug!(
229        file_count = details.files.len(),
230        "PR details fetched successfully"
231    );
232
233    Ok(details)
234}
235
236/// Detects if a patch is truncated mid-hunk by GitHub API.
237///
238/// A patch is considered truncated if the last non-empty line starts with '+' or '-',
239/// indicating an incomplete hunk.
240fn is_patch_truncated(patch: &str) -> bool {
241    let lines: Vec<&str> = patch.lines().collect();
242
243    // Rule 1: Check if last non-empty line starts with '+' or '-' (mid-hunk cutoff)
244    if let Some(last_line) = lines.iter().rev().find(|line| !line.trim().is_empty())
245        && (last_line.starts_with('+') || last_line.starts_with('-'))
246    {
247        return true;
248    }
249
250    // Rule 2: Check if declared hunk size matches actual lines delivered
251    // Parse the last @@ -a,b +c,d @@ header and verify line count
252    if let Some(last_hunk_header) = lines.iter().rev().find(|line| line.contains("@@")) {
253        // Extract the +c,d part from the hunk header
254        if let Some(plus_part) = last_hunk_header.split('+').nth(1) {
255            // Extract the number after '+' and before the next space or @@
256            if let Some(size_str) = plus_part.split_whitespace().next() {
257                // Parse "c,d" format
258                if let Some(count_str) = size_str.split(',').nth(1)
259                    && let Ok(declared_count) = count_str.parse::<usize>()
260                {
261                    // Count actual lines after this hunk header (context + added lines)
262                    // Find the index of this hunk header
263                    if let Some(hunk_idx) = lines.iter().position(|&line| line == *last_hunk_header)
264                    {
265                        let lines_after_hunk = &lines[hunk_idx + 1..];
266                        // Count lines that are context (' '), additions ('+'), or deletions ('-')
267                        // Stop counting if we hit another hunk header
268                        let mut actual_count = 0;
269                        for line in lines_after_hunk {
270                            if line.starts_with("@@") {
271                                break;
272                            }
273                            if line.starts_with(' ')
274                                || line.starts_with('+')
275                                || line.starts_with('-')
276                            {
277                                actual_count += 1;
278                            }
279                        }
280                        // If actual count is less than declared, the hunk is truncated
281                        if actual_count < declared_count {
282                            return true;
283                        }
284                    }
285                }
286            }
287        }
288    }
289
290    false
291}
292
293/// Fetches a single file's content from GitHub Contents API as a fallback for truncated patches.
294///
295/// Returns the file content truncated to `max_chars`, or `None` if the file cannot be fetched.
296/// Non-fatal errors (404, rate limits) are logged as warnings.
297async fn fetch_file_contents_single(
298    client: &Octocrab,
299    owner: &str,
300    repo: &str,
301    filename: &str,
302    head_sha: &str,
303    max_chars: usize,
304) -> Result<Option<String>> {
305    match client
306        .repos(owner, repo)
307        .get_content()
308        .path(filename)
309        .r#ref(head_sha)
310        .send()
311        .await
312    {
313        Ok(content) => {
314            // Try to decode the first item (should be the file, not a directory listing)
315            if let Some(item) = content.items.first() {
316                if let Some(decoded) = item.decoded_content() {
317                    let truncated = if decoded.len() > max_chars {
318                        decoded.chars().take(max_chars).collect::<String>()
319                    } else {
320                        decoded
321                    };
322                    Ok(Some(truncated))
323                } else {
324                    tracing::warn!(
325                        "Failed to decode content for {}/{}/{} at {}",
326                        owner,
327                        repo,
328                        filename,
329                        head_sha
330                    );
331                    Ok(None)
332                }
333            } else {
334                tracing::warn!(
335                    "File content response was empty for {}/{}/{} at {}",
336                    owner,
337                    repo,
338                    filename,
339                    head_sha
340                );
341                Ok(None)
342            }
343        }
344        Err(e) => {
345            tracing::warn!(
346                "Failed to fetch content for {}/{}/{} at {}: {}",
347                owner,
348                repo,
349                filename,
350                head_sha,
351                e
352            );
353            Ok(None)
354        }
355    }
356}
357
358/// Fetches full file contents for PR files from GitHub Contents API.
359///
360/// Fetches content for eligible files up to a specified limit and truncates each to a character limit.
361/// Skips deleted files and files with empty patches. Per-file errors are non-fatal: they produce
362/// `None` entries and log warnings.
363///
364/// # Arguments
365///
366/// * `client` - Authenticated Octocrab client
367/// * `owner` - Repository owner
368/// * `repo` - Repository name
369/// * `files` - Slice of PR files to fetch
370/// * `head_sha` - PR head commit SHA to fetch from
371/// * `max_files` - Maximum number of files to fetch content for
372/// * `max_chars_per_file` - Truncate each file's content at this character limit
373///
374/// # Returns
375///
376/// Vector of `Option<String>` with one entry per input file (in order):
377/// - `Some(content)` if fetch succeeded
378/// - `None` if fetch failed, file was skipped, or file index exceeded `max_files`
379#[instrument(skip(client, files), fields(owner = %owner, repo = %repo, max_files = max_files))]
380async fn fetch_file_contents(
381    client: &Octocrab,
382    owner: &str,
383    repo: &str,
384    files: &[PrFile],
385    head_sha: &str,
386    max_files: usize,
387    max_chars_per_file: usize,
388) -> Vec<Option<String>> {
389    let mut results = Vec::with_capacity(files.len());
390    let mut fetched_count = 0usize;
391
392    for file in files {
393        if should_skip_file(&file.filename, &file.status, file.patch.as_ref()) {
394            results.push(None);
395            continue;
396        }
397
398        // Skip if beyond max_files cap (count only successfully-fetched files)
399        if fetched_count >= max_files {
400            debug!(
401                file = %file.filename,
402                fetched_count = fetched_count,
403                max_files = max_files,
404                "Fetched file count exceeds max_files cap"
405            );
406            results.push(None);
407            continue;
408        }
409
410        // Attempt to fetch file content
411        match client
412            .repos(owner, repo)
413            .get_content()
414            .path(&file.filename)
415            .r#ref(head_sha)
416            .send()
417            .await
418        {
419            Ok(content) => {
420                // Try to decode the first item (should be the file, not a directory listing)
421                if let Some(item) = content.items.first() {
422                    if let Some(decoded) = item.decoded_content() {
423                        let truncated = if decoded.len() > max_chars_per_file {
424                            decoded.chars().take(max_chars_per_file).collect::<String>()
425                        } else {
426                            decoded
427                        };
428                        debug!(
429                            file = %file.filename,
430                            content_len = truncated.len(),
431                            "File content fetched and truncated"
432                        );
433                        results.push(Some(truncated));
434                        fetched_count += 1;
435                    } else {
436                        tracing::warn!(
437                            file = %file.filename,
438                            "Failed to decode file content; skipping"
439                        );
440                        results.push(None);
441                    }
442                } else {
443                    tracing::warn!(
444                        file = %file.filename,
445                        "File content response was empty; skipping"
446                    );
447                    results.push(None);
448                }
449            }
450            Err(e) => {
451                tracing::warn!(
452                    file = %file.filename,
453                    err = %e,
454                    "Failed to fetch file content; skipping"
455                );
456                results.push(None);
457            }
458        }
459    }
460
461    results
462}
463
464/// Posts a PR review to GitHub.
465///
466/// Uses Octocrab's custom HTTP POST to create a review with the specified event type.
467/// Requires write access to the repository.
468///
469/// # Arguments
470///
471/// * `client` - Authenticated Octocrab client
472/// * `owner` - Repository owner
473/// * `repo` - Repository name
474/// * `number` - PR number
475/// * `body` - Review comment text
476/// * `event` - Review event type (Comment, Approve, or `RequestChanges`)
477/// * `comments` - Inline review comments to attach; entries with `line = None` are silently skipped
478/// * `commit_id` - Head commit SHA to associate with the review; omitted from payload if empty
479///
480/// # Returns
481///
482/// Review ID on success.
483///
484/// # Errors
485///
486/// Returns an error if the API call fails, user lacks write access, or PR is not found.
487#[allow(clippy::too_many_arguments)]
488#[instrument(skip(client, comments), fields(owner = %owner, repo = %repo, number = number, event = %event))]
489pub async fn post_pr_review(
490    client: &Octocrab,
491    owner: &str,
492    repo: &str,
493    number: u64,
494    body: &str,
495    event: ReviewEvent,
496    comments: &[PrReviewComment],
497    commit_id: &str,
498) -> Result<u64> {
499    debug!("Posting PR review");
500
501    let route = format!("/repos/{owner}/{repo}/pulls/{number}/reviews");
502
503    // Build inline comments array; skip entries without a line number.
504    let inline_comments: Vec<serde_json::Value> = comments
505        .iter()
506        // Comments without a line number cannot be anchored to the diff; skip silently.
507        .filter_map(|c| {
508            c.line.map(|line| {
509                serde_json::json!({
510                    "path": c.file,
511                    "line": line,
512                    // RIGHT = new version of the file (added/changed lines).
513                    // Use line (file line number) rather than the deprecated
514                    // position (diff hunk offset) so no hunk parsing is needed.
515                    "side": "RIGHT",
516                    "body": render_pr_review_comment_body(c),
517                })
518            })
519        })
520        .collect();
521
522    let mut payload = serde_json::json!({
523        "body": body,
524        "event": event.to_string(),
525        "comments": inline_comments,
526    });
527
528    // commit_id is optional; include only when non-empty.
529    if !commit_id.is_empty() {
530        payload["commit_id"] = serde_json::Value::String(commit_id.to_string());
531    }
532
533    #[derive(serde::Deserialize)]
534    struct ReviewResponse {
535        id: u64,
536    }
537
538    let response: ReviewResponse = client.post(route, Some(&payload)).await.with_context(|| {
539        format!(
540            "Failed to post review to PR #{number} in {owner}/{repo}. \
541                 Check that you have write access to the repository."
542        )
543    })?;
544
545    debug!(review_id = response.id, "PR review posted successfully");
546
547    Ok(response.id)
548}
549
550/// Deletes a PR review comment.
551///
552/// # Errors
553///
554/// Returns an error if the API request fails. 404 errors (comment not found)
555/// are treated as success (idempotent).
556#[instrument(skip(client), fields(owner = %owner, repo = %repo, comment_id = comment_id))]
557pub async fn delete_pr_review_comment(
558    client: &Octocrab,
559    owner: &str,
560    repo: &str,
561    comment_id: u64,
562) -> Result<()> {
563    debug!("Deleting PR review comment");
564
565    let route = format!("/repos/{owner}/{repo}/pulls/comments/{comment_id}");
566
567    // Use generic delete method; needs explicit empty object body type
568    let empty_body = serde_json::json!({});
569    let result: std::result::Result<serde_json::Value, _> =
570        client.delete(&route, Some(&empty_body)).await;
571
572    match result {
573        Ok(_) => {
574            debug!("PR review comment deleted successfully");
575            Ok(())
576        }
577        Err(e)
578            if let octocrab::Error::GitHub { source, .. } = &e
579                && source.status_code.as_u16() == 404 =>
580        {
581            debug!("PR review comment already deleted (404); treating as success");
582            Ok(())
583        }
584        Err(e) => {
585            Err(e).with_context(|| format!("Failed to delete PR review comment #{comment_id}"))
586        }
587    }
588}
589
590/// Extract labels from PR metadata (title and file paths).
591///
592/// Parses conventional commit prefix from PR title and maps file paths to scope labels.
593/// Returns a vector of label names to apply to the PR.
594///
595/// # Arguments
596/// * `title` - PR title (may contain conventional commit prefix)
597/// * `file_paths` - List of file paths changed in the PR
598///
599/// # Returns
600/// Vector of label names to apply
601#[must_use]
602pub fn labels_from_pr_metadata(title: &str, file_paths: &[String]) -> Vec<String> {
603    let mut labels = std::collections::HashSet::new();
604
605    // Extract conventional commit prefix from title
606    // Handle both "feat: ..." and "feat(scope): ..." formats
607    let prefix = title
608        .split(':')
609        .next()
610        .unwrap_or("")
611        .split('(')
612        .next()
613        .unwrap_or("")
614        .trim();
615
616    // Map conventional commit type to label
617    let type_label = match prefix {
618        "feat" | "perf" => Some("enhancement"),
619        "fix" => Some("bug"),
620        "docs" => Some("documentation"),
621        "refactor" => Some("refactor"),
622        _ => None,
623    };
624
625    if let Some(label) = type_label {
626        labels.insert(label.to_string());
627    }
628
629    // Map file paths to scope labels
630    for path in file_paths {
631        let scope = if path.starts_with("crates/aptu-cli/") {
632            Some("cli")
633        } else if path.starts_with("docs/") {
634            Some("documentation")
635        } else {
636            None
637        };
638
639        if let Some(label) = scope {
640            labels.insert(label.to_string());
641        }
642    }
643
644    labels.into_iter().collect()
645}
646
647/// Creates a pull request on GitHub.
648///
649/// # Arguments
650///
651/// * `client` - Authenticated Octocrab client
652/// * `owner` - Repository owner
653/// * `repo` - Repository name
654/// * `title` - PR title
655/// * `head_branch` - Head branch (the branch with changes)
656/// * `base_branch` - Base branch (the branch to merge into)
657/// * `body` - Optional PR body text
658///
659/// # Returns
660///
661/// `PrCreateResult` with PR metadata.
662///
663/// # Errors
664///
665/// Returns an error if the API call fails or the user lacks write access.
666#[instrument(skip(client), fields(owner = %owner, repo = %repo, head = %head_branch, base = %base_branch))]
667#[allow(clippy::too_many_arguments)]
668pub async fn create_pull_request(
669    client: &Octocrab,
670    owner: &str,
671    repo: &str,
672    title: &str,
673    head_branch: &str,
674    base_branch: &str,
675    body: Option<&str>,
676    draft: bool,
677) -> anyhow::Result<PrCreateResult> {
678    debug!("Creating pull request");
679
680    let pr = client
681        .pulls(owner, repo)
682        .create(title, head_branch, base_branch)
683        .body(body.unwrap_or_default())
684        .draft(draft)
685        .send()
686        .await
687        .with_context(|| {
688            format!("Failed to create PR in {owner}/{repo} ({head_branch} -> {base_branch})")
689        })?;
690
691    let result = PrCreateResult {
692        pr_number: pr.number,
693        url: pr.html_url.to_string(),
694        branch: pr.head.ref_field,
695        base: pr.base.ref_field,
696        title: pr.title.clone(),
697        draft: pr.draft.unwrap_or(false),
698        files_changed: u32::try_from(pr.changed_files).unwrap_or(u32::MAX),
699        additions: pr.additions,
700        deletions: pr.deletions,
701    };
702
703    debug!(
704        pr_number = result.pr_number,
705        "Pull request created successfully"
706    );
707
708    Ok(result)
709}
710
711/// Determines whether a file should be skipped during fetch based on status and patch.
712/// Emits a debug log with the skip reason. Returns true if the file should be skipped
713/// (removed status or no patch), false otherwise.
714fn should_skip_file(filename: &str, status: &str, patch: Option<&String>) -> bool {
715    if status.to_lowercase().contains("removed") {
716        debug!(file = %filename, "Skipping removed file");
717        return true;
718    }
719    if patch.is_none_or(String::is_empty) {
720        debug!(file = %filename, "Skipping file with empty patch");
721        return true;
722    }
723    false
724}
725
726#[cfg(test)]
727mod tests {
728    use super::*;
729    use crate::ai::types::CommentSeverity;
730
731    fn decode_content(encoded: &str, max_chars: usize) -> Option<String> {
732        use base64::Engine;
733        let engine = base64::engine::general_purpose::STANDARD;
734        let decoded_bytes = engine.decode(encoded).ok()?;
735        let decoded_str = String::from_utf8(decoded_bytes).ok()?;
736
737        if decoded_str.len() <= max_chars {
738            Some(decoded_str)
739        } else {
740            Some(decoded_str.chars().take(max_chars).collect::<String>())
741        }
742    }
743
744    #[test]
745    fn test_pr_create_result_fields() {
746        // Arrange / Act: construct directly (no network call needed)
747        let result = PrCreateResult {
748            pr_number: 42,
749            url: "https://github.com/owner/repo/pull/42".to_string(),
750            branch: "feat/my-feature".to_string(),
751            base: "main".to_string(),
752            title: "feat: add feature".to_string(),
753            draft: false,
754            files_changed: 3,
755            additions: 100,
756            deletions: 10,
757        };
758
759        // Assert
760        assert_eq!(result.pr_number, 42);
761        assert_eq!(result.url, "https://github.com/owner/repo/pull/42");
762        assert_eq!(result.branch, "feat/my-feature");
763        assert_eq!(result.base, "main");
764        assert_eq!(result.title, "feat: add feature");
765        assert!(!result.draft);
766        assert_eq!(result.files_changed, 3);
767        assert_eq!(result.additions, 100);
768        assert_eq!(result.deletions, 10);
769    }
770
771    // ---------------------------------------------------------------------------
772    // post_pr_review payload construction
773    // ---------------------------------------------------------------------------
774
775    /// Helper: build the inline comments JSON array using the same logic as
776    /// `post_pr_review`, without making a live HTTP call.
777    fn build_inline_comments(comments: &[PrReviewComment]) -> Vec<serde_json::Value> {
778        comments
779            .iter()
780            .filter_map(|c| {
781                c.line.map(|line| {
782                    serde_json::json!({
783                        "path": c.file,
784                        "line": line,
785                        "side": "RIGHT",
786                        "body": render_pr_review_comment_body(c),
787                    })
788                })
789            })
790            .collect()
791    }
792
793    #[test]
794    fn test_post_pr_review_payload_with_comments() {
795        // Arrange
796        let comments = vec![PrReviewComment {
797            file: "src/main.rs".to_string(),
798            line: Some(42),
799            comment: "Consider using a match here.".to_string(),
800            severity: CommentSeverity::Suggestion,
801            suggested_code: None,
802        }];
803
804        // Act
805        let inline = build_inline_comments(&comments);
806
807        // Assert
808        assert_eq!(inline.len(), 1);
809        assert_eq!(inline[0]["path"], "src/main.rs");
810        assert_eq!(inline[0]["line"], 42);
811        assert_eq!(inline[0]["side"], "RIGHT");
812        assert_eq!(inline[0]["body"], "Consider using a match here.");
813    }
814
815    #[test]
816    fn test_post_pr_review_skips_none_line_comments() {
817        // Arrange: one comment with a line, one without.
818        let comments = vec![
819            PrReviewComment {
820                file: "src/lib.rs".to_string(),
821                line: None,
822                comment: "General file comment.".to_string(),
823                severity: CommentSeverity::Info,
824                suggested_code: None,
825            },
826            PrReviewComment {
827                file: "src/lib.rs".to_string(),
828                line: Some(10),
829                comment: "Inline comment.".to_string(),
830                severity: CommentSeverity::Warning,
831                suggested_code: None,
832            },
833        ];
834
835        // Act
836        let inline = build_inline_comments(&comments);
837
838        // Assert: only the comment with a line is included.
839        assert_eq!(inline.len(), 1);
840        assert_eq!(inline[0]["line"], 10);
841    }
842
843    #[test]
844    fn test_post_pr_review_empty_comments() {
845        // Arrange
846        let comments: Vec<PrReviewComment> = vec![];
847
848        // Act
849        let inline = build_inline_comments(&comments);
850
851        // Assert: empty slice produces empty array, which serializes as [].
852        assert!(inline.is_empty());
853        let serialized = serde_json::to_string(&inline).unwrap();
854        assert_eq!(serialized, "[]");
855    }
856
857    // ---------------------------------------------------------------------------
858    // Existing tests
859    // ---------------------------------------------------------------------------
860
861    // Smoke test to verify parse_pr_reference delegates correctly.
862    // Comprehensive parsing tests are in github/mod.rs.
863    #[test]
864    fn test_parse_pr_reference_delegates_to_shared() {
865        let (owner, repo, number) =
866            parse_pr_reference("https://github.com/block/goose/pull/123", None).unwrap();
867        assert_eq!(owner, "block");
868        assert_eq!(repo, "goose");
869        assert_eq!(number, 123);
870    }
871
872    #[test]
873    fn test_title_prefix_to_label_mapping() {
874        let cases = vec![
875            (
876                "feat: add new feature",
877                vec!["enhancement"],
878                "feat should map to enhancement",
879            ),
880            ("fix: resolve bug", vec!["bug"], "fix should map to bug"),
881            (
882                "docs: update readme",
883                vec!["documentation"],
884                "docs should map to documentation",
885            ),
886            (
887                "refactor: improve code",
888                vec!["refactor"],
889                "refactor should map to refactor",
890            ),
891            (
892                "perf: optimize",
893                vec!["enhancement"],
894                "perf should map to enhancement",
895            ),
896            (
897                "chore: update deps",
898                vec![],
899                "chore should produce no labels",
900            ),
901        ];
902
903        for (title, expected_labels, msg) in cases {
904            let labels = labels_from_pr_metadata(title, &[]);
905            for expected in &expected_labels {
906                assert!(
907                    labels.contains(&expected.to_string()),
908                    "{msg}: expected '{expected}' in {labels:?}",
909                );
910            }
911            if expected_labels.is_empty() {
912                assert!(labels.is_empty(), "{msg}: expected empty, got {labels:?}");
913            }
914        }
915    }
916
917    #[test]
918    fn test_file_path_to_scope_mapping() {
919        let cases = vec![
920            (
921                "feat: cli",
922                vec!["crates/aptu-cli/src/main.rs"],
923                vec!["enhancement", "cli"],
924                "cli path should map to cli scope",
925            ),
926            (
927                "feat: docs",
928                vec!["docs/GITHUB_ACTION.md"],
929                vec!["enhancement", "documentation"],
930                "docs path should map to documentation scope",
931            ),
932            (
933                "feat: workflow",
934                vec![".github/workflows/test.yml"],
935                vec!["enhancement"],
936                "workflow path should be ignored",
937            ),
938        ];
939
940        for (title, paths, expected_labels, msg) in cases {
941            let labels = labels_from_pr_metadata(
942                title,
943                &paths
944                    .iter()
945                    .map(std::string::ToString::to_string)
946                    .collect::<Vec<_>>(),
947            );
948            for expected in expected_labels {
949                assert!(
950                    labels.contains(&expected.to_string()),
951                    "{msg}: expected '{expected}' in {labels:?}",
952                );
953            }
954        }
955    }
956
957    #[test]
958    fn test_combined_title_and_paths() {
959        let labels = labels_from_pr_metadata(
960            "feat: multi",
961            &[
962                "crates/aptu-cli/src/main.rs".to_string(),
963                "docs/README.md".to_string(),
964            ],
965        );
966        assert!(
967            labels.contains(&"enhancement".to_string()),
968            "should include enhancement from feat prefix"
969        );
970        assert!(
971            labels.contains(&"cli".to_string()),
972            "should include cli from path"
973        );
974        assert!(
975            labels.contains(&"documentation".to_string()),
976            "should include documentation from path"
977        );
978    }
979
980    #[test]
981    fn test_no_match_returns_empty() {
982        let cases = vec![
983            (
984                "Random title",
985                vec![],
986                "unrecognized prefix should return empty",
987            ),
988            (
989                "chore: update",
990                vec![],
991                "ignored prefix should return empty",
992            ),
993        ];
994
995        for (title, paths, msg) in cases {
996            let labels = labels_from_pr_metadata(title, &paths);
997            assert!(labels.is_empty(), "{msg}: got {labels:?}");
998        }
999    }
1000
1001    #[test]
1002    fn test_scoped_prefix_extracts_type() {
1003        let labels = labels_from_pr_metadata("feat(cli): add new feature", &[]);
1004        assert!(
1005            labels.contains(&"enhancement".to_string()),
1006            "scoped prefix should extract type from feat(cli)"
1007        );
1008    }
1009
1010    #[test]
1011    fn test_duplicate_labels_deduplicated() {
1012        let labels = labels_from_pr_metadata("docs: update", &["docs/README.md".to_string()]);
1013        assert_eq!(
1014            labels.len(),
1015            1,
1016            "should have exactly one label when title and path both map to documentation"
1017        );
1018        assert!(
1019            labels.contains(&"documentation".to_string()),
1020            "should contain documentation label"
1021        );
1022    }
1023
1024    #[test]
1025    fn test_should_skip_file_respects_fetched_count_cap() {
1026        // Test that should_skip_file correctly identifies files to skip.
1027        // Files with removed status or no patch should be skipped.
1028        let removed_file = PrFile {
1029            filename: "removed.rs".to_string(),
1030            status: "removed".to_string(),
1031            additions: 0,
1032            deletions: 5,
1033            patch: None,
1034            patch_truncated: false,
1035            full_content: None,
1036        };
1037        let modified_file = PrFile {
1038            filename: "file_0.rs".to_string(),
1039            status: "modified".to_string(),
1040            additions: 1,
1041            deletions: 0,
1042            patch: Some("+ new code".to_string()),
1043            patch_truncated: false,
1044            full_content: None,
1045        };
1046        let no_patch_file = PrFile {
1047            filename: "file_1.rs".to_string(),
1048            status: "modified".to_string(),
1049            additions: 1,
1050            deletions: 0,
1051            patch: None,
1052            patch_truncated: false,
1053            full_content: None,
1054        };
1055
1056        // Assert: removed files are skipped
1057        assert!(
1058            should_skip_file(
1059                &removed_file.filename,
1060                &removed_file.status,
1061                removed_file.patch.as_ref()
1062            ),
1063            "removed files should be skipped"
1064        );
1065
1066        // Assert: modified files with patch are not skipped
1067        assert!(
1068            !should_skip_file(
1069                &modified_file.filename,
1070                &modified_file.status,
1071                modified_file.patch.as_ref()
1072            ),
1073            "modified files with patch should not be skipped"
1074        );
1075
1076        // Assert: files without patch are skipped
1077        assert!(
1078            should_skip_file(
1079                &no_patch_file.filename,
1080                &no_patch_file.status,
1081                no_patch_file.patch.as_ref()
1082            ),
1083            "files without patch should be skipped"
1084        );
1085    }
1086
1087    #[test]
1088    fn test_decode_content_valid_base64() {
1089        // Arrange: valid base64-encoded string
1090        use base64::Engine;
1091        let engine = base64::engine::general_purpose::STANDARD;
1092        let original = "Hello, World!";
1093        let encoded = engine.encode(original);
1094
1095        // Act: decode with sufficient max_chars
1096        let result = decode_content(&encoded, 1000);
1097
1098        // Assert: decoding succeeds and matches original
1099        assert_eq!(
1100            result,
1101            Some(original.to_string()),
1102            "valid base64 should decode successfully"
1103        );
1104    }
1105
1106    #[test]
1107    fn test_decode_content_invalid_base64() {
1108        // Arrange: invalid base64 string
1109        let invalid_base64 = "!!!invalid!!!";
1110
1111        // Act: attempt to decode
1112        let result = decode_content(invalid_base64, 1000);
1113
1114        // Assert: decoding fails gracefully
1115        assert_eq!(result, None, "invalid base64 should return None");
1116    }
1117
1118    #[test]
1119    fn test_decode_content_truncates_at_max_chars() {
1120        // Arrange: multi-byte UTF-8 string (Japanese characters)
1121        use base64::Engine;
1122        let engine = base64::engine::general_purpose::STANDARD;
1123        let original = "こんにちは".repeat(10); // 50 characters total
1124        let encoded = engine.encode(&original);
1125        let max_chars = 10;
1126
1127        // Act: decode with max_chars limit
1128        let result = decode_content(&encoded, max_chars);
1129
1130        // Assert: result is truncated to max_chars on character boundary
1131        assert!(result.is_some(), "decoding should succeed");
1132        let decoded = result.unwrap();
1133        assert_eq!(
1134            decoded.chars().count(),
1135            max_chars,
1136            "output should be truncated to max_chars on character boundary"
1137        );
1138        assert!(
1139            decoded.is_char_boundary(decoded.len()),
1140            "output should be valid UTF-8 (truncated on char boundary)"
1141        );
1142    }
1143
1144    #[test]
1145    fn test_list_files_pagination_collects_all_pages() {
1146        // Arrange: simulate pagination with two pages
1147        // Page 1: 100 items with next_link set
1148        let mut page1_items = Vec::new();
1149        for i in 0..100 {
1150            page1_items.push(PrFile {
1151                filename: format!("file{}.rs", i),
1152                status: "modified".to_string(),
1153                additions: 1,
1154                deletions: 0,
1155                patch: Some("@@ -1,1 +1,1 @@\n-old\n+new".to_string()),
1156                patch_truncated: false,
1157                full_content: None,
1158            });
1159        }
1160
1161        // Page 2: 50 items with no next_link
1162        let mut page2_items = Vec::new();
1163        for i in 100..150 {
1164            page2_items.push(PrFile {
1165                filename: format!("file{}.rs", i),
1166                status: "modified".to_string(),
1167                additions: 1,
1168                deletions: 0,
1169                patch: Some("@@ -1,1 +1,1 @@\n-old\n+new".to_string()),
1170                patch_truncated: false,
1171                full_content: None,
1172            });
1173        }
1174
1175        // Act: collect all items (simulating pagination loop)
1176        let mut all_files = Vec::new();
1177        all_files.extend(page1_items);
1178        all_files.extend(page2_items);
1179
1180        // Assert: total collected == 150
1181        assert_eq!(
1182            all_files.len(),
1183            150,
1184            "pagination should collect all items from both pages"
1185        );
1186    }
1187
1188    #[test]
1189    fn test_list_files_pagination_respects_300_file_cap() {
1190        // Arrange: build a Vec of 301 PrFile items
1191        let mut files = Vec::new();
1192        for i in 0..301 {
1193            files.push(PrFile {
1194                filename: format!("file{}.rs", i),
1195                status: "modified".to_string(),
1196                additions: 1,
1197                deletions: 0,
1198                patch: Some("@@ -1,1 +1,1 @@\n-old\n+new".to_string()),
1199                patch_truncated: false,
1200                full_content: None,
1201            });
1202        }
1203
1204        // Act: apply the 300-file cap (simulating the truncate logic)
1205        if files.len() >= 300 {
1206            files.truncate(300);
1207        }
1208
1209        // Assert: result.len() == 300
1210        assert_eq!(files.len(), 300, "pagination should enforce 300-file cap");
1211    }
1212
1213    #[test]
1214    fn test_is_patch_truncated_detects_mid_hunk_plus() {
1215        // Test: patch ending with '+' (mid-hunk truncation)
1216        let truncated_patch = "@@ -1,3 +1,4 @@\n line1\n line2\n+";
1217        assert!(
1218            is_patch_truncated(truncated_patch),
1219            "patch ending with + should be detected as truncated"
1220        );
1221    }
1222
1223    #[test]
1224    fn test_is_patch_truncated_detects_mid_hunk_minus() {
1225        // Test: patch ending with '-' (mid-hunk truncation)
1226        let truncated_patch = "@@ -1,3 +1,4 @@\n line1\n line2\n-";
1227        assert!(
1228            is_patch_truncated(truncated_patch),
1229            "patch ending with - should be detected as truncated"
1230        );
1231    }
1232
1233    #[test]
1234    fn test_is_patch_truncated_clean_patch_context_line() {
1235        // Test: patch ending with ' ' (context line, not truncated)
1236        let clean_patch = "@@ -1,3 +1,3 @@\n line1\n line2\n line3";
1237        assert!(
1238            !is_patch_truncated(clean_patch),
1239            "patch ending with context line should not be detected as truncated"
1240        );
1241    }
1242
1243    #[test]
1244    fn test_is_patch_truncated_correct_hunk_line_count() {
1245        // Test: patch with correct hunk line count (declared 3, actual 3)
1246        let clean_patch = "@@ -1,3 +1,3 @@\n line1\n line2\n line3";
1247        assert!(
1248            !is_patch_truncated(clean_patch),
1249            "patch with correct hunk line count should not be detected as truncated"
1250        );
1251    }
1252
1253    #[test]
1254    fn test_is_patch_truncated_declared_hunk_size_larger_than_delivered() {
1255        // Test: patch with declared hunk size larger than delivered lines
1256        // Declared: +1,4 (4 lines in new file), Actual: only 2 lines delivered
1257        let truncated_patch = "@@ -1,3 +1,4 @@\n line1\n line2";
1258        assert!(
1259            is_patch_truncated(truncated_patch),
1260            "patch with declared hunk size larger than delivered should be detected as truncated"
1261        );
1262    }
1263
1264    #[test]
1265    fn test_is_patch_truncated_no_hunk_header_but_last_line_plus() {
1266        // Test: patch with no @@ header but last line is '+'
1267        let truncated_patch = "line1\nline2\n+";
1268        assert!(
1269            is_patch_truncated(truncated_patch),
1270            "patch with no @@ header but ending with + should be detected as truncated"
1271        );
1272    }
1273
1274    #[test]
1275    fn test_is_patch_truncated_empty_patch() {
1276        // Test: empty patch
1277        let empty_patch = "";
1278        assert!(
1279            !is_patch_truncated(empty_patch),
1280            "empty patch should not be detected as truncated"
1281        );
1282    }
1283
1284    #[test]
1285    fn test_is_patch_truncated_multiple_hunks_last_hunk_truncated() {
1286        // Test: multiple hunks where the last hunk is truncated
1287        let truncated_patch = "@@ -1,2 +1,2 @@\n line1\n line2\n@@ -5,3 +5,4 @@\n line5\n line6";
1288        assert!(
1289            is_patch_truncated(truncated_patch),
1290            "patch with last hunk truncated should be detected as truncated"
1291        );
1292    }
1293
1294    #[test]
1295    fn test_fetch_file_contents_fallback_on_truncated_patch() {
1296        // Note: The Contents API network call cannot be unit-tested without a mock.
1297        // The fallback is exercised in integration tests via the full fetch_pr_details flow.
1298        // Unit tests for is_patch_truncated are above.
1299    }
1300}