diff --git a/internal/reviewplan/summary.go b/internal/reviewplan/summary.go index a99af6d..224fdbc 100644 --- a/internal/reviewplan/summary.go +++ b/internal/reviewplan/summary.go @@ -375,7 +375,7 @@ func ReviewersProducedResults(coverage []ReviewerCoverageSummary) map[string]boo func coverageResultProduced(status string) bool { switch strings.TrimSpace(status) { - case "complete_broad", "complete_constrained", "incomplete_skipped": + case "complete_broad", "complete_constrained", "incomplete_skipped", "incomplete_tool": return true default: return false diff --git a/internal/reviewplan/summary_test.go b/internal/reviewplan/summary_test.go index 94ccc24..7b45adf 100644 --- a/internal/reviewplan/summary_test.go +++ b/internal/reviewplan/summary_test.go @@ -456,6 +456,32 @@ func TestRollupSummaryRendering(t *testing.T) { } }) + t.Run("incomplete tool preserves produced results", func(t *testing.T) { + req := summaryRequest() + req.RunSummary.ReviewerCoverage = []ReviewerCoverageSummary{ + {AgentID: "go:implementation-tests", Status: "incomplete_tool", Scope: []string{"main.go"}, InspectedFiles: []string{"main.go"}}, + {AgentID: "policies:conventions", Status: "incomplete_tool", Scope: []string{"main.go"}, InspectedFiles: []string{"main.go"}}, + {AgentID: "failed", Status: "incomplete_failed", Scope: []string{"main.go"}}, + } + req.RunSummary.SelectedReviewers = append(req.RunSummary.SelectedReviewers, "failed") + plan, err := Build(req) + if err != nil { + t.Fatalf("Build: %v", err) + } + for _, want := range []string{ + "| go:implementation-tests | 2 |", + "| policies:conventions | 0 |", + "| failed | ⚠️ did not run |", + "- `go:implementation-tests` — ⚠️ incomplete (tool failure); skipped: none; constraints: none\n", + "- `policies:conventions` — ⚠️ incomplete (tool failure); skipped: none; constraints: none\n", + "- `failed` — ⚠️ failed\n", + } { + if !strings.Contains(plan.RollupMarkdown, want) { + t.Errorf("rollup missing %q:\n%s", want, plan.RollupMarkdown) + } + } + }) + t.Run("incomplete tool reviewer coverage force comment", func(t *testing.T) { req := baseRequest() req.Findings = nil diff --git a/internal/view/review_reviewer_ran_test.go b/internal/view/review_reviewer_ran_test.go index 389317c..fe40063 100644 --- a/internal/view/review_reviewer_ran_test.go +++ b/internal/view/review_reviewer_ran_test.go @@ -18,11 +18,15 @@ func TestReviewSummaryJSONDistinguishesFailedReviewer(t *testing.T) { {Name: "security:code-auditor", Findings: 0}, {Name: "documentation:docs", Findings: 0}, {Name: "policies:conventions", Findings: 0}, + {Name: "tool-zero", Findings: 0}, + {Name: "tool-findings", Findings: 2}, }, Run: reviewplan.RunSummary{ ReviewerCoverage: []reviewplan.ReviewerCoverageSummary{ {AgentID: "security:code-auditor", Status: "incomplete_failed"}, {AgentID: "documentation:docs", Status: "complete_broad"}, + {AgentID: "tool-zero", Status: "incomplete_tool"}, + {AgentID: "tool-findings", Status: "incomplete_tool"}, // policies:conventions absent: status unknown. }, }, @@ -40,6 +44,14 @@ func TestReviewSummaryJSONDistinguishesFailedReviewer(t *testing.T) { if !strings.Contains(got, `"name":"documentation:docs","findings":0,"ran":true`) { t.Fatalf("completed reviewer must serialize ran:true, got:\n%s", got) } + for _, want := range []string{ + `"name":"tool-zero","findings":0,"ran":true`, + `"name":"tool-findings","findings":2,"ran":true`, + } { + if !strings.Contains(got, want) { + t.Errorf("tool-incomplete reviewer must preserve its result %s, got:\n%s", want, got) + } + } // Unknown coverage omits the field rather than guessing a failure. if !strings.Contains(got, `"name":"policies:conventions","findings":0}`) { t.Fatalf("unknown coverage must omit ran, got:\n%s", got)