From 84e0f78257364d3d805b11aef65409616dd421d1 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 12 Jul 2026 15:41:14 -0700 Subject: [PATCH 1/3] fix(llm): bound prompt diffs to a 100k-byte budget Commit-message, squash, and summary prompts embedded diffs nearly unbounded: under the 400k threshold the diff passed through whole, which for syntax-dense content (hashes, minified assets, ~2 bytes/token) can exceed a 200k-token context window outright. Past the threshold, the 50-lines-per-file truncation had no line-length cap, so a single-line minified file passed through whole, and the diffstat was never reduced at all. prepare_diff now caps the stat at 200 lines (head plus trailing summary line), passes diffs at or under 100k bytes through unchanged, and otherwise drops lock files then accumulates per-file-truncated sections (50 lines, 500 bytes per line) in diff order until the budget is spent. The accumulation loop is the output bound; the per-section caps keep any one file from eating the whole budget. Worst case is ~30k tokens. Co-Authored-By: Claude Fable 5 --- src/llm.rs | 215 ++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 163 insertions(+), 52 deletions(-) diff --git a/src/llm.rs b/src/llm.rs index e7e3c0602b..a43991333d 100644 --- a/src/llm.rs +++ b/src/llm.rs @@ -130,14 +130,25 @@ fn format_reproduction_command(base_cmd: &str, llm_command: &str) -> String { /// Track whether template-file deprecation warning has been shown this session static TEMPLATE_FILE_WARNING_SHOWN: AtomicBool = AtomicBool::new(false); -/// Maximum diff size in characters before filtering kicks in -const DIFF_SIZE_THRESHOLD: usize = 400_000; +/// Byte budget for the diff embedded in a prompt. ~25k tokens for typical +/// code; syntax-dense content (hashes, minified assets) tokenizes as low as +/// ~2 bytes/token, so the worst case is ~50k tokens — bounded well under +/// small-model context windows, where the old 400k threshold was not. +const DIFF_BUDGET: usize = 100_000; -/// Maximum lines per file after truncation +/// Maximum lines per file section once the diff exceeds the budget const MAX_LINES_PER_FILE: usize = 50; -/// Maximum number of files to include after truncation -const MAX_FILES: usize = 50; +/// Maximum bytes per line once the diff exceeds the budget. A line-count cap +/// alone passes single-line minified/generated files through whole; this +/// bounds each section to `MAX_LINES_PER_FILE * MAX_LINE_LEN` bytes so no +/// one file eats the whole budget. +const MAX_LINE_LEN: usize = 500; + +/// Maximum diffstat lines, applied at any diff size. The stat is one line +/// per file, so a many-thousand-file diff would otherwise dwarf the +/// truncated diff itself. +const STAT_MAX_LINES: usize = 200; /// Lock file patterns that are filtered out when diff is too large const LOCK_FILE_PATTERNS: &[&str] = &[".lock", "-lock.json", "-lock.yaml", ".lock.hcl"]; @@ -201,11 +212,27 @@ fn parse_diff_sections(diff: &str) -> Vec<(&str, &str)> { sections } -/// Truncate a diff section to max lines, keeping the header +/// Cap a line at [`MAX_LINE_LEN`] bytes, cutting at a char boundary +fn truncate_line(line: &str) -> Cow<'_, str> { + if line.len() <= MAX_LINE_LEN { + return Cow::Borrowed(line); + } + let mut end = MAX_LINE_LEN; + while !line.is_char_boundary(end) { + end -= 1; + } + Cow::Owned(format!("{}... (line truncated)", &line[..end])) +} + +/// Truncate a diff section to max lines (keeping the header) and cap each +/// surviving line at [`MAX_LINE_LEN`] bytes fn truncate_diff_section(section: &str, max_lines: usize) -> String { let lines: Vec<&str> = section.lines().collect(); if lines.len() <= max_lines { - return section.to_string(); + return lines + .iter() + .map(|l| format!("{}\n", truncate_line(l))) + .collect(); } // Find where the actual diff content starts (after the @@ line) @@ -218,7 +245,7 @@ fn truncate_diff_section(section: &str, max_lines: usize) -> String { let mut result: String = lines .iter() .take(total_lines) - .map(|l| format!("{}\n", l)) + .map(|l| format!("{}\n", truncate_line(l))) .collect(); let omitted = lines.len() - total_lines; if omitted > 0 { @@ -228,19 +255,47 @@ fn truncate_diff_section(section: &str, max_lines: usize) -> String { result } -/// Prepare diff for LLM consumption, applying filtering if needed +/// Cap the diffstat at [`STAT_MAX_LINES`], keeping the head and the trailing +/// `N files changed, ...` summary line with an omission marker between +fn truncate_stat(stat: String) -> String { + let lines: Vec<&str> = stat.lines().collect(); + if lines.len() <= STAT_MAX_LINES { + return stat; + } + + let kept_head = STAT_MAX_LINES - 1; + let omitted = lines.len() - kept_head - 1; + let mut out: String = lines[..kept_head] + .iter() + .map(|l| format!("{}\n", l)) + .collect(); + out.push_str(&format!("... ({} more files)\n", omitted)); + out.push_str(lines[lines.len() - 1]); + out.push('\n'); + out +} + +/// Prepare diff for LLM consumption, applying filtering if needed. +/// +/// The stat is always capped at [`STAT_MAX_LINES`]. A diff at or under +/// [`DIFF_BUDGET`] passes through unchanged. Over budget, lock-file sections +/// are dropped; if that isn't enough, each section is truncated (line count +/// and line length) and sections accumulate in diff order until the budget +/// is spent — the accumulation is the output bound, the per-section caps +/// only keep one file from eating the whole budget. pub(crate) fn prepare_diff(diff: String, stat: String) -> PreparedDiff { - // If under threshold, pass through unchanged - if diff.len() < DIFF_SIZE_THRESHOLD { + let stat = truncate_stat(stat); + + if diff.len() <= DIFF_BUDGET { return PreparedDiff { diff, stat }; } tracing::debug!( count = diff.len(), - threshold = DIFF_SIZE_THRESHOLD, - "Diff size ({} chars) exceeds threshold ({}), filtering", + budget = DIFF_BUDGET, + "Diff size ({} bytes) exceeds budget ({}), filtering", diff.len(), - DIFF_SIZE_THRESHOLD + DIFF_BUDGET ); // Step 1: Filter out lock files @@ -259,43 +314,52 @@ pub(crate) fn prepare_diff(diff: String, stat: String) -> PreparedDiff { ); } - let filtered_diff: String = filtered_sections + let filtered_len: usize = filtered_sections .iter() - .map(|(_, content)| *content) - .collect(); + .map(|(_, content)| content.len()) + .sum(); - // If filtering lock files brought us under threshold, we're done - if filtered_diff.len() < DIFF_SIZE_THRESHOLD { + // If filtering lock files brought us under budget, we're done + if filtered_len <= DIFF_BUDGET { return PreparedDiff { - diff: filtered_diff, + diff: filtered_sections + .iter() + .map(|(_, content)| *content) + .collect(), stat, }; } - // Step 2: Truncate each file and limit file count + // Step 2: Truncate each section, accumulating until the budget is spent tracing::debug!( - count = filtered_diff.len(), - "Still too large ({} chars), truncating to {} lines/file, {} files max", - filtered_diff.len(), + count = filtered_len, + "Still too large ({} bytes), truncating to {} lines/file within a {} byte budget", + filtered_len, MAX_LINES_PER_FILE, - MAX_FILES + DIFF_BUDGET ); - let truncated: String = filtered_sections - .iter() - .take(MAX_FILES) - .map(|(_, content)| truncate_diff_section(content, MAX_LINES_PER_FILE)) - .collect(); + let mut truncated = String::new(); + let mut included = 0; + for (_, content) in &filtered_sections { + let section = truncate_diff_section(content, MAX_LINES_PER_FILE); + if truncated.len() + section.len() > DIFF_BUDGET { + break; + } + truncated.push_str(§ion); + included += 1; + } - let files_omitted = filtered_sections.len().saturating_sub(MAX_FILES); - let final_diff = if files_omitted > 0 { - format!("{}\n... ({} files omitted)\n", truncated, files_omitted) - } else { - truncated - }; + let files_omitted = filtered_sections.len() - included; + if files_omitted > 0 { + truncated.push_str(&format!( + "\n... ({} files omitted, see diffstat)\n", + files_omitted + )); + } PreparedDiff { - diff: final_diff, + diff: truncated, stat, } } @@ -2085,45 +2149,92 @@ index abc..def 100644 #[test] fn test_prepare_diff_filters_lock_files() { - // Create a diff just over the threshold with a lock file - let regular_content = "x".repeat(100_000); - let lock_content = "y".repeat(350_000); + // Over budget only because of the lock file: filtering it restores + // the regular content untouched (multi-line, so line caps would show) + let regular_content = "line of code\n".repeat(3_000); + let lock_content = "y".repeat(150_000); let diff = format!( - r#"diff --git a/src/main.rs b/src/main.rs -{} -diff --git a/Cargo.lock b/Cargo.lock -{} -"#, + "diff --git a/src/main.rs b/src/main.rs\n{}diff --git a/Cargo.lock b/Cargo.lock\n{}\n", regular_content, lock_content ); let stat = "2 files changed".to_string(); let prepared = prepare_diff(diff, stat); - // Lock file should be filtered out assert!(!prepared.diff.contains("Cargo.lock")); assert!(prepared.diff.contains("src/main.rs")); + // The surviving section is under budget, so it passes through whole + assert!(prepared.diff.contains(®ular_content)); } #[test] - fn test_prepare_diff_filters_then_truncates() { - // Create many non-lock files that exceed threshold even after lock filtering + fn test_prepare_diff_accumulates_sections_up_to_budget() { + // Many multi-line files that exceed the budget even after per-file + // truncation: sections accumulate in diff order until the budget is + // spent, and the rest are reported against the diffstat let mut diff = String::new(); for i in 0..100 { diff.push_str(&format!( - "diff --git a/file{}.rs b/file{}.rs\n{}\n", + "diff --git a/file{}.rs b/file{}.rs\n{}", i, i, - "x".repeat(5000) + "line of code that is well under the length cap\n".repeat(200) )); } + assert!(diff.len() > DIFF_BUDGET); let stat = "100 files changed".to_string(); let prepared = prepare_diff(diff, stat); - // Should be truncated (max 50 files) - assert!(prepared.diff.contains("files omitted")); + assert!(prepared.diff.contains("file0.rs")); + assert!(prepared.diff.contains("lines omitted")); + assert!(prepared.diff.contains("files omitted, see diffstat")); + // The omission marker is the only content past the budget + assert!(prepared.diff.len() < DIFF_BUDGET + 100); + } + + #[test] + fn test_prepare_diff_caps_single_line_files() { + // A minified single-line file slips through a line-count cap; the + // line-length cap must bound it. Multi-byte content exercises the + // char-boundary cut. + let minified = format!("+{}", "测".repeat(80_000)); // ~240k bytes, one line + let diff = format!( + "diff --git a/src/lib.rs b/src/lib.rs\n+fn real_change() {{}}\ndiff --git a/dist/bundle.min.js b/dist/bundle.min.js\n{}\n", + minified + ); + let stat = "2 files changed".to_string(); + + let prepared = prepare_diff(diff, stat); + + assert!(prepared.diff.contains("real_change")); + assert!(prepared.diff.contains("line truncated")); + assert!(prepared.diff.len() < DIFF_BUDGET + 100); + } + + #[test] + fn test_truncate_stat_keeps_head_and_summary() { + // Small stat passes through unchanged + let small = " src/main.rs | 4 ++++\n 1 file changed, 4 insertions(+)\n".to_string(); + assert_eq!(truncate_stat(small.clone()), small); + + // Oversized stat keeps the head, a marker, and the trailing summary + let mut stat = String::new(); + for i in 0..500 { + stat.push_str(&format!(" src/file{}.rs | 2 +-\n", i)); + } + stat.push_str(" 500 files changed, 500 insertions(+), 500 deletions(-)\n"); + + let truncated = truncate_stat(stat); + let lines: Vec<&str> = truncated.lines().collect(); + assert_eq!(lines.len(), STAT_MAX_LINES + 1); + assert_eq!(lines[0], " src/file0.rs | 2 +-"); + assert_eq!(lines[STAT_MAX_LINES - 1], "... (301 more files)"); + assert_eq!( + lines[STAT_MAX_LINES], + " 500 files changed, 500 insertions(+), 500 deletions(-)" + ); } #[test] From ca6582d2f9f6b13bca14ab2c7eb0b394d059a4f8 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 12 Jul 2026 15:55:21 -0700 Subject: [PATCH 2/3] fix(llm): close review gaps in prompt truncation - Cap squashed-commit subjects at 200 per prompt, with a synthetic "(N earlier commits omitted)" entry standing in for the older tail: the diff budget did not bound the squash prompt's commit list. - Drop truncate_stat's last-line special case: the summary path concatenates two --stat blocks (branch + working tree), so "the last line is the total" does not hold; keep head plus omission marker. - Force a/ b/ diff prefixes in compute_combined_diff so summary diffs stay parseable into sections under diff.noprefix, the same guard the commit and squash paths already carry. - Render sections without the full-section Vec or per-line allocations. Co-Authored-By: Claude Fable 5 --- src/llm.rs | 143 +++++++++++++++++++++++++++++++++---------------- src/summary.rs | 23 ++++++-- 2 files changed, 116 insertions(+), 50 deletions(-) diff --git a/src/llm.rs b/src/llm.rs index a43991333d..304e4fd0bf 100644 --- a/src/llm.rs +++ b/src/llm.rs @@ -150,6 +150,13 @@ const MAX_LINE_LEN: usize = 500; /// truncated diff itself. const STAT_MAX_LINES: usize = 200; +/// Maximum squashed commits rendered into a squash prompt; a synthetic +/// "(N earlier commits omitted)" entry stands in for the older tail. This +/// bounds the one prompt input the diff budget doesn't cover — subjects from +/// a branch with thousands of commits would otherwise grow the prompt +/// without limit. +const MAX_SQUASH_COMMITS: usize = 200; + /// Lock file patterns that are filtered out when diff is too large const LOCK_FILE_PATTERNS: &[&str] = &[".lock", "-lock.json", "-lock.yaml", ".lock.hcl"]; @@ -224,30 +231,27 @@ fn truncate_line(line: &str) -> Cow<'_, str> { Cow::Owned(format!("{}... (line truncated)", &line[..end])) } -/// Truncate a diff section to max lines (keeping the header) and cap each -/// surviving line at [`MAX_LINE_LEN`] bytes +/// Truncate a diff section to max lines (keeping at least the header lines, +/// through the first `@@` hunk marker) and cap each surviving line at +/// [`MAX_LINE_LEN`] bytes. Output is LF-normalized. fn truncate_diff_section(section: &str, max_lines: usize) -> String { - let lines: Vec<&str> = section.lines().collect(); - if lines.len() <= max_lines { - return lines - .iter() - .map(|l| format!("{}\n", truncate_line(l))) - .collect(); - } - - // Find where the actual diff content starts (after the @@ line) - let header_end = lines.iter().position(|l| l.starts_with("@@")).unwrap_or(0); - let header_lines = header_end + 1; // Include the first @@ line - - let content_lines = max_lines.saturating_sub(header_lines); - let total_lines = header_lines + content_lines; + let line_count = section.lines().count(); + let total_lines = if line_count <= max_lines { + line_count + } else { + let header_lines = section + .lines() + .position(|l| l.starts_with("@@")) + .map_or(1, |i| i + 1); + max_lines.max(header_lines) + }; - let mut result: String = lines - .iter() - .take(total_lines) - .map(|l| format!("{}\n", truncate_line(l))) - .collect(); - let omitted = lines.len() - total_lines; + let mut result = String::new(); + for line in section.lines().take(total_lines) { + result.push_str(&truncate_line(line)); + result.push('\n'); + } + let omitted = line_count - total_lines; if omitted > 0 { result.push_str(&format!("\n... ({} lines omitted)\n", omitted)); } @@ -255,23 +259,25 @@ fn truncate_diff_section(section: &str, max_lines: usize) -> String { result } -/// Cap the diffstat at [`STAT_MAX_LINES`], keeping the head and the trailing -/// `N files changed, ...` summary line with an omission marker between +/// Cap the diffstat at [`STAT_MAX_LINES`] lines plus an omission marker. No +/// line is special-cased: the summary path concatenates two `--stat` blocks +/// (branch + working tree), so "the last line is the total" doesn't hold in +/// general. fn truncate_stat(stat: String) -> String { - let lines: Vec<&str> = stat.lines().collect(); - if lines.len() <= STAT_MAX_LINES { + let line_count = stat.lines().count(); + if line_count <= STAT_MAX_LINES { return stat; } - let kept_head = STAT_MAX_LINES - 1; - let omitted = lines.len() - kept_head - 1; - let mut out: String = lines[..kept_head] - .iter() - .map(|l| format!("{}\n", l)) - .collect(); - out.push_str(&format!("... ({} more files)\n", omitted)); - out.push_str(lines[lines.len() - 1]); - out.push('\n'); + let mut out = String::new(); + for line in stat.lines().take(STAT_MAX_LINES) { + out.push_str(line); + out.push('\n'); + } + out.push_str(&format!( + "... ({} more files)\n", + line_count - STAT_MAX_LINES + )); out } @@ -588,6 +594,8 @@ fn load_template( /// Squash-specific variables (empty for regular commits): /// - `commit_details`: Commits being squashed. Each element renders as its /// subject when printed bare and exposes `.subject` / `.body` properties. +/// Capped at [`MAX_SQUASH_COMMITS`]; the older tail is represented by one +/// synthetic "(N earlier commits omitted)" entry. /// - `commits`: Commit subjects being squashed (deprecated — see #2984; /// `wt config update` rewrites it to `commit_details`) /// - `target_branch`: Target branch for merge @@ -641,16 +649,30 @@ fn build_prompt( // element renders as its subject (see `CommitDetailValue`), so a migrated // `{% for c in commit_details %}{{ c }}` reads identically to the old // `{% for c in commits %}{{ c }}`. - let commits_chronological: Vec<&String> = context + // + // The list is capped at `MAX_SQUASH_COMMITS` — the one prompt input the + // diff budget doesn't bound. Details arrive newest-first, so the newest + // are kept and a synthetic entry stands in for the older tail; it renders + // first in the chronological order below. + let omitted_commits = context .commit_details + .len() + .saturating_sub(MAX_SQUASH_COMMITS); + let synthetic_tail = (omitted_commits > 0).then(|| CommitMessageDetail { + subject: format!("... ({} earlier commits omitted)", omitted_commits), + body: String::new(), + }); + let kept_details = &context.commit_details[..context.commit_details.len() - omitted_commits]; + let details_chronological: Vec<&CommitMessageDetail> = synthetic_tail + .iter() + .chain(kept_details.iter().rev()) + .collect(); + let commits_chronological: Vec<&String> = details_chronological .iter() - .rev() .map(|detail| &detail.subject) .collect(); - let commit_details_chronological: Vec = context - .commit_details + let commit_details_chronological: Vec = details_chronological .iter() - .rev() .map(|detail| { Value::from_object(CommitDetailValue { subject: detail.subject.clone(), @@ -2214,12 +2236,12 @@ index abc..def 100644 } #[test] - fn test_truncate_stat_keeps_head_and_summary() { + fn test_truncate_stat_caps_lines() { // Small stat passes through unchanged let small = " src/main.rs | 4 ++++\n 1 file changed, 4 insertions(+)\n".to_string(); assert_eq!(truncate_stat(small.clone()), small); - // Oversized stat keeps the head, a marker, and the trailing summary + // Oversized stat keeps the head plus an omission marker let mut stat = String::new(); for i in 0..500 { stat.push_str(&format!(" src/file{}.rs | 2 +-\n", i)); @@ -2230,11 +2252,38 @@ index abc..def 100644 let lines: Vec<&str> = truncated.lines().collect(); assert_eq!(lines.len(), STAT_MAX_LINES + 1); assert_eq!(lines[0], " src/file0.rs | 2 +-"); - assert_eq!(lines[STAT_MAX_LINES - 1], "... (301 more files)"); - assert_eq!( - lines[STAT_MAX_LINES], - " 500 files changed, 500 insertions(+), 500 deletions(-)" - ); + assert_eq!(lines[STAT_MAX_LINES - 1], " src/file199.rs | 2 +-"); + assert_eq!(lines[STAT_MAX_LINES], "... (301 more files)"); + } + + #[test] + fn test_build_squash_prompt_caps_commit_count() { + // Details arrive newest-first; over MAX_SQUASH_COMMITS the newest are + // kept and a synthetic entry for the older tail renders first in the + // chronological list. + let config = CommitGenerationConfig { + squash_template: Some( + "{% for c in commit_details %}- {{ c }}\n{% endfor %}".to_string(), + ), + ..Default::default() + }; + let commit_details: Vec = (0..MAX_SQUASH_COMMITS + 50) + .map(|i| CommitMessageDetail { + subject: format!("commit {}", i), + body: String::new(), + }) + .collect(); + let context = squash_context("diff", "feature", None, "repo", &commit_details, "main"); + + let prompt = build_prompt(&config, TemplateType::Squash, &context).unwrap(); + + assert!(prompt.starts_with("- ... (50 earlier commits omitted)\n")); + // Newest (index 0) renders last; the oldest kept (index 199) follows + // the marker; the 50 oldest are gone. + assert!(prompt.ends_with("- commit 0\n")); + assert!(prompt.contains("- commit 199\n")); + assert!(!prompt.contains("- commit 200\n")); + assert_eq!(prompt.lines().count(), MAX_SQUASH_COMMITS + 1); } #[test] diff --git a/src/summary.rs b/src/summary.rs index c440c64819..cba20cc9a4 100644 --- a/src/summary.rs +++ b/src/summary.rs @@ -240,8 +240,17 @@ pub(crate) fn compute_combined_diff( if !is_default_branch && let Some(spec) = repo.branch_diff_spec(head) { // `--end-of-options` guards a base name that could begin with `-`; the // stat and full-diff invocations differ only by the `--stat` flag. + // The `-c` flags force the `a/`/`b/` prefix format regardless of user + // git config — `prepare_diff` parses the diff into per-file sections + // by those prefixes (same guard as `build_commit_prompt`). let run_branch_diff = |opts: &[&str], out: &mut String| { - let mut args = vec!["diff"]; + let mut args = vec![ + "-c", + "diff.noprefix=false", + "-c", + "diff.mnemonicPrefix=false", + "diff", + ]; args.extend_from_slice(opts); args.push("--end-of-options"); args.extend(spec.revs.iter().map(String::as_str)); @@ -261,8 +270,16 @@ pub(crate) fn compute_combined_diff( { stat.push_str(&wt_stat); } - if let Ok(wt_diff) = repo.run_command(&["-C", &path, "diff", "HEAD"]) - && !wt_diff.trim().is_empty() + if let Ok(wt_diff) = repo.run_command(&[ + "-C", + &path, + "-c", + "diff.noprefix=false", + "-c", + "diff.mnemonicPrefix=false", + "diff", + "HEAD", + ]) && !wt_diff.trim().is_empty() { diff.push_str(&wt_diff); } From b7d488e22caaa46cce0fc90ff1a3039c0a9838c9 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 12 Jul 2026 16:14:15 -0700 Subject: [PATCH 3/3] fix(llm): pin diff.srcPrefix/dstPrefix and share the prefix guard Review caught that git >= 2.45's diff.srcPrefix/diff.dstPrefix bypass the diff.noprefix/diff.mnemonicPrefix overrides, so a user setting them still defeats parse_diff_sections and an over-budget diff collapses to an empty prompt diff. All four prompt-diff invocations now share one DIFF_PREFIX_OVERRIDES constant pinning all four keys; unknown config keys are ignored by older git. Co-Authored-By: Claude Fable 5 --- src/llm.rs | 44 ++++++++++++++++++++++---------------------- src/summary.rs | 31 ++++++++++--------------------- 2 files changed, 32 insertions(+), 43 deletions(-) diff --git a/src/llm.rs b/src/llm.rs index 304e4fd0bf..f64a9787da 100644 --- a/src/llm.rs +++ b/src/llm.rs @@ -160,6 +160,23 @@ const MAX_SQUASH_COMMITS: usize = 200; /// Lock file patterns that are filtered out when diff is too large const LOCK_FILE_PATTERNS: &[&str] = &[".lock", "-lock.json", "-lock.yaml", ".lock.hcl"]; +/// Git `-c` overrides forcing the `a/`/`b/` diff prefix format regardless of +/// user config: `diff.noprefix`, `diff.mnemonicPrefix`, and (git >= 2.45) +/// `diff.srcPrefix`/`diff.dstPrefix` can all change the header format that +/// [`parse_diff_sections`] splits file sections on, so every full diff +/// destined for [`prepare_diff`] must carry these flags. Unknown config keys +/// are ignored by older git. +pub(crate) const DIFF_PREFIX_OVERRIDES: [&str; 8] = [ + "-c", + "diff.noprefix=false", + "-c", + "diff.mnemonicPrefix=false", + "-c", + "diff.srcPrefix=a/", + "-c", + "diff.dstPrefix=b/", +]; + /// Prepared diff output with optional filtering applied pub(crate) struct PreparedDiff { /// The diff content (possibly filtered/truncated) @@ -852,18 +869,9 @@ pub(crate) fn build_commit_prompt( let repo = Repository::current()?; let cwd = repo.discovery_path().to_path_buf(); - // Use -c flags to ensure consistent format regardless of user's git config - // (diff.noprefix, diff.mnemonicPrefix, etc. could break our parsing) let mut diff_cmd = Cmd::new("git") - .args([ - "-c", - "diff.noprefix=false", - "-c", - "diff.mnemonicPrefix=false", - "--no-pager", - "diff", - "--staged", - ]) + .args(DIFF_PREFIX_OVERRIDES) + .args(["--no-pager", "diff", "--staged"]) .current_dir(&cwd); let mut diff_stat_cmd = Cmd::new("git") .args(["--no-pager", "diff", "--staged", "--stat"]) @@ -967,17 +975,9 @@ pub(crate) fn build_squash_prompt( let repo = Repository::current()?; // Get the combined diff and diffstat for all commits being squashed - // Use -c flags to ensure consistent format regardless of user's git config - let diff_output = repo.run_command(&[ - "-c", - "diff.noprefix=false", - "-c", - "diff.mnemonicPrefix=false", - "--no-pager", - "diff", - merge_base, - "HEAD", - ])?; + let mut diff_args: Vec<&str> = DIFF_PREFIX_OVERRIDES.to_vec(); + diff_args.extend(["--no-pager", "diff", merge_base, "HEAD"]); + let diff_output = repo.run_command(&diff_args)?; let diff_stat = repo.run_command(&["--no-pager", "diff", merge_base, "HEAD", "--stat"])?; // Prepare diff (may filter if too large) diff --git a/src/summary.rs b/src/summary.rs index cba20cc9a4..8700eafb6a 100644 --- a/src/summary.rs +++ b/src/summary.rs @@ -41,7 +41,7 @@ use worktrunk::styling::INFO_SYMBOL; use worktrunk::sync::Semaphore; use worktrunk::utils::epoch_now; -use crate::llm::{execute_llm_command, prepare_diff}; +use crate::llm::{DIFF_PREFIX_OVERRIDES, execute_llm_command, prepare_diff}; /// Limits concurrent LLM calls to avoid overwhelming the network / LLM /// provider. 8 permits balances parallelism with resource usage — LLM calls @@ -240,17 +240,11 @@ pub(crate) fn compute_combined_diff( if !is_default_branch && let Some(spec) = repo.branch_diff_spec(head) { // `--end-of-options` guards a base name that could begin with `-`; the // stat and full-diff invocations differ only by the `--stat` flag. - // The `-c` flags force the `a/`/`b/` prefix format regardless of user - // git config — `prepare_diff` parses the diff into per-file sections - // by those prefixes (same guard as `build_commit_prompt`). + // The prefix overrides keep the diff parseable into per-file sections + // by `prepare_diff` (same guard as `build_commit_prompt`). let run_branch_diff = |opts: &[&str], out: &mut String| { - let mut args = vec![ - "-c", - "diff.noprefix=false", - "-c", - "diff.mnemonicPrefix=false", - "diff", - ]; + let mut args = DIFF_PREFIX_OVERRIDES.to_vec(); + args.push("diff"); args.extend_from_slice(opts); args.push("--end-of-options"); args.extend(spec.revs.iter().map(String::as_str)); @@ -270,16 +264,11 @@ pub(crate) fn compute_combined_diff( { stat.push_str(&wt_stat); } - if let Ok(wt_diff) = repo.run_command(&[ - "-C", - &path, - "-c", - "diff.noprefix=false", - "-c", - "diff.mnemonicPrefix=false", - "diff", - "HEAD", - ]) && !wt_diff.trim().is_empty() + let mut wt_diff_args = vec!["-C", &path]; + wt_diff_args.extend(DIFF_PREFIX_OVERRIDES); + wt_diff_args.extend(["diff", "HEAD"]); + if let Ok(wt_diff) = repo.run_command(&wt_diff_args) + && !wt_diff.trim().is_empty() { diff.push_str(&wt_diff); }