D-037: test: cover ChangeStatus Deleted and Renamed variants

tests/context.rs

@@ -1407,6 +1407,89 @@ fn format_files_shows_renamed_marker() {
         "renamed file should show similarity: {}",
         ctx.file_breakdown
     );
+    // old_path must be respected in the formatted output — the old path appears
+    // on the left of the `->` arrow so reviewers can see what was renamed from.
+    assert!(
+        ctx.file_breakdown.contains("src/old_name.rs"),
+        "renamed file should include the old path in output: {}",
+        ctx.file_breakdown
+    );
+    assert!(
+        ctx.file_breakdown.contains("->"),
+        "renamed file should show an arrow between old and new path: {}",
+        ctx.file_breakdown
+    );
+    assert!(
+        ctx.file_breakdown.contains("src/new_name.rs"),
+        "renamed file should include the new path in output: {}",
+        ctx.file_breakdown
+    );
+}
+
+#[test]
+fn format_files_mixed_statuses_show_all_markers() {
+    // One of each ChangeStatus variant in the same StagedChanges — verify every
+    // code path in `format_files` fires and produces the expected status prefix.
+    let changes = make_staged_changes(vec![
+        make_file_change("src/added.rs", ChangeStatus::Added, "+pub fn n() {}", 1, 0),
+        make_file_change(
+            "src/modified.rs",
+            ChangeStatus::Modified,
+            "-old\n+new",
+            1,
+            1,
+        ),
+        make_file_change(
+            "src/deleted.rs",
+            ChangeStatus::Deleted,
+            "-pub fn g() {}",
+            0,
+            1,
+        ),
+        make_renamed_file("src/old.rs", "src/new.rs", 90),
+    ]);
+    let ctx = ContextBuilder::build(&changes, &[], &[], &default_config());
+    assert!(
+        ctx.file_breakdown.contains("[+] src/added.rs"),
+        "added file should use [+] marker: {}",
+        ctx.file_breakdown
+    );
+    assert!(
+        ctx.file_breakdown.contains("[M] src/modified.rs"),
+        "modified file should use [M] marker: {}",
+        ctx.file_breakdown
+    );
+    assert!(
+        ctx.file_breakdown.contains("[-] src/deleted.rs"),
+        "deleted file should use [-] marker: {}",
+        ctx.file_breakdown
+    );
+    assert!(
+        ctx.file_breakdown.contains("[R] src/old.rs -> src/new.rs"),
+        "renamed file should use [R] with old -> new formatting: {}",
+        ctx.file_breakdown
+    );
+    // The change-summary count aggregates per status; verify it observed each.
+    assert!(
+        ctx.change_summary.contains("1 added"),
+        "change_summary should count added file: {}",
+        ctx.change_summary
+    );
+    assert!(
+        ctx.change_summary.contains("1 modified"),
+        "change_summary should count modified file: {}",
+        ctx.change_summary
+    );
+    assert!(
+        ctx.change_summary.contains("1 deleted"),
+        "change_summary should count deleted file: {}",
+        ctx.change_summary
+    );
+    assert!(
+        ctx.change_summary.contains("1 renamed"),
+        "change_summary should count renamed file: {}",
+        ctx.change_summary
+    );
 }
 
 // ─── Test Coverage: classify_span_change None path (#39) ──────────────────────

tests/helpers.rs

@@ -32,12 +32,27 @@ pub fn make_file_change(
 /// Create a renamed FileChange for testing
 #[allow(dead_code)]
 pub fn make_renamed_file(old_path: &str, new_path: &str, similarity: u8) -> FileChange {
+    make_renamed_file_with_diff(old_path, new_path, similarity, "", 0, 0)
+}
+
+/// Create a renamed FileChange with a diff body and explicit add/delete counts.
+///
+/// Useful for splitter tests that exercise diff-shape grouping on renames.
+#[allow(dead_code)]
+pub fn make_renamed_file_with_diff(
+    old_path: &str,
+    new_path: &str,
+    similarity: u8,
+    diff: &str,
+    additions: usize,
+    deletions: usize,
+) -> FileChange {
     FileChange {
         path: PathBuf::from(new_path),
         status: ChangeStatus::Renamed,
-        diff: Arc::from(""),
-        additions: 0,
-        deletions: 0,
+        diff: Arc::from(diff),
+        additions,
+        deletions,
         category: FileCategory::from_path(&PathBuf::from(new_path)),
         is_binary: false,
         old_path: Some(PathBuf::from(old_path)),

tests/splitter.rs

@@ -8,7 +8,9 @@ use std::path::PathBuf;
 
 use commitbee::domain::{ChangeStatus, CodeSymbol, SymbolKind};
 use commitbee::services::splitter::{CommitSplitter, SplitSuggestion};
-use helpers::{make_file_change, make_staged_changes};
+use helpers::{
+    make_file_change, make_renamed_file, make_renamed_file_with_diff, make_staged_changes,
+};
 
 // ─── Helpers ─────────────────────────────────────────────────────────────────
 
@@ -550,3 +552,226 @@ fn subset_empty_paths_returns_empty() {
     assert!(subset.files.is_empty());
     assert_eq!(subset.stats.files_changed, 0);
 }
+
+// ─── ChangeStatus::Deleted fixtures (audit D-037) ───────────────────────────
+
+/// Baseline: a deleted file must not be silently dropped. Either the splitter
+/// returns SingleCommit (where all files are implicitly in the one group) or
+/// the deletion lands in exactly one split group.
+#[test]
+fn deleted_file_is_represented_in_splitter_output() {
+    let changes = make_staged_changes(vec![
+        make_file_change(
+            "src/services/llm/ollama.rs",
+            ChangeStatus::Modified,
+            "@@ -10,3 +10,3 @@\n-let old = 1;\n+let new = 2;\n",
+            1,
+            1,
+        ),
+        make_file_change(
+            "src/services/llm/legacy.rs",
+            ChangeStatus::Deleted,
+            "@@ -1,10 +0,0 @@\n-pub fn retired() {}\n-pub fn also_retired() {}\n",
+            0,
+            10,
+        ),
+    ]);
+
+    let result = CommitSplitter::analyze(&changes, &[]);
+    let deleted_path = PathBuf::from("src/services/llm/legacy.rs");
+
+    match &result {
+        SplitSuggestion::SuggestSplit(groups) => {
+            let occurrences = groups
+                .iter()
+                .filter(|g| g.files.contains(&deleted_path))
+                .count();
+            assert_eq!(
+                occurrences, 1,
+                "deleted file must appear in exactly one split group: {:?}",
+                groups
+            );
+        }
+        SplitSuggestion::SingleCommit => {
+            // Single-commit means all files fall into the one implicit group
+            // — the deleted file is still present in the input, so no further
+            // assertion is needed beyond verifying the input shape.
+            assert!(changes.files.iter().any(|f| f.path == deleted_path));
+        }
+    }
+}
+
+/// With a symbol-bearing addition in an unrelated module, the splitter is
+/// likely to propose a split — the deletion must still land in exactly one
+/// group (never duplicated, never dropped).
+#[test]
+fn deleted_file_is_placed_into_a_splitter_group() {
+    let changes = make_staged_changes(vec![
+        make_file_change(
+            "src/services/llm/anthropic.rs",
+            ChangeStatus::Modified,
+            "@@ -0,0 +1,20 @@\n+pub fn brand_new_api() {}\n",
+            20,
+            0,
+        ),
+        make_file_change(
+            "src/services/sanitizer.rs",
+            ChangeStatus::Deleted,
+            "@@ -1,5 +0,0 @@\n-pub fn removed_sanitiser() {}\n",
+            0,
+            5,
+        ),
+    ]);
+
+    let symbols = vec![make_symbol(
+        "brand_new_api",
+        SymbolKind::Function,
+        "src/services/llm/anthropic.rs",
+        true,
+        true,
+    )];
+
+    let result = CommitSplitter::analyze(&changes, &symbols);
+    let deleted_path = PathBuf::from("src/services/sanitizer.rs");
+
+    match result {
+        SplitSuggestion::SuggestSplit(groups) => {
+            let occurrences = groups
+                .iter()
+                .filter(|g| g.files.contains(&deleted_path))
+                .count();
+            assert_eq!(
+                occurrences, 1,
+                "deleted file should appear in exactly one split group: groups={:?}",
+                groups
+            );
+        }
+        SplitSuggestion::SingleCommit => {
+            // A single commit is also acceptable — but the deleted file must
+            // have been surfaced via `changes.files` iteration (the splitter
+            // does not drop files from the input), so we assert the input
+            // invariant for clarity.
+            assert!(changes.files.iter().any(|f| f.path == deleted_path));
+        }
+    }
+}
+
+// ─── ChangeStatus::Renamed fixtures (audit D-037) ───────────────────────────
+
+/// The splitter must key off the *new* path (`path`) for module detection,
+/// not the stale `old_path`. A rename + sibling modification under the same
+/// new module should collapse into a single commit.
+#[test]
+fn renamed_file_grouped_by_new_path_module() {
+    let changes = make_staged_changes(vec![
+        make_renamed_file_with_diff(
+            "src/services/old_module/helper.rs",
+            "src/services/llm/helper.rs",
+            95,
+            "@@ -1,3 +1,3 @@\n-use old_name;\n+use new_name;\n",
+            1,
+            1,
+        ),
+        make_file_change(
+            "src/services/llm/ollama.rs",
+            ChangeStatus::Modified,
+            "@@ -1,3 +1,3 @@\n-use old_name;\n+use new_name;\n",
+            1,
+            1,
+        ),
+    ]);
+
+    let result = CommitSplitter::analyze(&changes, &[]);
+
+    // Rename lives in the same module (`llm`) as the modification under its
+    // new path — splitter should produce a single commit. This asserts the
+    // splitter consults the new path (`path`) for module detection, not the
+    // stale `old_path`.
+    assert!(
+        matches!(result, SplitSuggestion::SingleCommit),
+        "renamed file under the same new-module as a sibling modification should group into a single commit, got {:?}",
+        result
+    );
+
+    // Confirm old_path survived the pipeline unchanged (the splitter uses
+    // `changes.subset` internally which clones FileChange records — so the
+    // original `old_path` must still be present on the input).
+    let renamed = changes
+        .files
+        .iter()
+        .find(|f| f.status == ChangeStatus::Renamed)
+        .expect("renamed file should be present");
+    assert_eq!(
+        renamed.old_path.as_deref(),
+        Some(std::path::Path::new("src/services/old_module/helper.rs")),
+        "old_path must be preserved verbatim",
+    );
+}
+
+/// With a rename beside an unrelated addition, the renamed file must be
+/// referenced by its *new* path in whichever group holds it, and `old_path`
+/// / `rename_similarity` must round-trip unchanged through the splitter input.
+#[test]
+fn renamed_file_is_placed_into_a_splitter_group_with_old_path_preserved() {
+    let rename = make_renamed_file("src/parser/old.rs", "src/parser/new.rs", 88);
+    let changes = make_staged_changes(vec![
+        rename,
+        make_file_change(
+            "src/services/llm/anthropic.rs",
+            ChangeStatus::Modified,
+            "@@ -0,0 +1,20 @@\n+pub fn brand_new_api() {}\n",
+            20,
+            0,
+        ),
+    ]);
+
+    let symbols = vec![make_symbol(
+        "brand_new_api",
+        SymbolKind::Function,
+        "src/services/llm/anthropic.rs",
+        true,
+        true,
+    )];
+
+    let result = CommitSplitter::analyze(&changes, &symbols);
+    let renamed_path = PathBuf::from("src/parser/new.rs");
+
+    // Whether the result is SingleCommit or SuggestSplit, the renamed file
+    // must be represented by its *new* path in the splitter output, and the
+    // original `old_path` must still be reachable via the input `changes`.
+    match &result {
+        SplitSuggestion::SuggestSplit(groups) => {
+            let occurrences = groups
+                .iter()
+                .filter(|g| g.files.contains(&renamed_path))
+                .count();
+            assert_eq!(
+                occurrences, 1,
+                "renamed file should appear under its new path in exactly one group: {:?}",
+                groups
+            );
+        }
+        SplitSuggestion::SingleCommit => {
+            assert!(changes.files.iter().any(|f| f.path == renamed_path));
+        }
+    }
+
+    // old_path must be retrievable from the original StagedChanges the caller
+    // passed in — the splitter borrows, it does not mutate, so this is a
+    // regression guard rather than a behaviour assertion.
+    let renamed = changes
+        .files
+        .iter()
+        .find(|f| f.path == renamed_path)
+        .expect("renamed file should be reachable via new path");
+    assert_eq!(
+        renamed.old_path.as_deref(),
+        Some(std::path::Path::new("src/parser/old.rs")),
+        "old_path for the rename must still be the original path",
+    );
+    assert_eq!(
+        renamed.rename_similarity,
+        Some(88),
+        "rename_similarity must round-trip through the splitter input",
+    );
+}