Merge pull request 'feat(ai-review 統計): 將統計改為新舊問題分列' (#41) from develop into master
Reviewed-on: actions/code-review#41
This commit was merged in pull request #41.
This commit is contained in:
@@ -392,5 +392,11 @@
|
|||||||
"role": "Assassin",
|
"role": "Assassin",
|
||||||
"original_finding": "此測試明確證實了 `OPENCODE_SKIP_TLS_VERIFY` 環境變數的寬鬆判斷邏輯,導致 OpenCode LLM 連線的 TLS 驗證容易被關閉。這是「關閉 TLS 驗證」的不安全預設,極大地增加了中間人攻擊的風險。",
|
"original_finding": "此測試明確證實了 `OPENCODE_SKIP_TLS_VERIFY` 環境變數的寬鬆判斷邏輯,導致 OpenCode LLM 連線的 TLS 驗證容易被關閉。這是「關閉 TLS 驗證」的不安全預設,極大地增加了中間人攻擊的風險。",
|
||||||
"reason": "誤判/既有設計。OpenCode server 目前支援自簽或內部服務情境,action input 與 README 均明確標示 OPENCODE_SKIP_TLS_VERIFY 預設跳過 TLS 驗證;本 PR 只補測試與 Review comment 內容,未新增或放寬此安全行為。"
|
"reason": "誤判/既有設計。OpenCode server 目前支援自簽或內部服務情境,action input 與 README 均明確標示 OPENCODE_SKIP_TLS_VERIFY 預設跳過 TLS 驗證;本 PR 只補測試與 Review comment 內容,未新增或放寬此安全行為。"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"location": "app/comments.test.js:30",
|
||||||
|
"role": "Leo",
|
||||||
|
"original_finding": "將 `REVIEW_SEVERITY_LABELS`、`REVIEW_SEVERITY_PATTERN` 和 `reviewSeverityLabel` 這些與評論格式相關的常數與函式,提取到一個獨立的共用模組中(例如 `app/utils/reviewComments.js`),並讓測試檔案和任何需要用到它們的應用程式邏輯都從該模組匯入。這樣能確保「評論格式」的定義只有一個來源,提升可維護性。",
|
||||||
|
"reason": "誤判。這些常數與 `reviewSeverityLabel` 只用於 `app/comments.test.js` 內部驗證 review comment body 格式,production code 沒有使用同一段解析邏輯;抽成共用模組會把測試專用輔助程式提升為正式 API,增加不必要的維護負擔。"
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -1,18 +1 @@
|
|||||||
[
|
[]
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Leo",
|
|
||||||
"location": "app/comments.test.js:30",
|
|
||||||
"problem": "常數 `REVIEW_SEVERITY_LABELS`、`REVIEW_SEVERITY_PATTERN` 以及輔助函式 `reviewSeverityLabel` 被定義在測試檔案的 `describe` 區塊內。如果實際的應用程式邏輯(例如產生或處理這些評論的程式碼)也需要用到這些資訊,那麼這會造成邏輯重複或知識分散。未來若評論格式或嚴重等級標籤有變動,將需要同時修改多處,增加維護成本與出錯風險。",
|
|
||||||
"suggestion": "將 `REVIEW_SEVERITY_LABELS`、`REVIEW_SEVERITY_PATTERN` 和 `reviewSeverityLabel` 這些與評論格式相關的常數與函式,提取到一個獨立的共用模組中(例如 `app/utils/reviewComments.js`),並讓測試檔案和任何需要用到它們的應用程式邏輯都從該模組匯入。這樣能確保「評論格式」的定義只有一個來源,提升可維護性。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "info",
|
|
||||||
"role": "Bard",
|
|
||||||
"location": "app/comments.test.js:192",
|
|
||||||
"problem": "此處新增的 `reviewSeverityLabel` 函式,雖其意圖在上下文脈絡中尚稱清晰,但若能為其添上一筆簡潔的 JSDoc 註解,闡明其參數與回傳值的語義,將使這段樂章更臻完善,即便在測試檔案中,亦能提升未來維護者的閱讀體驗,使程式碼的旋律更加和諧。",
|
|
||||||
"suggestion": "建議為 `reviewSeverityLabel` 函式加上 JSDoc 註解,例如:\n```javascript\n /**\n * 從評論物件中提取嚴重等級標籤。\n * @param {object | null | undefined} comment - 評論物件,預期包含 `body` 屬性。\n * @returns {string | undefined} 嚴重等級標籤字串(如 '🔴 嚴重'),若無匹配或輸入無效則回傳 undefined。\n */\n function reviewSeverityLabel(comment) {\n return comment?.body?.match(REVIEW_SEVERITY_PATTERN)?.[1];\n }\n```",
|
|
||||||
"is_new": true
|
|
||||||
}
|
|
||||||
]
|
|
||||||
|
|||||||
+27
-6
@@ -60,16 +60,35 @@ function countBy(findings, predicate) {
|
|||||||
return findings.filter(predicate).length;
|
return findings.filter(predicate).length;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function newFindingsOnly(findings) {
|
||||||
|
return findings.filter(f => f.is_new !== false);
|
||||||
|
}
|
||||||
|
|
||||||
|
export function formatFindingsStats(findings) {
|
||||||
|
const oldFindings = findings.filter(f => f.is_new === false);
|
||||||
|
const newFindings = newFindingsOnly(findings);
|
||||||
|
const row = (label, items) => `| ${label} | ${countBy(items, f => f.level === 'critical')} 筆 | ${countBy(items, f => f.level === 'warning')} 筆 | ${countBy(items, f => f.level === 'info')} 筆 |`;
|
||||||
|
|
||||||
|
return [
|
||||||
|
'| 類型 | 🔴 嚴重 | 🟡 警告 | 🔵 建議 |',
|
||||||
|
'| --- | --- | --- | --- |',
|
||||||
|
row('舊問題', oldFindings),
|
||||||
|
row('新問題', newFindings),
|
||||||
|
].join('\n');
|
||||||
|
}
|
||||||
|
|
||||||
|
export function formatFindingsStatsLine(findings) {
|
||||||
|
const oldFindings = findings.filter(f => f.is_new === false);
|
||||||
|
const newFindings = newFindingsOnly(findings);
|
||||||
|
const row = items => `嚴重${countBy(items, f => f.level === 'critical')} / 警告${countBy(items, f => f.level === 'warning')} / 建議${countBy(items, f => f.level === 'info')}`;
|
||||||
|
return `舊: ${row(oldFindings)};新: ${row(newFindings)}`;
|
||||||
|
}
|
||||||
|
|
||||||
function buildReviewSummary(findings) {
|
function buildReviewSummary(findings) {
|
||||||
const criticalCount = countBy(findings, f => f.level === 'critical');
|
|
||||||
const warningCount = countBy(findings, f => f.level === 'warning');
|
|
||||||
const infoCount = countBy(findings, f => f.level === 'info');
|
|
||||||
return [
|
return [
|
||||||
'## AI Code Review 統計',
|
'## AI Code Review 統計',
|
||||||
'',
|
'',
|
||||||
'| 🔴 嚴重 | 🟡 警告 | 🔵 建議 |',
|
formatFindingsStats(findings),
|
||||||
'| --- | --- | --- |',
|
|
||||||
`| ${criticalCount} 筆 | ${warningCount} 筆 | ${infoCount} 筆 |`,
|
|
||||||
].join('\n');
|
].join('\n');
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -99,6 +118,8 @@ export async function postFindingsReview(findings, deps = {}) {
|
|||||||
const body = buildReviewSummary(summaryFindings);
|
const body = buildReviewSummary(summaryFindings);
|
||||||
await postReview({ body, comments });
|
await postReview({ body, comments });
|
||||||
ok(`review 發布: summary=${summaryFindings.length} total=${sortedComments.length} commentable=${comments.length}`);
|
ok(`review 發布: summary=${summaryFindings.length} total=${sortedComments.length} commentable=${comments.length}`);
|
||||||
|
line(`review summary 統計: ${formatFindingsStatsLine(summaryFindings)}`);
|
||||||
|
line(`review comments 統計: ${formatFindingsStatsLine(sortedComments)}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
+54
-5
@@ -3,7 +3,7 @@ import assert from 'node:assert/strict';
|
|||||||
import fs from 'node:fs';
|
import fs from 'node:fs';
|
||||||
import os from 'node:os';
|
import os from 'node:os';
|
||||||
import path from 'node:path';
|
import path from 'node:path';
|
||||||
import { saveFindings, parseLocation, postNewCriticalComments, postFindingsReview } from './comments.js';
|
import { saveFindings, parseLocation, postNewCriticalComments, postFindingsReview, formatFindingsStats, formatFindingsStatsLine } from './comments.js';
|
||||||
import { FINDINGS_PATH } from './config.js';
|
import { FINDINGS_PATH } from './config.js';
|
||||||
|
|
||||||
describe('saveFindings', () => {
|
describe('saveFindings', () => {
|
||||||
@@ -96,6 +96,33 @@ describe('parseLocation', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('formatFindingsStats', () => {
|
||||||
|
const statsFindings = [
|
||||||
|
{ level: 'critical', is_new: false },
|
||||||
|
{ level: 'warning', is_new: true },
|
||||||
|
{ level: 'info' },
|
||||||
|
{ level: 'custom', is_new: true },
|
||||||
|
];
|
||||||
|
|
||||||
|
it('formats old and new findings by severity', () => {
|
||||||
|
const stats = formatFindingsStats(statsFindings);
|
||||||
|
|
||||||
|
assert.equal(stats, [
|
||||||
|
'| 類型 | 🔴 嚴重 | 🟡 警告 | 🔵 建議 |',
|
||||||
|
'| --- | --- | --- | --- |',
|
||||||
|
'| 舊問題 | 1 筆 | 0 筆 | 0 筆 |',
|
||||||
|
'| 新問題 | 0 筆 | 1 筆 | 1 筆 |',
|
||||||
|
].join('\n'));
|
||||||
|
});
|
||||||
|
|
||||||
|
it('formats compact one-line stats for action logs', () => {
|
||||||
|
assert.equal(
|
||||||
|
formatFindingsStatsLine(statsFindings),
|
||||||
|
'舊: 嚴重1 / 警告0 / 建議0;新: 嚴重0 / 警告1 / 建議1',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('postNewCriticalComments', () => {
|
describe('postNewCriticalComments', () => {
|
||||||
const critical = { level: 'critical', role: 'Rex', location: 'app/preflight.js:19', suggestion: '修這個', is_new: true };
|
const critical = { level: 'critical', role: 'Rex', location: 'app/preflight.js:19', suggestion: '修這個', is_new: true };
|
||||||
|
|
||||||
@@ -190,6 +217,11 @@ describe('postFindingsReview', () => {
|
|||||||
const REVIEW_SEVERITY_LABELS = ['🔴 嚴重', '🟡 警告', '🔵 建議'];
|
const REVIEW_SEVERITY_LABELS = ['🔴 嚴重', '🟡 警告', '🔵 建議'];
|
||||||
const REVIEW_SEVERITY_PATTERN = new RegExp(`\\*\\*嚴重等級\\*\\*:(${REVIEW_SEVERITY_LABELS.join('|')})(?:\\n|$)`);
|
const REVIEW_SEVERITY_PATTERN = new RegExp(`\\*\\*嚴重等級\\*\\*:(${REVIEW_SEVERITY_LABELS.join('|')})(?:\\n|$)`);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 從 review comment body 擷取嚴重等級標籤。
|
||||||
|
* @param {object | null | undefined} comment - 預期包含 body 欄位的 review comment。
|
||||||
|
* @returns {string | undefined} 嚴重等級標籤;格式不符時回傳 undefined。
|
||||||
|
*/
|
||||||
function reviewSeverityLabel(comment) {
|
function reviewSeverityLabel(comment) {
|
||||||
return comment?.body?.match(REVIEW_SEVERITY_PATTERN)?.[1];
|
return comment?.body?.match(REVIEW_SEVERITY_PATTERN)?.[1];
|
||||||
}
|
}
|
||||||
@@ -221,14 +253,15 @@ describe('postFindingsReview', () => {
|
|||||||
];
|
];
|
||||||
|
|
||||||
await postFindingsReview(findings, {
|
await postFindingsReview(findings, {
|
||||||
summaryFindings: findings.filter(f => f.is_new !== false),
|
summaryFindings: findings,
|
||||||
commentFindings: findings,
|
commentFindings: findings,
|
||||||
postReview: async (args) => { reviewCalls.push(args); },
|
postReview: async (args) => { reviewCalls.push(args); },
|
||||||
});
|
});
|
||||||
|
|
||||||
assert.equal(reviewCalls.length, 1);
|
assert.equal(reviewCalls.length, 1);
|
||||||
assert.match(reviewCalls[0].body, /\| 🔴 嚴重 \| 🟡 警告 \| 🔵 建議 \|/);
|
assert.match(reviewCalls[0].body, /\| 類型 \| 🔴 嚴重 \| 🟡 警告 \| 🔵 建議 \|/);
|
||||||
assert.match(reviewCalls[0].body, /\| 0 筆 \| 1 筆 \| 1 筆 \|/);
|
assert.match(reviewCalls[0].body, /\| 舊問題 \| 1 筆 \| 0 筆 \| 0 筆 \|/);
|
||||||
|
assert.match(reviewCalls[0].body, /\| 新問題 \| 0 筆 \| 1 筆 \| 1 筆 \|/);
|
||||||
assert.deepEqual(
|
assert.deepEqual(
|
||||||
reviewCalls[0].comments.map(c => c.path),
|
reviewCalls[0].comments.map(c => c.path),
|
||||||
['app/a.js', 'app/b.js', 'app/c.js'],
|
['app/a.js', 'app/b.js', 'app/c.js'],
|
||||||
@@ -248,6 +281,22 @@ describe('postFindingsReview', () => {
|
|||||||
assert.match(reviewCalls[0].comments[0].body, /建議.*C/s);
|
assert.match(reviewCalls[0].comments[0].body, /建議.*C/s);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('separates old and new findings in default review statistics', async () => {
|
||||||
|
const reviewCalls = [];
|
||||||
|
await postFindingsReview([
|
||||||
|
{ level: 'critical', role: 'Rex', location: 'app/a.js:10', suggestion: 'old critical', is_new: false },
|
||||||
|
{ level: 'warning', role: 'Leo', location: 'app/b.js:20', suggestion: 'new warning', is_new: true },
|
||||||
|
{ level: 'info', role: 'Maya', location: 'app/c.js:30', suggestion: 'new info' },
|
||||||
|
], {
|
||||||
|
postReview: async (args) => { reviewCalls.push(args); },
|
||||||
|
});
|
||||||
|
|
||||||
|
assert.equal(reviewCalls.length, 1);
|
||||||
|
assert.match(reviewCalls[0].body, /\| 舊問題 \| 1 筆 \| 0 筆 \| 0 筆 \|/);
|
||||||
|
assert.match(reviewCalls[0].body, /\| 新問題 \| 0 筆 \| 1 筆 \| 1 筆 \|/);
|
||||||
|
assert.equal(reviewCalls[0].comments.length, 3);
|
||||||
|
});
|
||||||
|
|
||||||
it('only adds comments for findings with parseable file and line', async () => {
|
it('only adds comments for findings with parseable file and line', async () => {
|
||||||
const reviewCalls = [];
|
const reviewCalls = [];
|
||||||
await postFindingsReview([
|
await postFindingsReview([
|
||||||
@@ -258,7 +307,7 @@ describe('postFindingsReview', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
assert.equal(reviewCalls.length, 1);
|
assert.equal(reviewCalls.length, 1);
|
||||||
assert.match(reviewCalls[0].body, /\| 1 筆 \| 1 筆 \| 0 筆 \|/);
|
assert.match(reviewCalls[0].body, /\| 新問題 \| 1 筆 \| 1 筆 \| 0 筆 \|/);
|
||||||
assert.equal(reviewCalls[0].comments.length, 1);
|
assert.equal(reviewCalls[0].comments.length, 1);
|
||||||
assert.equal(reviewCalls[0].comments[0].path, 'app/b.js');
|
assert.equal(reviewCalls[0].comments[0].path, 'app/b.js');
|
||||||
});
|
});
|
||||||
|
|||||||
+18
-4
@@ -3,7 +3,7 @@ import { GITEA_REPOSITORY, PR_NUMBER, PR_HEAD_BRANCH, PR_BASE_BRANCH, getLLMConf
|
|||||||
import { loadRoles, getRoleIntro } from './roles.js';
|
import { loadRoles, getRoleIntro } from './roles.js';
|
||||||
import { getPRDiff, postComment, getCommitMessageBySha, getBotReviewOutcome, shouldSkipBotCommit } from './gitea.js';
|
import { getPRDiff, postComment, getCommitMessageBySha, getBotReviewOutcome, shouldSkipBotCommit } from './gitea.js';
|
||||||
import { analyzeWithRole, loadOldFindings, mergeFindings, sortByLevel, deduplicateWithAI, loadExclusions, applyExclusions, filterFalsePositivesWithAI } from './findings.js';
|
import { analyzeWithRole, loadOldFindings, mergeFindings, sortByLevel, deduplicateWithAI, loadExclusions, applyExclusions, filterFalsePositivesWithAI } from './findings.js';
|
||||||
import { saveFindings, postFindingsReview } from './comments.js';
|
import { saveFindings, postFindingsReview, formatFindingsStatsLine } from './comments.js';
|
||||||
import { cloneRepo, commitAndPush, getRepoState } from './git.js';
|
import { cloneRepo, commitAndPush, getRepoState } from './git.js';
|
||||||
import { validateJSONArrayFile, ensureJSONArrayFileExists } from './json.js';
|
import { validateJSONArrayFile, ensureJSONArrayFileExists } from './json.js';
|
||||||
import { runPreflight } from './preflight.js';
|
import { runPreflight } from './preflight.js';
|
||||||
@@ -11,6 +11,10 @@ import { section, step, line, ok, warn, error } from './log.js';
|
|||||||
|
|
||||||
const WORKSPACE = process.env.GITHUB_WORKSPACE || '/workspace';
|
const WORKSPACE = process.env.GITHUB_WORKSPACE || '/workspace';
|
||||||
|
|
||||||
|
function logFindingsStats(label, findings) {
|
||||||
|
line(`${label}: ${formatFindingsStatsLine(findings)}`);
|
||||||
|
}
|
||||||
|
|
||||||
async function main() {
|
async function main() {
|
||||||
section('AI Code Review Pipeline');
|
section('AI Code Review Pipeline');
|
||||||
step('Step1', 'Pipeline 啟動');
|
step('Step1', 'Pipeline 啟動');
|
||||||
@@ -82,6 +86,7 @@ async function main() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
ok(`Step3 完成: 新 findings 總計 ${newFindings.length} 筆`);
|
ok(`Step3 完成: 新 findings 總計 ${newFindings.length} 筆`);
|
||||||
|
logFindingsStats('Step3 統計', newFindings);
|
||||||
|
|
||||||
step('Step4', 'Findings 合併與語意去重');
|
step('Step4', 'Findings 合併與語意去重');
|
||||||
let repoDir;
|
let repoDir;
|
||||||
@@ -95,25 +100,34 @@ async function main() {
|
|||||||
line(`repo 狀態: branch=${repoState.branch || 'detached'} commit=${repoState.shortSha || 'unknown'} commit_time=${repoState.commitTime || 'unknown'} path=${repoState.repoDir}`);
|
line(`repo 狀態: branch=${repoState.branch || 'detached'} commit=${repoState.shortSha || 'unknown'} commit_time=${repoState.commitTime || 'unknown'} path=${repoState.repoDir}`);
|
||||||
}
|
}
|
||||||
const oldFindings = loadOldFindings(repoDir || WORKSPACE);
|
const oldFindings = loadOldFindings(repoDir || WORKSPACE);
|
||||||
|
logFindingsStats('Step4 舊 findings 統計', oldFindings);
|
||||||
|
logFindingsStats('Step4 新 findings 統計', newFindings);
|
||||||
const mergedFindings = mergeFindings(oldFindings, newFindings);
|
const mergedFindings = mergeFindings(oldFindings, newFindings);
|
||||||
ok(`Step4 merged findings total=${mergedFindings.length}`);
|
ok(`Step4 merged findings total=${mergedFindings.length}`);
|
||||||
|
logFindingsStats('Step4 合併後統計', mergedFindings);
|
||||||
const deduped = await deduplicateWithAI(mergedFindings);
|
const deduped = await deduplicateWithAI(mergedFindings);
|
||||||
|
logFindingsStats('Step4 AI 去重後統計', deduped);
|
||||||
const sorted = sortByLevel(deduped);
|
const sorted = sortByLevel(deduped);
|
||||||
ok(`Step4 去重完成: ${mergedFindings.length} -> ${sorted.length} 筆 (critical=${sorted.filter(f=>f.level==='critical').length} warning=${sorted.filter(f=>f.level==='warning').length} info=${sorted.filter(f=>f.level==='info').length})`);
|
ok(`Step4 去重完成: ${mergedFindings.length} -> ${sorted.length} 筆`);
|
||||||
|
logFindingsStats('Step4 排序後統計', sorted);
|
||||||
|
|
||||||
step('Step5', 'AI 排除問題過濾');
|
step('Step5', 'AI 排除問題過濾');
|
||||||
const exclusions = loadExclusions(repoDir || WORKSPACE, repoState, WORKSPACE);
|
const exclusions = loadExclusions(repoDir || WORKSPACE, repoState, WORKSPACE);
|
||||||
const ruleFiltered = applyExclusions(sorted, exclusions);
|
const ruleFiltered = applyExclusions(sorted, exclusions);
|
||||||
|
logFindingsStats('Step5 規則排除後統計', ruleFiltered);
|
||||||
const filtered = await filterFalsePositivesWithAI(ruleFiltered, exclusions);
|
const filtered = await filterFalsePositivesWithAI(ruleFiltered, exclusions);
|
||||||
|
logFindingsStats('Step5 AI 誤報過濾後統計', filtered);
|
||||||
ok(`Step5 完成: findings total=${filtered.length}`);
|
ok(`Step5 完成: findings total=${filtered.length}`);
|
||||||
|
|
||||||
step('Step6', 'Findings 寫入與 Review 發布');
|
step('Step6', 'Findings 寫入與 Review 發布');
|
||||||
const reviewDir = repoDir || WORKSPACE;
|
const reviewDir = repoDir || WORKSPACE;
|
||||||
saveFindings(WORKSPACE, filtered, reviewDir);
|
saveFindings(WORKSPACE, filtered, reviewDir);
|
||||||
try {
|
try {
|
||||||
const newFindings = filtered.filter(f => f.is_new !== false);
|
logFindingsStats('Step6 儲存 findings 統計', filtered);
|
||||||
|
logFindingsStats('Step6 review summary 統計', filtered);
|
||||||
|
logFindingsStats('Step6 review comments 統計', filtered);
|
||||||
await postFindingsReview(filtered, {
|
await postFindingsReview(filtered, {
|
||||||
summaryFindings: newFindings,
|
summaryFindings: filtered,
|
||||||
commentFindings: filtered,
|
commentFindings: filtered,
|
||||||
});
|
});
|
||||||
ok('Step6 完成');
|
ok('Step6 完成');
|
||||||
|
|||||||
Reference in New Issue
Block a user