Skip to content

Commit a8b8dba

Browse files
committed
fix(context): use separate old/new spans in classify_span_change
Tracks old-file and new-file line ranges independently so whitespace detection works when symbols shift positions between HEAD and staged. Also fixes false positive in public_api_removed_count that counted modified public symbols as "removed", causing spurious breaking_change validator violations.
1 parent 51dc97e commit a8b8dba

2 files changed

Lines changed: 68 additions & 10 deletions

File tree

src/services/context.rs

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,29 @@ impl ContextBuilder {
9191
.cloned()
9292
.collect();
9393

94-
// Populate is_whitespace_only by comparing diff content within each symbol's span
94+
// Populate is_whitespace_only by comparing diff content within each symbol's span.
95+
// Uses separate old/new line ranges since the same symbol may be at different
96+
// line numbers in HEAD vs staged (e.g., lines added above it shift everything).
9597
for symbol in &mut modified_symbols {
9698
if let Some(file_change) = changes.files.iter().find(|f| f.path == symbol.file) {
97-
symbol.is_whitespace_only =
98-
Self::classify_span_change(&file_change.diff, symbol.line, symbol.end_line);
99+
// Find the old-side counterpart for its line range
100+
let old_sym = symbols.iter().find(|s| {
101+
!s.is_added
102+
&& s.name == symbol.name
103+
&& s.kind == symbol.kind
104+
&& s.file == symbol.file
105+
});
106+
let (old_start, old_end) = old_sym
107+
.map(|s| (s.line, s.end_line))
108+
.unwrap_or((symbol.line, symbol.end_line));
109+
110+
symbol.is_whitespace_only = Self::classify_span_change(
111+
&file_change.diff,
112+
symbol.line,
113+
symbol.end_line,
114+
old_start,
115+
old_end,
116+
);
99117
}
100118
}
101119

@@ -161,12 +179,14 @@ impl ContextBuilder {
161179
// Evidence flags for constraint-based anti-hallucination
162180
let is_mechanical = Self::detect_mechanical_transform(changes, &symbols_deduped);
163181
let has_bug_evidence = Self::detect_bug_evidence(changes);
164-
// RemovedOnly public symbols + modified public symbols both contribute to breaking risk
182+
// Only genuinely removed public symbols count as "removed API".
183+
// Modified public symbols (same name in old+new) are NOT removals — their
184+
// signatures may have changed but the API still exists. Counting them as
185+
// removed triggers false "breaking_change required" validator violations.
165186
let public_api_removed_count = symbols_deduped
166187
.iter()
167188
.filter(|s| !s.is_added && s.is_public)
168-
.count()
169-
+ modified_symbols.iter().filter(|s| s.is_public).count();
189+
.count();
170190
let has_new_public_api = symbols_deduped.iter().any(|s| s.is_added && s.is_public);
171191
let is_dependency_only = Self::detect_dependency_only(changes);
172192

@@ -196,10 +216,21 @@ impl ContextBuilder {
196216
}
197217

198218
/// Classify whether changes within a symbol span are whitespace-only.
199-
/// Tracks both old-file and new-file line numbers independently.
219+
///
220+
/// Tracks old-file and new-file line numbers independently, using separate
221+
/// spans for each: `new_start..new_end` for added lines, `old_start..old_end`
222+
/// for removed lines. This correctly handles cases where the same symbol is
223+
/// at different line numbers in HEAD vs staged (e.g., lines added above it).
224+
///
200225
/// Returns `None` if no changes in span, `Some(true)` if whitespace-only,
201226
/// `Some(false)` if semantic changes detected.
202-
fn classify_span_change(diff: &str, start_line: usize, end_line: usize) -> Option<bool> {
227+
fn classify_span_change(
228+
diff: &str,
229+
new_start: usize,
230+
new_end: usize,
231+
old_start: usize,
232+
old_end: usize,
233+
) -> Option<bool> {
203234
use crate::services::analyzer::DiffHunk;
204235

205236
let hunks = DiffHunk::parse_from_diff(diff);
@@ -227,13 +258,13 @@ impl ContextBuilder {
227258
}
228259

229260
if let Some(content) = line.strip_prefix('+') {
230-
let in_new_span = current_new_line >= start_line && current_new_line <= end_line;
261+
let in_new_span = current_new_line >= new_start && current_new_line <= new_end;
231262
if in_new_span {
232263
added_in_span.push(content);
233264
}
234265
current_new_line += 1;
235266
} else if let Some(content) = line.strip_prefix('-') {
236-
let in_old_span = current_old_line >= start_line && current_old_line <= end_line;
267+
let in_old_span = current_old_line >= old_start && current_old_line <= old_end;
237268
if in_old_span {
238269
removed_in_span.push(content);
239270
}

tests/context.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -941,6 +941,33 @@ fn all_symbols_whitespace_only_suggests_style() {
941941
);
942942
}
943943

944+
#[test]
945+
fn whitespace_detection_works_with_shifted_lines() {
946+
// Symbol `process` was at line 2 in HEAD, now at line 5 in staged
947+
// (3 lines added above it). The whitespace change is inside the function.
948+
// classify_span_change must use separate old/new spans to detect this correctly.
949+
let changes = make_staged_changes(vec![make_file_change(
950+
"src/lib.rs",
951+
ChangeStatus::Modified,
952+
"@@ -2,3 +5,3 @@\n fn process() {\n- do_thing()\n+ do_thing()\n }",
953+
1,
954+
1,
955+
)]);
956+
let mut sym_old = make_symbol("process", SymbolKind::Function, "src/lib.rs", true, false);
957+
sym_old.line = 2;
958+
sym_old.end_line = 4;
959+
let mut sym_new = make_symbol("process", SymbolKind::Function, "src/lib.rs", true, true);
960+
sym_new.line = 5;
961+
sym_new.end_line = 7;
962+
let ctx = ContextBuilder::build(&changes, &[sym_old, sym_new], &default_config());
963+
// Should detect as whitespace-only despite line shift
964+
assert!(
965+
!ctx.symbols_modified.contains("process"),
966+
"whitespace-only change with shifted lines should not appear in symbols_modified: {}",
967+
ctx.symbols_modified
968+
);
969+
}
970+
944971
// ─── Multi-file diff truncation ──────────────────────────────────────────────
945972

946973
// ─── Locale support ──────────────────────────────────────────────────────────

0 commit comments

Comments
 (0)