Skip to content

【WIP】feat: add ocr scan for full-file code review #93

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
css521 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

【WIP】feat: add ocr scan for full-file code review #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 7 additions & 1 deletion cmd/opencodereview/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ func dispatch() error {
return nil
case "review", "r":
return runReview(args[1:])
case "scan", "s":
return runScan(args[1:])
case "config":
return runConfig(args[1:])
case "llm":
Expand All @@ -69,7 +71,8 @@ Usage:
ocr [command]

Commands:
review, r Start a code review
review, r Start a diff-based code review
scan, s Scan entire files (no diff required)
rules Inspect and debug review rules
config Manage configuration settings
llm LLM utility commands
Expand All @@ -79,11 +82,14 @@ Commands:
Examples:
ocr review --from master --to dev Review diff range
ocr review --commit abc123 Review a single commit
ocr scan --all Scan every reviewable file in the repo
ocr scan --path internal/agent Scan a single directory
ocr config set llm.model opus-4-6 Set a config value
ocr llm test Test LLM connectivity
ocr version Show version info

Use "ocr review -h" for more information about review.
Use "ocr scan -h" for more information about scan.
Use "ocr rules -h" for more information about rules.
Use "ocr config" for more information about config.
Use "ocr llm" for more information about LLM utilities.
Expand Down
2 changes: 2 additions & 0 deletions cmd/opencodereview/output.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,8 @@ func statusBadge(status string) string {
return "\033[36m[R]\033[0m"
case "binary":
return "\033[35m[B]\033[0m"
case "scan":
return "\033[34m[S]\033[0m"
default:
return "[?]"
}
Expand Down
151 changes: 29 additions & 122 deletions cmd/opencodereview/review_cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,134 +8,76 @@ import (
"time"

"github.com/open-code-review/open-code-review/internal/agent"
"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"
"github.com/open-code-review/open-code-review/internal/diff"
"github.com/open-code-review/open-code-review/internal/gitcmd"
"github.com/open-code-review/open-code-review/internal/llm"
"github.com/open-code-review/open-code-review/internal/stdout"
"github.com/open-code-review/open-code-review/internal/telemetry"
"github.com/open-code-review/open-code-review/internal/tool"
)

func runReview(args []string) error {
opts, err := parseReviewFlags(args)
if err != nil {
return fmt.Errorf("parse flags: %w", err)
// parseReviewFlags already wraps with "parse flags: %w" — return as-is.
return err
Comment on lines +18 to +19

@github-actions github-actions Bot Jun 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Incorrect comment / inconsistent error wrapping. The comment claims parseReviewFlags already wraps all errors with "parse flags: %w", but that's only true for the a.Parse(args) error (flags.go:136). Validation errors returned later in parseReviewFlags (e.g., "only one review mode allowed", "--to is required when --from is specified", "invalid --audience value") are returned without the "parse flags:" prefix.

The old code wrapped all errors from parseReviewFlags uniformly with fmt.Errorf("parse flags: %w", err), providing consistent context. Removing this wrapper means those validation errors now lose the "parse flags:" prefix, creating inconsistent error messages.

Either restore the wrapper here (it's harmless to double-wrap the parse error), or add the prefix to every error return in parseReviewFlags.

}
if opts.showHelp {
printReviewUsage()
return nil
}

if err := requireGitRepo(opts.repoDir); err != nil {
return err
}

tpl, err := template.LoadDefault()
if err != nil {
return fmt.Errorf("load default template: %w", err)
}
if opts.maxTools > 0 {
tpl.MaxToolRequestTimes = opts.maxTools
}
if err := tpl.Validate(); err != nil {
return fmt.Errorf("invalid config: %w", err)
}

repoDir, err := resolveRepoDir(opts.repoDir)
cc, err := loadCommonContext(opts.repoDir, opts.rulePath, opts.maxTools, opts.maxGitProcs)
if err != nil {
return fmt.Errorf("resolve repo: %w", err)
return err
}

if opts.commit != "" && opts.background == "" {
if msg, err := getCommitMessage(repoDir, opts.commit); err == nil && msg != "" {
if msg, err := getCommitMessage(cc.RepoDir, opts.commit); err == nil && msg != "" {
opts.background = msg
}
}

resolver, fileFilter, err := rules.NewResolver(repoDir, opts.rulePath)
if err != nil {
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)
return runPreview(cc, opts)
}
planToolDefs := agent.BuildToolDefs(toolEntries, true)
mainToolDefs := agent.BuildToolDefs(toolEntries, false)

cfgPath, err := defaultConfigPath()
rt, err := loadLLMRuntime(cc.Template, opts.toolConfigPath)
if err != nil {
return err
}

appCfg, err := LoadAppConfig(cfgPath)
if err != nil {
return fmt.Errorf("load app config: %w", err)
}
if appCfg != nil {
tpl.ApplyLanguage(appCfg.Language)
}

ep, err := llm.ResolveEndpoint(cfgPath)
if err != nil {
return fmt.Errorf("resolve LLM endpoint: %w", err)
}

llmClient := llm.NewLLMClient(ep)
model := ep.Model

gitRunner := gitcmd.New(opts.maxGitProcs)

collector := tool.NewCommentCollector()
mode := tool.ParseReviewMode(opts.from, opts.to, opts.commit)
ref, _ := mode.RefValue(opts.to, opts.commit)
fileReader := &tool.FileReader{
RepoDir: repoDir,
RepoDir: cc.RepoDir,
Mode: mode,
Ref: ref,
Runner: gitRunner,
Runner: cc.GitRunner,
}
tools := buildToolRegistry(collector, fileReader)
tools := buildToolRegistry(rt.Collector, fileReader)

ag := agent.New(agent.Args{
RepoDir: repoDir,
RepoDir: cc.RepoDir,
From: opts.from,
To: opts.to,
Commit: opts.commit,
Template: *tpl,
SystemRule: resolver,
FileFilter: fileFilter,
LLMClient: llmClient,
Template: *cc.Template,
SystemRule: cc.Resolver,
FileFilter: cc.FileFilter,
LLMClient: rt.Client,
Tools: tools,
PlanToolDefs: planToolDefs,
MainToolDefs: mainToolDefs,
CommentCollector: collector,
PlanToolDefs: rt.PlanToolDefs,
MainToolDefs: rt.MainToolDefs,
CommentCollector: rt.Collector,
CommentWorkerPool: agent.NewCommentWorkerPool(opts.concurrency),
MaxConcurrency: opts.concurrency,
ConcurrentTaskTimeout: opts.perFileTimeout,
Model: model,
Model: rt.Model,
Background: opts.background,
GitRunner: gitRunner,
GitRunner: cc.GitRunner,
})

// Silence progress output during execution; restore before Summary in agent mode.
var unsilence func()
if opts.outputFormat == "json" || opts.audience == "agent" {
unsilence = stdout.Quiet()
defer func() {
if unsilence != nil {
unsilence()
}
}()
}
// Silence progress output during execution; restored before the trace
// summary in agent-text mode (and on function exit otherwise).
q := newQuietHandle(opts.outputFormat, opts.audience)
defer q.Restore()

ctx, span := telemetry.StartSpan(context.Background(), "review.run")
defer span.End()
Expand All @@ -147,41 +89,7 @@ func runReview(args []string) error {
return fmt.Errorf("review failed: %w", err)
}

// Resolve line numbers by matching existing_code against diff hunks.
comments = diff.ResolveLineNumbers(comments, ag.Diffs())

// Record summary metrics (files_reviewed is refined by agent.Run).
duration := time.Since(startTime)
telemetry.RecordReviewDuration(ctx, duration)
if len(comments) > 0 {
telemetry.RecordCommentsGenerated(ctx, int64(len(comments)))
}

// If no files were reviewed (e.g. workspace has no changes), inform the caller in JSON mode.
if opts.outputFormat == "json" && len(comments) == 0 && ag.FilesReviewed() == 0 {
return outputJSONNoFiles()
}

// In agent mode (text output), restore stdout so Summary reaches the terminal.
if opts.audience == "agent" && opts.outputFormat != "json" && unsilence != nil {
unsilence()
unsilence = nil
}

if opts.outputFormat != "json" {
telemetry.PrintTraceSummary(ag.FilesReviewed(), int64(len(comments)), ag.TotalInputTokens(), ag.TotalOutputTokens(), ag.TotalTokensUsed(), ag.TotalCacheReadTokens(), ag.TotalCacheWriteTokens(), duration)
}

if opts.outputFormat == "json" {
return outputJSONWithWarnings(comments, ag.Warnings(), ag.FilesReviewed(), ag.TotalInputTokens(), ag.TotalOutputTokens(), ag.TotalTokensUsed(), ag.TotalCacheReadTokens(), ag.TotalCacheWriteTokens(), duration)
}
if opts.audience == "agent" {
outputTextWithWarnings(comments, ag.Warnings())
return nil
}
outputTextWithWarnings(comments, ag.Warnings())

return nil
return emitRunResult(ctx, ag, comments, startTime, opts.outputFormat, opts.audience, q)
}

func resolveRepoDir(input string) (string, error) {
Expand Down Expand Up @@ -216,15 +124,14 @@ func requireGitRepo(dir string) error {
return nil
}

func runPreview(repoDir string, opts reviewOptions, fileFilter *rules.FileFilter) error {
gitRunner := gitcmd.New(opts.maxGitProcs)
func runPreview(cc *commonContext, opts reviewOptions) error {
ag := agent.New(agent.Args{
RepoDir: repoDir,
RepoDir: cc.RepoDir,
From: opts.from,
To: opts.to,
Commit: opts.commit,
FileFilter: fileFilter,
GitRunner: gitRunner,
FileFilter: cc.FileFilter,
GitRunner: cc.GitRunner,
})

preview, err := ag.Preview(context.Background())
Expand Down
Loading