feat: add review filter to remove provably incorrect comments after subtask

- Add REVIEW_FILTER_TASK template and LLM conversation config
- Implement executeReviewFilter to post-process comments per file
- Add CommentsForPath and RemoveByPathAndIndices to CommentCollector
- Apply timeout from template config to review filter LLM call
- Await CommentWorkerPool before running filter to prevent race condition
- Include raw response preview in filter JSON parse error logs
This commit is contained in:
kite 2026-06-09 20:41:42 +08:00
parent cf32900ca1
commit 01c9c85456
5 changed files with 163 additions and 1 deletions

View file

@ -580,7 +580,117 @@ func (a *Agent) executeSubtask(ctx context.Context, d model.Diff) error {
return nil
}
return a.performLlmCodeReview(ctx, messages, newPath)
err := a.performLlmCodeReview(ctx, messages, newPath)
if err == nil {
if a.args.CommentWorkerPool != nil {
a.args.CommentWorkerPool.Await()
}
a.executeReviewFilter(ctx, d, newPath)
}
return err
}
// executeReviewFilter runs the REVIEW_FILTER_TASK to remove comments that are
// provably incorrect based solely on the diff. Errors are logged and silently ignored.
func (a *Agent) executeReviewFilter(ctx context.Context, d model.Diff, newPath string) {
ft := a.args.Template.ReviewFilterTask
if ft == nil || len(ft.Messages) == 0 {
return
}
comments := a.args.CommentCollector.CommentsForPath(newPath)
if len(comments) == 0 {
return
}
commentsJSON := buildFilterCommentsJSON(comments)
messages := make([]llm.Message, 0, len(ft.Messages))
for _, m := range ft.Messages {
content := m.Content
content = strings.ReplaceAll(content, "{{path}}", newPath)
content = strings.ReplaceAll(content, "{{diff}}", d.Diff)
content = strings.ReplaceAll(content, "{{comments}}", commentsJSON)
messages = append(messages, llm.NewTextMessage(m.Role, content))
}
if ft.Timeout > 0 {
var cancel context.CancelFunc
ctx, cancel = context.WithTimeout(ctx, time.Duration(ft.Timeout)*time.Second)
defer cancel()
}
fs := a.session.GetOrCreateFileSession(newPath)
rec := fs.AppendTaskRecord(session.ReviewFilterTask, messages)
startTime := time.Now()
resp, err := a.args.LLMClient.CompletionsWithCtx(ctx, llm.ChatRequest{
Model: a.args.Model,
Messages: messages,
MaxTokens: a.args.Template.MaxTokens,
})
if err != nil {
rec.SetError(err, time.Since(startTime))
fmt.Fprintf(stdout.Writer(), "[ocr] Review filter failed for %s: %v\n", newPath, err)
return
}
rec.SetResponse(resp, time.Since(startTime))
if resp.Usage != nil {
atomic.AddInt64(&a.totalInputTokens, resp.Usage.PromptTokens)
atomic.AddInt64(&a.totalOutputTokens, resp.Usage.CompletionTokens)
atomic.AddInt64(&a.totalCacheReadTokens, resp.Usage.CacheReadTokens)
atomic.AddInt64(&a.totalCacheWriteTokens, resp.Usage.CacheWriteTokens)
}
indices := parseFilterResponse(resp.Content(), len(comments))
if len(indices) == 0 {
return
}
a.args.CommentCollector.RemoveByPathAndIndices(newPath, indices)
fmt.Fprintf(stdout.Writer(), "[ocr] Review filter removed %d comment(s) for %s\n", len(indices), newPath)
}
// buildFilterCommentsJSON serializes comments into a JSON array with generated IDs.
func buildFilterCommentsJSON(comments []model.LlmComment) string {
type filterComment struct {
ID string `json:"id"`
Content string `json:"content"`
ExistingCode string `json:"existing_code,omitempty"`
}
items := make([]filterComment, len(comments))
for i, cm := range comments {
items[i] = filterComment{
ID: fmt.Sprintf("c-%d", i),
Content: cm.Content,
ExistingCode: cm.ExistingCode,
}
}
data, _ := json.Marshal(items)
return string(data)
}
// parseFilterResponse extracts comment indices from the LLM filter response.
// Returns a set of 0-based indices. Invalid IDs or out-of-range indices are ignored.
func parseFilterResponse(raw string, total int) map[int]struct{} {
raw = stripMarkdownFences(raw)
var ids []string
if err := json.Unmarshal([]byte(raw), &ids); err != nil {
preview := raw
if len(preview) > 200 {
preview = preview[:200] + "..."
}
fmt.Fprintf(stdout.Writer(), "[ocr] Review filter: failed to parse LLM response: %v, raw: %s\n", err, preview)
return nil
}
indices := make(map[int]struct{})
for _, id := range ids {
var idx int
if _, err := fmt.Sscanf(id, "c-%d", &idx); err == nil && idx >= 0 && idx < total {
indices[idx] = struct{}{}
}
}
return indices
}
// buildChangeFilesExcept returns a formatted list of changed files except the given path.

View file

@ -38,6 +38,19 @@
],
"timeout": 120
},
"REVIEW_FILTER_TASK": {
"messages": [
{
"role": "system",
"content": "You are a fact-checker for code review comments.\n\nThese review comments come from an Agent that can invoke tools to obtain the full code context. You can currently only see the code diff.\n\nTherefore, your task is NOT to verify whether all review comments are correct, but to **filter out only those review comments that can be confirmed as incorrect based solely on the current diff**.\n\nFor review comments whose correctness cannot be determined from the diff alone, even if you find them suspicious, you should let them pass — because the Agent may have access to context that you cannot see."
},
{
"role": "user",
"content": "### Task\n\nGiven a code diff and a set of review comments, identify those that are **provably incorrect based solely on the diff**.\n\n### Evaluation Principles\n\n**Core principle: You need to falsify, not verify.**\n\n- ✅ Should flag: The diff contains **direct counter-evidence** that proves the key claim of the review comment is wrong\n- ❌ Should NOT flag: The review comment references context not visible in the diff (may have been obtained by the Agent via tools)\n- ❌ Should NOT flag: You merely \"cannot verify\" but also cannot disprove the review comment\n\n### Evaluation Method\n\nFor each review comment, perform the following two steps:\n\n#### Step 1: Fact Check (Veto Rule)\n\n- Only verify claims that are verifiable within the diff\n- Only determine a comment as incorrect when the diff provides counter-evidence. **If a claim involves information outside the diff (such as logic in other files, business semantics, runtime behavior), and the diff contains no evidence contradicting it, do not make a determination.**\n\n⚠ Fact check fails → Immediately determine as incorrect, skip Step 2.\n\n#### Step 2: Issue Classification\n\nAfter confirming that the facts visible in the diff are accurate, determine whether the description contains a **significant deviation that can be disproved from the diff**:\n\n- Does it misidentify clearly normal code in the diff as a defect?\n- Does it attribute behavior visible in the diff in a way that contradicts the code?\n\n⚠ Only determine as incorrect when the diff can directly prove the description is wrong.\n\n### Code Diff\n\n```{{path}}\n{{diff}}\n```\n\n### Review Comments\n\n{{comments}}\n\n### Output\n\nReturn all incorrect review comment IDs directly, without any explanation. Use JSON array format:\n\n```json\n[\"id-xxx\", \"id-yyy\"]\n```\n\nIf there are no review comments that can be confirmed as incorrect, return an empty array:\n\n```json\n[]\n```"
}
],
"timeout": 60
},
"TOOL_REQUEST_WAIT_TIME_MS": 10000,
"MAX_TOOL_REQUEST_TIMES": 30,
"MAX_SUBTASK_EXECUTION_TIME_MINUTES": 5,

View file

@ -19,6 +19,7 @@ type Template struct {
MaxSubtaskExecMinutes int `json:"MAX_SUBTASK_EXECUTION_TIME_MINUTES"`
PlanModeLineThreshold int `json:"PLAN_MODE_LINE_THRESHOLD"`
ReLocationTask *LlmConversation `json:"RE_LOCATION_TASK,omitempty"`
ReviewFilterTask *LlmConversation `json:"REVIEW_FILTER_TASK,omitempty"`
}
//go:embed task_template.json

View file

@ -20,6 +20,7 @@ const (
MainTask TaskType = "main_task"
MemoryCompressionTask TaskType = "memory_compression_task"
ReLocationTask TaskType = "re_location_task"
ReviewFilterTask TaskType = "review_filter_task"
)
const (

View file

@ -33,3 +33,40 @@ func (c *CommentCollector) Comments() []model.LlmComment {
copy(out, c.comments)
return out
}
// CommentsForPath returns a copy of comments whose Path matches the given path.
func (c *CommentCollector) CommentsForPath(path string) []model.LlmComment {
c.mu.Lock()
defer c.mu.Unlock()
var out []model.LlmComment
for _, cm := range c.comments {
if cm.Path == path {
out = append(out, cm)
}
}
return out
}
// RemoveByPathAndIndices removes comments for a given path whose per-path index
// (0-based position among all comments with that path) is in the indices set.
func (c *CommentCollector) RemoveByPathAndIndices(path string, indices map[int]struct{}) {
c.mu.Lock()
defer c.mu.Unlock()
kept := c.comments[:0]
pathIdx := 0
for _, cm := range c.comments {
if cm.Path == path {
if _, remove := indices[pathIdx]; remove {
pathIdx++
continue
}
pathIdx++
}
kept = append(kept, cm)
}
tail := c.comments[len(kept):]
for i := range tail {
tail[i] = model.LlmComment{}
}
c.comments = kept
}