From e11ab5eafedfff71b72a5d924aa343a63100a776 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Wed, 3 Jun 2026 00:57:40 -0700 Subject: [PATCH] feat: add structured category and severity fields to review findings Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 11 ++ README.zh-CN.md | 9 ++ cmd/opencodereview/output.go | 18 ++++ internal/config/template/task_template.json | 2 +- internal/config/toolsconfig/tools.json | 8 ++ internal/model/review.go | 6 ++ internal/tool/code_comment.go | 6 ++ internal/tool/code_comment_test.go | 108 ++++++++++++++++++++ 8 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 internal/tool/code_comment_test.go diff --git a/README.md b/README.md index 32e073b..7cf6881 100644 --- a/README.md +++ b/README.md @@ -214,6 +214,17 @@ ocr review \ The `--format json` and `--audience agent` flags output machine-readable results suitable for parsing in CI scripts. +Each finding in the JSON output may include two optional structured fields so CI +integrations can sort, group, or gate builds without re-parsing comment text: + +| Field | Allowed values | Notes | +|-------|----------------|-------| +| `category` | `bug`, `security`, `performance`, `maintainability`, `test`, `style`, `documentation`, `other` | Classifies the kind of issue. | +| `severity` | `critical`, `high`, `medium`, `low`, `info` | Indicates the importance of the issue. | + +Both fields are optional and emitted only when the model populates them, so existing +consumers that ignore them are unaffected (the keys are omitted entirely when empty). + See the [`examples/`](./examples/) directory for integration examples: - [`github_actions/`](./examples/github_actions/) — GitHub Actions integration example diff --git a/README.zh-CN.md b/README.zh-CN.md index 04b2b3a..248c0e0 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -214,6 +214,15 @@ ocr review \ `--format json` 和 `--audience agent` 参数输出适合 CI 脚本解析的机器可读结果。 +JSON 输出中的每条评审结果可包含两个可选的结构化字段，便于 CI 集成在无需解析评论文本的情况下排序、分组或卡点构建： + +| 字段 | 允许的取值 | 说明 | +|------|-----------|------| +| `category` | `bug`、`security`、`performance`、`maintainability`、`test`、`style`、`documentation`、`other` | 标识问题的类别。 | +| `severity` | `critical`、`high`、`medium`、`low`、`info` | 标识问题的严重程度。 | + +这两个字段均为可选，仅在模型填充时才会输出，因此忽略它们的现有消费方不受影响（字段为空时会被完全省略）。 + 集成示例请参见 [`examples/`](./examples/) 目录： - [`github_actions/`](./examples/github_actions/) — GitHub Actions 集成示例 diff --git a/cmd/opencodereview/output.go b/cmd/opencodereview/output.go index 603a7ea..43f46ca 100644 --- a/cmd/opencodereview/output.go +++ b/cmd/opencodereview/output.go @@ -58,6 +58,10 @@ func renderComment(comment model.LlmComment) { fmt.Printf("\n\033[2m─── %s:%d-%d ───\033[0m\n", comment.Path, comment.StartLine, comment.EndLine) + if badge := buildBadge(comment); badge != "" { + fmt.Printf("%s\n", badge) + } + if comment.Content != "" { for _, ln := range wrapByRunes(comment.Content, 100) { fmt.Printf("%s\n", ln) @@ -81,6 +85,20 @@ func renderComment(comment model.LlmComment) { fmt.Println() } +// buildBadge renders a compact "[severity:high] [category:security]" prefix line +// for a comment. It returns an empty string when both fields are absent, so existing +// text output is unchanged for findings without structured metadata. +func buildBadge(comment model.LlmComment) string { + var parts []string + if comment.Severity != "" { + parts = append(parts, fmt.Sprintf("[severity:%s]", comment.Severity)) + } + if comment.Category != "" { + parts = append(parts, fmt.Sprintf("[category:%s]", comment.Category)) + } + return strings.Join(parts, " ") +} + // 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") diff --git a/internal/config/template/task_template.json b/internal/config/template/task_template.json index 5964dc0..d4b9b78 100644 --- a/internal/config/template/task_template.json +++ b/internal/config/template/task_template.json @@ -3,7 +3,7 @@ "messages": [ { "role": "system", - "content": "## Role\nYou are a code review assistant developed by Alibaba. You are skilled at code review in the software development process and are responsible for providing professional review feedback for code changes that are about to be submitted. Your feedback perfectly combines detailed analysis with contextual explanations.\nYou are working in an IDE with editor concepts for open files and an integrated terminal. The user's developed code is stored in the IDE's staging area.\nBefore users commit staged code to remote repositories, they will send you tasks to help them complete the process successfully. Each time a user sends a task, it will be placed in , and you will use to interact with the real world when executing tasks.\nPlease keep your responses concise and objective.\n\n## Capabilities\n- Think step by step progressively.\n- First understand the code changes to be reviewed. Code changes are provided in Unified Diff format, where lines starting with `-` indicate deleted code, lines starting with `+` indicate added code, consecutive `-` and `+` lines represent modified code, and other lines represent unchanged code.\n- Be objective and neutral, make judgments based on facts and logic, avoid subjective assumptions. When the context is unclear, use tools to obtain contextual information rather than judging based on assumptions.\n- For the current code changes, provide feedback opinions, pointing out areas for improvement or potential issues. Focus on issues in newly added code.\n- Avoid commenting on correct code or unchanged code.\n- Avoid commenting on deleted code; deleted code serves only as reference context.\n- Focus on clarity, practicality, and comprehensiveness.\n- Use developer-friendly terminology and analogies in explanations.\n- Focus primarily on the actual code logic and functionality. Avoid commenting on or providing feedback about non-functional elements such as code comments, tool-generated indicators (like @Generated annotations), or other metadata, unless the user explicitly requests you to review these elements.\n\n## Strict Focus Rules\n- Context tools are for understanding purposes only. Findings from other files must NOT become the subject of your comments.\n- If you discover a potential issue in another file while gathering context, ignore it — your task is limited to the current diffs.\n\n## Reply limit\n- If the current code review task is complete, call `task_done` to end the task.\n- If a code issue has been identified and confirmed, call the `code_comment` tool to provide feedback.\n- If additional context is needed to confirm the issue, call the appropriate context tool." + "content": "## Role\nYou are a code review assistant developed by Alibaba. You are skilled at code review in the software development process and are responsible for providing professional review feedback for code changes that are about to be submitted. Your feedback perfectly combines detailed analysis with contextual explanations.\nYou are working in an IDE with editor concepts for open files and an integrated terminal. The user's developed code is stored in the IDE's staging area.\nBefore users commit staged code to remote repositories, they will send you tasks to help them complete the process successfully. Each time a user sends a task, it will be placed in , and you will use to interact with the real world when executing tasks.\nPlease keep your responses concise and objective.\n\n## Capabilities\n- Think step by step progressively.\n- First understand the code changes to be reviewed. Code changes are provided in Unified Diff format, where lines starting with `-` indicate deleted code, lines starting with `+` indicate added code, consecutive `-` and `+` lines represent modified code, and other lines represent unchanged code.\n- Be objective and neutral, make judgments based on facts and logic, avoid subjective assumptions. When the context is unclear, use tools to obtain contextual information rather than judging based on assumptions.\n- For the current code changes, provide feedback opinions, pointing out areas for improvement or potential issues. Focus on issues in newly added code.\n- Avoid commenting on correct code or unchanged code.\n- Avoid commenting on deleted code; deleted code serves only as reference context.\n- Focus on clarity, practicality, and comprehensiveness.\n- Use developer-friendly terminology and analogies in explanations.\n- Focus primarily on the actual code logic and functionality. Avoid commenting on or providing feedback about non-functional elements such as code comments, tool-generated indicators (like @Generated annotations), or other metadata, unless the user explicitly requests you to review these elements.\n- When reporting an issue with the `code_comment` tool, set its `category` field to one of: bug, security, performance, maintainability, test, style, documentation, other; and set its `severity` field to one of: critical, high, medium, low, info. Choose the value that best fits the finding so consumers can sort and filter results.\n\n## Strict Focus Rules\n- Context tools are for understanding purposes only. Findings from other files must NOT become the subject of your comments.\n- If you discover a potential issue in another file while gathering context, ignore it — your task is limited to the current diffs.\n\n## Reply limit\n- If the current code review task is complete, call `task_done` to end the task.\n- If a code issue has been identified and confirmed, call the `code_comment` tool to provide feedback.\n- If additional context is needed to confirm the issue, call the appropriate context tool." }, { "role": "user", diff --git a/internal/config/toolsconfig/tools.json b/internal/config/toolsconfig/tools.json index 1f984a0..d711d40 100644 --- a/internal/config/toolsconfig/tools.json +++ b/internal/config/toolsconfig/tools.json @@ -51,6 +51,14 @@ "suggestion_code": { "type": "string", "description": "Corresponding suggested code snippet, maintaining consistent code style." + }, + "category": { + "type": "string", + "description": "Issue category. One of: bug, security, performance, maintainability, test, style, documentation, other." + }, + "severity": { + "type": "string", + "description": "Issue severity. One of: critical, high, medium, low, info." } }, "required": [ diff --git a/internal/model/review.go b/internal/model/review.go index 914d51c..e9d3719 100644 --- a/internal/model/review.go +++ b/internal/model/review.go @@ -9,6 +9,12 @@ type LlmComment struct { StartLine int `json:"start_line"` EndLine int `json:"end_line"` Thinking string `json:"thinking,omitempty"` + // Category is the optional finding classification. Allowed values: + // bug, security, performance, maintainability, test, style, documentation, other. + Category string `json:"category,omitempty"` + // Severity is the optional finding importance. Allowed values: + // critical, high, medium, low, info. + Severity string `json:"severity,omitempty"` } // CodeReviewResult holds raw LLM-generated review suggestion for a code segment. diff --git a/internal/tool/code_comment.go b/internal/tool/code_comment.go index 1bf4f82..48b7ded 100644 --- a/internal/tool/code_comment.go +++ b/internal/tool/code_comment.go @@ -65,6 +65,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 } diff --git a/internal/tool/code_comment_test.go b/internal/tool/code_comment_test.go new file mode 100644 index 0000000..6db5b45 --- /dev/null +++ b/internal/tool/code_comment_test.go @@ -0,0 +1,108 @@ +package tool + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/open-code-review/open-code-review/internal/model" +) + +// TestParseComments_CategorySeverityParsed verifies that the optional structured +// category and severity fields are read off each comment object when present. +func TestParseComments_CategorySeverityParsed(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") + } +} + +// TestParseComments_CategorySeverityOptional verifies that a finding without the +// new fields is still accepted and leaves them zero-valued (backward compatible). +func TestParseComments_CategorySeverityOptional(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_JSONOmitsEmptyCategorySeverity verifies that omitempty drops the +// keys entirely when unset, so existing JSON consumers are unaffected. +func TestLlmComment_JSONOmitsEmptyCategorySeverity(t *testing.T) { + cm := model.LlmComment{ + Path: "main.go", + Content: "no metadata", + } + b, err := json.Marshal(cm) + 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) + } +} + +// TestLlmComment_JSONSerializesCategorySeverity verifies the fields serialize when set. +func TestLlmComment_JSONSerializesCategorySeverity(t *testing.T) { + cm := model.LlmComment{ + Path: "main.go", + Content: "sql injection", + Category: "security", + Severity: "critical", + } + b, err := json.Marshal(cm) + 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) + } +}