Skip to content

fix(review): suppress phantom [index-breaking-change] when the diff redefines the symbol - #535

Merged
ajianaz merged 1 commit into
developfrom
fix/stale-index-breaking-fp
Aug 27, 2026
Merged

fix(review): suppress phantom [index-breaking-change] when the diff redefines the symbol#535
ajianaz merged 1 commit into
developfrom
fix/stale-index-breaking-fp

Conversation

@ajianaz

@ajianaz ajianaz commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Fixes #533

What

  • scan_breaking_changes now builds the post-image definition set of the diff (symbol names matched on added lines, same regex set as the removal scan, diff-global across chunks) before scanning for removals
  • A removal candidate whose name still exists anywhere in the post-image is treated as signature drift / move / definition-line rewording, not a removal, and is skipped
  • True removals (name absent from the post-image) are still reported — including genuine renames, where only the old name fires
  • Internal scan_breaking_changes_with(conn, ...) split so the full contract is testable against an in-memory index
  • 5 new tests, including the exact acceptance fixture from the issue

Why

While landing #531 the pre-commit hook reported [MAJOR] [index-breaking-change] Removing build_review_prompt breaks 2 caller(s) even though nothing was removed — a signature-only edit shows the old definition as a - line, and a stale index made it look like a phantom removal. The scanner compared removed lines against a possibly-outdated index without ever checking whether the symbol still exists in the change itself (#533).

Testing

  • Acceptance fixture from the issue: signature-drift diff against a stale index with 1 caller → no finding (was: MAJOR false positive)
  • Genuine removal against the same index → still fires with index-breaking-change / Major
  • Rename (old_namenew_name) → fires only for old_name
  • Cross-file move of a called function → no finding (post-image is diff-global)
  • collect_added_definitions unit test: added names collected, removed/context names not
  • cargo fmt --all -- --check clean; cargo clippy --all-targets --features tree-sitter -- -D warnings 0 warnings; cargo test --features tree-sitter 931 passed, 0 failed (909 unit + 16 CLI + 6 config)

Note: the pre-existing make_chunk test helper stores diff content with the +/- marker, while the real parser strips it — the old removal-scan tests passed vacuously against anchored regexes. New tests use a chunk_lines helper matching parser semantics.

All commits signed-off (DCO).

…edefines the symbol

Signed-off-by: Anaz S. Aji <aji.anaz@gmail.com>
@ajianaz
ajianaz merged commit 5a0b684 into develop Aug 27, 2026
15 checks passed
@ajianaz
ajianaz deleted the fix/stale-index-breaking-fp branch August 27, 2026 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

review: signature-only refactor flagged as [index-breaking-change] symbol removal (stale-index FP)

1 participant