fix(review): Preserve deletion metadata in reconstructed diffs

internal/gitprovider/github/rest.go

@@ -179,9 +179,21 @@ func reconstructUnifiedDiff(files []pullFileResponse) string {
 		}
 		if f.Patch == "" {
 			// A pure rename has no patch and is already fully described above.
-			// Anything else without a patch is a binary or oversized file; mark
-			// it so the change stays visible even though there are no hunks.
-			if f.Status != "renamed" {
+			// A removed file also has no patch when it is binary or individually
+			// oversized. Preserve its deletion metadata instead of falling through
+			// to the generic binary marker; otherwise the diff parser classifies
+			// the removal as a modified file and reviewer coverage includes the
+			// deleted path.
+			switch f.Status {
+			case "removed":
+				fmt.Fprintf(&b, "--- a/%s\n", oldPath)
+				b.WriteString("+++ /dev/null\n")
+			case "renamed":
+				// The rename metadata above fully describes a pure rename.
+			default:
+				// Anything else without a patch is a binary or oversized file;
+				// mark it so the change stays visible even though there are no
+				// hunks.
 				fmt.Fprintf(&b, "Binary files a/%s and b/%s differ\n", oldPath, newPath)
 			}
 			continue

internal/gitprovider/github/rest_test.go

@@ -459,6 +459,8 @@ func TestReconstructUnifiedDiff(t *testing.T) {
 		{Filename: "mod.go", Status: "modified", Patch: "@@ -1,2 +1,2 @@\n line\n-old\n+new"},
 		{Filename: "rn.go", PreviousFilename: "ro.go", Status: "renamed"}, // pure rename: no patch
 		{Filename: "bin.dat", Status: "added"},                            // added, no patch: binary
+		{Filename: "removed.txt", Status: "removed"},                      // removed, no patch: oversized text
+		{Filename: "removed.bin", Status: "removed"},                      // removed, no patch: binary
 	})
 	cases := []struct {
 		substr string
@@ -468,7 +470,11 @@ func TestReconstructUnifiedDiff(t *testing.T) {
 		{"diff --git a/ro.go b/rn.go\nrename from ro.go\nrename to rn.go\n", true},
 		{"Binary files a/ro.go", false}, // a pure rename must not be marked binary
 		{"diff --git a/bin.dat b/bin.dat\nBinary files a/bin.dat and b/bin.dat differ\n", true},
-		{"+y", false}, // the empty-filename entry must be dropped entirely
+		{"diff --git a/removed.txt b/removed.txt\n--- a/removed.txt\n+++ /dev/null\n", true},
+		{"diff --git a/removed.bin b/removed.bin\nBinary files a/removed.bin and b/removed.bin differ\n", false},
+		{"diff --git a/removed.bin b/removed.bin\n--- a/removed.bin\n+++ /dev/null\n", true},
+		{"deleted file mode", false}, // no provider mode data: never fabricate one
+		{"+y", false},                // the empty-filename entry must be dropped entirely
 	}
 	for _, c := range cases {
 		if strings.Contains(got, c.substr) != c.want {

internal/pipeline/diff_test.go

@@ -156,3 +156,28 @@ func TestParseUnifiedDiffRejectsBadHunkHeader(t *testing.T) {
 		t.Fatal("parseUnifiedDiff error = nil, want bad hunk failure")
 	}
 }
+
+func TestParseUnifiedDiffPreservesPatchlessRemovedStatus(t *testing.T) {
+	raw := strings.Join([]string{
+		"diff --git a/removed.txt b/removed.txt",
+		"--- a/removed.txt",
+		"+++ /dev/null",
+		"diff --git a/removed.bin b/removed.bin",
+		"--- a/removed.bin",
+		"+++ /dev/null",
+		"",
+	}, "\n")
+
+	parsed, err := parseUnifiedDiff(raw)
+	if err != nil {
+		t.Fatalf("parseUnifiedDiff: %v", err)
+	}
+	if len(parsed.Patches) != 2 {
+		t.Fatalf("patches = %#v, want two patchless deletions", parsed.Patches)
+	}
+	for _, patch := range parsed.Patches {
+		if !patch.Deleted || patch.Binary || patch.Path == "" || patch.OldPath != patch.Path || len(patch.Hunks) != 0 {
+			t.Fatalf("patchless deletion = %#v, want Deleted with no Binary flag or hunks", patch)
+		}
+	}
+}

internal/pipeline/pipeline.go

@@ -780,7 +780,7 @@ func executeLLMPhases(ctx context.Context, opts Options, req Request, mode execu
 	}
 
 	cohortScope := ledger.ReviewerCohortScope{PRKey: prepared.prKey, Profile: req.ProfileName, PostingIdentity: runlifecycle.PostingKey(req.PostingIdentity)}
-	selection, reviewerResumeIDs, reusedCohort, err := loadReviewerCohort(ctx, opts, req, cohortScope, prepared.catalog, prepared.changedFiles, maxAgents)
+	selection, reviewerResumeIDs, reusedCohort, err := loadReviewerCohort(ctx, opts, req, cohortScope, prepared.catalog, reviewablePatchPaths(prepared.parsed.Patches), maxAgents)
 	if err != nil {
 		return executionPhaseFailure(err)
 	}
@@ -1254,6 +1254,14 @@ func runSelectionPhase(ctx context.Context, opts Options, req selectionPhaseRequ
 	if err != nil {
 		return llm.Selection{}, sessionDraft{}, ledger.Session{}, err
 	}
+	// Deleted files remain in the dossier so the change is visible to the
+	// orchestrator, but they are not reviewer obligations: there is no file at
+	// the head for a reviewer to inspect. Keep them out of the assignment
+	// contract and the post-selection backstops, matching buildReviewerCoverage.
+	reviewerFiles := reviewablePatchPaths(req.ParsedDiff.Patches)
+	// Citing a removed or pre-rename path must not fail the whole selection.
+	selectableFiles := append(append([]string(nil), reviewerFiles...), mentionableExtraPaths(req.ParsedDiff.Patches)...)
+	promptInput.ChangedFiles = append([]string(nil), reviewerFiles...)
 	dependencyTaskIDs := []string{dossier.SummaryTaskID}
 	fingerprintDeps := append(append([]string(nil), dependencyTaskIDs...), promptDeps...)
 	selectionPrompt, err := buildSelectionPrompt(req.Catalog, promptInput, req.MaxAgents, req.SelectionPromptInstructions)
@@ -1270,7 +1278,7 @@ func runSelectionPhase(ctx context.Context, opts Options, req selectionPhaseRequ
 	decode := func(data []byte) (llm.Selection, error) {
 		return llm.DecodeSelection(data, llm.SelectionOptions{
 			KnownAgents:  knownAgents(req.Catalog),
-			ChangedFiles: changedFiles(req.ParsedDiff.Patches),
+			ChangedFiles: stringSet(selectableFiles),
 			KnownThreads: knownThreadIDs,
 		})
 	}
@@ -1303,7 +1311,8 @@ func runSelectionPhase(ctx context.Context, opts Options, req selectionPhaseRequ
 	if err != nil {
 		return llm.Selection{}, selectionSession, ledgerSession, err
 	}
-	changed := patchPaths(req.ParsedDiff.Patches)
+	changed := reviewerFiles
+	selection = filterSelectedReviewerAssignments(selection, changed)
 	selection = ensureRequiredOnMatchAgents(selection, req.Catalog, changed)
 	selection, err = opts.capSelectionAgents(selection, req.Catalog, changed, req.MaxAgents)
 	if err != nil {
@@ -2056,8 +2065,10 @@ func runReviewer(ctx context.Context, opts Options, req Request, runID string, p
 		return llm.Findings{}, sessionDraft{}, ledger.Session{}, nil, Failure(FailureTerminal, err)
 	}
 	model, effort := runtimeConfig.model, runtimeConfig.effort
-	changedFilePaths := patchPaths(parsed.Patches)
-	assignmentScope := reviewerAssignmentScope(selected, changedFilePaths)
+	changedFilePaths := reviewablePatchPaths(parsed.Patches)
+	selected = filterSelectedReviewerAssignment(selected, changedFilePaths)
+	// A reviewer may cite its own assignment plus unassignable paths, nothing else.
+	citableFiles := append(append([]string(nil), reviewerAssignmentScope(selected, changedFilePaths)...), mentionableExtraPaths(parsed.Patches)...)
 	prompt, promptDeps, err := buildReviewerPrompt(artifacts, pr, selected, agent, changedFilePaths, resumeState.discussion)
 	if err != nil {
 		return llm.Findings{}, sessionDraft{}, ledger.Session{}, nil, Failure(FailureTerminal, err)
@@ -2108,7 +2119,7 @@ func runReviewer(ctx context.Context, opts Options, req Request, runID string, p
 	}, func(data []byte) (llm.Findings, error) {
 		return llm.DecodeFindings(data, llm.FindingsOptions{
 			KnownAgents:  map[string]bool{agent.ID: true},
-			ChangedFiles: stringSet(assignmentScope),
+			ChangedFiles: stringSet(citableFiles),
 			NewFindingID: opts.newFindingID,
 		})
 	})
@@ -2622,6 +2633,49 @@ func deletedPatchPaths(patches []FilePatch) map[string]bool {
 	return deleted
 }
 
+// renamedPatchOldPaths returns the pre-rename paths still present in the diff.
+// They are not reviewer obligations, but an orchestrator may cite one because
+// the "rename from" header is visible in the dossier diff.
+func renamedPatchOldPaths(patches []FilePatch) []string {
+	paths := make([]string, 0, len(patches))
+	for _, patch := range patches {
+		if patch.Deleted || patch.OldPath == "" || patch.OldPath == patch.Path {
+			continue
+		}
+		paths = append(paths, patch.OldPath)
+	}
+	sort.Strings(paths)
+	return paths
+}
+
+// mentionableExtraPaths returns paths a model may cite but is never assigned:
+// removed files and the pre-rename sources still visible in the diff.
+func mentionableExtraPaths(patches []FilePatch) []string {
+	deleted := deletedPatchPaths(patches)
+	seen := make(map[string]bool, len(deleted))
+	paths := make([]string, 0, len(patches))
+	for path := range deleted {
+		seen[path] = true
+		paths = append(paths, path)
+	}
+	for _, path := range renamedPatchOldPaths(patches) {
+		if seen[path] {
+			continue
+		}
+		seen[path] = true
+		paths = append(paths, path)
+	}
+	sort.Strings(paths)
+	return paths
+}
+
+// reviewablePatchPaths returns changed paths that a reviewer can inspect at
+// the head. Deleted paths remain in ParsedDiff and the dossier, but are not a
+// reviewer assignment or coverage obligation.
+func reviewablePatchPaths(patches []FilePatch) []string {
+	return excludeFiles(patchPaths(patches), deletedPatchPaths(patches))
+}
+
 // excludeFiles returns values with any member of exclude removed, preserving order.
 func excludeFiles(values []string, exclude map[string]bool) []string {
 	if len(exclude) == 0 {
@@ -2636,6 +2690,43 @@ func excludeFiles(values []string, exclude map[string]bool) []string {
 	return out
 }
 
+func filterAssignmentFiles(files, changedFiles []string) []string {
+	changed := stringSet(changedFiles)
+	var filtered []string
+	for _, file := range files {
+		if changed[file] && !slices.Contains(filtered, file) {
+			filtered = append(filtered, file)
+		}
+	}
+	return copySortedStrings(filtered)
+}
+
+func filterSelectedReviewerAssignment(selected llm.SelectedAgent, changedFiles []string) llm.SelectedAgent {
+	selected.Files = filterAssignmentFiles(selected.Files, changedFiles)
+	selected.AllowedFiles = filterAssignmentFiles(selected.AllowedFiles, changedFiles)
+	return selected
+}
+
+// filterSelectedReviewerAssignments removes deleted paths from explicit
+// assignments and drops a selected reviewer whose only assignment was
+// deleted. Broad selections remain broad when reviewable paths exist.
+func filterSelectedReviewerAssignments(selection llm.Selection, changedFiles []string) llm.Selection {
+	filtered := selection
+	filtered.SelectedAgents = nil
+	if len(changedFiles) == 0 {
+		return filtered
+	}
+	for _, selected := range selection.SelectedAgents {
+		hadExplicitAssignment := len(selected.Files) > 0 || len(selected.AllowedFiles) > 0
+		selected = filterSelectedReviewerAssignment(selected, changedFiles)
+		if hadExplicitAssignment && len(selected.Files) == 0 && len(selected.AllowedFiles) == 0 {
+			continue
+		}
+		filtered.SelectedAgents = append(filtered.SelectedAgents, selected)
+	}
+	return filtered
+}
+
 func buildReviewerCoverage(selected []llm.SelectedAgent, results []llm.Findings, failures []ReviewerFailure, changedFiles []string, deleted map[string]bool, toolEvidence ...map[string]*llm.ReviewerToolEvidence) []reviewplan.ReviewerCoverageSummary {
 	if len(selected) == 0 && len(changedFiles) == 0 {
 		return nil
@@ -3217,19 +3308,6 @@ func knownAgents(catalog agents.Catalog) map[string]bool {
 	return setBy(catalog.Agents, func(agent agents.Agent) string { return agent.ID })
 }
 
-func changedFiles(patches []FilePatch) map[string]bool {
-	paths := make([]string, 0, len(patches)*2)
-	for _, patch := range patches {
-		if patch.Path != "" {
-			paths = append(paths, patch.Path)
-		}
-		if patch.OldPath != "" {
-			paths = append(paths, patch.OldPath)
-		}
-	}
-	return stringSet(paths)
-}
-
 func knownThreads(threads []gitprovider.InlineThread) map[string]bool {
 	return threadIDSet(threads, func(thread gitprovider.InlineThread) gitprovider.ThreadID { return thread.ID })
 }