feat(ai-review 對話收斂): 讀 PR review 留言判斷解決狀態並收斂 findings #42
@@ -46,10 +46,12 @@ async function main() {
|
||||
}
|
||||
|
||||
step('Step2', 'PR 對話收斂');
|
||||
let reconcile = { resolvedFindings: [], carriedFindings: [], resolvedCount: 0, unresolvedCount: 0 };
|
||||
let reconcile = { resolvedFindings: [], carriedFindings: [], resolvedCount: 0, duplicateCount: 0, unresolvedCount: 0 };
|
||||
try {
|
||||
reconcile = await reconcileConversations();
|
||||
ok(`Step2 完成: resolved=${reconcile.resolvedCount} unresolved=${reconcile.unresolvedCount} 加回=${reconcile.carriedFindings.length}`);
|
||||
// 載入來源分支既有的舊問題,供對話收斂判斷「未修復但已存在於舊問題」的情況
|
||||
const oldFindingsForReconcile = loadOldFindings(WORKSPACE);
|
||||
reconcile = await reconcileConversations({ oldFindings: oldFindingsForReconcile });
|
||||
ok(`Step2 完成: resolved=${reconcile.resolvedCount} duplicate=${reconcile.duplicateCount} 加回=${reconcile.carriedFindings.length}`);
|
||||
} catch (e) {
|
||||
warn(`Step2 對話收斂失敗(繼續執行): ${e.message}`);
|
||||
}
|
||||
|
||||
@@ -134,11 +134,12 @@ function isSafeRepoPath(p) {
|
||||
}
|
||||
|
||||
/**
|
||||
* 對話收斂主流程:
|
||||
* 1. 取得 PR 所有行內 review comment,收斂成對話,跳過已 resolve 的;
|
||||
* 2. 取每個對話所在檔案的最新內容,請 AI 判斷問題是否已解決;
|
||||
* 3. 已解決者呼叫 Gitea resolve API 解決對話,並記錄其 finding(供移除舊問題);
|
||||
* 4. 未解決且可解析為 bot finding 者,收集為「加回問題列表」清單。
|
||||
* 對話收斂主流程:取得 PR 所有行內 review comment、收斂成對話、跳過已 resolve 的,
|
||||
* 取最新程式碼請 AI 判斷後,對每個待判斷對話做下列處置:
|
||||
* 1. 程式碼已修復(AI 判定已解決)→ 解決對話,並記錄其 finding 供從舊問題移除;
|
||||
* 2. 未修復但已存在於舊問題(以檔案+建議簽章比對)→ 解決對話(已被追蹤,不重複加回);
|
||||
* 3. 未修復且不在舊問題 → 不解決對話,將其 finding 加入舊問題集合(carriedFindings)。
|
||||
* deps.oldFindings 提供來源分支既有的舊問題清單以供第 2 步比對。
|
||||
|
admin marked this conversation as resolved
Outdated
|
||||
* 任一外部呼叫失敗都降級處理(保守視為未解決),不中斷整體 pipeline。
|
||||
*/
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Maya
**問題**:函式 `reconcileConversations` 缺少對 `judge` 拋出錯誤情境的明確測試。雖然程式碼有 `try-catch` 處理,但應有專門的測試案例來驗證此失敗路徑的行為。
**建議**:請新增測試案例,模擬 `judge` 函式拋出錯誤時,確認 `reconcileConversations` 能正確捕獲錯誤,記錄警告,並將所有待判斷的對話都視為未解決(即 `verdicts` 應全部為 `resolved: false`)。
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Maya
**問題**:缺少關鍵邊界條件與異常路徑的測試案例。包含 `judge` 拋出錯誤、`chatFn` 解析異常、`levelRaw` 或 `suggestion` 空值、`getFileContent` 失敗以及混合正確/錯誤的判斷數據等場景。
**建議**:請在 `app/resolve.test.js` 中新增這些邊界條件的測試案例,確保系統在面對 AI 異常輸出、API 失敗、或輸入欄位缺失時,仍能穩健處理並符合預期行為。
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Maya
**問題**:函式 `reconcileConversations` 缺少對 `judge` 拋出錯誤情境的明確測試。雖然程式碼有 `try-catch` 處理,但應有專門的測試案例來驗證此失敗路徑的行為。
**建議**:請新增測試案例,模擬 `judge` 函式拋出錯誤時,確認 `reconcileConversations` 能正確捕獲錯誤,記錄警告,並將所有待判斷的對話都視為未解決(即 `verdicts` 應全部為 `resolved: false`)。
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Maya
**問題**:函式 `reconcileConversations` 缺少對 `judge` 拋出錯誤情境的明確測試。雖然程式碼有 `try-catch` 處理,但應有專門的測試案例來驗證此失敗路徑的行為。
**建議**:請新增測試案例,模擬 `judge` 函式拋出錯誤時,確認 `reconcileConversations` 能正確捕獲錯誤,記錄警告,並將所有待判斷的對話都視為未解決(即 `verdicts` 應全部為 `resolved: false`)。
|
||||
export async function reconcileConversations(deps = {}) {
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🔴 嚴重 **嚴重等級**:🔴 嚴重
**審查員**:Assassin
**問題**:在 `judgeConversationsResolved` 函式中,`thread`(來自 Gitea comment 內容)和 `code`(來自 PR 檔案內容)被直接拼接進傳給 LLM 的 `payload` 中。如果攻擊者能夠控制這些內容,他們可以透過注入惡意指令來劫持 LLM 的行為,例如使其始終將特定問題判斷為已解決,或嘗試從 LLM 獲取敏感資訊(提示詞注入)。
**建議**:對所有傳遞給 LLM 的外部輸入(如 `thread` 和 `code`)進行嚴格的淨化和隔離。考慮使用結構化輸入而非直接拼接字串,並在 LLM 提示詞中明確指示其忽略任何試圖改變其行為的指令。對於敏感操作,應建立多層驗證機制,不單純依賴 LLM 的判斷。
admin
commented
嚴重等級:🔴 嚴重 **嚴重等級**:🔴 嚴重
**審查員**:Assassin
**問題**:LLM 提示詞注入風險:在 `judgeConversationsResolved` 函式中,外部來源的 `thread` 和 `code` 被直接拼接進傳給 LLM 的 `payload` 中,攻擊者可能注入惡意指令來劫持 LLM 行為。
**建議**:對所有傳遞給 LLM 的外部輸入進行嚴格的淨化和隔離。使用結構化輸入而非直接拼接字串,並在提示詞中明確指示 AI 忽略任何試圖下達指令的內容,僅對邏輯進行判斷。對於敏感操作,應建立多層驗證機制。
|
||||
@@ -147,6 +148,7 @@ export async function reconcileConversations(deps = {}) {
|
||||
resolveComment = resolvePullReviewComment,
|
||||
getFileContent = getFileContentAtRef,
|
||||
judge = judgeConversationsResolved,
|
||||
oldFindings = [],
|
||||
} = deps;
|
||||
|
||||
let comments;
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🔴 嚴重 **嚴重等級**:🔴 嚴重
**審查員**:Rogue
**問題**:這裡又在浪費時間!`reconcileConversations` 函式在取得所有獨特的檔案路徑後,又在迴圈裡對每個檔案路徑依序呼叫 `getFileContent`。如果有很多檔案需要檢查,這會導致 `F` 次遠端 API 呼叫依序執行,嚴重拖慢整體流程。
**建議**:改用 `Promise.all` 或 `Promise.allSettled` 來並行發送所有 `getFileContent` 的請求。這樣可以大幅減少等待時間,讓檔案內容的取得幾乎同時完成。
|
||||
@@ -196,39 +198,63 @@ export async function reconcileConversations(deps = {}) {
|
||||
verdicts = items.map(it => ({ idx: it.idx, resolved: false }));
|
||||
}
|
||||
const resolvedSet = new Set(verdicts.filter(v => v.resolved).map(v => v.idx));
|
||||
const oldSigs = new Set((oldFindings || []).map(findingSig));
|
||||
|
||||
const resolvedFindings = [];
|
||||
const carriedFindings = [];
|
||||
// 分類每個待判斷對話:
|
||||
// - 'resolved':程式碼已修復 → 解決對話,並從舊問題移除
|
||||
// - 'duplicate':未修復但已存在於舊問題 → 解決對話(不重複加回)
|
||||
// - 'carry':未修復且不在舊問題 → 加入舊問題集合(不解決對話)
|
||||
const dispositions = open.map((c, i) => {
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Leo
**問題**:函式 `normalizeKey` 對建議內容進行了非常積極的正規化,移除了所有標點符號、符號和空白字元。雖然這有助於避免行號漂移和微小措辭差異造成的重複判斷,但過度正規化可能會導致不同但語意相近的建議被視為相同,進而影響問題追蹤的精確性。
**建議**:請評估這種積極正規化是否會導致誤判。如果發現有不同建議被錯誤合併的情況,可以考慮放寬正規化規則,例如只移除空白字元和部分標點符號,或加入其他判斷維度(如關鍵字比對)來提高精確度。
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Leo
**問題**:正規化邏輯(`normalizeKey` 等)過於激進且未快取,既可能導致語意相近建議被誤判為相同,也在頻繁比較時造成效能浪費。
**建議**:請評估目前的正規化規則,若發現誤判,放寬規則或加入關鍵字比對。將簽章產生邏輯抽離為獨立 Helper 函式,並在產生時進行快取(Memoize)以提升效能。
|
||||
if (resolvedSet.has(i)) return 'resolved';
|
||||
const sig = c.botFinding ? findingSig(c.botFinding) : null;
|
||||
if (sig && oldSigs.has(sig)) return 'duplicate';
|
||||
return 'carry';
|
||||
});
|
||||
|
||||
// 並行 resolve 所有 AI 判定已解決的對話(allSettled:個別失敗不中斷其他)
|
||||
// 並行對 resolved 與 duplicate 的對話呼叫 resolve API(allSettled:個別失敗不中斷其他)
|
||||
const resolveTargets = open
|
||||
.map((c, i) => ({ c, i }))
|
||||
.filter(({ i }) => resolvedSet.has(i));
|
||||
.filter(({ i }) => dispositions[i] === 'resolved' || dispositions[i] === 'duplicate');
|
||||
const settled = await Promise.allSettled(
|
||||
resolveTargets.map(({ c }) => resolveComment(c.commentIds[0])),
|
||||
);
|
||||
const resolveOutcome = new Map();
|
||||
resolveTargets.forEach(({ i }, j) => resolveOutcome.set(i, settled[j]));
|
||||
|
||||
const resolvedFindings = [];
|
||||
const carriedFindings = [];
|
||||
let resolvedCount = 0;
|
||||
let duplicateCount = 0;
|
||||
for (let i = 0; i < open.length; i++) {
|
||||
const c = open[i];
|
||||
const outcome = resolveOutcome.get(i);
|
||||
if (outcome?.status === 'fulfilled') {
|
||||
resolvedCount += 1;
|
||||
if (c.botFinding) resolvedFindings.push({ ...c.botFinding, is_new: false });
|
||||
ok(`對話已解決並 resolve: ${c.path}:${c.line}`);
|
||||
const disp = dispositions[i];
|
||||
|
||||
if (disp === 'carry') {
|
||||
pushCarried(carriedFindings, c);
|
||||
continue;
|
||||
}
|
||||
if (outcome?.status === 'rejected') {
|
||||
warn(`resolve 對話失敗(保留為未解決): ${c.path}:${c.line} error=${outcome.reason?.message}`);
|
||||
|
||||
const outcome = resolveOutcome.get(i);
|
||||
if (outcome?.status === 'fulfilled') {
|
||||
if (disp === 'resolved') {
|
||||
resolvedCount += 1;
|
||||
if (c.botFinding) resolvedFindings.push({ ...c.botFinding, is_new: false });
|
||||
ok(`對話已解決並 resolve(程式碼已修復): ${c.path}:${c.line}`);
|
||||
} else {
|
||||
duplicateCount += 1;
|
||||
ok(`對話已解決並 resolve(已存在於舊問題): ${c.path}:${c.line}`);
|
||||
}
|
||||
|
admin marked this conversation as resolved
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Maya
**問題**:`reconcileConversations` 中的 `reconcile` 流程包含多個步驟(取得 comments、group、判斷、resolve),一旦中間有外部呼叫失敗就降級。目前的測試案例主要覆蓋了「全部成功」或「特定某個失敗」,但缺乏對「部分 resolve 成功,部分 resolve 失敗」這種狀態的驗證。
**建議**:補充測試案例,模擬部分 `resolveComment` 成功、部分失敗的情境,驗證最終回傳的 `closedCount` 與 `resolvedFindings` 等統計數據是否正確計算。
|
||||
continue;
|
||||
}
|
||||
pushCarried(carriedFindings, c);
|
||||
|
||||
warn(`resolve 對話失敗: ${c.path}:${c.line} error=${outcome?.reason?.message}`);
|
||||
// resolved 但無法關閉對話時,保守加回舊問題避免遺漏;duplicate 本就在舊問題中,無須加回
|
||||
if (disp === 'resolved') pushCarried(carriedFindings, c);
|
||||
}
|
||||
|
||||
const unresolvedCount = open.length - resolvedCount;
|
||||
ok(`對話收斂完成: resolved=${resolvedCount} unresolved=${unresolvedCount} 加回 findings=${carriedFindings.length}`);
|
||||
return { resolvedFindings, carriedFindings, resolvedCount, unresolvedCount };
|
||||
ok(`對話收斂完成: 已修復 resolved=${resolvedCount} 已存在舊問題 duplicate=${duplicateCount} 加入舊問題 carried=${carriedFindings.length}`);
|
||||
return { resolvedFindings, carriedFindings, resolvedCount, duplicateCount, unresolvedCount };
|
||||
}
|
||||
|
||||
function fileOf(location) {
|
||||
|
||||
嚴重等級:🔴 嚴重
審查員:Maya
問題:
reconcileConversations核心流程中,對於getFileContent失敗或內容為空的處理邏輯,直接降級為空字串並視為未解決,但若檔案內容實際上非空且未解決,這可能導致判斷偏差。建議:補測試案例,模擬
getFileContent拋出錯誤時,reconcileConversations是否正確地將對話保留為未解決,且後續統計數字(carriedFindings)是否正確。