diff --git a/CHANGELOG.md b/CHANGELOG.md index 438b28591..789bec5a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ All notable changes to rtk (Rust Token Killer) will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Bug Fixes + +* **diff:** correct truncation overflow count in condense_unified_diff ([#833](https://github.com/rtk-ai/rtk/pull/833)) ([5399f83](https://github.com/rtk-ai/rtk/commit/5399f83)) +* **git:** replace vague truncation markers with exact counts in log and grep output ([#833](https://github.com/rtk-ai/rtk/pull/833)) ([185fb97](https://github.com/rtk-ai/rtk/commit/185fb97)) + + ## [0.33.0-rc.54](https://github.com/rtk-ai/rtk/compare/v0.32.0-rc.54...v0.33.0-rc.54) (2026-03-24) diff --git a/src/diff_cmd.rs b/src/diff_cmd.rs index 15b433a8b..4da3f7664 100644 --- a/src/diff_cmd.rs +++ b/src/diff_cmd.rs @@ -168,6 +168,10 @@ fn condense_unified_diff(diff: &str) -> String { for c in &changes { result.push(format!(" {}", c)); } + let total = added + removed; + if total > 10 { + result.push(format!(" ... +{} more", total - 10)); + } } current_file = line .trim_start_matches("+++ ") @@ -192,6 +196,10 @@ fn condense_unified_diff(diff: &str) -> String { for c in &changes { result.push(format!(" {}", c)); } + let total = added + removed; + if total > 10 { + result.push(format!(" ... +{} more", total - 10)); + } } result.join("\n") @@ -334,9 +342,57 @@ diff --git a/b.rs b/b.rs assert!(result.is_empty()); } + // --- truncation accuracy --- + + fn make_large_unified_diff(added: usize, removed: usize) -> String { + let mut lines = vec![ + "diff --git a/config.yaml b/config.yaml".to_string(), + "--- a/config.yaml".to_string(), + "+++ b/config.yaml".to_string(), + "@@ -1,200 +1,200 @@".to_string(), + ]; + for i in 0..removed { + lines.push(format!("-old_value_{}", i)); + } + for i in 0..added { + lines.push(format!("+new_value_{}", i)); + } + lines.join("\n") + } + + #[test] + fn test_condense_unified_diff_overflow_count_accuracy() { + // 100 added + 100 removed = 200 total changes, only 10 shown + // True overflow = 200 - 10 = 190 + // Bug: changes vec capped at 15, so old code showed "+5 more" (15-10) instead of "+190 more" + let diff = make_large_unified_diff(100, 100); + let result = condense_unified_diff(&diff); + assert!( + result.contains("+190 more"), + "Expected '+190 more' but got:\n{}", + result + ); + assert!( + !result.contains("+5 more"), + "Bug still present: showing '+5 more' instead of true overflow" + ); + } + + #[test] + fn test_condense_unified_diff_no_false_overflow() { + // 8 changes total — all fit within the 10-line display cap, no overflow message + let diff = make_large_unified_diff(4, 4); + let result = condense_unified_diff(&diff); + assert!( + !result.contains("more"), + "No overflow message expected for 8 changes, got:\n{}", + result + ); + } + #[test] fn test_no_truncation_large_diff() { - // Verify all changes are shown, not truncated + // Verify compute_diff returns all changes without truncation let mut a = Vec::new(); let mut b = Vec::new(); for i in 0..500 { @@ -351,9 +407,11 @@ diff --git a/b.rs b/b.rs let b_refs: Vec<&str> = b.iter().map(|s| s.as_str()).collect(); let result = compute_diff(&a_refs, &b_refs); - // Should have ~167 changes (every 3rd line), all present - assert!(result.changes.len() > 100, "Expected 100+ changes, got {}", result.changes.len()); - // No truncation — changes count matches what we generate + assert!( + result.changes.len() > 100, + "Expected 100+ changes, got {}", + result.changes.len() + ); assert!(!result.changes.is_empty()); } @@ -363,7 +421,6 @@ diff --git a/b.rs b/b.rs let a = vec![long_line.as_str()]; let b = vec!["short"]; let result = compute_diff(&a, &b); - // The removed line should contain the full 500-char string match &result.changes[0] { DiffChange::Removed(_, content) | DiffChange::Added(_, content) => { assert_eq!(content.len(), 500, "Line was truncated!"); @@ -373,24 +430,4 @@ diff --git a/b.rs b/b.rs } } } - - #[test] - fn test_condense_unified_no_truncation() { - // Generate a large unified diff - let mut lines = Vec::new(); - lines.push("diff --git a/big.yaml b/big.yaml".to_string()); - lines.push("--- a/big.yaml".to_string()); - lines.push("+++ b/big.yaml".to_string()); - for i in 0..200 { - lines.push(format!("+added_line_{}", i)); - } - let diff = lines.join("\n"); - let result = condense_unified_diff(&diff); - - // All 200 added lines should be present - assert!(result.contains("added_line_0")); - assert!(result.contains("added_line_199")); - assert!(!result.contains("not shown"), "Should not truncate"); - assert!(!result.contains("more"), "Should not have '... more'"); - } } diff --git a/src/filter.rs b/src/filter.rs index d6d9d19bc..857ec9384 100644 --- a/src/filter.rs +++ b/src/filter.rs @@ -491,4 +491,49 @@ fn main() { assert!(!result.contains("// This is a comment")); assert!(result.contains("fn main()")); } + + // --- truncation accuracy --- + + #[test] + fn test_smart_truncate_overflow_count_exact() { + // 200 plain-text lines with max_lines=20. + // smart_truncate keeps the first max_lines/2=10 lines, then skips the rest. + // The overflow message "// ... N more lines (total: T)" must satisfy: + // kept_count + N == T + let total_lines = 200usize; + let max_lines = 20usize; + let content: String = (0..total_lines) + .map(|i| format!("plain text line number {}", i)) + .collect::>() + .join("\n"); + + let output = smart_truncate(&content, max_lines, &Language::Rust); + + // Extract the overflow message + let overflow_line = output + .lines() + .find(|l| l.contains("more lines")) + .unwrap_or_else(|| panic!("No overflow message found in:\n{}", output)); + + // Parse "// ... N more lines (total: T)" + let reported_more: usize = overflow_line + .split_whitespace() + .find(|w| w.parse::().is_ok()) + .and_then(|w| w.parse().ok()) + .unwrap_or_else(|| panic!("Could not parse overflow count from: {}", overflow_line)); + + let kept_count = output + .lines() + .filter(|l| !l.contains("more lines") && !l.contains("omitted")) + .count(); + + assert_eq!( + kept_count + reported_more, + total_lines, + "kept ({}) + reported_more ({}) must equal total ({})", + kept_count, + reported_more, + total_lines + ); + } } diff --git a/src/git.rs b/src/git.rs index 4bb7f6745..9b4b757ad 100644 --- a/src/git.rs +++ b/src/git.rs @@ -296,13 +296,19 @@ pub(crate) fn compact_diff(diff: &str, max_lines: usize) -> String { let mut added = 0; let mut removed = 0; let mut in_hunk = false; - let mut hunk_lines = 0; + let mut hunk_shown = 0; + let mut hunk_skipped = 0usize; let max_hunk_lines = 100; let mut was_truncated = false; for line in diff.lines() { if line.starts_with("diff --git") { - // New file + // Flush hunk truncation before starting a new file + if hunk_skipped > 0 { + result.push(format!(" ... ({} lines truncated)", hunk_skipped)); + was_truncated = true; + hunk_skipped = 0; + } if !current_file.is_empty() && (added > 0 || removed > 0) { result.push(format!(" +{} -{}", added, removed)); } @@ -311,38 +317,42 @@ pub(crate) fn compact_diff(diff: &str, max_lines: usize) -> String { added = 0; removed = 0; in_hunk = false; + hunk_shown = 0; } else if line.starts_with("@@") { - // New hunk + // Flush hunk truncation before starting a new hunk + if hunk_skipped > 0 { + result.push(format!(" ... ({} lines truncated)", hunk_skipped)); + was_truncated = true; + hunk_skipped = 0; + } in_hunk = true; - hunk_lines = 0; + hunk_shown = 0; let hunk_info = line.split("@@").nth(1).unwrap_or("").trim(); result.push(format!(" @@ {} @@", hunk_info)); } else if in_hunk { if line.starts_with('+') && !line.starts_with("+++") { added += 1; - if hunk_lines < max_hunk_lines { + if hunk_shown < max_hunk_lines { result.push(format!(" {}", line)); - hunk_lines += 1; + hunk_shown += 1; + } else { + hunk_skipped += 1; } } else if line.starts_with('-') && !line.starts_with("---") { removed += 1; - if hunk_lines < max_hunk_lines { + if hunk_shown < max_hunk_lines { result.push(format!(" {}", line)); - hunk_lines += 1; + hunk_shown += 1; + } else { + hunk_skipped += 1; } - } else if hunk_lines < max_hunk_lines && !line.starts_with("\\") { + } else if hunk_shown < max_hunk_lines && !line.starts_with("\\") { // Context line - if hunk_lines > 0 { + if hunk_shown > 0 { result.push(format!(" {}", line)); - hunk_lines += 1; + hunk_shown += 1; } } - - if hunk_lines == max_hunk_lines { - result.push(" ... (truncated)".to_string()); - hunk_lines += 1; - was_truncated = true; - } } if result.len() >= max_lines { @@ -352,6 +362,12 @@ pub(crate) fn compact_diff(diff: &str, max_lines: usize) -> String { } } + // Flush last hunk + if hunk_skipped > 0 { + result.push(format!(" ... ({} lines truncated)", hunk_skipped)); + was_truncated = true; + } + if !current_file.is_empty() && (added > 0 || removed > 0) { result.push(format!(" +{} -{}", added, removed)); } @@ -533,23 +549,27 @@ fn filter_log_output( None => continue, }; // Remaining lines are the body — keep up to 3 non-empty, non-trailer lines - let body_lines: Vec<&str> = lines + let all_body_lines: Vec<&str> = lines .map(|l| l.trim()) .filter(|l| { !l.is_empty() && !l.starts_with("Signed-off-by:") && !l.starts_with("Co-authored-by:") }) - .take(3) .collect(); + let body_omitted = all_body_lines.len().saturating_sub(3); + let body_lines = &all_body_lines[..all_body_lines.len().min(3)]; if body_lines.is_empty() { result.push(header); } else { let mut entry = header; - for body in &body_lines { + for body in body_lines { entry.push_str(&format!("\n {}", truncate_line(body, truncate_width))); } + if body_omitted > 0 { + entry.push_str(&format!("\n [+{} lines omitted]", body_omitted)); + } result.push(entry); } } @@ -2304,4 +2324,84 @@ no changes added to commit (use "git add" and/or "git commit -a") let _ = std::fs::remove_dir_all(&tmp); } + + // --- truncation accuracy --- + + #[test] + fn test_format_status_overflow_count_exact() { + // 25 staged files, default status_max_files = 15 + // Should show 15, overflow = 25 - 15 = 10, report "+10 more" + let mut porcelain = String::from("## main...origin/main\n"); + for i in 0..25 { + porcelain.push_str(&format!("M staged_file_{}.rs\n", i)); + } + let result = format_status_output(&porcelain); + assert!( + result.contains("+10 more"), + "Expected '+10 more' for 25 staged files (max_files=15), got:\n{}", + result + ); + assert!( + result.contains("Staged: 25 files"), + "Expected 'Staged: 25 files', got:\n{}", + result + ); + } + + #[test] + fn test_compact_diff_recovery_hint_present() { + // A hunk with 110 lines exceeds max_hunk_lines (100), triggers truncation + // The recovery hint must appear so LLMs can re-fetch the full diff + let mut diff = String::new(); + diff.push_str("diff --git a/large.rs b/large.rs\n"); + diff.push_str("--- a/large.rs\n"); + diff.push_str("+++ b/large.rs\n"); + diff.push_str("@@ -1,150 +1,150 @@\n"); + for i in 0..110 { + diff.push_str(&format!("+added line {}\n", i)); + } + let result = compact_diff(&diff, 500); + assert!( + result.contains("[full diff: rtk git diff --no-compact]"), + "Expected recovery hint when hunk is truncated, got:\n{}", + result + ); + } + + #[test] + fn test_compact_diff_hunk_truncation_count_accurate() { + // 150 change lines in one hunk: 100 shown, 50 silently dropped + // Must report the exact count, not just "(truncated)" + let mut diff = String::from( + "diff --git a/large.rs b/large.rs\n--- a/large.rs\n+++ b/large.rs\n@@ -1,150 +1,150 @@\n", + ); + for i in 0..150 { + diff.push_str(&format!("+line {}\n", i)); + } + let result = compact_diff(&diff, 500); + assert!( + result.contains("50 lines truncated"), + "Expected '50 lines truncated' (150 - 100 = 50), got:\n{}", + result + ); + } + + #[test] + fn test_filter_log_output_body_omission_indicator() { + // Commit with 6 meaningful body lines: only 3 shown, must signal "+3 lines omitted" + let body_lines = (1..=6) + .map(|i| format!("body line {}", i)) + .collect::>() + .join("\n"); + let output = format!( + "abc1234 feat: big change (1 day ago) \n{}\n---END---\n", + body_lines + ); + let result = filter_log_output(&output, 10, false, false); + assert!( + result.contains("+3 lines omitted"), + "Expected '+3 lines omitted' when 6 body lines truncated to 3, got:\n{}", + result + ); + } } diff --git a/src/grep_cmd.rs b/src/grep_cmd.rs index c1819dded..23592441a 100644 --- a/src/grep_cmd.rs +++ b/src/grep_cmd.rs @@ -280,6 +280,27 @@ mod tests { assert_eq!(filtered[0], "-i"); } + // --- truncation accuracy --- + + #[test] + fn test_grep_overflow_uses_uncapped_total() { + // Confirm the grep overflow invariant: matches vec is never capped before overflow calc. + // If total_matches > per_file, overflow = total_matches - per_file (not capped). + // This documents that grep_cmd.rs avoids the diff_cmd bug (cap at N then compute N-10). + let per_file = config::limits().grep_max_per_file; + let total_matches = per_file + 42; + let overflow = total_matches - per_file; + assert_eq!(overflow, 42, "overflow must equal true suppressed count"); + // Demonstrate why capping before subtraction is wrong: + let hypothetical_cap = per_file + 5; + let capped = total_matches.min(hypothetical_cap); + let wrong_overflow = capped - per_file; + assert_ne!( + wrong_overflow, overflow, + "capping before subtraction gives wrong overflow" + ); + } + // Verify line numbers are always enabled in rg invocation (grep_cmd.rs:24). // The -n/--line-numbers clap flag in main.rs is a no-op accepted for compat. #[test] diff --git a/src/init.rs b/src/init.rs index 5bf5071b6..53bbe70cb 100644 --- a/src/init.rs +++ b/src/init.rs @@ -2356,12 +2356,16 @@ pub fn run_copilot(verbose: u8) -> Result<()> { let github_dir = Path::new(".github"); let hooks_dir = github_dir.join("hooks"); - fs::create_dir_all(&hooks_dir) - .context("Failed to create .github/hooks/ directory")?; + fs::create_dir_all(&hooks_dir).context("Failed to create .github/hooks/ directory")?; // 1. Write hook config let hook_path = hooks_dir.join("rtk-rewrite.json"); - write_if_changed(&hook_path, COPILOT_HOOK_JSON, "Copilot hook config", verbose)?; + write_if_changed( + &hook_path, + COPILOT_HOOK_JSON, + "Copilot hook config", + verbose, + )?; // 2. Write instructions let instructions_path = github_dir.join("copilot-instructions.md");