From 0275a29961f7c4acfd1206c99ed2e6a567a8934c Mon Sep 17 00:00:00 2001 From: Andrew Anderson Date: Fri, 18 Sep 2026 00:53:34 -0400 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20feat(dashboard):=20expose=20the=20p?= =?UTF-8?q?er-PR=20review=20perspective=20cap=20in=20the=20Review=20Gate?= =?UTF-8?q?=20dialog?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit review.max_perspectives_per_pr shipped with no way to set it from the dashboard: the Review Gate section renders require_approval, fan_out, max_parallel_reviews, reviewer_agents, all_authors, acknowledge_no_findings, fixer_agent and human_decision_label, and the PUT handler accepts exactly those keys. The cap was reachable only by hand-editing YAML, which on a hosted spoke means editing a file the dashboard overlay then rewrites. That made the lever effectively unavailable on the deployment that needs it most. Parallel review slots are a fixed budget spent in PR order, so without a cap the first PR in a deep queue absorbs every slot for its own perspectives — adding reviewers buys more opinions on one PR instead of coverage across many, and that one PR collects every comment at once now that reviewers publish. Adds the field to the PUT body with the same >= 0 validation max_parallel_reviews uses, and a number input to the Review Gate section. 0 or empty is unchanged: no cap, fan every perspective out at once. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Andrew Anderson --- .../added-review-max-perspectives-ui.md | 1 + src/pkg/dashboard/api_config_review.go | 9 +++++ src/pkg/dashboard/api_config_sections_test.go | 33 +++++++++++++++++++ src/pkg/dashboard/static/index.html | 5 +++ 4 files changed, 48 insertions(+) create mode 100644 changelog.d/added-review-max-perspectives-ui.md 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 000000000..26d303c4b --- /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 a81cb6f42..60b015627 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 23def6a79..643ec99f3 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 e8f5171cc..7668df471 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)); },