fix(pipeline): exclude deleted files from reviewer coverage

internal/pipeline/pipeline.go

@@ -861,7 +861,7 @@ func executeLLMPhases(ctx context.Context, opts Options, req Request, mode execu
 	result.Findings = findings
 	result.ReviewerFailures = reviewerFailures
 	result.reviewerFastDelivered = reviewerFastDelivery(prepared.fastRequested, reviewerSessions)
-	reviewerCoverage := buildReviewerCoverage(selection.SelectedAgents, reviewerResults, reviewerFailures, prepared.changedFiles, reviewerToolEvidenceByAgent(reviewerSessions))
+	reviewerCoverage := buildReviewerCoverage(selection.SelectedAgents, reviewerResults, reviewerFailures, prepared.changedFiles, deletedPatchPaths(prepared.parsed.Patches), reviewerToolEvidenceByAgent(reviewerSessions))
 	result.ReviewerCoverage = reviewerCoverage
 	result.Sessions = appendSessionsIfPresent(result.Sessions, reviewerLedgerSessions...)
 
@@ -2486,13 +2486,46 @@ func filterReviewableFiles(files []string) []string {
 	return out
 }
 
-func buildReviewerCoverage(selected []llm.SelectedAgent, results []llm.Findings, failures []ReviewerFailure, changedFiles []string, toolEvidence ...map[string]*llm.ReviewerToolEvidence) []reviewplan.ReviewerCoverageSummary {
+// deletedPatchPaths returns the set of paths removed by the diff. A deleted file
+// has no content at head for a reviewer to inspect, so it is exempt from
+// coverage accounting just like a generated lockfile — otherwise a skipped
+// deletion marks the reviewer incomplete_skipped and blocks approval on an
+// otherwise clean review.
+func deletedPatchPaths(patches []FilePatch) map[string]bool {
+	var deleted map[string]bool
+	for _, patch := range patches {
+		if patch.Deleted {
+			if deleted == nil {
+				deleted = map[string]bool{}
+			}
+			deleted[patch.Path] = true
+		}
+	}
+	return deleted
+}
+
+// excludeFiles returns values with any member of exclude removed, preserving order.
+func excludeFiles(values []string, exclude map[string]bool) []string {
+	if len(exclude) == 0 {
+		return values
+	}
+	out := make([]string, 0, len(values))
+	for _, value := range values {
+		if !exclude[value] {
+			out = append(out, value)
+		}
+	}
+	return out
+}
+
+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
 	}
-	// Generated lockfiles are not a review obligation: exclude them so neither a
-	// reviewer that skips one nor an unassigned lockfile blocks approval.
-	changedFiles = filterReviewableFiles(changedFiles)
+	// Generated lockfiles and deleted files are not a review obligation: exclude
+	// them so neither a reviewer that skips one nor an unassigned one blocks
+	// approval. (Deleted files have no content at head to inspect.)
+	changedFiles = excludeFiles(filterReviewableFiles(changedFiles), deleted)
 	resultByAgent := make(map[string]llm.Findings, len(results))
 	for _, result := range results {
 		resultByAgent[result.AgentID] = result
@@ -2504,9 +2537,10 @@ func buildReviewerCoverage(selected []llm.SelectedAgent, results []llm.Findings,
 	assigned := map[string]bool{}
 	out := make([]reviewplan.ReviewerCoverageSummary, 0, len(selected)+1)
 	for _, agent := range selected {
-		// A lockfile explicitly assigned to an agent is exempt too — the scope
-		// is what the reviewer is held to, and lockfiles are not reviewable.
-		scope := filterReviewableFiles(reviewerAssignmentScope(agent, changedFiles))
+		// A lockfile or deleted file explicitly assigned to an agent is exempt
+		// too — the scope is what the reviewer is held to, and neither is
+		// reviewable content at head.
+		scope := excludeFiles(filterReviewableFiles(reviewerAssignmentScope(agent, changedFiles)), deleted)
 		for _, file := range scope {
 			assigned[file] = true
 		}

internal/pipeline/pipeline_test.go

@@ -5060,6 +5060,7 @@ func TestBuildReviewerCoverageStatuses(t *testing.T) {
 		[]llm.Findings{{AgentID: "harness:broad", InspectedFiles: []string{"main.go", "other.go"}}},
 		nil,
 		[]string{"main.go", "other.go"},
+		nil,
 	)
 	if len(broad) != 1 || broad[0].Status != reviewerCoverageCompleteBroad {
 		t.Fatalf("broad coverage = %#v, want complete broad", broad)
@@ -5076,7 +5077,7 @@ func TestBuildReviewerCoverageStatuses(t *testing.T) {
 	}
 	failures := []ReviewerFailure{{AgentID: "harness:failed", Error: "model failed"}}
 
-	got := buildReviewerCoverage(selected, results, failures, []string{"api.go", "db.sql", "unassigned.go", "worker.go"})
+	got := buildReviewerCoverage(selected, results, failures, []string{"api.go", "db.sql", "unassigned.go", "worker.go"}, nil)
 	byAgent := map[string]reviewplan.ReviewerCoverageSummary{}
 	for _, entry := range got {
 		byAgent[entry.AgentID] = entry
@@ -5108,7 +5109,7 @@ func TestBuildReviewerCoverageExemptsGeneratedLockfiles(t *testing.T) {
 	results := []llm.Findings{
 		{AgentID: "rust:impl", InspectedFiles: []string{"main.go"}, SkippedFiles: []string{"Cargo.lock"}},
 	}
-	got := buildReviewerCoverage(selected, results, nil, []string{"main.go", "Cargo.lock", "yarn.lock"})
+	got := buildReviewerCoverage(selected, results, nil, []string{"main.go", "Cargo.lock", "yarn.lock"}, nil)
 	if len(got) != 1 {
 		t.Fatalf("coverage = %#v, want a single reviewer entry (no lockfile coverage rows)", got)
 	}
@@ -5128,12 +5129,42 @@ func TestBuildReviewerCoverageExemptsGeneratedLockfiles(t *testing.T) {
 		[]llm.Findings{{AgentID: "rust:impl", InspectedFiles: []string{"Cargo.lock", "main.go"}}},
 		nil,
 		[]string{"main.go", "Cargo.lock"},
+		nil,
 	)
 	if len(inspectedLock) != 1 || !reflect.DeepEqual(inspectedLock[0].InspectedFiles, []string{"main.go"}) {
 		t.Fatalf("inspected files = %#v, want [main.go] with the lockfile dropped", inspectedLock)
 	}
 }
 
+func TestBuildReviewerCoverageExemptsDeletedFiles(t *testing.T) {
+	// A reviewer that skips a deleted file (no content at head to inspect) is
+	// complete, not incomplete_skipped, and a deleted file left unassigned is
+	// not incomplete_unassigned — same coverage exemption as a lockfile.
+	deleted := map[string]bool{"removed.go": true, "gone.go": true}
+	got := buildReviewerCoverage(
+		[]llm.SelectedAgent{{AgentID: "harness:reviewer", Files: []string{"main.go", "removed.go"}}},
+		[]llm.Findings{{
+			AgentID:        "harness:reviewer",
+			InspectedFiles: []string{"main.go"},
+			SkippedFiles:   []string{"removed.go"},
+		}},
+		nil,
+		[]string{"main.go", "removed.go", "gone.go"},
+		deleted,
+	)
+	if len(got) != 1 {
+		t.Fatalf("coverage = %#v, want a single reviewer entry (no deleted-file rows)", got)
+	}
+	if got[0].AgentID != "harness:reviewer" || got[0].Status != reviewerCoverageCompleteBroad {
+		t.Fatalf("coverage = %#v, want complete despite skipped deletion", got)
+	}
+	if len(got[0].SkippedFiles) != 0 {
+		t.Fatalf("skipped files = %#v, want none (removed.go is exempt from coverage)", got[0].SkippedFiles)
+	}
+	// gone.go (deleted, unassigned) must not surface as an incomplete_unassigned
+	// coverage row that would block approval.
+}
+
 func TestEnsureSelectedGlobCoverageSkipsLockfiles(t *testing.T) {
 	// A changed lockfile that matches an agent's globs must NOT be force-assigned
 	// into that agent's scope — it is exempt from the coverage universe. Without
@@ -5163,6 +5194,7 @@ func TestBuildReviewerCoverageUsesTypedToolEvidenceInsteadOfModelConstraint(t *t
 		}},
 		nil,
 		[]string{"main.go"},
+		nil,
 		map[string]*llm.ReviewerToolEvidence{
 			"harness:reviewer": {DiffStatus: llm.DiffToolStatusSucceeded},
 		},
@@ -5201,6 +5233,7 @@ func TestBuildReviewerCoverageMarksAssignedScopeMissing(t *testing.T) {
 		[]llm.Findings{{AgentID: "harness:reviewer", InspectedFiles: []string{"main.go"}}},
 		nil,
 		[]string{"main.go", "other.go"},
+		nil,
 	)
 	if len(got) != 1 {
 		t.Fatalf("coverage entries = %#v", got)
@@ -5326,6 +5359,7 @@ func TestRebaseReviewerCohortLeavesInherentlyUnmatchedFilesUnassigned(t *testing
 		[]llm.Findings{{AgentID: "repo:go", InspectedFiles: []string{"main.go"}}},
 		nil,
 		[]string{"main.go", ".github/workflows/ci.yml"},
+		nil,
 	)
 	if len(coverage) != 2 || coverage[1].AgentID != "unassigned" || coverage[1].Status != reviewerCoverageIncompleteUnassigned ||
 		!reflect.DeepEqual(coverage[1].SkippedFiles, []string{".github/workflows/ci.yml"}) {
@@ -5370,7 +5404,7 @@ func TestRebaseReviewerCohortOnlyInherentlyUnmatchedFileRemainsUnassigned(t *tes
 	if len(selection.SelectedAgents) != 0 || len(resumes) != 0 {
 		t.Fatalf("reused cohort selection = %#v resumes = %#v, want no assigned reviewers or resumes", selection.SelectedAgents, resumes)
 	}
-	coverage := buildReviewerCoverage(selection.SelectedAgents, nil, nil, changedFiles)
+	coverage := buildReviewerCoverage(selection.SelectedAgents, nil, nil, changedFiles, nil)
 	wantCoverage := []reviewplan.ReviewerCoverageSummary{{
 		AgentID:      "unassigned",
 		Status:       reviewerCoverageIncompleteUnassigned,
@@ -5526,6 +5560,7 @@ func TestBuildReviewerCoverageAllowsBroadReviewerSplitAssignments(t *testing.T)
 		},
 		nil,
 		[]string{"main.go", "other.go"},
+		nil,
 	)
 	if len(got) != 2 {
 		t.Fatalf("coverage entries = %#v, want two reviewer entries", got)
@@ -5543,6 +5578,7 @@ func TestBuildReviewerCoverageIgnoresSkippedFilesOutsideAssignmentScope(t *testi
 		[]llm.Findings{{AgentID: "harness:reviewer", InspectedFiles: []string{"main.go"}, SkippedFiles: []string{"other.go"}}},
 		nil,
 		[]string{"main.go", "other.go"},
+		nil,
 	)
 	if len(got) != 2 {
 		t.Fatalf("coverage entries = %#v, want reviewer plus unassigned other.go", got)