mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-08-21 06:34:29 +00:00
* fix(review-filter): add `submit_filter_result` func for review filter stage At review filter stage, LLM(test GLM5.2) may return natural language content such as: """ Looking at each comment... - commit 1: ... - commit 2: ... Summary: ```json [...]``` """ add function calling for LLM will get more stable result for some model of providers. and this tool only appears at review filter stage, so it does no harm for review quality. * fix(filter): log tool call argument parse failures for observability * refactor(filter): two-tool design with required choice for review filter - Replace single submit_filter_result tool with two mutually exclusive tools: report_incorrect_comments and approve_all_comments - Add ToolChoice field to ChatRequest, wired through OpenAI Chat, OpenAI Responses, and Anthropic client paths - Set ToolChoice to 'required' so the model must always make an explicit decision (no silent text-only fallback) - Aggregate results across multiple tool calls for robustness - Update prompt to instruct exactly-one-tool usage - Preserve text-based fallback for providers without tool support * fix(filter): stop the review filter from deleting real findings An A/B benchmark over 194 identical commit ranges (50 OSS repos, claude-opus-4-6) showed the filter removing 22 comments at 36% precision: 8 of them reported real defects — a heap overflow, an ignored LOCKMODE parameter, a static/non-static linkage conflict, a dropped success status, double URL encoding, an always-true condition. Two causes, one of them structural. Field order in report_incorrect_comments. Go serializes the parameter map alphabetically, so comment_ids was emitted before any reasoning field and the model had to commit before it had finished thinking. Replaying recorded sessions with a diagnostic field made this visible: it wrote "this is a protected subject, I should not remove it" while the id stayed in the list it had already produced. Adding an "analysis" array — alphabetically first, required — lets it reason before concluding. parseFilterToolCalls is unchanged; it reads comment_ids and ignores the rest. Prompt scope. Step 2 "Issue Classification" asked whether the comment "misidentifies clearly normal code as a defect", which invited a value judgement and was the entry point for most wrong removals. It is gone. Removal now needs one of two grounds: the code the comment targets is absent from the diff, or a single diff line literally contradicts its central claim. Protected subjects (memory safety, concurrency, linkage, behavioral change, unused parameters) and style-only comments that state something true are vetoed as the first two steps of the method, not as prose the model reads and then skips. Replaying all 455 recorded filter calls with the same inputs: precision 36% -> 88%, real findings deleted 8 -> 0, at +37% filter tokens and +7% mean latency. Caveat: the grounds were derived from this same dataset, so the figure is a training-set result and wants a hold-out range set before it is trusted. --------- Co-authored-by: kite <lizhengfeng.lzf@alibaba-inc.com> |
||
|---|---|---|
| .. | ||
| prompts | ||
| scan_template.json | ||
| task_template.json | ||
| template.go | ||
| template_test.go | ||