feat(ai-review 對話收斂): 讀 PR review 留言判斷解決狀態並收斂 findings #42
@@ -482,5 +482,11 @@
|
||||
"role": "Bard",
|
||||
"original_finding": "`fetchAccountQuota` 中的 `QUOTA_STRATEGIES` 物件定義龐大,將所有平台策略硬編碼於此,未來新增供應商難以維護;建議抽離至獨立檔案或策略模式。",
|
||||
"reason": "過早最佳化(與先前已排除的 usage.js SRP 拆檔建議等價)。目前 QUOTA_STRATEGIES 為精簡的查表物件、各平台策略短小且集中易讀;在供應商數量出現實際膨脹痛點前抽檔,徒增檔案與匯入複雜度。"
|
||||
},
|
||||
{
|
||||
"location": "app/usage.js",
|
||||
"role": "Assassin",
|
||||
"original_finding": "extractUsage 對不預期 payload 僅返回 null,過度信任 API 回應結構,可能導致計費或配額相關監控被繞過;建議增加 Schema Validation、異常明確記錄。",
|
||||
"reason": "誤判/過度設計。extractUsage 僅用於「使用量顯示統計」,非計費或配額強制;回傳 null 是「此回應無可辨識 usage 資訊」的正確訊號,呼叫端以 0 計入並降級顯示,不影響任何金流或門檻判斷。對 best-effort 顯示統計加 schema validation 與錯誤記錄屬過度設計。"
|
||||
}
|
||||
]
|
||||
|
||||
@@ -1,34 +1,10 @@
|
||||
[
|
||||
{
|
||||
"level": "critical",
|
||||
"role": "Maya",
|
||||
"location": "app/comments.test.js",
|
||||
"problem": "在 `postFindingsReview` 測試中,雖然增加了驗證 usageSection 的案例,但對於舊問題(is_new: false)不應被標註的邏輯,缺乏針對「舊問題數量是否正確統計進總計」的邊界測試。",
|
||||
"suggestion": "補上一個測試案例:驗證當存在新問題與舊問題時,統計表格中「新問題」與「舊問題」的行數與數字皆正確,且舊問題確實沒有產生對應的行內評論。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Assassin",
|
||||
"location": "app/usage.test.js",
|
||||
"problem": "`extractUsage` 對不預期 payload 僅返回 `null`,過度信任 API 回應結構,可能導致計費或配額相關監控被繞過。",
|
||||
"suggestion": "增加嚴格結構驗證(Schema Validation),異常時應明確記錄並標示,而非默默忽略。",
|
||||
"is_new": false
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Maya",
|
||||
"location": "app/findings.test.js",
|
||||
"problem": "在 `appendExclusions` 測試中,測試案例僅檢查了「檔案路徑 + 原文」的去重,未測試當只有檔案路徑相同、但原問題內容不同時的行為(理應視為不同排除條目)。",
|
||||
"suggestion": "補上測試案例:輸入兩條路徑相同但原問題不同的 exclusion,確保兩者皆被成功寫入。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Maya",
|
||||
"location": "app/findings.test.js",
|
||||
"problem": "在 `filterFalsePositivesWithAI` 的測試中,測試了 LLM 呼叫失敗時會保守保留,但未測試當 `chatFn` 回傳結構不完整(例如缺少 verdict 欄位)時,是否真的有正確過濾或保留。",
|
||||
"suggestion": "增加一個測試案例,模擬 `chatFn` 回傳一個包含錯誤 verdict 格式的物件,驗證該問題是否如預期被保守保留。",
|
||||
"location": "app/findings.test.js:283",
|
||||
"problem": "在測試 `resolveMissingLineNumbers` 時,僅測試了 `chatFn` 成功回傳有效或無效行號的情況,但未測試 `chatFn` 拋出例外(Exception)的失敗情境。",
|
||||
"suggestion": "補上 `chatFn` throw error 的測試案例,驗證該函數是否能妥善處理例外並正確記錄警告資訊,而非讓整個執行流程中斷。",
|
||||
"is_new": true
|
||||
}
|
||||
]
|
||||
|
||||
@@ -44,6 +44,7 @@
|
||||
- 帳號額度:`fetchAccountQuota` 依平台採不同策略——OpenRouter(`openai` slot 指向 openrouter.ai 時)以 `GET /auth/key` 取得 USD credits 已用/上限/剩餘;Ollama、OpenCode 為本地/自架服務回報「不適用」;OpenAI、Claude、Gemini、Amazon Q 的帳號額度需 org/admin 權限,API key 無法取得時誠實回報原因
|
||||
- 兩種來源皆無法取得(例如帳號無上限且回應無速率 header)時,降級為「無法計算百分比」並附原因,不中斷流程;`formatUsageStats` 產生 Review 本文區塊,`formatUsageStatsLine` 產生單行 log 摘要
|
||||
14. 誤報判斷套用「防守方」角色:`app/findings.js` 的 `filterFalsePositivesWithAI` 以 `app/roles.js` 的 `loadRole('Paladin')` 載入防守方角色,並用 `buildVerdictPrompt(role, exclusionHint)` 組出帶其個性與裁決準則的 system prompt;對每一條 finding 各派一個防守方 sub-agent(`judgeFindingIsFalsePositive`)裁決 `confirmed`/`false_positive`,多條問題時以 `Promise.all` 平行處理;判為誤報者剔除、成立者保留,任一 sub-agent 失敗(含解析失敗)保守視為成立保留,不中斷流程。角色檔遺失時 `buildVerdictPrompt(null)` 退回通用裁判 prompt。
|
||||
15. location 行號強制:`buildAnalysisPrompt` 明確要求每條問題的 `location` 必須是 `檔案路徑:行號`(單一行號、不可只給檔名),否則該問題無法在 Review 行內標註、只剩統計數字。Step5 角色分析後由 `resolveMissingLineNumbers` 把關:對「只有檔名、缺行號」的新問題,用 `buildLocateLinePrompt(role)` 反問**原角色**、附該檔 diff 區段(`extractFileDiff`)請它回 `{"line": 數字}`,重複嘗試到取得有效行號為止(每條最多 `MAX_LOCATE_ATTEMPTS=3` 次,避免無限迴圈);成功補成 `檔案:行號`,連續失敗則記錄警告並保留檔名。
|
||||
|
||||
# 使用說明
|
||||
|
||||
|
||||
@@ -307,6 +307,22 @@ describe('postFindingsReview', () => {
|
||||
assert.equal(reviewCalls[0].body, reviewCalls[0].body.trimEnd());
|
||||
});
|
||||
|
||||
it('counts both new and old findings in the summary but only inline-comments new ones', async () => {
|
||||
const reviewCalls = [];
|
||||
await postFindingsReview([
|
||||
{ level: 'critical', role: 'Rex', location: 'app/a.js:10', suggestion: 'old crit', is_new: false },
|
||||
{ level: 'warning', role: 'Leo', location: 'app/b.js:20', suggestion: 'new warn', is_new: true },
|
||||
{ level: 'info', role: 'Maya', location: 'app/c.js:30', suggestion: 'new info', is_new: true },
|
||||
], { postReview: async (a) => { reviewCalls.push(a); } });
|
||||
|
||||
const body = reviewCalls[0].body;
|
||||
assert.match(body, /\| 新問題 \| 0 筆 \| 1 筆 \| 1 筆 \| 0 筆 \|/);
|
||||
assert.match(body, /\| 舊問題 \| 1 筆 \| 0 筆 \| 0 筆 \| 0 筆 \|/);
|
||||
// 舊問題 app/a.js 不產生行內 comment;只有新問題被標註
|
||||
assert.ok(!reviewCalls[0].comments.some(c => c.path === 'app/a.js'));
|
||||
assert.deepEqual(reviewCalls[0].comments.map(c => c.path), ['app/b.js', 'app/c.js']);
|
||||
});
|
||||
|
||||
it('separates old and new findings in default review statistics', async () => {
|
||||
const reviewCalls = [];
|
||||
await postFindingsReview([
|
||||
|
||||
+60
-1
@@ -1,7 +1,7 @@
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import { chatJSON } from './llm.js';
|
||||
import { buildAnalysisPrompt, loadRole, buildVerdictPrompt } from './roles.js';
|
||||
import { buildAnalysisPrompt, loadRole, buildVerdictPrompt, buildLocateLinePrompt } from './roles.js';
|
||||
import { FINDINGS_PATH, EXCLUSIONS_PATH } from './config.js';
|
||||
import { line, ok, warn } from './log.js';
|
||||
|
||||
@@ -244,6 +244,65 @@ function fallback(label, findings, e) {
|
||||
return findings;
|
||||
}
|
||||
|
||||
const MAX_LOCATE_ATTEMPTS = 3;
|
||||
|
||||
/** 從 location 取出行號;無 `檔案:行號`(或多檔逗號)時回 null。 */
|
||||
function findingLine(location) {
|
||||
const s = String(location || '').trim();
|
||||
if (!s || s.includes(',')) return null;
|
||||
const m = /^(.+?):(\d+)(?:-\d+)?$/.exec(s);
|
||||
return m ? Number(m[2]) : null;
|
||||
}
|
||||
|
||||
/** 從整份 unified diff 擷取指定檔案的區段,找不到時回退整份 diff。 */
|
||||
function extractFileDiff(diff, file) {
|
||||
const lines = String(diff || '').split('\n');
|
||||
const out = [];
|
||||
let capturing = false;
|
||||
for (const l of lines) {
|
||||
if (l.startsWith('diff --git ')) capturing = l.includes(`b/${file}`) || l.includes(`a/${file}`);
|
||||
if (capturing) out.push(l);
|
||||
}
|
||||
return out.length ? out.join('\n') : String(diff || '');
|
||||
}
|
||||
|
||||
/**
|
||||
* 對「只有檔名、缺行號」的 findings,反問原角色依該檔 diff 找出行號,
|
||||
* 重複嘗試直到取得有效行號(每條最多 maxAttempts 次,避免無限迴圈);
|
||||
* 成功則把 location 補成 `檔案:行號`,否則保留原檔名。
|
||||
*/
|
||||
export async function resolveMissingLineNumbers(findings, diff, deps = {}) {
|
||||
const { chatFn = chatJSON, getRole = loadRole, maxAttempts = MAX_LOCATE_ATTEMPTS } = deps;
|
||||
let resolved = 0;
|
||||
let pending = 0;
|
||||
for (const f of findings) {
|
||||
if (findingLine(f.location) != null) continue; // 已有行號
|
||||
const file = String(f.location || '').split(',')[0].split(':')[0].trim();
|
||||
if (!file) continue;
|
||||
pending += 1;
|
||||
const systemPrompt = buildLocateLinePrompt(getRole(f.role) || { name: f.role });
|
||||
const userContent = `${JSON.stringify({ file, problem: f.problem, suggestion: f.suggestion })}\n\n--- ${file} Git Diff ---\n${extractFileDiff(diff, file)}`;
|
||||
let located = null;
|
||||
for (let attempt = 1; attempt <= maxAttempts && located == null; attempt++) {
|
||||
try {
|
||||
const res = await chatFn(systemPrompt, userContent);
|
||||
const ln = Number(res?.line);
|
||||
if (Number.isInteger(ln) && ln > 0) located = ln;
|
||||
} catch (e) {
|
||||
warn(`[${f.role}] 行號定位失敗(第 ${attempt}/${maxAttempts} 次): ${e.message}`);
|
||||
}
|
||||
}
|
||||
if (located != null) {
|
||||
f.location = `${file}:${located}`;
|
||||
resolved += 1;
|
||||
} else {
|
||||
warn(`[${f.role}] ${maxAttempts} 次嘗試後仍無法定位行號,保留檔名: ${file}`);
|
||||
}
|
||||
}
|
||||
if (pending > 0) ok(`補行號: ${resolved}/${pending} 筆成功定位`);
|
||||
return findings;
|
||||
}
|
||||
|
||||
/** 只保留 AI 需要的欄位,減少 token 用量 */
|
||||
function toAIPayload(findings) {
|
||||
return findings.map(({ level, role, location, problem, suggestion }) => ({ level, role, location, problem, suggestion }));
|
||||
|
||||
+57
-1
@@ -3,7 +3,7 @@ import assert from 'node:assert/strict';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { loadOldFindings, loadExclusions, applyExclusions, filterFalsePositivesWithAI, appendExclusions } from './findings.js';
|
||||
import { loadOldFindings, loadExclusions, applyExclusions, filterFalsePositivesWithAI, appendExclusions, resolveMissingLineNumbers } from './findings.js';
|
||||
import { EXCLUSIONS_PATH, FINDINGS_PATH } from './config.js';
|
||||
|
||||
describe('findings exclusions', () => {
|
||||
@@ -59,6 +59,18 @@ describe('findings exclusions', () => {
|
||||
assert.equal(merged.length, 2);
|
||||
});
|
||||
|
||||
it('appendExclusions keeps same-path entries that have different original text', () => {
|
||||
const fullPath = path.join(workspace, EXCLUSIONS_PATH);
|
||||
fs.mkdirSync(path.dirname(fullPath), { recursive: true });
|
||||
fs.writeFileSync(fullPath, JSON.stringify([{ location: 'app/a.js:1', original_finding: '問題甲' }], null, 2));
|
||||
|
||||
appendExclusions(workspace, [{ location: 'app/a.js:5', original_finding: '問題乙', reason: 'r' }]);
|
||||
|
||||
const onDisk = JSON.parse(fs.readFileSync(fullPath, 'utf8'));
|
||||
assert.equal(onDisk.length, 2); // 同檔但原文不同 → 視為不同排除條目,兩者皆保留
|
||||
assert.deepEqual(onDisk.map(e => e.original_finding), ['問題甲', '問題乙']);
|
||||
});
|
||||
|
||||
it('writes appended exclusions to both workspace and mirror dir', () => {
|
||||
const repoRoot = path.join(workspace, 'repo');
|
||||
fs.mkdirSync(repoRoot, { recursive: true });
|
||||
@@ -225,6 +237,50 @@ describe('findings exclusions', () => {
|
||||
assert.equal(result.length, 2);
|
||||
});
|
||||
|
||||
it('keeps a finding when the defender returns an out-of-range verdict value', async () => {
|
||||
const findings = [{ level: 'warning', role: 'Mage', location: 'a.js:1', problem: 'p', suggestion: 's' }];
|
||||
const chatFn = async () => ({ verdict: 'maybe', reason: 'x' }); // 非 confirmed/false_positive
|
||||
const result = await filterFalsePositivesWithAI(findings, [], chatFn);
|
||||
assert.equal(result.length, 1); // 只有明確 false_positive 才剔除,其餘保守保留
|
||||
});
|
||||
|
||||
it('resolveMissingLineNumbers fills missing line numbers by re-asking the role', async () => {
|
||||
const findings = [
|
||||
{ level: 'critical', role: 'Maya', location: 'app/a.js', problem: 'p', suggestion: 's' },
|
||||
{ level: 'warning', role: 'Leo', location: 'app/b.js:20', problem: 'p', suggestion: 's' }, // 已有行號 → 不動
|
||||
];
|
||||
let calls = 0;
|
||||
const chatFn = async () => { calls += 1; return { line: 42 }; };
|
||||
|
||||
await resolveMissingLineNumbers(findings, 'diff --git a/app/a.js b/app/a.js\n@@ -1 +1 @@', { chatFn, getRole: () => ({ name: 'Maya' }) });
|
||||
|
||||
assert.equal(findings[0].location, 'app/a.js:42'); // 補上行號
|
||||
assert.equal(findings[1].location, 'app/b.js:20'); // 不變
|
||||
assert.equal(calls, 1); // 只對缺行號者呼叫
|
||||
});
|
||||
|
||||
it('resolveMissingLineNumbers retries until a valid line appears', async () => {
|
||||
const findings = [{ level: 'warning', role: 'Leo', location: 'app/x.js', problem: 'p', suggestion: 's' }];
|
||||
let n = 0;
|
||||
const chatFn = async () => { n += 1; return n < 3 ? { line: 0 } : { line: 7 }; };
|
||||
|
||||
await resolveMissingLineNumbers(findings, 'd', { chatFn, getRole: () => null, maxAttempts: 5 });
|
||||
|
||||
assert.equal(findings[0].location, 'app/x.js:7');
|
||||
assert.equal(n, 3); // 第三次才給出有效行號
|
||||
});
|
||||
|
||||
it('resolveMissingLineNumbers keeps the filename after exhausting retries', async () => {
|
||||
const findings = [{ level: 'warning', role: 'Leo', location: 'app/y.js', problem: 'p', suggestion: 's' }];
|
||||
let n = 0;
|
||||
const chatFn = async () => { n += 1; return { line: 0 }; };
|
||||
|
||||
await resolveMissingLineNumbers(findings, 'd', { chatFn, getRole: () => null, maxAttempts: 3 });
|
||||
|
||||
assert.equal(findings[0].location, 'app/y.js'); // 仍保留檔名
|
||||
assert.equal(n, 3); // 嘗試 3 次後放棄
|
||||
});
|
||||
|
||||
it('logs exclusions file metadata and repo state when loading exclusions', () => {
|
||||
const fullPath = path.join(workspace, EXCLUSIONS_PATH);
|
||||
fs.mkdirSync(path.dirname(fullPath), { recursive: true });
|
||||
|
||||
+3
-1
@@ -2,7 +2,7 @@ import path from 'path';
|
||||
import { GITEA_REPOSITORY, PR_NUMBER, PR_HEAD_BRANCH, PR_BASE_BRANCH, getLLMConfig, FINDINGS_PATH, EXCLUSIONS_PATH } from './config.js';
|
||||
import { loadRoles, getRoleIntro } from './roles.js';
|
||||
import { getPRDiff, postComment, getCommitMessageBySha, getBotReviewOutcome, shouldSkipBotCommit } from './gitea.js';
|
||||
import { analyzeWithRole, loadOldFindings, mergeFindings, sortByLevel, deduplicateWithAI, loadExclusions, applyExclusions, filterFalsePositivesWithAI, appendExclusions } from './findings.js';
|
||||
import { analyzeWithRole, loadOldFindings, mergeFindings, sortByLevel, deduplicateWithAI, loadExclusions, applyExclusions, filterFalsePositivesWithAI, appendExclusions, resolveMissingLineNumbers } from './findings.js';
|
||||
import { reconcileConversations, dropResolvedFindings, addCarriedFindings } from './resolve.js';
|
||||
import { saveFindings, postFindingsReview, formatFindingsStatsLine } from './comments.js';
|
||||
import { getRunUsage, getRateLimit, fetchAccountQuota, formatUsageStats, formatUsageStatsLine } from './usage.js';
|
||||
@@ -88,6 +88,8 @@ async function main() {
|
||||
if (analyses[i].status === 'fulfilled') newFindings.push(...analyses[i].value);
|
||||
else warn(`[${roles[i].name}] 分析失敗(跳過): ${analyses[i].reason?.message}`);
|
||||
}
|
||||
// 對只有檔名、缺行號的問題,反問原角色補上行號(最多重試數次),確保後續能行內標註
|
||||
await resolveMissingLineNumbers(newFindings, diff);
|
||||
output(`新 findings ${newFindings.length} 筆(${formatFindingsStatsLine(newFindings)})`);
|
||||
|
||||
// Step6 合併與去重:舊問題 + 對話收斂結果 + 新問題 → 語意去重
|
||||
|
||||
+20
-1
@@ -70,7 +70,7 @@ export function buildAnalysisPrompt(role) {
|
||||
'{',
|
||||
' "level": "critical|warning|info",',
|
||||
` "role": "${role.name}",`,
|
||||
' "location": "檔案路徑:行號 或 檔案路徑",',
|
||||
' "location": "檔案路徑:行號(行號為必填,例如 app/foo.js:42)",',
|
||||
' "problem": "繁體中文(台灣用語)說明審查員認為這裡有問題的原因,不要只填檔案路徑或行號",',
|
||||
' "suggestion": "繁體中文(台灣用語)的具體修改建議"',
|
||||
'}',
|
||||
@@ -80,10 +80,29 @@ export function buildAnalysisPrompt(role) {
|
||||
'- warning:建議修正的問題',
|
||||
'- info:可選的改善建議',
|
||||
'',
|
||||
'location 規則(務必遵守):',
|
||||
'- **每一條問題都必須帶行號**,格式一律為 `檔案路徑:行號`(單一行號,例如 `app/foo.js:42`)。',
|
||||
'- 嚴禁只給檔名而省略行號;行號請取該問題在 Git Diff 新增/修改處的實際行號。',
|
||||
'- 一條問題只對應一個檔案與一個行號,不要用逗號列多個檔案。',
|
||||
'',
|
||||
'只回傳 JSON 陣列,不要有其他文字。如果沒有問題,回傳空陣列 []。',
|
||||
].filter(l => l !== '').join('\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* 由角色定義組出「補行號」的 system prompt:
|
||||
* 當該角色先前提出的問題只有檔名、缺行號時,請它對照 Git Diff 找出實際行號。
|
||||
*/
|
||||
export function buildLocateLinePrompt(role) {
|
||||
const name = role?.name || 'AI Review';
|
||||
const badge = role?.badge ? `${role.badge} ` : '';
|
||||
return [
|
||||
`你是 ${badge}${name}${role?.focus ? `(負責「${role.focus}」面向)` : ''}。`,
|
||||
'你先前提出了一個問題,但 location 只給了檔名、沒有行號。請對照下方提供的該檔案 Git Diff,找出這個問題對應的**實際行號**(新增/修改處在該檔案中的行號)。',
|
||||
'只回傳 JSON 物件:{"line": 數字},不要有其他文字。若 diff 中確實找不到對應行,回傳 {"line": 0}。',
|
||||
].join('\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* 由防守方角色定義組出「單條 finding 誤報裁決」的 system prompt:
|
||||
* 套用其個性與裁決準則本文,要求對一條 finding 判定成立或誤報,回固定 JSON 物件。
|
||||
|
||||
+24
-1
@@ -1,6 +1,6 @@
|
||||
import { describe, it } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { parseRoleFile, loadRoles, loadRole, buildAnalysisPrompt, getRoleIntro } from './roles.js';
|
||||
import { parseRoleFile, loadRoles, loadRole, buildAnalysisPrompt, buildLocateLinePrompt, getRoleIntro } from './roles.js';
|
||||
|
||||
const SAMPLE = `---
|
||||
name: Tester
|
||||
@@ -81,6 +81,29 @@ describe('buildAnalysisPrompt', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildAnalysisPrompt 行號要求', () => {
|
||||
it('requires a line number in location', () => {
|
||||
const prompt = buildAnalysisPrompt(parseRoleFile(SAMPLE));
|
||||
assert.match(prompt, /行號為必填/);
|
||||
assert.match(prompt, /每一條問題都必須帶行號/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildLocateLinePrompt', () => {
|
||||
it('asks the same role to return a JSON line number', () => {
|
||||
const prompt = buildLocateLinePrompt({ name: 'Maya', badge: '🧪', focus: 'testing' });
|
||||
assert.match(prompt, /Maya/);
|
||||
assert.match(prompt, /找出.*行號|實際行號/);
|
||||
assert.match(prompt, /\{"line": 數字\}/);
|
||||
});
|
||||
|
||||
it('tolerates a bare role object without badge/focus', () => {
|
||||
const prompt = buildLocateLinePrompt({ name: 'Leo' });
|
||||
assert.match(prompt, /Leo/);
|
||||
assert.doesNotMatch(prompt, /undefined/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getRoleIntro', () => {
|
||||
it('renders a table row per role with its badge', () => {
|
||||
const intro = getRoleIntro([parseRoleFile(SAMPLE)]);
|
||||
|
||||
Reference in New Issue
Block a user