diff --git a/changelog.d/added-review-max-perspectives-ui.md b/changelog.d/added-review-max-perspectives-ui.md new file mode 100644 index 0000000000..26d303c4bb --- /dev/null +++ b/changelog.d/added-review-max-perspectives-ui.md @@ -0,0 +1 @@ +- Added a **Max perspectives per PR** control to Settings → Features → Review Gate, so the per-PR perspective cap can be set from the dashboard instead of only in YAML. Without it the cap shipped unreachable: parallel review slots are spent in PR order, so the first PR in a deep queue absorbs every slot and adding reviewers buys more opinions on one PR instead of coverage across many. 0 or empty keeps the existing no-cap default. diff --git a/src/pkg/dashboard/api_config_review.go b/src/pkg/dashboard/api_config_review.go index a81cb6f42f..60b0156271 100644 --- a/src/pkg/dashboard/api_config_review.go +++ b/src/pkg/dashboard/api_config_review.go @@ -29,6 +29,7 @@ func (s *Server) handleReviewConfigPut(w http.ResponseWriter, r *http.Request) { RequireApproval *bool `json:"require_approval"` FanOut *bool `json:"fan_out"` MaxParallelReviews *int `json:"max_parallel_reviews"` + MaxPerspectives *int `json:"max_perspectives_per_pr"` ReviewerAgents *[]string `json:"reviewer_agents"` FixerAgent *string `json:"fixer_agent"` AllAuthors *bool `json:"all_authors"` @@ -45,6 +46,11 @@ func (s *Server) handleReviewConfigPut(w http.ResponseWriter, r *http.Request) { return } + if body.MaxPerspectives != nil && *body.MaxPerspectives < 0 { + jsonError(w, "max_perspectives_per_pr must be >= 0", http.StatusBadRequest) + return + } + cfg := s.deps.Config if body.RequireApproval != nil { cfg.Review.RequireApproval = *body.RequireApproval @@ -55,6 +61,9 @@ func (s *Server) handleReviewConfigPut(w http.ResponseWriter, r *http.Request) { if body.MaxParallelReviews != nil { cfg.Review.MaxParallelReviews = *body.MaxParallelReviews } + if body.MaxPerspectives != nil { + cfg.Review.MaxPerspectivesPerPR = *body.MaxPerspectives + } if body.ReviewerAgents != nil { agents := make([]string, 0, len(*body.ReviewerAgents)) for _, a := range *body.ReviewerAgents { diff --git a/src/pkg/dashboard/api_config_sections_test.go b/src/pkg/dashboard/api_config_sections_test.go index 23def6a799..643ec99f36 100644 --- a/src/pkg/dashboard/api_config_sections_test.go +++ b/src/pkg/dashboard/api_config_sections_test.go @@ -253,6 +253,39 @@ func TestReviewConfigPut_ValidatesAndApplies(t *testing.T) { } } +func TestReviewConfigPut_MaxPerspectivesPerPR(t *testing.T) { + s := covApiServer(t) + + if rec := doPut(s, "/api/config/review", map[string]any{"max_perspectives_per_pr": -1}); rec.Code != http.StatusBadRequest { + t.Fatalf("negative max_perspectives_per_pr: expected 400, got %d", rec.Code) + } + if s.deps.Config.Review.MaxPerspectivesPerPR != 0 { + t.Fatalf("rejected write still mutated config: %d", s.deps.Config.Review.MaxPerspectivesPerPR) + } + + if rec := doPut(s, "/api/config/review", map[string]any{"max_perspectives_per_pr": 1}); rec.Code != http.StatusOK { + t.Fatalf("valid put: expected 200, got %d: %s", rec.Code, rec.Body.String()) + } + if s.deps.Config.Review.MaxPerspectivesPerPR != 1 { + t.Fatalf("cap not applied: %d", s.deps.Config.Review.MaxPerspectivesPerPR) + } + + // Absent key leaves the cap untouched; an explicit 0 clears it back to + // "no cap", which is the documented way to turn the cap off. + if rec := doPut(s, "/api/config/review", map[string]any{}); rec.Code != http.StatusOK { + t.Fatalf("empty put: expected 200, got %d", rec.Code) + } + if s.deps.Config.Review.MaxPerspectivesPerPR != 1 { + t.Fatalf("empty put mutated the cap: %d", s.deps.Config.Review.MaxPerspectivesPerPR) + } + if rec := doPut(s, "/api/config/review", map[string]any{"max_perspectives_per_pr": 0}); rec.Code != http.StatusOK { + t.Fatalf("clearing put: expected 200, got %d", rec.Code) + } + if s.deps.Config.Review.MaxPerspectivesPerPR != 0 { + t.Fatalf("cap not cleared: %d", s.deps.Config.Review.MaxPerspectivesPerPR) + } +} + func TestReviewConfigPut_RejectsNonOwner(t *testing.T) { s := covApiServer(t) if rec := doPutNoRole(s, "/api/config/review", `{"require_approval":true}`); rec.Code != http.StatusForbidden { diff --git a/src/pkg/dashboard/static/index.html b/src/pkg/dashboard/static/index.html index e8f5171cc6..7668df4711 100644 --- a/src/pkg/dashboard/static/index.html +++ b/src/pkg/dashboard/static/index.html @@ -23550,6 +23550,10 @@

Escalation Breaker

+
+ + +
@@ -26650,6 +26654,7 @@

Escalation Breaker

gh72: function (event, A) { markDirty('auto-merge','required_checks',this.value.split(',').map(s=>s.trim()).filter(Boolean)); }, gh73: function (event, A) { markDirty('review','max_parallel_reviews',Number(this.value)||0); }, gh74: function (event, A) { markDirty('review','reviewer_agents',this.value.split(',').map(s=>s.trim()).filter(Boolean)); }, + gh81: function (event, A) { markDirty('review','max_perspectives_per_pr',Number(this.value)||0); }, gh77: function (event, A) { projectObservabilityPlatformsChanged(this); }, gh78: function (event, A) { projectObservabilityReferenceChanged(this); }, gh79: function (event, A) { markDirty('repos','maxIssuesPerKick',Number(this.value)); },