Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4a82861
docs: spec for min_severity cascading + review-level filter
cpcloud Apr 7, 2026
4a7c91f
docs: address review findings on min_severity spec
cpcloud Apr 7, 2026
bba3056
docs: address normalization, worktree-path, task-skip findings on min…
cpcloud Apr 7, 2026
dc4a55b
docs: address rebase, daemon-boundary, legacy-task findings on min_se…
cpcloud Apr 7, 2026
bcabfff
docs: implementation plan for min-severity cascading
cpcloud Apr 7, 2026
5ee28ae
docs: remove git commit steps from min-severity implementation plan
cpcloud Apr 7, 2026
aec659e
fix(spec): use normalizeMinSeverityForWrite at all write sites in SQL…
cpcloud Apr 7, 2026
13b40d5
fix(spec,plan): add BatchUpsertJobs, CI batch path, and worktree tests
cpcloud Apr 7, 2026
1229692
fix(plan): correct BatchUpsertJobs column order, worktree and legacy …
cpcloud Apr 7, 2026
40e1651
feat(config): add global min-severity fields and ResolveReviewMinSeve…
cpcloud Apr 7, 2026
503560a
feat: implement min-severity cascading and review-level filter
cpcloud Apr 7, 2026
4e19f75
fix(lint): use assert.Empty for empty string checks
cpcloud Apr 7, 2026
6bf0983
fix(storage): scan min_severity into hydration fields in GetJobsWithR…
cpcloud Apr 7, 2026
afb7379
fix(ci): separate review-level and synthesis min-severity in CI path
cpcloud Apr 7, 2026
001effc
fix(ci): use stricter of review and CI severity for prompt injection
cpcloud Apr 7, 2026
baa0000
fix(ci): route single-result CI through synthesis when min-severity set
cpcloud Apr 7, 2026
23a3ad5
fix(review): treat low min-severity same as empty in single-result path
cpcloud Apr 7, 2026
6a726cb
fix(review): treat SEVERITY_THRESHOLD_MET as pass in single-result fo…
cpcloud Apr 7, 2026
3ab44ee
fix(review): use ParseVerdict for pass detection in single-result for…
cpcloud Apr 7, 2026
6141603
fix(verdict): require SEVERITY_THRESHOLD_MET marker to stand alone
cpcloud Apr 7, 2026
6d9f02b
fix: support single * and _ italic wrappers in IsMarkerOnlyOutput
cpcloud Apr 7, 2026
c3ff148
fix(lint): use strings.CutPrefix in IsMarkerOnlyOutput
cpcloud Apr 7, 2026
26c5072
fix(lint): use strings.Cut for newline split in IsMarkerOnlyOutput
cpcloud Apr 7, 2026
f338590
Remove specs
wesm Apr 7, 2026
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
21 changes: 15 additions & 6 deletions cmd/roborev/ci.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,31 +189,40 @@ func runCIReview(ctx context.Context, opts ciReviewOpts) error {
return err
}

// Resolve min severity
minSev, err := config.ResolveCIMinSeverity(
// Resolve min severity for synthesis output filtering
ciMinSev, err := config.ResolveCIMinSeverity(
opts.minSeverity, repoCfg, globalCfg)
if err != nil {
return err
}

// Resolve review-level min severity for prompt injection
reviewMinSev, err := config.ResolveReviewMinSeverity(
"", root, globalCfg)
if err != nil {
return err
}

// Resolve synthesis agent
synthAgent := config.ResolveCISynthesisAgent(
opts.synthesisAgent, repoCfg, globalCfg)

log.Printf(
"ci review: ref=%s agents=%v types=%v "+
"reasoning=%s min_severity=%s",
"reasoning=%s ci_min_severity=%s review_min_severity=%s",
gitRef, agents, reviewTypes,
reasoningLevel, minSev)
reasoningLevel, ciMinSev, reviewMinSev)

// Run batch
// Run batch with review-level severity for prompt injection.
// CI-level severity is applied separately during synthesis.
batchCfg := review.BatchConfig{
RepoPath: root,
GitRef: gitRef,
Agents: agents,
ReviewTypes: reviewTypes,
Reasoning: reasoningLevel,
GlobalConfig: globalCfg,
MinSeverity: reviewMinSev,
}

results := review.RunBatch(ctx, batchCfg)
Expand All @@ -225,7 +234,7 @@ func runCIReview(ctx context.Context, opts ciReviewOpts) error {
comment, synthErr := review.Synthesize(
ctx, results, review.SynthesizeOpts{
Agent: synthAgent,
MinSeverity: minSev,
MinSeverity: ciMinSev,
RepoPath: root,
GitRef: gitRef,
HeadSHA: headSHA,
Expand Down
9 changes: 6 additions & 3 deletions cmd/roborev/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -799,8 +799,9 @@ func fixSingleJob(cmd *cobra.Command, repoRoot string, jobID int64, opts fixOpti
// task/analyze jobs have free-form output without severity labels)
var minSev string
if !job.IsTaskJob() {
fixCfg, _ := config.LoadGlobal()
minSev, err = config.ResolveFixMinSeverity(
opts.minSeverity, repoRoot,
opts.minSeverity, repoRoot, fixCfg,
)
if err != nil {
return fmt.Errorf("resolve min-severity: %w", err)
Expand Down Expand Up @@ -1011,11 +1012,14 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches,
return err
}

// Load global config for severity + prompt-size resolution.
cfg, _ := config.LoadGlobal()

// Resolve minimum severity filter. Suppress if any entry is a
// task job — task/analyze output has no severity labels, so the
// instruction would confuse the agent for those entries.
minSev, err := config.ResolveFixMinSeverity(
opts.minSeverity, roots.worktreeRoot,
opts.minSeverity, roots.worktreeRoot, cfg,
)
if err != nil {
return fmt.Errorf("resolve min-severity: %w", err)
Expand All @@ -1031,7 +1035,6 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches,

// Split into batches by prompt size (after severity resolution
// so the severity instruction overhead is accounted for)
cfg, _ := config.LoadGlobal()
maxSize := config.ResolveMaxPromptSize(roots.worktreeRoot, cfg)
batches := splitIntoBatches(entries, maxSize, minSev)

Expand Down
10 changes: 5 additions & 5 deletions cmd/roborev/refine.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ func runRefine(ctx RunContext, opts refineOptions) error {

// Resolve minimum severity filter
minSev, err := config.ResolveRefineMinSeverity(
opts.minSeverity, repoPath,
opts.minSeverity, repoPath, cfg,
)
if err != nil {
return fmt.Errorf("resolve min-severity: %w", err)
Expand Down Expand Up @@ -616,10 +616,10 @@ func runRefine(ctx RunContext, opts refineOptions) error {

// When severity filtering is active and the agent
// signals all findings are below threshold, treat as
// resolved rather than a fix failure.
if minSev != "" && strings.Contains(
output, config.SeverityThresholdMarker,
) {
// resolved rather than a fix failure. Require the
// marker to stand alone so prose findings echoed
// alongside it cannot silently close the review.
if minSev != "" && config.IsMarkerOnlyOutput(output) {
fmt.Println(
"All findings below severity " +
"threshold - closing review",
Expand Down
47 changes: 28 additions & 19 deletions cmd/roborev/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,22 @@ const MaxDirtyDiffSize = 200 * 1024

func reviewCmd() *cobra.Command {
var (
repoPath string
sha string
agent string
model string
reasoning string
reviewType string
fast bool
quiet bool
dirty bool
wait bool
branch string
baseBranch string
since string
local bool
provider string
repoPath string
sha string
agent string
model string
reasoning string
reviewType string
fast bool
quiet bool
dirty bool
wait bool
branch string
baseBranch string
since string
local bool
provider string
minSeverity string
)

cmd := &cobra.Command{
Expand Down Expand Up @@ -270,7 +271,7 @@ Examples:

// Handle --local mode: run agent directly without daemon
if local {
return runLocalReview(cmd, root, gitRef, diffContent, agent, model, provider, reasoning, reviewType, quiet)
return runLocalReview(cmd, root, gitRef, diffContent, agent, model, provider, reasoning, reviewType, quiet, minSeverity)
}

// Build request body
Expand All @@ -284,6 +285,7 @@ Examples:
Reasoning: reasoning,
ReviewType: reviewType,
DiffContent: diffContent,
MinSeverity: minSeverity,
}

reqBody, _ := json.Marshal(reqFields)
Expand Down Expand Up @@ -358,14 +360,15 @@ Examples:
cmd.Flags().BoolVar(&local, "local", false, "run review locally without daemon (streams output to console)")
cmd.Flags().StringVar(&reviewType, "type", "", "review type (security, design) — changes system prompt")
cmd.Flags().StringVar(&provider, "provider", "", "provider for pi agent (e.g. anthropic, openai)")
cmd.Flags().StringVar(&minSeverity, "min-severity", "", "minimum severity threshold: critical, high, medium, low")
registerAgentCompletion(cmd)
registerReasoningCompletion(cmd)

return cmd
}

// runLocalReview runs a review directly without the daemon
func runLocalReview(cmd *cobra.Command, repoPath, gitRef, diffContent, agentName, model, provider, reasoning, reviewType string, quiet bool) error {
func runLocalReview(cmd *cobra.Command, repoPath, gitRef, diffContent, agentName, model, provider, reasoning, reviewType string, quiet bool, minSeverity string) error {
// Load config
cfg, err := config.LoadGlobal()
if err != nil {
Expand All @@ -378,6 +381,12 @@ func runLocalReview(cmd *cobra.Command, repoPath, gitRef, diffContent, agentName
return fmt.Errorf("invalid reasoning: %w", err)
}

// Resolve review min-severity
resolvedMinSev, err := config.ResolveReviewMinSeverity(minSeverity, repoPath, cfg)
if err != nil {
return fmt.Errorf("invalid min-severity: %w", err)
}

// Map review_type to config workflow (matches daemon behavior)
workflow := "review"
if !config.IsDefaultReviewType(reviewType) {
Expand Down Expand Up @@ -435,13 +444,13 @@ func runLocalReview(cmd *cobra.Command, repoPath, gitRef, diffContent, agentName
var snapshotCleanup func()
if diffContent != "" {
// Dirty review
dirtyResult, dirtyErr := pb.BuildDirtyWithSnapshot(repoPath, diffContent, 0, cfg.ReviewContextCount, a.Name(), reviewType)
dirtyResult, dirtyErr := pb.BuildDirtyWithSnapshot(repoPath, diffContent, 0, cfg.ReviewContextCount, a.Name(), reviewType, resolvedMinSev)
reviewPrompt = dirtyResult.Prompt
snapshotCleanup = dirtyResult.Cleanup
err = dirtyErr
} else {
excludes := config.ResolveExcludePatterns(repoPath, cfg, reviewType)
result, buildErr := pb.BuildWithSnapshot(repoPath, gitRef, 0, cfg.ReviewContextCount, a.Name(), reviewType, excludes)
result, buildErr := pb.BuildWithSnapshot(repoPath, gitRef, 0, cfg.ReviewContextCount, a.Name(), reviewType, resolvedMinSev, excludes)
reviewPrompt = result.Prompt
snapshotCleanup = result.Cleanup
err = buildErr
Expand Down
125 changes: 109 additions & 16 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ type Config struct {
SecurityBackupModel string `toml:"security_backup_model"`
DesignBackupModel string `toml:"design_backup_model"`

// Minimum severity thresholds (global defaults)
ReviewMinSeverity string `toml:"review_min_severity" comment:"Minimum severity for reviews: critical, high, medium, or low. Empty disables filtering."`
RefineMinSeverity string `toml:"refine_min_severity" comment:"Minimum severity for refine: critical, high, medium, or low. Empty disables filtering."`
FixMinSeverity string `toml:"fix_min_severity" comment:"Minimum severity for fix: critical, high, medium, or low. Empty disables filtering."`

AllowUnsafeAgents *bool `toml:"allow_unsafe_agents"` // nil = not set, allows commands to choose their own default
DisableCodexSandbox bool `toml:"disable_codex_sandbox"` // use --full-auto instead of --sandbox read-only (for systems where bwrap is broken)
ReuseReviewSession *bool `toml:"reuse_review_session"` // nil = not set; when true, reuse prior branch review sessions when possible
Expand Down Expand Up @@ -621,8 +626,9 @@ type RepoConfig struct {
ReviewReasoning string `toml:"review_reasoning" comment:"Reasoning level for reviews in this repo: fast, standard, medium, thorough, or maximum."`
RefineReasoning string `toml:"refine_reasoning" comment:"Reasoning level for refine in this repo: fast, standard, medium, thorough, or maximum."`
FixReasoning string `toml:"fix_reasoning" comment:"Reasoning level for fix in this repo: fast, standard, medium, thorough, or maximum."`
FixMinSeverity string `toml:"fix_min_severity" comment:"Minimum severity for fix in this repo: critical, high, medium, or low."` // Minimum severity for fix: critical, high, medium, low
RefineMinSeverity string `toml:"refine_min_severity" comment:"Minimum severity for refine in this repo: critical, high, medium, low."` // Minimum severity for refine: critical, high, medium, low
FixMinSeverity string `toml:"fix_min_severity" comment:"Minimum severity for fix in this repo: critical, high, medium, or low."` // Minimum severity for fix: critical, high, medium, low
RefineMinSeverity string `toml:"refine_min_severity" comment:"Minimum severity for refine in this repo: critical, high, medium, low."` // Minimum severity for refine: critical, high, medium, low
ReviewMinSeverity string `toml:"review_min_severity" comment:"Minimum severity for reviews in this repo: critical, high, medium, low."` // Minimum severity for review: critical, high, medium, low
ExcludePatterns []string `toml:"exclude_patterns" comment:"Filenames or glob patterns to exclude from review diffs for this repo."`
PostCommitReview string `toml:"post_commit_review" comment:"Automatic post-commit review mode for this repo: commit or branch."` // "commit" (default) or "branch"
ReuseReviewSession *bool `toml:"reuse_review_session"`
Expand Down Expand Up @@ -1442,6 +1448,54 @@ var severityAbove = map[string]string{
// from "agent couldn't fix it."
const SeverityThresholdMarker = "SEVERITY_THRESHOLD_MET"

// IsMarkerOnlyOutput reports whether output is essentially the
// SeverityThresholdMarker by itself, allowing only whitespace and
// minimal markdown decoration: bold (**...** or __...__), italic
// (*...* or _..._), a fenced code block, a leading list bullet,
// and an optional trailing period. Any prose or other substantive
// content disqualifies the output, since we cannot reliably tell
// chatty narration from prose findings without severity labels.
//
// Callers that want to treat the marker as a "below threshold"
// signal should use this helper rather than substring matching,
// which is too easy to fool with marker-bearing prose findings.
func IsMarkerOnlyOutput(output string) bool {
s := strings.TrimSpace(output)
if s == "" {
return false
}

// Strip a fenced code block if the output is wrapped in one.
if rest, ok := strings.CutPrefix(s, "```"); ok {
if _, after, found := strings.Cut(rest, "\n"); found {
s = after
} else {
s = rest
}
s = strings.TrimSuffix(s, "```")
s = strings.TrimSpace(s)
}

// Strip a leading list marker (- or *) followed by space.
if len(s) >= 2 && (s[0] == '-' || s[0] == '*') && s[1] == ' ' {
s = strings.TrimSpace(s[2:])
}

// Strip surrounding bold/italic markers. Bold forms (** and __)
// are stripped before italic forms (* and _) so that **X** fully
// unwraps in one pass rather than degrading to *X*.
for _, wrap := range []string{"**", "__", "*", "_"} {
if strings.HasPrefix(s, wrap) && strings.HasSuffix(s, wrap) && len(s) >= 2*len(wrap) {
s = strings.TrimSpace(s[len(wrap) : len(s)-len(wrap)])
}
}

// Strip a trailing period.
s = strings.TrimSpace(strings.TrimSuffix(s, "."))

return s == SeverityThresholdMarker
}

// SeverityInstruction returns a prompt instruction telling the agent
// to focus only on findings at or above minSeverity. Returns "" for
// empty, "low", or unrecognized input (no filtering needed).
Expand Down Expand Up @@ -1567,37 +1621,76 @@ func validateRepoReasoningOverride(
}

// ResolveFixMinSeverity determines minimum severity for fix.
// Priority: explicit > per-repo config > "" (no filter)
func ResolveFixMinSeverity(
explicit string, repoPath string,
) (string, error) {
// Priority: explicit > per-repo config > global config > "" (no filter)
func ResolveFixMinSeverity(explicit string, repoPath string, globalCfg *Config) (string, error) {
if strings.TrimSpace(explicit) != "" {
return NormalizeMinSeverity(explicit)
}
if repoCfg, err := LoadRepoConfig(repoPath); err == nil &&
repoCfg != nil &&
strings.TrimSpace(repoCfg.FixMinSeverity) != "" {
if repoCfg, err := LoadRepoConfig(repoPath); err == nil && repoCfg != nil && strings.TrimSpace(repoCfg.FixMinSeverity) != "" {
return NormalizeMinSeverity(repoCfg.FixMinSeverity)
}
if globalCfg != nil && strings.TrimSpace(globalCfg.FixMinSeverity) != "" {
return NormalizeMinSeverity(globalCfg.FixMinSeverity)
}
return "", nil
}

// ResolveRefineMinSeverity determines minimum severity for refine.
// Priority: explicit > per-repo config > "" (no filter)
func ResolveRefineMinSeverity(
explicit string, repoPath string,
) (string, error) {
// Priority: explicit > per-repo config > global config > "" (no filter)
func ResolveRefineMinSeverity(explicit string, repoPath string, globalCfg *Config) (string, error) {
if strings.TrimSpace(explicit) != "" {
return NormalizeMinSeverity(explicit)
}
if repoCfg, err := LoadRepoConfig(repoPath); err == nil &&
repoCfg != nil &&
strings.TrimSpace(repoCfg.RefineMinSeverity) != "" {
if repoCfg, err := LoadRepoConfig(repoPath); err == nil && repoCfg != nil && strings.TrimSpace(repoCfg.RefineMinSeverity) != "" {
return NormalizeMinSeverity(repoCfg.RefineMinSeverity)
}
if globalCfg != nil && strings.TrimSpace(globalCfg.RefineMinSeverity) != "" {
return NormalizeMinSeverity(globalCfg.RefineMinSeverity)
}
return "", nil
}

// ResolveReviewMinSeverity determines minimum severity for review.
// Priority: explicit > per-repo config > global config > "" (no filter)
func ResolveReviewMinSeverity(explicit string, repoPath string, globalCfg *Config) (string, error) {
if strings.TrimSpace(explicit) != "" {
return NormalizeMinSeverity(explicit)
}
if repoCfg, err := LoadRepoConfig(repoPath); err == nil && repoCfg != nil && strings.TrimSpace(repoCfg.ReviewMinSeverity) != "" {
return NormalizeMinSeverity(repoCfg.ReviewMinSeverity)
}
if globalCfg != nil && strings.TrimSpace(globalCfg.ReviewMinSeverity) != "" {
return NormalizeMinSeverity(globalCfg.ReviewMinSeverity)
}
return "", nil
}

// severityRank returns a numeric rank for a severity level.
// Higher rank = stricter threshold (fewer findings pass).
func severityRank(s string) int {
switch s {
case "critical":
return 4
case "high":
return 3
case "medium":
return 2
case "low":
return 1
default:
return 0
}
}

// StricterSeverity returns whichever severity threshold is stricter
// (filters more). Empty string means "no filter" (least strict).
func StricterSeverity(a, b string) string {
if severityRank(a) >= severityRank(b) {
return a
}
return b
}

// ResolveModel determines which model to use based on config priority:
// 1. Explicit model parameter (if non-empty)
// 2. Per-repo config (model in .roborev.toml)
Expand Down
Loading
Loading