mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-08-16 20:24:02 +00:00
feat(cmd): add no-review cmd (#835)
* feat(cmd): add no-review cmd * docs(flags): improve --no-filter help text for clarity --------- Co-authored-by: kite <lizhengfeng.lzf@alibaba-inc.com>
This commit is contained in:
parent
980f21d6f2
commit
552dc95147
4 changed files with 189 additions and 0 deletions
|
|
@ -45,6 +45,7 @@ type reviewOptions struct {
|
|||
maxGitProcs int
|
||||
maxTokens int
|
||||
maxTokensBudget int
|
||||
noFilter bool
|
||||
preview bool
|
||||
}
|
||||
|
||||
|
|
@ -207,6 +208,7 @@ func executeReview(opts reviewOptions) error {
|
|||
GitRunner: cc.GitRunner,
|
||||
Resume: resumeState,
|
||||
MaxTokensBudget: int64(opts.maxTokensBudget),
|
||||
SkipFilter: opts.noFilter,
|
||||
RuntimeConfig: rt.RuntimeConfig,
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -178,6 +178,7 @@ func registerReviewFlags(cmd *cobra.Command, opts *reviewOptions) {
|
|||
addBackgroundFlags(cmd, &opts.background, &opts.backgroundFile)
|
||||
addProviderFlag(cmd, &opts.provider)
|
||||
addModelFlag(cmd, &opts.model)
|
||||
cmd.Flags().BoolVar(&opts.noFilter, "no-filter", false, "keep all review comments without LLM post-filtering")
|
||||
addPreviewFlag(cmd, &opts.preview)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -139,6 +139,10 @@ type Args struct {
|
|||
// would exceed it. 0 = unlimited. Mirrors scan.Args.MaxTokensBudget.
|
||||
MaxTokensBudget int64
|
||||
|
||||
// SkipFilter disables the REVIEW_FILTER_TASK even when the template
|
||||
// defines one. Set via the --no-filter CLI flag.
|
||||
SkipFilter bool
|
||||
|
||||
// RuntimeConfig carries the non-secret, allowlisted runtime settings that
|
||||
// identify how this run was configured, for the manifest's
|
||||
// runtime_config_sha256. It is populated by the cmd layer from the resolved
|
||||
|
|
@ -1238,6 +1242,12 @@ func (a *Agent) executeReviewFilter(ctx context.Context, d model.Diff, newPath s
|
|||
return
|
||||
}
|
||||
|
||||
if a.args.SkipFilter {
|
||||
telemetry.SetAttr(span, "skipped", true)
|
||||
fmt.Fprintf(stdout.Writer(), "[ocr] Review filter skipped for %s (--no-filter)\n", newPath)
|
||||
return
|
||||
}
|
||||
|
||||
comments := a.args.CommentCollector.CommentsForPath(newPath)
|
||||
if len(comments) == 0 {
|
||||
return
|
||||
|
|
|
|||
|
|
@ -346,6 +346,182 @@ func TestExecuteReviewFilter_LLMError(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestExecuteReviewFilter_SkipFilter(t *testing.T) {
|
||||
t.Run("AC-1: SkipFilter disables the filter", func(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
sess := session.New(tmpDir, "main", "test", session.SessionOptions{ReviewMode: "diff"})
|
||||
client := &fakeAgentClient{}
|
||||
collector := tool.NewCommentCollector()
|
||||
collector.Add(model.LlmComment{Path: "a.go", Content: "comment"})
|
||||
|
||||
a := New(Args{
|
||||
LLMClient: client,
|
||||
Model: "test",
|
||||
Session: sess,
|
||||
SkipFilter: true,
|
||||
CommentCollector: collector,
|
||||
Template: template.Template{
|
||||
ReviewFilterTask: &template.LlmConversation{
|
||||
Messages: []template.ChatMessage{{Role: "user", Content: "Filter: {{comments}}"}},
|
||||
},
|
||||
MaxTokens: 10000,
|
||||
MaxToolRequestTimes: 5,
|
||||
MainTask: template.LlmConversation{Messages: []template.ChatMessage{{Role: "user", Content: "t"}}},
|
||||
},
|
||||
})
|
||||
|
||||
a.executeReviewFilter(context.Background(), model.Diff{NewPath: "a.go", Diff: "+code"}, "a.go")
|
||||
|
||||
if client.calls != 0 {
|
||||
t.Errorf("no LLM calls expected when SkipFilter is true, got %d", client.calls)
|
||||
}
|
||||
comments := collector.CommentsForPath("a.go")
|
||||
if len(comments) != 1 {
|
||||
t.Errorf("comments should be unchanged when filter is skipped, got %d", len(comments))
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("AC-2: All comments preserved when skipped", func(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
sess := session.New(tmpDir, "main", "test", session.SessionOptions{ReviewMode: "diff"})
|
||||
client := &fakeAgentClient{}
|
||||
collector := tool.NewCommentCollector()
|
||||
collector.Add(model.LlmComment{Path: "a.go", Content: "comment 1"})
|
||||
collector.Add(model.LlmComment{Path: "a.go", Content: "comment 2"})
|
||||
collector.Add(model.LlmComment{Path: "a.go", Content: "comment 3"})
|
||||
|
||||
a := New(Args{
|
||||
LLMClient: client,
|
||||
Model: "test",
|
||||
Session: sess,
|
||||
SkipFilter: true,
|
||||
CommentCollector: collector,
|
||||
Template: template.Template{
|
||||
ReviewFilterTask: &template.LlmConversation{
|
||||
Messages: []template.ChatMessage{{Role: "user", Content: "Filter: {{comments}}"}},
|
||||
},
|
||||
MaxTokens: 10000,
|
||||
MaxToolRequestTimes: 5,
|
||||
MainTask: template.LlmConversation{Messages: []template.ChatMessage{{Role: "user", Content: "t"}}},
|
||||
},
|
||||
})
|
||||
|
||||
a.executeReviewFilter(context.Background(), model.Diff{NewPath: "a.go", Diff: "+code"}, "a.go")
|
||||
|
||||
comments := collector.CommentsForPath("a.go")
|
||||
if len(comments) != 3 {
|
||||
t.Fatalf("expected 3 comments when filter is skipped, got %d", len(comments))
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("AC-3: Default (no SkipFilter) still runs filter", func(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
sess := session.New(tmpDir, "main", "test", session.SessionOptions{ReviewMode: "diff"})
|
||||
|
||||
filterResp := `["c-1"]`
|
||||
client := &fakeAgentClient{
|
||||
responses: []*llm.ChatResponse{{
|
||||
Choices: []llm.Choice{{
|
||||
Message: llm.ResponseMessage{Content: &filterResp},
|
||||
}},
|
||||
Usage: &llm.UsageInfo{PromptTokens: 10, CompletionTokens: 5},
|
||||
}},
|
||||
}
|
||||
|
||||
collector := tool.NewCommentCollector()
|
||||
collector.Add(model.LlmComment{Path: "a.go", Content: "keep this"})
|
||||
collector.Add(model.LlmComment{Path: "a.go", Content: "remove this"})
|
||||
|
||||
a := New(Args{
|
||||
LLMClient: client,
|
||||
Model: "test",
|
||||
Session: sess,
|
||||
CommentCollector: collector,
|
||||
Template: template.Template{
|
||||
ReviewFilterTask: &template.LlmConversation{
|
||||
Messages: []template.ChatMessage{{Role: "user", Content: "Filter: {{comments}} path={{path}} diff={{diff}}"}},
|
||||
},
|
||||
MaxTokens: 10000,
|
||||
MaxToolRequestTimes: 5,
|
||||
MainTask: template.LlmConversation{Messages: []template.ChatMessage{{Role: "user", Content: "t"}}},
|
||||
},
|
||||
})
|
||||
|
||||
a.executeReviewFilter(context.Background(), model.Diff{NewPath: "a.go", Diff: "+code"}, "a.go")
|
||||
|
||||
if client.calls == 0 {
|
||||
t.Error("LLM client should have been called when SkipFilter is false (default)")
|
||||
}
|
||||
comments := collector.CommentsForPath("a.go")
|
||||
if len(comments) != 1 {
|
||||
t.Errorf("expected 1 comment after filter, got %d", len(comments))
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("AC-4: SkipFilter is reached when ReviewFilterTask is non-nil", func(t *testing.T) {
|
||||
// After the nil-template guard, SkipFilter is the next early-return.
|
||||
// With a non-nil ReviewFilterTask + zero comments, the function would
|
||||
// normally fall through to the LLM call; SkipFilter must short-circuit it.
|
||||
tmpDir := t.TempDir()
|
||||
sess := session.New(tmpDir, "main", "test", session.SessionOptions{ReviewMode: "diff"})
|
||||
client := &fakeAgentClient{}
|
||||
collector := tool.NewCommentCollector()
|
||||
collector.Add(model.LlmComment{Path: "a.go", Content: "comment"})
|
||||
|
||||
a := New(Args{
|
||||
LLMClient: client,
|
||||
Model: "test",
|
||||
Session: sess,
|
||||
SkipFilter: true,
|
||||
CommentCollector: collector,
|
||||
Template: template.Template{
|
||||
ReviewFilterTask: &template.LlmConversation{
|
||||
Messages: []template.ChatMessage{{Role: "user", Content: "Filter: {{comments}}"}},
|
||||
},
|
||||
MaxTokens: 10000,
|
||||
MaxToolRequestTimes: 5,
|
||||
MainTask: template.LlmConversation{Messages: []template.ChatMessage{{Role: "user", Content: "t"}}},
|
||||
},
|
||||
})
|
||||
|
||||
a.executeReviewFilter(context.Background(), model.Diff{NewPath: "a.go", Diff: "+x"}, "a.go")
|
||||
|
||||
if client.calls != 0 {
|
||||
t.Errorf("no LLM calls expected when SkipFilter is true, got %d", client.calls)
|
||||
}
|
||||
if len(collector.CommentsForPath("a.go")) != 1 {
|
||||
t.Errorf("comments should be unchanged when filter is skipped")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("AC-5: Skip takes priority over no comments", func(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
sess := session.New(tmpDir, "main", "test", session.SessionOptions{ReviewMode: "diff"})
|
||||
client := &fakeAgentClient{}
|
||||
|
||||
a := New(Args{
|
||||
LLMClient: client,
|
||||
Model: "test",
|
||||
Session: sess,
|
||||
SkipFilter: true,
|
||||
Template: template.Template{
|
||||
ReviewFilterTask: &template.LlmConversation{
|
||||
Messages: []template.ChatMessage{{Role: "user", Content: "Filter: {{comments}}"}},
|
||||
},
|
||||
MaxTokens: 10000,
|
||||
MaxToolRequestTimes: 5,
|
||||
MainTask: template.LlmConversation{Messages: []template.ChatMessage{{Role: "user", Content: "t"}}},
|
||||
},
|
||||
})
|
||||
|
||||
a.executeReviewFilter(context.Background(), model.Diff{NewPath: "a.go", Diff: "+x"}, "a.go")
|
||||
|
||||
if client.calls != 0 {
|
||||
t.Errorf("no LLM calls expected when SkipFilter is true, got %d", client.calls)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestExecutePlanPhase(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
sess := session.New(tmpDir, "main", "test", session.SessionOptions{ReviewMode: "diff"})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue