Skip to content

Commit 282279d

Browse files
shenlongScarletCarpet
shenlong
authored and
ScarletCarpet
committed
feat(doc|cli|review|scan|prompt): add structured category and severity fields to review findings
add category and serverity feature which forked from [pr-29](alibaba#29) and add or fix some extra change @mvanhorn: - Use fun call instead of modify system prompt - Add colored severity badge to terminal text output for quick visual scanning - Add --level and --category CLI flags to filter findings at client-side - Normalize severity values: only high, medium, low - Remove `test` category and add `imporvement` category - Sort findings by severity (high -> medium -> low) then by category - Update README (en/zh-CN) with filter flag documentation and examples - Add comprehensive tests for normalization, filtering, sorting, badges
1 parent 0aa6ac2 commit 282279d

17 files changed

Lines changed: 765 additions & 39 deletions

‎README.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,17 @@ The `--from` flag accepts a branch ref (e.g., `origin/main`) or commit SHA as th
406406

407407
The `--format json` flag outputs machine-readable results suitable for parsing in CI scripts.
408408

409+
Each finding in the JSON output may include two optional structured fields so CI
410+
integrations can sort, group, or gate builds without re-parsing comment text:
411+
412+
| Field | Allowed values | Notes |
413+
|-------|----------------|-------|
414+
| `severity` | `high`, `medium`, `low` | Indicates the importance of the issue. |
415+
| `category` | `bug`, `security`, `performance`, `maintainability`, `improvement`, `style`, `documentation`, `other` | Classifies the kind of issue. |
416+
417+
Both fields are optional and emitted only when the model populates them, so existing
418+
consumers that ignore them are unaffected (the keys are omitted entirely when empty).
419+
409420
See the [`examples/`](./examples/) directory for integration examples:
410421

411422
- [`github_actions/`](./examples/github_actions/) — GitHub Actions integration example
@@ -448,6 +459,10 @@ See the [`examples/`](./examples/) directory for integration examples:
448459
| `--max-tools` | — | built-in | Max tool call rounds per file; only takes effect when greater than template default |
449460
| `--max-git-procs` | — | built-in | Max concurrent git subprocesses |
450461
| `--tools` | — | — | Path to custom JSON tools config |
462+
| `--level` | — | all | Comma-separated severity levels to include. (e.g. `--level high,medium`) |
463+
| `--category` | — | all | Comma-separated categories to include. (e.g. `--category bug,security`) |
464+
465+
When `--level` or `--category` is set, only findings whose normalized severity or category match one of the given values are shown. Combining both filters acts as a logical AND. Filtering is applied client-side (after the LLM finishes), so it does not reduce API cost — it only controls what appears in the output.
451466

452467
### `ocr scan` Flags
453468

@@ -469,6 +484,8 @@ non-git directories too (it falls back to a filesystem walk that honors `.gitign
469484
| `--concurrency` | — | `8` | Max concurrent file scans |
470485
| `--rule` | — | — | Path to custom JSON review rules |
471486
| `--repo` | — | current dir | Repository or directory root to scan |
487+
| `--level` | — | all | Comma-separated severity levels to include. (e.g. `--level high,medium`) |
488+
| `--category` | — | all | Comma-separated categories to include. (e.g. `--category bug,security`) |
472489

473490
Before each run, `ocr scan` prints a rough token-cost estimate. Use `--preview` to see the
474491
file list first, and `--max-tokens-budget` to cap spend on large repositories.
@@ -526,6 +543,10 @@ ocr scan --repo /path/to/plain/dir --format json
526543
# Fastest scan: skip planning, dedup, and the project summary
527544
ocr scan --no-plan --no-dedup --no-summary
528545

546+
# Filter output to show only high and medium severity findings
547+
ocr review --level high,medium --category bug
548+
ocr scan --level high,medium --category bug
549+
529550
# View review session history in browser
530551
ocr viewer
531552
ocr viewer --addr :3000

‎README.zh-CN.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,15 @@ ocr review \
404404

405405
`--format json` 参数输出适合 CI 脚本解析的机器可读结果。
406406

407+
JSON 输出中的每条评审结果可包含两个可选的结构化字段，便于 CI 集成在无需解析评论文本的情况下排序、分组或卡点构建：
408+
409+
| 字段 | 允许的取值 | 说明 |
410+
|------|-----------|------|
411+
| `severity` | `high`、`medium`、`low` | 标识问题的严重程度。 |
412+
| `category` | `bug`、`security`、`performance`、`maintainability`、`test`、`style`、`documentation`、`improvement`、`other` | 标识问题的类别。 |
413+
414+
这两个字段均为可选，仅在模型填充时才会输出，因此忽略它们的现有消费方不受影响（字段为空时会被完全省略）。
415+
407416
集成示例请参见 [`examples/`](./examples/) 目录：
408417

409418
- [`github_actions/`](./examples/github_actions/) — GitHub Actions 集成示例
@@ -446,6 +455,10 @@ ocr review \
446455
| `--max-tools` | — | 内置默认 | 每个文件的最大工具调用轮次；仅在大于模板默认值时生效 |
447456
| `--max-git-procs` | — | 内置默认 | 最大并发 git 子进程数 |
448457
| `--tools` | — | — | 自定义 JSON 工具配置路径 |
458+
| `--level` | — | 全部 | 以逗号分隔的要包含的严重级别（例如 `--level high,medium`） |
459+
| `--category` | — | 全部 | 以逗号分隔的要包含的分类（例如 `--category bug,security`） |
460+
461+
设置 `--level` 或 `--category` 后，仅显示归一化后的严重级别或分类匹配给定值的问题。两者组合使用时为逻辑与（AND）关系。过滤在 LLM 完成后客户端执行，因此不会降低 API 费用——仅控制输出的内容。
449462

450463
### `ocr scan` 参数
451464

@@ -465,6 +478,8 @@ ocr review \
465478
| `--concurrency` | — | `8` | 最大并发文件扫描数 |
466479
| `--rule` | — | — | 自定义 JSON 审查规则路径 |
467480
| `--repo` | — | 当前目录 | 要扫描的仓库或目录根路径 |
481+
| `--level` | — | 全部 | 以逗号分隔的要包含的严重级别（例如 `--level high,medium`） |
482+
| `--category` | — | 全部 | 以逗号分隔的要包含的分类（例如 `--category bug,security`） |
468483

469484
每次运行前，`ocr scan` 会打印粗略的 token 费用估算。使用 `--preview` 先查看文件列表，使用 `--max-tokens-budget` 限制大型仓库的开销。
470485

@@ -521,6 +536,10 @@ ocr scan --repo /path/to/plain/dir --format json
521536
# 最快扫描：跳过规划、去重和项目总结
522537
ocr scan --no-plan --no-dedup --no-summary
523538

539+
# 过滤输出，只显示高和中严重级别的问题
540+
ocr review --level high,medium --category bug
541+
ocr scan --level high,medium --category bug
542+
524543
# 在浏览器中查看审查会话历史
525544
ocr viewer
526545
ocr viewer --addr :3000

‎cmd/opencodereview/emit_run_result_test.go‎

Lines changed: 104 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func (m *mockResultProvider) ToolCalls() map[string]int64 { return m.toolCall
3838
func TestEmitRunResult_JSONNoFiles(t *testing.T) {
3939
ag := &mockResultProvider{filesReviewed: 0}
4040
got := captureStdout(t, func() {
41-
err := emitRunResult(context.Background(), ag, nil, time.Now(), "json", "developer", nil)
41+
err := emitRunResult(context.Background(), ag, nil, time.Now(), "json", "developer", nil, nil)
4242
if err != nil {
4343
t.Fatalf("unexpected error: %v", err)
4444
}
@@ -63,7 +63,7 @@ func TestEmitRunResult_JSONWithComments(t *testing.T) {
6363
}
6464
comments := []model.LlmComment{{Path: "main.go", Content: "fix", StartLine: 1, EndLine: 2}}
6565
got := captureStdout(t, func() {
66-
err := emitRunResult(context.Background(), ag, comments, time.Now(), "json", "developer", nil)
66+
err := emitRunResult(context.Background(), ag, comments, time.Now(), "json", "developer", nil, nil)
6767
if err != nil {
6868
t.Fatalf("unexpected error: %v", err)
6969
}
@@ -83,7 +83,7 @@ func TestEmitRunResult_JSONWithComments(t *testing.T) {
8383
func TestEmitRunResult_TextNoComments(t *testing.T) {
8484
ag := &mockResultProvider{filesReviewed: 2}
8585
got := captureStdout(t, func() {
86-
err := emitRunResult(context.Background(), ag, nil, time.Now(), "text", "developer", nil)
86+
err := emitRunResult(context.Background(), ag, nil, time.Now(), "text", "developer", nil, nil)
8787
if err != nil {
8888
t.Fatalf("unexpected error: %v", err)
8989
}
@@ -97,7 +97,7 @@ func TestEmitRunResult_TextWithComments(t *testing.T) {
9797
ag := &mockResultProvider{filesReviewed: 1}
9898
comments := []model.LlmComment{{Path: "a.go", Content: "rename", StartLine: 5, EndLine: 10}}
9999
got := captureStdout(t, func() {
100-
err := emitRunResult(context.Background(), ag, comments, time.Now(), "text", "developer", nil)
100+
err := emitRunResult(context.Background(), ag, comments, time.Now(), "text", "developer", nil, nil)
101101
if err != nil {
102102
t.Fatalf("unexpected error: %v", err)
103103
}
@@ -116,7 +116,7 @@ func TestEmitRunResult_TextWithProjectSummary(t *testing.T) {
116116
projectSummary: "All tests pass, code quality is good.",
117117
}
118118
got := captureStdout(t, func() {
119-
err := emitRunResult(context.Background(), ag, nil, time.Now(), "text", "developer", nil)
119+
err := emitRunResult(context.Background(), ag, nil, time.Now(), "text", "developer", nil, nil)
120120
if err != nil {
121121
t.Fatalf("unexpected error: %v", err)
122122
}
@@ -133,7 +133,7 @@ func TestEmitRunResult_AgentTextRestoresQuiet(t *testing.T) {
133133
ag := &mockResultProvider{filesReviewed: 1}
134134
q := newQuietHandle("text", "agent")
135135
got := captureStdout(t, func() {
136-
err := emitRunResult(context.Background(), ag, nil, time.Now(), "text", "agent", q)
136+
err := emitRunResult(context.Background(), ag, nil, time.Now(), "text", "agent", q, nil)
137137
if err != nil {
138138
t.Fatalf("unexpected error: %v", err)
139139
}
@@ -153,7 +153,7 @@ func TestEmitRunResult_AgentJSONDoesNotRestore(t *testing.T) {
153153
}
154154
q := newQuietHandle("json", "agent")
155155
got := captureStdout(t, func() {
156-
err := emitRunResult(context.Background(), ag, nil, time.Now(), "json", "agent", q)
156+
err := emitRunResult(context.Background(), ag, nil, time.Now(), "json", "agent", q, nil)
157157
if err != nil {
158158
t.Fatalf("unexpected error: %v", err)
159159
}
@@ -168,10 +168,106 @@ func TestEmitRunResult_AgentJSONDoesNotRestore(t *testing.T) {
168168
func TestEmitRunResult_NilQuietHandle(t *testing.T) {
169169
ag := &mockResultProvider{filesReviewed: 1}
170170
got := captureStdout(t, func() {
171-
err := emitRunResult(context.Background(), ag, nil, time.Now(), "text", "agent", nil)
171+
err := emitRunResult(context.Background(), ag, nil, time.Now(), "text", "agent", nil, nil)
172172
if err != nil {
173173
t.Fatalf("unexpected error: %v", err)
174174
}
175175
})
176176
_ = got
177177
}
178+
179+
func TestEmitRunResult_FilteredNoMatchShowsMessage(t *testing.T) {
180+
fc := parseFilterFlags("low", "")
181+
ag := &mockResultProvider{filesReviewed: 1}
182+
comments := []model.LlmComment{
183+
{Path: "a.go", Severity: "high", Category: "bug", Content: "should be filtered out"},
184+
}
185+
got := captureStdout(t, func() {
186+
err := emitRunResult(context.Background(), ag, comments, time.Now(), "text", "developer", nil, fc)
187+
if err != nil {
188+
t.Fatalf("unexpected error: %v", err)
189+
}
190+
})
191+
if !strings.Contains(got, "No comments generated. Looks good to me.\n") {
192+
t.Errorf("expected 'No comments match', got %q", got)
193+
}
194+
}
195+
196+
func TestEmitRunResult_SortedBySeverityAndCategory(t *testing.T) {
197+
fc := parseFilterFlags("", "")
198+
ag := &mockResultProvider{filesReviewed: 1}
199+
comments := []model.LlmComment{
200+
{Path: "c.go", Severity: "low", Category: "bug", Content: "low bug", StartLine: 1, EndLine: 2},
201+
{Path: "a.go", Severity: "high", Category: "security", Content: "high sec", StartLine: 1, EndLine: 2},
202+
{Path: "b.go", Severity: "medium", Category: "bug", Content: "mid bug", StartLine: 1, EndLine: 2},
203+
}
204+
got := captureStdout(t, func() {
205+
err := emitRunResult(context.Background(), ag, comments, time.Now(), "text", "developer", nil, fc)
206+
if err != nil {
207+
t.Fatalf("unexpected error: %v", err)
208+
}
209+
})
210+
// a.go (high·security) should appear first, then b.go (medium·bug), then c.go (low·bug)
211+
aIdx := strings.Index(got, "a.go")
212+
bIdx := strings.Index(got, "b.go")
213+
cIdx := strings.Index(got, "c.go")
214+
if aIdx < 0 || bIdx < 0 || cIdx < 0 {
215+
t.Fatal("expected all three files in output")
216+
}
217+
if !(aIdx < bIdx && bIdx < cIdx) {
218+
t.Errorf("expected order a.go -> b.go -> c.go, got a=%d b=%d c=%d", aIdx, bIdx, cIdx)
219+
}
220+
}
221+
222+
func TestEmitRunResult_FilteredWithCountHint(t *testing.T) {
223+
fc := parseFilterFlags("high", "")
224+
ag := &mockResultProvider{filesReviewed: 1}
225+
comments := []model.LlmComment{
226+
{Path: "a.go", Severity: "high", Category: "bug", Content: "keep"},
227+
{Path: "b.go", Severity: "medium", Category: "style", Content: "hidden"},
228+
{Path: "c.go", Severity: "low", Category: "docs", Content: "hidden"},
229+
}
230+
got := captureStdout(t, func() {
231+
err := emitRunResult(context.Background(), ag, comments, time.Now(), "text", "developer", nil, fc)
232+
if err != nil {
233+
t.Fatalf("unexpected error: %v", err)
234+
}
235+
})
236+
if !strings.Contains(got, "a.go") {
237+
t.Errorf("expected a.go in output, got %q", got)
238+
}
239+
if strings.Contains(got, "b.go") || strings.Contains(got, "c.go") {
240+
t.Errorf("did not expect filtered-out files in output, got %q", got)
241+
}
242+
}
243+
244+
func TestEmitRunResult_JSONFilteredByLevel(t *testing.T) {
245+
fc := parseFilterFlags("high", "")
246+
ag := &mockResultProvider{
247+
filesReviewed: 2,
248+
inputTokens: 100,
249+
outputTokens: 50,
250+
totalTokens: 150,
251+
toolCalls: map[string]int64{"file_read": 1},
252+
}
253+
comments := []model.LlmComment{
254+
{Path: "a.go", Severity: "high", Category: "bug", Content: "keep"},
255+
{Path: "b.go", Severity: "medium", Category: "style", Content: "hidden"},
256+
}
257+
got := captureStdout(t, func() {
258+
err := emitRunResult(context.Background(), ag, comments, time.Now(), "json", "developer", nil, fc)
259+
if err != nil {
260+
t.Fatalf("unexpected error: %v", err)
261+
}
262+
})
263+
var out jsonOutput
264+
if err := json.Unmarshal([]byte(got), &out); err != nil {
265+
t.Fatalf("unmarshal: %v", err)
266+
}
267+
if len(out.Comments) != 1 {
268+
t.Errorf("expected 1 comment after filter, got %d", len(out.Comments))
269+
}
270+
if out.Comments[0].Path != "a.go" {
271+
t.Errorf("expected a.go, got %s", out.Comments[0].Path)
272+
}
273+
}

‎cmd/opencodereview/flags.go‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"flag"
55
"fmt"
66
"os"
7+
"strings"
78
"time"
89
)
910

@@ -112,6 +113,8 @@ type reviewOptions struct {
112113
maxGitProcs int
113114
preview bool
114115
showHelp bool
116+
level string // --level: comma-separated severity levels to show
117+
category string // --category: comma-separated categories to show
115118
}
116119

117120
func parseReviewFlags(args []string) (reviewOptions, error) {
@@ -135,6 +138,8 @@ func parseReviewFlags(args []string) (reviewOptions, error) {
135138
a.IntVar(&opts.maxTools, "max-tools", 0, "max tool call rounds per file (0 = template default; min 10)")
136139
a.IntVar(&opts.maxGitProcs, "max-git-procs", 16, "max concurrent git subprocesses")
137140
a.BoolVarP(&opts.preview, "preview", "p", false, "preview which files will be reviewed without running the LLM")
141+
a.StringVar(&opts.level, "level", "", "comma-separated severity levels to include: high,medium,low (defaults to all)")
142+
a.StringVar(&opts.category, "category", "", "comma-separated categories to include (defaults to all)")
138143

139144
if err := a.Parse(args); err != nil {
140145
return opts, fmt.Errorf("parse flags: %w", err)
@@ -182,6 +187,19 @@ func parseReviewFlags(args []string) (reviewOptions, error) {
182187
return opts, fmt.Errorf("--max-git-procs must be a non-negative integer (0 means use default 16)")
183188
}
184189

190+
for _, l := range splitCSV(opts.level) {
191+
l = strings.ToLower(l)
192+
if !allowedLevels[l] {
193+
return opts, fmt.Errorf("invalid --level value %q", l)
194+
}
195+
}
196+
for _, c := range splitCSV(opts.category) {
197+
c = strings.ToLower(c)
198+
if !allowedCategories[c] {
199+
return opts, fmt.Errorf("invalid --category value %q", c)
200+
}
201+
}
202+
185203
return opts, nil
186204
}
187205

@@ -229,7 +247,9 @@ Flags:
229247
--rule string path to JSON file with system review rules
230248
--timeout int concurrent task timeout in minutes (default 10)
231249
--to string target ref to end diff at (e.g., 'feature-branch')
232-
--tools string path to JSON tools config file (default: embedded)`)
250+
--tools string path to JSON tools config file (default: embedded)
251+
--level string comma-separated severity levels to include: high,medium,low (defaults to all)
252+
--category string comma-separated categories to include (defaults to all)`)
233253
}
234254

235255
// --- config subcommand ---

0 commit comments

Comments
 (0)