-
Notifications
You must be signed in to change notification settings - Fork 6
Feature/improve-line-comments #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,222 @@ | ||
| /** | ||
| * 本地验证 diff 解析逻辑 | ||
| * 运行: cd .opencode-review && bun run gitea-pr-diff.test.ts | ||
| */ | ||
|
|
||
| // 模拟 parseDiff 和 formatDiffForReview 函数 | ||
| interface ParsedChange { | ||
| type: "add" | "del" | "normal" | ||
| content: string | ||
| oldLine?: number | ||
| newLine?: number | ||
| } | ||
|
|
||
| interface ParsedHunk { | ||
| oldStart: number | ||
| newStart: number | ||
| changes: ParsedChange[] | ||
| } | ||
|
|
||
| interface ParsedFile { | ||
| path: string | ||
| status: string | ||
| hunks: ParsedHunk[] | ||
| } | ||
|
|
||
| function parseDiff(diffText: string): ParsedFile[] { | ||
| const files: ParsedFile[] = [] | ||
| const lines = diffText.split("\n") | ||
| let i = 0 | ||
|
|
||
| while (i < lines.length) { | ||
| const line = lines[i] | ||
|
|
||
| if (line.startsWith("diff --git")) { | ||
| const pathMatch = line.match(/b\/(.+)$/) | ||
| const path = pathMatch ? pathMatch[1] : "unknown" | ||
|
|
||
| let status = "modified" | ||
| while (i < lines.length && !lines[i].startsWith("@@")) { | ||
| if (lines[i].startsWith("new file")) status = "added" | ||
| else if (lines[i].startsWith("deleted file")) status = "deleted" | ||
| else if (lines[i].startsWith("rename")) status = "renamed" | ||
| i++ | ||
| } | ||
|
|
||
| const hunks: ParsedHunk[] = [] | ||
|
|
||
| while (i < lines.length && !lines[i].startsWith("diff --git")) { | ||
| if (lines[i].startsWith("@@")) { | ||
| const hunkMatch = lines[i].match(/@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/) | ||
| if (hunkMatch) { | ||
| const oldStart = parseInt(hunkMatch[1], 10) | ||
| const newStart = parseInt(hunkMatch[2], 10) | ||
| const changes: ParsedChange[] = [] | ||
| let oldLine = oldStart | ||
| let newLine = newStart | ||
|
|
||
| i++ | ||
| while (i < lines.length && !lines[i].startsWith("@@") && !lines[i].startsWith("diff --git")) { | ||
| const content = lines[i].slice(1) | ||
| if (lines[i].startsWith("+")) { | ||
| changes.push({ type: "add", content, newLine: newLine++ }) | ||
| } else if (lines[i].startsWith("-")) { | ||
| changes.push({ type: "del", content, oldLine: oldLine++ }) | ||
| } else if (lines[i].startsWith(" ")) { | ||
| changes.push({ type: "normal", content, oldLine: oldLine++, newLine: newLine++ }) | ||
| } | ||
| i++ | ||
| } | ||
|
|
||
| hunks.push({ oldStart, newStart, changes }) | ||
| } else { | ||
| i++ | ||
| } | ||
| } else { | ||
| i++ | ||
| } | ||
| } | ||
|
|
||
| files.push({ path, status, hunks }) | ||
| } else { | ||
| i++ | ||
| } | ||
| } | ||
|
|
||
| return files | ||
| } | ||
|
|
||
| function formatDiffForReview(files: ParsedFile[]): string { | ||
| const parts: string[] = [] | ||
|
|
||
| for (const file of files) { | ||
| parts.push(`\n## ${file.path} (${file.status})\n`) | ||
|
|
||
| for (const hunk of file.hunks) { | ||
| parts.push(`@@ starting at line ${hunk.newStart} (old: ${hunk.oldStart}) @@`) | ||
|
|
||
| for (const change of hunk.changes) { | ||
| if (change.type === "add") { | ||
| parts.push(`[NEW:${change.newLine}] +${change.content}`) | ||
| } else if (change.type === "del") { | ||
| parts.push(`[OLD:${change.oldLine}] -${change.content}`) | ||
| } else { | ||
| parts.push(`[NEW:${change.newLine}] ${change.content}`) | ||
| } | ||
| } | ||
| parts.push("") | ||
| } | ||
| } | ||
|
|
||
| return parts.join("\n") | ||
| } | ||
|
|
||
| // ============ 测试用例 ============ | ||
|
|
||
| const testDiff = `diff --git a/src/app.ts b/src/app.ts | ||
| index 1234567..abcdefg 100644 | ||
| --- a/src/app.ts | ||
| +++ b/src/app.ts | ||
| @@ -10,7 +10,8 @@ function main() { | ||
| const config = loadConfig() | ||
| console.log("Starting app") | ||
| - const oldValue = "deprecated" | ||
| + const newValue = "updated" | ||
| + const extraLine = "added" | ||
| return config | ||
| } | ||
| ` | ||
|
|
||
| console.log("=== 测试 Diff 解析 ===\n") | ||
|
|
||
| const parsed = parseDiff(testDiff) | ||
| console.log("解析结果:") | ||
| console.log(JSON.stringify(parsed, null, 2)) | ||
|
|
||
| console.log("\n=== 格式化输出 ===\n") | ||
|
|
||
| const formatted = formatDiffForReview(parsed) | ||
| console.log(formatted) | ||
|
|
||
| console.log("\n=== 验证检查 ===\n") | ||
|
|
||
| // 验证删除行有 oldLine | ||
| const delChange = parsed[0]?.hunks[0]?.changes.find(c => c.type === "del") | ||
| if (delChange && delChange.oldLine) { | ||
| console.log(`✅ 删除行有 oldLine: ${delChange.oldLine}`) | ||
| } else { | ||
| console.log("❌ 删除行缺少 oldLine") | ||
| } | ||
|
|
||
| // 验证添加行有 newLine | ||
| const addChange = parsed[0]?.hunks[0]?.changes.find(c => c.type === "add") | ||
| if (addChange && addChange.newLine) { | ||
| console.log(`✅ 添加行有 newLine: ${addChange.newLine}`) | ||
| } else { | ||
| console.log("❌ 添加行缺少 newLine") | ||
| } | ||
|
|
||
| // 验证格式化输出包含 [OLD:X] 和 [NEW:X] | ||
| if (formatted.includes("[OLD:") && formatted.includes("[NEW:")) { | ||
| console.log("✅ 格式化输出包含 [OLD:X] 和 [NEW:X] 格式") | ||
| } else { | ||
| console.log("❌ 格式化输出格式不正确") | ||
| } | ||
|
|
||
| console.log("\n=== 模拟 Review 评论映射 ===\n") | ||
|
|
||
| // 模拟 gitea-review 的评论映射逻辑 | ||
| interface Comment { | ||
| path: string | ||
| line?: number | ||
| old_line?: number | ||
| body: string | ||
| } | ||
|
|
||
| function mapComment(c: Comment) { | ||
| const result: { path: string; body: string; new_position?: number; old_position?: number } = { | ||
| path: c.path, | ||
| body: c.body, | ||
| } | ||
|
|
||
| if (c.old_line !== undefined && c.old_line > 0) { | ||
| result.old_position = c.old_line | ||
| result.new_position = 0 | ||
| } else if (c.line !== undefined && c.line > 0) { | ||
| result.new_position = c.line | ||
| result.old_position = 0 | ||
| } | ||
|
|
||
| return result | ||
| } | ||
|
|
||
| // 测试评论映射 | ||
| const testComments: Comment[] = [ | ||
| { path: "src/app.ts", line: 13, body: "Consider const here" }, | ||
| { path: "src/app.ts", old_line: 12, body: "This was important logic" }, | ||
| ] | ||
|
|
||
| console.log("输入评论:") | ||
| console.log(JSON.stringify(testComments, null, 2)) | ||
|
|
||
| console.log("\n映射到 Gitea API 格式:") | ||
| const mapped = testComments.map(mapComment) | ||
| console.log(JSON.stringify(mapped, null, 2)) | ||
|
|
||
| // 验证映射结果 | ||
| const newLineComment = mapped.find(m => m.new_position && m.new_position > 0) | ||
| const oldLineComment = mapped.find(m => m.old_position && m.old_position > 0) | ||
|
|
||
| if (newLineComment && newLineComment.old_position === 0) { | ||
| console.log("\n✅ 新行评论正确映射: new_position=" + newLineComment.new_position + ", old_position=0") | ||
| } else { | ||
| console.log("\n❌ 新行评论映射错误") | ||
| } | ||
|
|
||
| if (oldLineComment && oldLineComment.new_position === 0) { | ||
| console.log("✅ 旧行评论正确映射: old_position=" + oldLineComment.old_position + ", new_position=0") | ||
| } else { | ||
| console.log("❌ 旧行评论映射错误") | ||
| } | ||
|
|
||
| console.log("\n=== 测试完成 ===") | ||
|
Comment on lines
+1
to
+222
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,15 +128,18 @@ function formatDiffForReview(files: ParsedFile[]): string { | |
| parts.push(`\n## ${file.path} (${file.status})\n`) | ||
|
|
||
| for (const hunk of file.hunks) { | ||
| parts.push(`@@ starting at line ${hunk.newStart} @@`) | ||
| parts.push(`@@ starting at line ${hunk.newStart} (old: ${hunk.oldStart}) @@`) | ||
|
|
||
| for (const change of hunk.changes) { | ||
| if (change.type === "add") { | ||
| parts.push(`[${change.newLine}] +${change.content}`) | ||
| // New line: [NEW:行号] +内容 | ||
| parts.push(`[NEW:${change.newLine}] +${change.content}`) | ||
| } else if (change.type === "del") { | ||
| parts.push(`[DEL] -${change.content}`) | ||
| // Deleted line: [OLD:行号] -内容 (用于 old_position 评论) | ||
| parts.push(`[OLD:${change.oldLine}] -${change.content}`) | ||
| } else { | ||
| parts.push(`[${change.newLine}] ${change.content}`) | ||
| // Context line: [NEW:行号] 内容 | ||
| parts.push(`[NEW:${change.newLine}] ${change.content}`) | ||
|
Comment on lines
+135
to
+142
|
||
| } | ||
| } | ||
| parts.push("") | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -80,12 +80,13 @@ export default tool({ | |||||||||||||||||||||||||
| .array( | ||||||||||||||||||||||||||
| tool.schema.object({ | ||||||||||||||||||||||||||
| path: tool.schema.string().describe("File path"), | ||||||||||||||||||||||||||
| line: tool.schema.number().describe("Line number in the new file"), | ||||||||||||||||||||||||||
| line: tool.schema.number().optional().describe("Line number in the NEW file (for added/context lines). Use this OR old_line, not both."), | ||||||||||||||||||||||||||
| old_line: tool.schema.number().optional().describe("Line number in the OLD file (for deleted lines). Use this OR line, not both."), | ||||||||||||||||||||||||||
| body: tool.schema.string().describe("Comment content"), | ||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| .optional() | ||||||||||||||||||||||||||
| .describe("Line-level review comments"), | ||||||||||||||||||||||||||
| .describe("Line-level review comments. Use 'line' for new/context lines, 'old_line' for deleted lines."), | ||||||||||||||||||||||||||
| approval: tool.schema | ||||||||||||||||||||||||||
| .enum(["comment", "approve", "request_changes"]) | ||||||||||||||||||||||||||
| .optional() | ||||||||||||||||||||||||||
|
|
@@ -113,11 +114,30 @@ export default tool({ | |||||||||||||||||||||||||
| .replace(/\\t/g, "\t") | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Format review comments for API - also process escaped characters in comments | ||||||||||||||||||||||||||
| const reviewComments = comments.map((c) => ({ | ||||||||||||||||||||||||||
| path: c.path, | ||||||||||||||||||||||||||
| body: c.body.replace(/\\n/g, "\n").replace(/\\t/g, "\t"), | ||||||||||||||||||||||||||
| new_position: c.line, | ||||||||||||||||||||||||||
| })) | ||||||||||||||||||||||||||
| // Per Gitea API: use new_position for new file lines, old_position for deleted lines | ||||||||||||||||||||||||||
| const reviewComments = comments.map((c) => { | ||||||||||||||||||||||||||
| const comment: { | ||||||||||||||||||||||||||
| path: string | ||||||||||||||||||||||||||
| body: string | ||||||||||||||||||||||||||
| new_position?: number | ||||||||||||||||||||||||||
| old_position?: number | ||||||||||||||||||||||||||
| } = { | ||||||||||||||||||||||||||
| path: c.path, | ||||||||||||||||||||||||||
| body: c.body.replace(/\\n/g, "\n").replace(/\\t/g, "\t"), | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (c.old_line !== undefined && c.old_line > 0) { | ||||||||||||||||||||||||||
| // Comment on deleted line (old file) | ||||||||||||||||||||||||||
| comment.old_position = c.old_line | ||||||||||||||||||||||||||
| comment.new_position = 0 | ||||||||||||||||||||||||||
| } else if (c.line !== undefined && c.line > 0) { | ||||||||||||||||||||||||||
| // Comment on new/context line (new file) | ||||||||||||||||||||||||||
| comment.new_position = c.line | ||||||||||||||||||||||||||
| comment.old_position = 0 | ||||||||||||||||||||||||||
|
Comment on lines
+130
to
+136
|
||||||||||||||||||||||||||
| // Comment on deleted line (old file) | |
| comment.old_position = c.old_line | |
| comment.new_position = 0 | |
| } else if (c.line !== undefined && c.line > 0) { | |
| // Comment on new/context line (new file) | |
| comment.new_position = c.line | |
| comment.old_position = 0 | |
| // Comment on deleted line (old file) - only set old_position | |
| comment.old_position = c.old_line | |
| } else if (c.line !== undefined && c.line > 0) { | |
| // Comment on new/context line (new file) - only set new_position | |
| comment.new_position = c.line |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing validation for comments that have both 'line' and 'old_line' defined, or neither defined. According to the documentation, users should use one OR the other, but there's no enforcement. If both are provided, the code will use old_line (due to the if-else ordering), which may not be the user's intent. If neither is provided, a comment will be created with no position fields, which could cause API errors or unexpected behavior.
Consider adding validation to ensure exactly one of 'line' or 'old_line' is provided, and throw a descriptive error if the constraint is violated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test diff string has incorrect formatting. Line 119 shows
+++ b/src/app.ts(the standard git diff format for the new file path), but according to the diff view, there's an extra+character at line 119, making it++++ b/src/app.ts. This is incorrect git diff syntax and will likely cause the parseDiff function to fail or skip this file header.The correct format should be:
Not: