test(ai-review): 補使用量統計 NaN 安全測試並排除微優化誤報 #46

Merged
jiantw83 merged 14 commits from ai-review-resolve/20260623-163642 into develop 2026-06-23 09:35:21 +00:00
5 changed files with 654 additions and 543 deletions
File diff suppressed because it is too large Load Diff
+4 -12
View File
@@ -1,18 +1,10 @@
[
{
"level": "warning",
"level": "info",
"role": "Maya",
"location": "app/usage.test.js:240",
"problem": "`formatUsageStatsLine` 測試案例中,僅驗證了單一平台的格式,缺失了當 `quota` 或 `rate` 資料缺失或包含無效數字(如 `NaN`)時的處理測試。",
"suggestion": "補充針對 `quota` 或 `rate` 傳入異常資料(如 `limit: NaN`)的測試,驗證 `formatUsageStatsLine` 是否能產生安全的預設文字,而非輸出 `NaN` 或破壞版面。",
"is_new": true
},
{
"level": "warning",
"role": "Rogue",
"location": "app/usage.js:146",
"problem": "在 `recordRateLimit` 中頻繁呼叫 `lowerCaseKeys`,這會對每個請求的 headers 進行複製與轉換,增加記憶體分配開銷。",
"suggestion": "建議直接存取 headers 時改用不區分大小寫的存取函式,避免複製整個物件。",
"location": "app/usage.test.js:217",
"problem": "測試案例 `returns null percent when rate.remaining is null/undefined` 僅驗證了 remaining 為 null/undefined,但未驗證當 rate.limit 為 null/undefined 時的情況。雖然這可能由 `calculatePercent` 內部處理,但針對 `resolveRemainingPercent` 這一層級的整合測試仍不完整。",
"suggestion": "建議補上一個測試案例,明確測試當 `rate.limit` 為 null/undefined 時,`resolveRemainingPercent` 的行為是否符合預期。",
"is_new": true
}
]
+3 -1
View File
@@ -115,7 +115,9 @@ async function main() {
// Step7 過濾:套用排除規則 + 防守方 AI 誤報裁決
step('Step7', '排除規則與誤報過濾');
if (reconcile.excludedFindings.length > 0) {
appendExclusions(WORKSPACE, reconcile.excludedFindings, repoDir || WORKSPACE);
// 以 repoDir 為主(即將提交回去的來源分支副本),WORKSPACE 為鏡像;
// 順序須與下方 loadExclusions 一致,否則會讀到空的 WORKSPACE 而把既有排除規則覆蓋掉。
appendExclusions(repoDir || WORKSPACE, reconcile.excludedFindings, WORKSPACE);
}
const exclusions = loadExclusions(repoDir || WORKSPACE, repoState, WORKSPACE);
input(`待過濾 ${sorted.length} 筆;排除規則 ${exclusions.length}`);
+27 -10
View File
@@ -211,23 +211,40 @@ function round1(n) {
const RATE_KIND_LABEL = { tokens: 'token', requests: '次數' };
admin marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Rogue
問題:在函式內部重複呼叫 Number(n),造成不必要的型別轉換開銷,且在條件式之後又呼叫一次 Number(quota.limit),造成重複轉換。
建議:建議將 Number(n) 的結果暫存起來,或在進入條件式後立即將值賦值給一個變數並重複使用,減少重複轉換的成本。

**嚴重等級**:🟡 警告 **審查員**:Rogue **問題**:在函式內部重複呼叫 `Number(n)`,造成不必要的型別轉換開銷,且在條件式之後又呼叫一次 `Number(quota.limit)`,造成重複轉換。 **建議**:建議將 `Number(n)` 的結果暫存起來,或在進入條件式後立即將值賦值給一個變數並重複使用,減少重複轉換的成本。
/**
* 計算「剩餘百分比」= remaining / limit × 100。
admin marked this conversation as resolved Outdated
Outdated
Review

嚴重等級🟡 警告
審查員:Assassin
問題:在 isFinitePositive 函數中,使用 Number(n) 進行強制轉型。若 n 為物件(例如 {}[]),Number() 的行為在某些邊緣情況下可能會產生非預期的數字,雖然當前邏輯有做 Number.isFinite 檢查,但對於這種隱式轉型仍需保持警惕,且函數名稱 isFinitePositive 雖然清楚,但在這個上下文中,函式內部使用了 Number(n) 強制轉型,這在 JavaScript 中可能會隱蔽掉原本資料型態的不一致。
建議:建議在 isFinitePositive 內部補上對 nullundefined 的顯式檢查,並直接在外部嚴格限制輸入類型或使用更明確的檢查(例如 typeof n === 'number')取代隱式轉型,避免隱式轉型帶來的不可控副作用。

**嚴重等級**:🟡 警告 **審查員**:Assassin **問題**:在 `isFinitePositive` 函數中,使用 `Number(n)` 進行強制轉型。若 `n` 為物件(例如 `{}` 或 `[]`),`Number()` 的行為在某些邊緣情況下可能會產生非預期的數字,雖然當前邏輯有做 `Number.isFinite` 檢查,但對於這種隱式轉型仍需保持警惕,且函數名稱 `isFinitePositive` 雖然清楚,但在這個上下文中,函式內部使用了 `Number(n)` 強制轉型,這在 JavaScript 中可能會隱蔽掉原本資料型態的不一致。 **建議**:建議在 `isFinitePositive` 內部補上對 `null` 或 `undefined` 的顯式檢查,並直接在外部嚴格限制輸入類型或使用更明確的檢查(例如 `typeof n === 'number'`)取代隱式轉型,避免隱式轉型帶來的不可控副作用。
* limit 或 remaining 為 nullundefinedNaNInfinity,或 limit ≤ 0 時回 null
admin marked this conversation as resolved Outdated
Outdated
Review

嚴重等級🟡 警告
審查員:Maya
問題:新增的 isFinitePositive 函數雖然確保了大於 0,但當 n 是類似 "100" 的數字字串時,Number(n) 可以正常運作,然而如果是 nullundefinedNumber(n) 分別會變成 0NaN。雖然在目前的使用邏輯下(limit != null)似乎安全,但這個檢查函數本身沒有對 null/undefined 做明確的防禦性處理,容易在未來被錯誤複用。
建議:建議在 isFinitePositive 內部補上對 nullundefined 的顯式檢查,或是增加對輸入型別的限制,確保該函數作為基礎邏輯檢查更健壯。

**嚴重等級**:🟡 警告 **審查員**:Maya **問題**:新增的 `isFinitePositive` 函數雖然確保了大於 0,但當 `n` 是類似 "100" 的數字字串時,`Number(n)` 可以正常運作,然而如果是 `null` 或 `undefined`,`Number(n)` 分別會變成 `0` 和 `NaN`。雖然在目前的使用邏輯下(`limit != null`)似乎安全,但這個檢查函數本身沒有對 `null`/`undefined` 做明確的防禦性處理,容易在未來被錯誤複用。 **建議**:建議在 `isFinitePositive` 內部補上對 `null` 或 `undefined` 的顯式檢查,或是增加對輸入型別的限制,確保該函數作為基礎邏輯檢查更健壯。
* 避免算出 Infinity%/NaN%/負百分比或除以零。
*/
function calculatePercent(remaining, limit) {
if (remaining == null || limit == null) return null;
const rem = Number(remaining);
const lim = Number(limit);
admin marked this conversation as resolved Outdated
Outdated
Review

嚴重等級🟡 警告
審查員:Leo
問題:resolveRemainingPercent 函式同時處理了 quota 和 rate 的邏輯,導致檢查 limit 和 remaining 的計算與驗證邏輯在兩處重複,未來若要調整百分比算法,必須兩處同步修改,增加維護負擔。
建議:建議將百分比計算邏輯抽象為獨立的輔助函式(如 calculatePercent(remaining, limit)),讓 resolveRemainingPercent 僅負責選擇計算基準,降低重複性並提升封裝度。

**嚴重等級**:🟡 警告 **審查員**:Leo **問題**:resolveRemainingPercent 函式同時處理了 quota 和 rate 的邏輯,導致檢查 limit 和 remaining 的計算與驗證邏輯在兩處重複,未來若要調整百分比算法,必須兩處同步修改,增加維護負擔。 **建議**:建議將百分比計算邏輯抽象為獨立的輔助函式(如 calculatePercent(remaining, limit)),讓 resolveRemainingPercent 僅負責選擇計算基準,降低重複性並提升封裝度。
if (!Number.isFinite(rem) || !Number.isFinite(lim) || lim <= 0) return null;
return round1((rem / lim) * 100);
}
admin marked this conversation as resolved Outdated
Outdated
Review

嚴重等級🔵 建議
審查員:Bard
問題:註解中提到「落到 { percent: null, reason }」,但程式碼實際邏輯是在 if 判斷式內直接 return,且 resolveRemainingPercent 函式中檢查分散在兩個 if 區塊,產生了重複的邏輯結構。
建議:建議將註解簡化為:「上限或剩餘為無效值時跳過計算」,並將計算百分比的邏輯抽離成一個獨立的輔助函式,統一處理 isFinite 的檢查與百分比運算。

**嚴重等級**:🔵 建議 **審查員**:Bard **問題**:註解中提到「落到 { percent: null, reason }」,但程式碼實際邏輯是在 if 判斷式內直接 return,且 `resolveRemainingPercent` 函式中檢查分散在兩個 if 區塊,產生了重複的邏輯結構。 **建議**:建議將註解簡化為:「上限或剩餘為無效值時跳過計算」,並將計算百分比的邏輯抽離成一個獨立的輔助函式,統一處理 `isFinite` 的檢查與百分比運算。
/**
* 計算「剩餘可用百分比」,依優先序擇一:
* 1. 帳號額度(quota 有上限)→ 剩餘 credits / 上限;
* 2. 速率配額(rate limit header)→ 當前視窗剩餘 / 上限;
* 皆無法取得時回傳 { percent: null, reason }。
* 1. 帳號額度(quota 有有效上限)→ 剩餘 credits / 上限;
* 2. 速率配額(rate limit header,有有效上限)→ 當前視窗剩餘 / 上限;
* 上限或剩餘為無效值(null/0/負數/NaN/Infinity)時跳過計算,落到 { percent: null, reason }。
admin marked this conversation as resolved Outdated
Outdated
Review

嚴重等級🔴 嚴重
審查員:Mage
問題:在計算 remaining 時,remaining = Number(rate.remaining); 之後直接檢查 Number.isFinite(remaining),但若 rate.remainingundefinednullNumber() 會轉為 0。這意味著如果 rate.remaining 缺失,會被錯誤地視為「剩餘 0」而不是「無效值」,進而導致計算出 0% 的錯誤結果,而非預期的落到 { percent: null }
建議:應先檢查 rate.remaining 是否為 null/undefined,或者使用更嚴格的轉換方式,確保只有在確實是有限數字時才進行後續運算。

**嚴重等級**:🔴 嚴重 **審查員**:Mage **問題**:在計算 `remaining` 時,`remaining = Number(rate.remaining);` 之後直接檢查 `Number.isFinite(remaining)`,但若 `rate.remaining` 是 `undefined` 或 `null`,`Number()` 會轉為 `0`。這意味著如果 `rate.remaining` 缺失,會被錯誤地視為「剩餘 0」而不是「無效值」,進而導致計算出 `0%` 的錯誤結果,而非預期的落到 `{ percent: null }`。 **建議**:應先檢查 `rate.remaining` 是否為 null/undefined,或者使用更嚴格的轉換方式,確保只有在確實是有限數字時才進行後續運算。
*/
export function resolveRemainingPercent(quota, rate) {
if (quota?.available && quota.limit != null && Number(quota.limit) > 0) {
if (quota?.available && quota.limit != null) {
const limit = Number(quota.limit);
const remaining = quota.remaining == null ? limit - num(quota.used) : Number(quota.remaining);
return { percent: round1((remaining / limit) * 100), basis: '帳號額度', remaining, limit, unit: quota.currency || '' };
const percent = calculatePercent(remaining, limit);
if (percent != null) {
return { percent, basis: '帳號額度', remaining, limit, unit: quota.currency || '' };
}
}
if (rate?.hasData && Number(rate.limit) > 0) {
const limit = Number(rate.limit);
const remaining = Number(rate.remaining);
const kindLabel = RATE_KIND_LABEL[rate.kind] || rate.kind;
return { percent: round1((remaining / limit) * 100), basis: `速率配額(當前視窗,${kindLabel}`, remaining, limit, unit: '' };
if (rate?.hasData) {
const percent = calculatePercent(rate.remaining, rate.limit);
if (percent != null) {
const kindLabel = RATE_KIND_LABEL[rate.kind] || rate.kind;
return { percent, basis: `速率配額(當前視窗,${kindLabel}`, remaining: Number(rate.remaining), limit: Number(rate.limit), unit: '' };
}
}
let reason;
if (quota?.available && quota.limit == null) reason = '帳號額度無上限,無法計算百分比';
+52
View File
@@ -196,6 +196,32 @@ describe('resolveRemainingPercent', () => {
assert.match(pct.reason, /無上限/);
});
it('returns null percent for non-finite or non-positive quota limits', () => {
for (const limit of [0, -5, Infinity, NaN, undefined]) {
admin marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Bard
問題:測試迴圈中的陣列 [0, -5, Infinity, NaN, undefined] 直接混用了不同型別與邊界值,雖然測試目的明確,但在測試碼中,將邏輯測試與邊界測試分開撰寫會更具可讀性。
建議:建議將測試案例拆分,例如分開測試「非數字類型」、「負數」與「邊界值(Infinity/NaN)」,這樣在測試失敗時能更快速釐清是哪種輸入類型導致的問題。

**嚴重等級**:🟡 警告 **審查員**:Bard **問題**:測試迴圈中的陣列 `[0, -5, Infinity, NaN, undefined]` 直接混用了不同型別與邊界值,雖然測試目的明確,但在測試碼中,將邏輯測試與邊界測試分開撰寫會更具可讀性。 **建議**:建議將測試案例拆分,例如分開測試「非數字類型」、「負數」與「邊界值(Infinity/NaN)」,這樣在測試失敗時能更快速釐清是哪種輸入類型導致的問題。
const pct = resolveRemainingPercent({ available: true, used: 0, limit, remaining: limit, currency: 'USD' }, null);
assert.equal(pct.percent, null, `quota.limit=${limit} 應算不出百分比`);
}
});
it('returns null percent for non-finite or non-positive rate limits', () => {
for (const limit of [0, -1, Infinity, NaN]) {
const pct = resolveRemainingPercent({ available: false, reason: 'x' }, { hasData: true, remaining: limit, limit, kind: 'tokens' });
assert.equal(pct.percent, null, `rate.limit=${limit} 應算不出百分比`);
}
});
it('returns null percent when rate.remaining is null/undefined', () => {
for (const remaining of [null, undefined]) {
const pct = resolveRemainingPercent({ available: false, reason: 'x' }, { hasData: true, remaining, limit: 200000, kind: 'tokens' });
assert.equal(pct.percent, null, `rate.remaining=${remaining} 應算不出百分比`);
}
Review

嚴重等級🔵 建議
審查員:Maya
問題:測試案例 returns null percent when rate.remaining is null/undefined 僅驗證了 remaining 為 null/undefined,但未驗證當 rate.limit 為 null/undefined 時的情況。雖然這可能由 calculatePercent 內部處理,但針對 resolveRemainingPercent 這一層級的整合測試仍不完整。
建議:建議補上一個測試案例,明確測試當 rate.limit 為 null/undefined 時,resolveRemainingPercent 的行為是否符合預期。

**嚴重等級**:🔵 建議 **審查員**:Maya **問題**:測試案例 `returns null percent when rate.remaining is null/undefined` 僅驗證了 remaining 為 null/undefined,但未驗證當 rate.limit 為 null/undefined 時的情況。雖然這可能由 `calculatePercent` 內部處理,但針對 `resolveRemainingPercent` 這一層級的整合測試仍不完整。 **建議**:建議補上一個測試案例,明確測試當 `rate.limit` 為 null/undefined 時,`resolveRemainingPercent` 的行為是否符合預期。
});
it('returns null percent when limit is finite but remaining is non-finite', () => {
const pct = resolveRemainingPercent({ available: true, used: 0, limit: 100, remaining: Infinity, currency: 'USD' }, null);
assert.equal(pct.percent, null);
});
it('does not divide by zero when quota.limit is 0', () => {
const pct = resolveRemainingPercent({ available: true, used: 5, limit: 0, currency: 'USD' }, { hasData: false });
assert.equal(pct.percent, null); // limit > 0 守衛擋掉除以零
4
@@ -244,4 +270,30 @@ describe('formatUsageStatsLine', () => {
const line = formatUsageStatsLine('ollama', 'llama3', usage, { available: false, reason: '本地服務,無帳號額度概念' }, { hasData: false });
assert.match(line, /;剩餘可用: 無法計算(本地服務,無帳號額度概念)/);
});
it('produces safe text (no NaN) when quota/rate carry invalid numbers', () => {
// quota.limit 為 NaN、rate.limit 為 NaN → 不應算出百分比、不得輸出 NaN
const line = formatUsageStatsLine('openai', 'm', usage,
{ available: true, used: 5, limit: NaN, currency: 'USD' },
{ hasData: true, remaining: NaN, limit: NaN, kind: 'tokens' });
assert.doesNotMatch(line, /NaN/);
assert.match(line, /剩餘可用: 無法計算/);
});
it('does not output Infinity/NaN/negative percent for invalid quota numbers', () => {
const ownUsage = { calls: 1, promptTokens: 1, completionTokens: 1, totalTokens: 2 };
for (const limit of [Infinity, 0, -5, NaN]) {
const line = formatUsageStatsLine('openai', 'm', ownUsage, { available: true, used: 0, limit, remaining: limit, currency: 'USD' }, null);
assert.doesNotMatch(line, /NaN|Infinity|-\d+%/);
assert.match(line, /剩餘可用: 無法計算/);
}
});
it('falls back to a valid rate percent when only the quota limit is invalid', () => {
const ownUsage = { calls: 1, promptTokens: 1, completionTokens: 1, totalTokens: 2 };
const line = formatUsageStatsLine('openai', 'm', ownUsage,
{ available: true, used: 0, limit: Infinity, currency: 'USD' }, // quota 無效
{ hasData: true, remaining: 150000, limit: 200000, kind: 'tokens' }); // rate 有效 → 75%
assert.match(line, /剩餘可用: 75%(速率配額/);
});
});