Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion cmd/prr/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,13 @@ func runAudit(debug bool, args []string) {
opts.SiblingClustering = true
} else if arg == "--bug-priors" {
opts.BugPriors = true
} else if after, ok := strings.CutPrefix(arg, "--min-severity="); ok {
sev, err := review.ParseMinSeverity(after)
if err != nil {
printError(fmt.Errorf("--min-severity=%s: %w", after, err))
os.Exit(1)
}
opts.MinSeverity = sev
} else if arg == "--help" || arg == "-h" {
printAuditUsage()
os.Exit(0)
Expand Down Expand Up @@ -455,6 +462,8 @@ func printAuditUsage() {
fmt.Fprintf(os.Stderr, " --audit-recent=<n> Restrict audit to files touched in the last <n> commits\n")
fmt.Fprintf(os.Stderr, " --sibling-cluster Enable Phase 2.5 sibling-outlier detection (experimental)\n")
fmt.Fprintf(os.Stderr, " --bug-priors Inject recent fix-shaped commits as known-failure priors\n")
fmt.Fprintf(os.Stderr, " --min-severity=<lvl> Drop deep-review findings below <lvl> before recheck\n")
fmt.Fprintf(os.Stderr, " and later phases (critical|high|medium|low|nit)\n")
fmt.Fprintf(os.Stderr, "\n")
fmt.Fprintf(os.Stderr, " %s\n", dim.Render("Available categories:"))
fmt.Fprintf(os.Stderr, " authentication, authorization, input-validation, data-integrity,\n")
Expand All @@ -476,6 +485,7 @@ func runReview(debug bool, args []string) {
post := false
postYes := false
var reviewModeStr string
var minSeverityStr string

// Parse flags and find the PR number
var prNumber string
Expand All @@ -498,6 +508,8 @@ func runReview(debug bool, args []string) {
postYes = true
} else if after, ok := strings.CutPrefix(arg, "--review-mode="); ok {
reviewModeStr = after
} else if after, ok := strings.CutPrefix(arg, "--min-severity="); ok {
minSeverityStr = after
} else if arg == "--help" || arg == "-h" {
printReviewUsage()
os.Exit(0)
Expand All @@ -512,6 +524,12 @@ func runReview(debug bool, args []string) {
os.Exit(1)
}

minSeverity, err := review.ParseMinSeverity(minSeverityStr)
if err != nil {
printError(fmt.Errorf("--min-severity=%s: %w", minSeverityStr, err))
os.Exit(1)
}

if prNumber == "" {
printReviewUsage()
printError(fmt.Errorf("PR number is required: prr review <number>"))
Expand Down Expand Up @@ -583,6 +601,7 @@ func runReview(debug bool, args []string) {
Debug: reviewDebug,
BugPriors: bugPriors,
ReviewMode: reviewMode,
MinSeverity: minSeverity,
}

// Default: shared progress TUI (same as `prr audit`). Falls back
Expand Down Expand Up @@ -873,7 +892,9 @@ func printReviewUsage() {
fmt.Fprintf(os.Stderr, " --bug-priors Inject recent fix-shaped commits as known-failure priors\n")
fmt.Fprintf(os.Stderr, " --review-mode=<mode> full (default) reviews every file; aoi-only skips files without AOIs\n")
fmt.Fprintf(os.Stderr, " --post Post findings to GitHub as a batch review (event=COMMENT)\n")
fmt.Fprintf(os.Stderr, " --yes, -y Skip the confirmation prompt for --post (required in non-TTY contexts)\n\n")
fmt.Fprintf(os.Stderr, " --yes, -y Skip the confirmation prompt for --post (required in non-TTY contexts)\n")
fmt.Fprintf(os.Stderr, " --min-severity=<lvl> Drop deep-review findings below <lvl> before recheck\n")
fmt.Fprintf(os.Stderr, " and later phases (critical|high|medium|low|nit)\n\n")
}

func createAIClient(cfg *config.Config) ai.Client {
Expand Down
22 changes: 22 additions & 0 deletions internal/audit/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ type Options struct {
// into every Phase 3 deep-review prompt and recheck pass. Off by
// default — opt in via the --bug-priors CLI flag.
BugPriors bool

// MinSeverity drops Phase 3 deep-review findings below this
// severity before recheck and all later phases run. One of
// "critical", "high", "medium", "low", "nit". Empty = no filtering.
MinSeverity string
}

// ConcurrencyConfig holds per-phase concurrency caps. Each field is the
Expand Down Expand Up @@ -168,6 +173,11 @@ type Result struct {
// Phase 0 would not reach the executive summary).
ProjectContext string

// MinSeverity records the --min-severity threshold this run was
// filtered at. Empty when no filter was applied. Surfaced in the
// report and consumed by the TUI to render a partial-run badge.
MinSeverity string

// TokenUsage tracks actual token consumption per phase.
Usage PhaseUsage
}
Expand Down Expand Up @@ -611,6 +621,7 @@ func Run(
Routing: routing,
ProjectContext: projectContext,
SkippedSubcategories: routing.SkippedSubcategories(opts.MaxReviews),
MinSeverity: opts.MinSeverity,
}

if routing.TotalAOIs == 0 {
Expand Down Expand Up @@ -666,6 +677,17 @@ func Run(
log.Printf("Phase 3 complete: %d call(s) (%d failed), %d findings, %d dismissals in %v",
len(calls), failed, len(findings), dismissals, time.Since(phase3Start).Round(time.Second))

// Drop findings below the requested minimum severity before recheck
// and every later phase, so the user pays no recheck/synthesis cost
// for findings they've chosen to ignore.
if opts.MinSeverity != "" {
var dropped int
findings, dropped = review.FilterByMinSeverity(findings, opts.MinSeverity)
if dropped > 0 {
log.Printf("Filtered out %d finding(s) below severity %q", dropped, opts.MinSeverity)
}
}

// ── Phase 3b: Recheck — deduplicate and filter findings ─────
dbgw.Phase("PHASE 3b: Recheck")
if len(findings) == 0 {
Expand Down
51 changes: 51 additions & 0 deletions internal/review/execute.go
Original file line number Diff line number Diff line change
Expand Up @@ -1082,3 +1082,54 @@ func severityRank(s string) int {
return 4
}
}

// minSeverityLevels is the canonical severity ladder, ordered
// most-to-least severe. Kept package-private so callers can't mutate
// the shared slice — use AllSeverities for read access.
var minSeverityLevels = []string{"critical", "high", "medium", "low", "nit"}

// AllSeverities returns a copy of the valid severity values
// (most-to-least severe) for the --min-severity flag and any UI
// that needs to render the canonical ladder.
func AllSeverities() []string {
result := make([]string, len(minSeverityLevels))
copy(result, minSeverityLevels)
return result
}

// ParseMinSeverity normalizes and validates a --min-severity value.
// An empty string means "no filtering" and returns "" with no error.
func ParseMinSeverity(s string) (string, error) {
s = strings.TrimSpace(strings.ToLower(s))
if s == "" {
return "", nil
}
switch s {
case "critical", "high", "medium", "low", "nit":
return s, nil
default:
return "", fmt.Errorf("invalid severity %q (valid: %s)", s, strings.Join(minSeverityLevels, ", "))
}
}

// FilterByMinSeverity drops findings whose severity is below min. An
// empty min keeps every finding. The returned slice is always a fresh
// allocation independent of the caller's input — modifying it never
// touches the original. A finding with an unrecognized or empty
// severity ranks as the lowest level ("nit"), matching severityRank.
func FilterByMinSeverity(findings []state.DeepFinding, min string) (kept []state.DeepFinding, dropped int) {
kept = make([]state.DeepFinding, 0, len(findings))
if min == "" {
kept = append(kept, findings...)
return kept, 0
}
threshold := severityRank(min)
for _, f := range findings {
if severityRank(f.Severity) <= threshold {
kept = append(kept, f)
} else {
dropped++
}
}
return kept, dropped
}
97 changes: 97 additions & 0 deletions internal/review/min_severity_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package review

import (
"testing"

"github.com/andreujuanc/prr/internal/state"
)

func TestParseMinSeverity(t *testing.T) {
cases := []struct {
in string
want string
wantErr bool
}{
{"", "", false},
{"high", "high", false},
{"HIGH", "high", false},
{" critical ", "critical", false},
{"nit", "nit", false},
{"bogus", "", true},
{"warning", "", true},
}
for _, c := range cases {
got, err := ParseMinSeverity(c.in)
if c.wantErr {
if err == nil {
t.Errorf("ParseMinSeverity(%q): expected error, got nil", c.in)
}
continue
}
if err != nil {
t.Errorf("ParseMinSeverity(%q): unexpected error %v", c.in, err)
}
if got != c.want {
t.Errorf("ParseMinSeverity(%q) = %q, want %q", c.in, got, c.want)
}
}
}

func TestFilterByMinSeverity(t *testing.T) {
findings := []state.DeepFinding{
{FindingID: "F-1", Severity: "critical"},
{FindingID: "F-2", Severity: "high"},
{FindingID: "F-3", Severity: "medium"},
{FindingID: "F-4", Severity: "low"},
{FindingID: "F-5", Severity: "nit"},
{FindingID: "F-6", Severity: ""}, // ranks as nit
}

t.Run("empty min keeps all and returns an independent slice", func(t *testing.T) {
kept, dropped := FilterByMinSeverity(findings, "")
if dropped != 0 || len(kept) != len(findings) {
t.Fatalf("empty min: kept %d dropped %d, want kept %d dropped 0", len(kept), dropped, len(findings))
}
// Mutating the returned slice must not touch the input. This is
// the contract callers rely on; the previous implementation
// returned the input slice unchanged, which aliased its storage.
kept[0].FindingID = "mutated"
if findings[0].FindingID == "mutated" {
t.Fatalf("empty-min path aliases the input slice; modifying kept[0] mutated findings[0]")
}
})

t.Run("high keeps critical and high", func(t *testing.T) {
kept, dropped := FilterByMinSeverity(findings, "high")
if len(kept) != 2 || dropped != 4 {
t.Fatalf("min high: kept %d dropped %d, want kept 2 dropped 4", len(kept), dropped)
}
for _, f := range kept {
if f.Severity != "critical" && f.Severity != "high" {
t.Errorf("min high kept unexpected severity %q", f.Severity)
}
}
})

t.Run("low drops nit and empty", func(t *testing.T) {
kept, dropped := FilterByMinSeverity(findings, "low")
// critical, high, medium, low kept; nit and "" dropped.
if len(kept) != 4 || dropped != 2 {
t.Fatalf("min low: kept %d dropped %d, want kept 4 dropped 2", len(kept), dropped)
}
})

t.Run("nit keeps all including empty severity", func(t *testing.T) {
kept, dropped := FilterByMinSeverity(findings, "nit")
if len(kept) != len(findings) || dropped != 0 {
t.Fatalf("min nit: kept %d dropped %d, want kept %d dropped 0", len(kept), dropped, len(findings))
}
})

t.Run("critical keeps only critical", func(t *testing.T) {
kept, dropped := FilterByMinSeverity(findings, "critical")
if len(kept) != 1 || dropped != 5 || kept[0].Severity != "critical" {
t.Fatalf("min critical: kept %d dropped %d, want kept 1 dropped 5", len(kept), dropped)
}
})
}
Loading
Loading