mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-07-09 17:28:58 +00:00
feat(review): add structured category and severity to findings (#311)
Some checks are pending
CI / test (push) Waiting to run
Some checks are pending
CI / test (push) Waiting to run
Add two structured fields, category and severity, to every review finding so CI integrations can sort, group, filter, or gate builds without re-parsing natural-language comment text. - Tool schema (tools.json): add category/severity as enum-constrained, required properties of code_comment. severity is limited to critical/high/medium/low (info dropped, since LLMs struggle to distinguish low from info). - System prompt (task_template.json) is intentionally left untouched to avoid the review-quality regression observed on the benchmark suite; the tool schema alone drives field population. - JSON output: category/severity are flat siblings of content/start_line, omitted entirely when empty (backward compatible). - CLI output: render an inline [category - severity] badge before the comment, colored by severity. - Sync docs across all five README locales.
This commit is contained in:
parent
39eb0f3984
commit
a2e08b77a1
11 changed files with 304 additions and 3 deletions
|
|
@ -404,6 +404,15 @@ ocr review \
|
|||
|
||||
`--format json`フラグは、CIスクリプトでのパースに適した機械可読な結果を出力します。
|
||||
|
||||
各指摘には2つの構造化フィールドが付与され、CI統合はコメント本文を再パースせずに並べ替え・グループ化・フィルタリング・ビルドのゲート判定を行えます:
|
||||
|
||||
| フィールド | 許可される値 | 説明 |
|
||||
|-----------|-------------|------|
|
||||
| `category` | `bug`、`security`、`performance`、`maintainability`、`test`、`style`、`documentation`、`other` | 指摘が属するカテゴリ。 |
|
||||
| `severity` | `critical`、`high`、`medium`、`low` | 指摘の重要度。 |
|
||||
|
||||
JSON出力ではこの2つのフィールドは`content`や`start_line`などと同じ階層に並びます。ターミナルでは、コメントの前にインラインの`[category · severity]`バッジとして表示され、重要度に応じて色分けされます。
|
||||
|
||||
統合例は[`examples/`](./examples/)ディレクトリを参照してください:
|
||||
|
||||
- [`github_actions/`](./examples/github_actions/) — GitHub Actions統合の例
|
||||
|
|
|
|||
|
|
@ -404,6 +404,15 @@ ocr review \
|
|||
|
||||
`--format json` flag는 CI script에서 파싱하기 좋은 machine-readable 결과를 출력합니다.
|
||||
|
||||
각 finding에는 두 개의 구조화된 field가 포함되어, CI 통합에서 comment 텍스트를 다시 파싱하지 않고도 정렬·그룹화·필터링하거나 build를 gate할 수 있습니다:
|
||||
|
||||
| Field | 허용 값 | 설명 |
|
||||
|-------|--------|------|
|
||||
| `category` | `bug`, `security`, `performance`, `maintainability`, `test`, `style`, `documentation`, `other` | 이슈가 속한 카테고리. |
|
||||
| `severity` | `critical`, `high`, `medium`, `low` | 이슈의 중요도. |
|
||||
|
||||
JSON 출력에서 두 field는 `content`, `start_line` 등과 같은 수준의 sibling으로 나타납니다. 터미널에서는 comment 앞에 인라인 `[category · severity]` badge로 표시되며 severity에 따라 색상이 지정됩니다.
|
||||
|
||||
통합 예시는 [`examples/`](./examples/) 디렉터리를 참고하세요.
|
||||
|
||||
- [`github_actions/`](./examples/github_actions/): GitHub Actions 통합 예시
|
||||
|
|
|
|||
|
|
@ -406,6 +406,15 @@ The `--from` flag accepts a branch ref (e.g., `origin/main`) or commit SHA as th
|
|||
|
||||
The `--format json` flag outputs machine-readable results suitable for parsing in CI scripts.
|
||||
|
||||
Each finding carries two structured fields so CI integrations can sort, group, filter, or gate builds without re-parsing comment text:
|
||||
|
||||
| Field | Allowed values | Notes |
|
||||
|-------|----------------|-------|
|
||||
| `category` | `bug`, `security`, `performance`, `maintainability`, `test`, `style`, `documentation`, `other` | The category the issue belongs to. |
|
||||
| `severity` | `critical`, `high`, `medium`, `low` | The importance of the issue. |
|
||||
|
||||
In JSON output the two fields appear as siblings alongside `content`, `start_line`, etc. In the terminal, they render as an inline `[category · severity]` badge before the comment, colored by severity.
|
||||
|
||||
See the [`examples/`](./examples/) directory for integration examples:
|
||||
|
||||
- [`github_actions/`](./examples/github_actions/) — GitHub Actions integration example
|
||||
|
|
|
|||
|
|
@ -406,6 +406,15 @@ ocr review \
|
|||
|
||||
Флаг `--format json` выводит машиночитаемый результат, удобный для разбора в CI-скриптах.
|
||||
|
||||
Каждое замечание содержит два структурированных поля, чтобы CI-интеграции могли сортировать, группировать, фильтровать замечания или блокировать сборку без повторного разбора текста комментария:
|
||||
|
||||
| Поле | Допустимые значения | Примечание |
|
||||
|------|---------------------|------------|
|
||||
| `category` | `bug`, `security`, `performance`, `maintainability`, `test`, `style`, `documentation`, `other` | Категория, к которой относится замечание. |
|
||||
| `severity` | `critical`, `high`, `medium`, `low` | Важность замечания. |
|
||||
|
||||
В JSON-выводе эти два поля располагаются рядом с `content`, `start_line` и др. В терминале они отображаются перед комментарием как встроенный бейдж `[category · severity]`, цвет которого определяется важностью.
|
||||
|
||||
Примеры интеграции — в каталоге [`examples/`](./examples/):
|
||||
|
||||
- [`github_actions/`](./examples/github_actions/) — пример интеграции с GitHub Actions
|
||||
|
|
|
|||
|
|
@ -404,6 +404,15 @@ ocr review \
|
|||
|
||||
`--format json` 参数输出适合 CI 脚本解析的机器可读结果。
|
||||
|
||||
每条评审结果都带有两个结构化字段,便于 CI 集成在无需解析评论文本的情况下排序、分组、过滤或卡点构建:
|
||||
|
||||
| 字段 | 允许的取值 | 说明 |
|
||||
|------|-----------|------|
|
||||
| `category` | `bug`、`security`、`performance`、`maintainability`、`test`、`style`、`documentation`、`other` | 问题所属的类别。 |
|
||||
| `severity` | `critical`、`high`、`medium`、`low` | 问题的严重程度。 |
|
||||
|
||||
在 JSON 输出中,这两个字段与 `content`、`start_line` 等平级;在终端中,它们会以内联的 `[category · severity]` 徽章形式显示在评论前,并按严重程度着色。
|
||||
|
||||
集成示例请参见 [`examples/`](./examples/) 目录:
|
||||
|
||||
- [`github_actions/`](./examples/github_actions/) — GitHub Actions 集成示例
|
||||
|
|
|
|||
|
|
@ -61,7 +61,19 @@ func renderComment(comment model.LlmComment) {
|
|||
fmt.Printf("\n\033[2m─── %s:%d-%d ───\033[0m\n", sanitizeTerminal(comment.Path), comment.StartLine, comment.EndLine)
|
||||
|
||||
if comment.Content != "" {
|
||||
for _, ln := range wrapByRunes(sanitizeTerminal(comment.Content), 100) {
|
||||
badge := buildBadge(comment)
|
||||
content := sanitizeTerminal(comment.Content)
|
||||
if badge != "" {
|
||||
// Prepend the plain badge text to the content so it wraps inline with
|
||||
// the first line, then colorize just the badge prefix after wrapping.
|
||||
content = badge + " " + content
|
||||
}
|
||||
lines := wrapByRunes(content, 100)
|
||||
for i, ln := range lines {
|
||||
if i == 0 && badge != "" && strings.HasPrefix(ln, badge) {
|
||||
color := severityColor(comment.Severity)
|
||||
ln = color + badge + "\033[0m" + ln[len(badge):]
|
||||
}
|
||||
fmt.Printf("%s\n", ln)
|
||||
}
|
||||
fmt.Println()
|
||||
|
|
@ -83,6 +95,41 @@ func renderComment(comment model.LlmComment) {
|
|||
fmt.Println()
|
||||
}
|
||||
|
||||
// buildBadge renders a compact "[category · severity]" tag for a finding. It returns
|
||||
// an empty string when neither structured field is present, so text output for findings
|
||||
// without metadata is unchanged.
|
||||
func buildBadge(comment model.LlmComment) string {
|
||||
category := sanitizeTerminal(comment.Category)
|
||||
severity := sanitizeTerminal(comment.Severity)
|
||||
switch {
|
||||
case category != "" && severity != "":
|
||||
return fmt.Sprintf("[%s · %s]", category, severity)
|
||||
case category != "":
|
||||
return fmt.Sprintf("[%s]", category)
|
||||
case severity != "":
|
||||
return fmt.Sprintf("[%s]", severity)
|
||||
default:
|
||||
return ""
|
||||
}
|
||||
}
|
||||
|
||||
// severityColor maps a finding severity to an ANSI color used for its badge.
|
||||
// Unknown or empty severities fall back to dim.
|
||||
func severityColor(severity string) string {
|
||||
switch severity {
|
||||
case "critical":
|
||||
return "\033[1;91m" // bold bright red
|
||||
case "high":
|
||||
return "\033[91m" // bright red
|
||||
case "medium":
|
||||
return "\033[93m" // bright yellow
|
||||
case "low":
|
||||
return "\033[94m" // bright blue
|
||||
default:
|
||||
return "\033[2m" // dim
|
||||
}
|
||||
}
|
||||
|
||||
// printDiffLine renders a single diff line with colored prefix and background on content.
|
||||
func printDiffLine(prefix, content, fgColor, bgColor string) {
|
||||
fmt.Printf("%s%s%s %s%s\033[0m\n", fgColor+bgColor, prefix, "\033[0m"+bgColor, content, "\033[0m")
|
||||
|
|
|
|||
|
|
@ -1,6 +1,78 @@
|
|||
package main
|
||||
|
||||
import "testing"
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/open-code-review/open-code-review/internal/model"
|
||||
)
|
||||
|
||||
func TestBuildBadge(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
comment model.LlmComment
|
||||
want string
|
||||
}{
|
||||
{"both fields", model.LlmComment{Category: "security", Severity: "high"}, "[security · high]"},
|
||||
{"category only", model.LlmComment{Category: "bug"}, "[bug]"},
|
||||
{"severity only", model.LlmComment{Severity: "low"}, "[low]"},
|
||||
{"neither", model.LlmComment{}, ""},
|
||||
{"strips control chars", model.LlmComment{Category: "bug\x1b[0m", Severity: "high"}, "[bug[0m · high]"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := buildBadge(tt.comment); got != tt.want {
|
||||
t.Errorf("buildBadge() = %q, want %q", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSeverityColor(t *testing.T) {
|
||||
// Each known severity must map to a distinct color; unknown falls back to dim.
|
||||
seen := map[string]string{}
|
||||
for _, sev := range []string{"critical", "high", "medium", "low"} {
|
||||
c := severityColor(sev)
|
||||
if c == "" {
|
||||
t.Errorf("severityColor(%q) is empty", sev)
|
||||
}
|
||||
if prev, ok := seen[c]; ok {
|
||||
t.Errorf("severityColor(%q) shares color with %q", sev, prev)
|
||||
}
|
||||
seen[c] = sev
|
||||
}
|
||||
if got := severityColor("bogus"); got != "\033[2m" {
|
||||
t.Errorf("severityColor(unknown) = %q, want dim", got)
|
||||
}
|
||||
if got := severityColor(""); got != "\033[2m" {
|
||||
t.Errorf("severityColor(empty) = %q, want dim", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRenderComment_BadgeInline verifies the badge is colorized and rendered inline
|
||||
// with the first line of the comment content.
|
||||
func TestRenderComment_BadgeInline(t *testing.T) {
|
||||
out := captureStdout(t, func() {
|
||||
renderComment(model.LlmComment{
|
||||
Path: "internal/mcp/client.go",
|
||||
StartLine: 27,
|
||||
EndLine: 27,
|
||||
Content: "Potential environment variable leak.",
|
||||
Category: "security",
|
||||
Severity: "high",
|
||||
})
|
||||
})
|
||||
if !strings.Contains(out, "[security · high]") {
|
||||
t.Errorf("expected badge in output, got:\n%s", out)
|
||||
}
|
||||
// severity high → bright red; the badge must be wrapped in the color + reset.
|
||||
if !strings.Contains(out, "\033[91m[security · high]\033[0m") {
|
||||
t.Errorf("expected colorized badge, got:\n%q", out)
|
||||
}
|
||||
if !strings.Contains(out, "Potential environment variable leak.") {
|
||||
t.Errorf("expected content in output, got:\n%s", out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeTerminal(t *testing.T) {
|
||||
tests := []struct {
|
||||
|
|
|
|||
|
|
@ -51,11 +51,37 @@
|
|||
"suggestion_code": {
|
||||
"type": "string",
|
||||
"description": "Corresponding suggested code snippet, maintaining consistent code style."
|
||||
},
|
||||
"category": {
|
||||
"type": "string",
|
||||
"enum": [
|
||||
"bug",
|
||||
"security",
|
||||
"performance",
|
||||
"maintainability",
|
||||
"test",
|
||||
"style",
|
||||
"documentation",
|
||||
"other"
|
||||
],
|
||||
"description": "The category the issue belongs to."
|
||||
},
|
||||
"severity": {
|
||||
"type": "string",
|
||||
"enum": [
|
||||
"critical",
|
||||
"high",
|
||||
"medium",
|
||||
"low"
|
||||
],
|
||||
"description": "The severity of the issue."
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
"content",
|
||||
"existing_code"
|
||||
"existing_code",
|
||||
"category",
|
||||
"severity"
|
||||
]
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,6 +9,12 @@ type LlmComment struct {
|
|||
StartLine int `json:"start_line"`
|
||||
EndLine int `json:"end_line"`
|
||||
Thinking string `json:"thinking,omitempty"`
|
||||
// Category classifies the finding. One of:
|
||||
// bug, security, performance, maintainability, test, style, documentation, other.
|
||||
Category string `json:"category,omitempty"`
|
||||
// Severity indicates the importance of the finding. One of:
|
||||
// critical, high, medium, low.
|
||||
Severity string `json:"severity,omitempty"`
|
||||
}
|
||||
|
||||
// CodeReviewResult holds raw LLM-generated review suggestion for a code segment.
|
||||
|
|
|
|||
|
|
@ -68,6 +68,12 @@ func ParseComments(args map[string]any) ([]model.LlmComment, string) {
|
|||
if thinking, ok := obj["thinking"].(string); ok {
|
||||
cm.Thinking = thinking
|
||||
}
|
||||
if category, ok := obj["category"].(string); ok {
|
||||
cm.Category = category
|
||||
}
|
||||
if severity, ok := obj["severity"].(string); ok {
|
||||
cm.Severity = severity
|
||||
}
|
||||
if path, ok := args["path"].(string); ok {
|
||||
cm.Path = path
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,7 +2,11 @@ package tool
|
|||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/open-code-review/open-code-review/internal/model"
|
||||
)
|
||||
|
||||
func TestParseComments(t *testing.T) {
|
||||
|
|
@ -133,6 +137,101 @@ func TestParseComments_Fields(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// TestParseComments_CategorySeverity verifies the structured category and severity
|
||||
// fields are read off each comment object when present, and left zero-valued when
|
||||
// absent (older/less-capable models that omit them still produce valid comments).
|
||||
func TestParseComments_CategorySeverity(t *testing.T) {
|
||||
t.Run("parsed when present", func(t *testing.T) {
|
||||
args := map[string]any{
|
||||
"path": "main.go",
|
||||
"comments": []any{
|
||||
map[string]any{
|
||||
"content": "Potential nil pointer dereference.",
|
||||
"existing_code": "x := *p",
|
||||
"category": "bug",
|
||||
"severity": "high",
|
||||
},
|
||||
},
|
||||
}
|
||||
comments, errMsg := ParseComments(args)
|
||||
if errMsg != "" {
|
||||
t.Fatalf("unexpected error message: %s", errMsg)
|
||||
}
|
||||
if len(comments) != 1 {
|
||||
t.Fatalf("expected 1 comment, got %d", len(comments))
|
||||
}
|
||||
if got := comments[0].Category; got != "bug" {
|
||||
t.Errorf("category = %q, want %q", got, "bug")
|
||||
}
|
||||
if got := comments[0].Severity; got != "high" {
|
||||
t.Errorf("severity = %q, want %q", got, "high")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("zero-valued when absent", func(t *testing.T) {
|
||||
args := map[string]any{
|
||||
"path": "main.go",
|
||||
"comments": []any{
|
||||
map[string]any{
|
||||
"content": "Consider renaming for clarity.",
|
||||
"existing_code": "a := 1",
|
||||
},
|
||||
},
|
||||
}
|
||||
comments, errMsg := ParseComments(args)
|
||||
if errMsg != "" {
|
||||
t.Fatalf("unexpected error message: %s", errMsg)
|
||||
}
|
||||
if len(comments) != 1 {
|
||||
t.Fatalf("expected 1 comment, got %d", len(comments))
|
||||
}
|
||||
if comments[0].Category != "" {
|
||||
t.Errorf("category = %q, want empty", comments[0].Category)
|
||||
}
|
||||
if comments[0].Severity != "" {
|
||||
t.Errorf("severity = %q, want empty", comments[0].Severity)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestLlmComment_JSONCategorySeverity verifies category and severity serialize as
|
||||
// flat siblings alongside content when set, and are omitted entirely when empty so
|
||||
// existing JSON consumers are unaffected.
|
||||
func TestLlmComment_JSONCategorySeverity(t *testing.T) {
|
||||
t.Run("omitted when empty", func(t *testing.T) {
|
||||
b, err := json.Marshal(model.LlmComment{Path: "main.go", Content: "no metadata"})
|
||||
if err != nil {
|
||||
t.Fatalf("marshal: %v", err)
|
||||
}
|
||||
out := string(b)
|
||||
if strings.Contains(out, "category") {
|
||||
t.Errorf("expected no category key, got %s", out)
|
||||
}
|
||||
if strings.Contains(out, "severity") {
|
||||
t.Errorf("expected no severity key, got %s", out)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("serialized when set", func(t *testing.T) {
|
||||
b, err := json.Marshal(model.LlmComment{
|
||||
Path: "main.go",
|
||||
Content: "sql injection",
|
||||
Category: "security",
|
||||
Severity: "critical",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("marshal: %v", err)
|
||||
}
|
||||
out := string(b)
|
||||
if !strings.Contains(out, `"category":"security"`) {
|
||||
t.Errorf("expected category in output, got %s", out)
|
||||
}
|
||||
if !strings.Contains(out, `"severity":"critical"`) {
|
||||
t.Errorf("expected severity in output, got %s", out)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestCodeCommentProvider_Execute(t *testing.T) {
|
||||
t.Run("adds comments to collector", func(t *testing.T) {
|
||||
collector := NewCommentCollector()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue