diff --git a/cmd/opencodereview/flags.go b/cmd/opencodereview/flags.go index 021891b..9da35e2 100644 --- a/cmd/opencodereview/flags.go +++ b/cmd/opencodereview/flags.go @@ -105,6 +105,7 @@ type reviewOptions struct { background string // --background: optional requirement context concurrency int perFileTimeout int + preview bool showHelp bool } @@ -124,6 +125,7 @@ func parseReviewFlags(args []string) (reviewOptions, error) { a.IntVar(&opts.perFileTimeout, "timeout", 10, "concurrent task timeout in minutes") a.StringVar(&opts.audience, "audience", "human", "output audience: human (show progress) or agent (summary only)") a.StringVarP(&opts.background, "background", "b", "", "optional requirement/business context for the review") + a.BoolVarP(&opts.preview, "preview", "p", false, "preview which files will be reviewed without running the LLM") if err := a.Parse(args); err != nil { return opts, fmt.Errorf("parse flags: %w", err) @@ -183,6 +185,10 @@ Examples: # Agent mode (summary only, no progress lines) ocr review --audience agent + # Preview which files will be reviewed + ocr review --preview + ocr review -c abc123 -p + Flags: --audience string output audience: human (show progress) or agent (summary only) (default "human") -b, --background string optional requirement/business context for the review @@ -190,6 +196,7 @@ Flags: -f, --format string output format: text or json (default "text") --concurrency int max concurrent file reviews (default 8) --from string source ref to start diff from (e.g., 'main') + -p, --preview preview which files will be reviewed without running the LLM --repo string root directory of the git repository (default: current dir) --rule string path to JSON file with system review rules --timeout int concurrent task timeout in minutes (default 10) diff --git a/cmd/opencodereview/output.go b/cmd/opencodereview/output.go index e95cc5a..18c3995 100644 --- a/cmd/opencodereview/output.go +++ b/cmd/opencodereview/output.go @@ -162,9 +162,9 @@ func buildDiffLines(comment model.LlmComment) []suggestdiff.DiffLine { } type jsonOutput struct { - Status string `json:"status"` - Message string `json:"message,omitempty"` - Comments []model.LlmComment `json:"comments"` + Status string `json:"status"` + Message string `json:"message,omitempty"` + Comments []model.LlmComment `json:"comments"` Warnings []agent.AgentWarning `json:"warnings,omitempty"` } @@ -216,3 +216,65 @@ func outputJSONNoFiles() error { enc.SetIndent("", " ") return enc.Encode(out) } + +func outputPreviewText(p *agent.DiffPreview) { + if p.TotalFiles == 0 { + fmt.Println("No files changed.") + return + } + + maxPathLen := 0 + for _, e := range p.Entries { + if len(e.Path) > maxPathLen { + maxPathLen = len(e.Path) + } + } + if maxPathLen < 20 { + maxPathLen = 20 + } + pathFmt := fmt.Sprintf("%%-%ds", maxPathLen) + + fmt.Printf("\nPreview: %d file(s) changed | \033[32m+%d\033[0m \033[31m-%d\033[0m\n", + p.TotalFiles, p.TotalInsertions, p.TotalDeletions) + + if p.ReviewableCount > 0 { + fmt.Printf("\n\033[1mWill review (%d):\033[0m\n", p.ReviewableCount) + for _, e := range p.Entries { + if !e.WillReview { + continue + } + fmt.Printf(" %s "+pathFmt+" \033[32m+%-4d\033[0m \033[31m-%-4d\033[0m\n", + statusBadge(e.Status), e.Path, e.Insertions, e.Deletions) + } + } + + if p.ExcludedCount > 0 { + fmt.Printf("\n\033[1mExcluded from review (%d):\033[0m\n", p.ExcludedCount) + for _, e := range p.Entries { + if e.WillReview { + continue + } + fmt.Printf(" %s "+pathFmt+" \033[2m(%s)\033[0m\n", + statusBadge(e.Status), e.Path, e.ExcludeReason) + } + } + + fmt.Println() +} + +func statusBadge(status string) string { + switch status { + case "added": + return "\033[32m[A]\033[0m" + case "modified": + return "\033[33m[M]\033[0m" + case "deleted": + return "\033[31m[D]\033[0m" + case "renamed": + return "\033[36m[R]\033[0m" + case "binary": + return "\033[35m[B]\033[0m" + default: + return "[?]" + } +} diff --git a/cmd/opencodereview/review_cmd.go b/cmd/opencodereview/review_cmd.go index f9f2875..6961447 100644 --- a/cmd/opencodereview/review_cmd.go +++ b/cmd/opencodereview/review_cmd.go @@ -50,6 +50,10 @@ func runReview(args []string) error { return fmt.Errorf("load rules: %w", err) } + if opts.preview { + return runPreview(repoDir, opts, fileFilter) + } + toolEntries, err := toolsconfig.Load(opts.toolConfigPath) if err != nil { return fmt.Errorf("load tools: %w", err) @@ -198,6 +202,24 @@ func requireGitRepo(dir string) error { return nil } +func runPreview(repoDir string, opts reviewOptions, fileFilter *rules.FileFilter) error { + ag := agent.New(agent.Args{ + RepoDir: repoDir, + From: opts.from, + To: opts.to, + Commit: opts.commit, + FileFilter: fileFilter, + }) + + preview, err := ag.Preview() + if err != nil { + return fmt.Errorf("preview failed: %w", err) + } + + outputPreviewText(preview) + return nil +} + func buildToolRegistry(collector *tool.CommentCollector, fr *tool.FileReader, diffMap map[string]string) tool.Registry { reg := tool.NewRegistry() reg.Register(tool.NewFileRead(fr)) diff --git a/internal/agent/agent.go b/internal/agent/agent.go index b5836c0..9557f92 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -10,7 +10,6 @@ import ( "sync/atomic" "time" - "github.com/open-code-review/open-code-review/internal/config/allowlist" "github.com/open-code-review/open-code-review/internal/config/rules" "github.com/open-code-review/open-code-review/internal/config/template" "github.com/open-code-review/open-code-review/internal/config/toolsconfig" @@ -575,11 +574,7 @@ func (a *Agent) filterLargeDiffs(diffs []model.Diff) []model.Diff { func (a *Agent) countReviewable(diffs []model.Diff) int { count := 0 for _, d := range diffs { - path := d.NewPath - if path == "/dev/null" { - path = d.OldPath - } - if !a.shouldReview(path) { + if !a.shouldReview(effectivePath(d)) { continue } if d.IsDeleted { @@ -590,34 +585,9 @@ func (a *Agent) countReviewable(diffs []model.Diff) int { return count } -// shouldReview applies the four-gate filter algorithm: -// 1. User exclude → skip -// 2. Default extension check → skip if not allowed -// 3. User include → keep (penetrates default path exclusion) -// 4. Default path exclusion → skip -// -// If none of the gates trigger, the file is kept for review. +// shouldReview applies the four-gate filter algorithm via whyExcluded. func (a *Agent) shouldReview(path string) bool { - f := a.args.FileFilter - - if f != nil && f.IsUserExcluded(path) { - return false - } - - ext := a.extFromPath(path) - if ext != "" && !allowedext.IsAllowedExt(ext) { - return false - } - - if f != nil && f.HasInclude() && f.IsUserIncluded(path) { - return true - } - - if allowedext.IsExcludedPath(path) { - return false - } - - return true + return a.whyExcluded(path) == ExcludeNone } // filterDiffs drops diffs that should not be reviewed based on user-configured @@ -627,10 +597,7 @@ func (a *Agent) filterDiffs(diffs []model.Diff) []model.Diff { skipped := 0 for _, d := range diffs { - path := d.NewPath - if path == "/dev/null" { - path = d.OldPath - } + path := effectivePath(d) if !a.shouldReview(path) { fmt.Fprintf(stdout.Writer(), "[ocr] Skipping %s — filtered by path/extension rules\n", path) skipped++ diff --git a/internal/agent/preview.go b/internal/agent/preview.go new file mode 100644 index 0000000..1447752 --- /dev/null +++ b/internal/agent/preview.go @@ -0,0 +1,128 @@ +package agent + +import ( + "fmt" + + allowedext "github.com/open-code-review/open-code-review/internal/config/allowlist" + "github.com/open-code-review/open-code-review/internal/model" +) + +// ExcludeReason describes why a file was excluded from review. +type ExcludeReason string + +const ( + ExcludeNone ExcludeReason = "" + ExcludeUserRule ExcludeReason = "user_exclude" + ExcludeExtension ExcludeReason = "unsupported_ext" + ExcludeDefaultPath ExcludeReason = "default_path" + ExcludeDeleted ExcludeReason = "deleted" +) + +// DiffPreviewEntry is one file's preview record. +type DiffPreviewEntry struct { + Path string `json:"path"` + Status string `json:"status"` + Insertions int64 `json:"insertions"` + Deletions int64 `json:"deletions"` + WillReview bool `json:"will_review"` + ExcludeReason ExcludeReason `json:"exclude_reason,omitempty"` +} + +// DiffPreview is the full preview result. +type DiffPreview struct { + Entries []DiffPreviewEntry `json:"files"` + TotalInsertions int64 `json:"total_insertions"` + TotalDeletions int64 `json:"total_deletions"` + TotalFiles int `json:"total_files"` + ReviewableCount int `json:"reviewable_count"` + ExcludedCount int `json:"excluded_count"` +} + +// whyExcluded applies the same four-gate algorithm as shouldReview but +// returns the specific reason a file is excluded. +func (a *Agent) whyExcluded(path string) ExcludeReason { + f := a.args.FileFilter + + if f != nil && f.IsUserExcluded(path) { + return ExcludeUserRule + } + + ext := a.extFromPath(path) + if ext != "" && !allowedext.IsAllowedExt(ext) { + return ExcludeExtension + } + + if f != nil && f.HasInclude() && f.IsUserIncluded(path) { + return ExcludeNone + } + + if allowedext.IsExcludedPath(path) { + return ExcludeDefaultPath + } + + return ExcludeNone +} + +// Preview loads diffs and applies the filter algorithm, returning structured +// preview data without dispatching any LLM calls. +func (a *Agent) Preview() (*DiffPreview, error) { + if err := a.loadDiffs(); err != nil { + return nil, fmt.Errorf("load diffs: %w", err) + } + + result := &DiffPreview{ + TotalInsertions: a.totalInsertions, + TotalDeletions: a.totalDeletions, + TotalFiles: len(a.diffs), + } + + for _, d := range a.diffs { + path := effectivePath(d) + entry := DiffPreviewEntry{ + Path: path, + Insertions: d.Insertions, + Deletions: d.Deletions, + Status: diffStatus(d), + } + + reason := a.whyExcluded(path) + if reason == ExcludeNone && d.IsDeleted { + reason = ExcludeDeleted + } + + entry.WillReview = reason == ExcludeNone + entry.ExcludeReason = reason + + if entry.WillReview { + result.ReviewableCount++ + } else { + result.ExcludedCount++ + } + + result.Entries = append(result.Entries, entry) + } + + return result, nil +} + +func effectivePath(d model.Diff) string { + if d.NewPath == "/dev/null" { + return d.OldPath + } + return d.NewPath +} + +func diffStatus(d model.Diff) string { + switch { + case d.IsBinary: + return "binary" + case d.IsNew: + return "added" + case d.IsDeleted: + return "deleted" + case d.OldPath != d.NewPath && d.OldPath != "" && d.OldPath != "/dev/null": + return "renamed" + default: + return "modified" + } +}