diff --git a/.gitea/ai-review/findings.json b/.gitea/ai-review/findings.json index 86036ff..ef85255 100644 --- a/.gitea/ai-review/findings.json +++ b/.gitea/ai-review/findings.json @@ -1,42 +1,18 @@ [ { "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:211", - "problem": "程式碼中直接嵌入了正規表達式 `/嚴重等級\\*\\*:(.+)/`,這是一個「魔術值」。它讓程式碼的意圖不夠清晰,且若未來需要修改此模式,會增加維護的困難度,也使得該行程式碼過於冗長,影響閱讀流暢性。", - "suggestion": "建議將此正規表達式提取為一個具名常數,以提升可讀性與可維護性。例如:\n```javascript\nconst SEVERITY_PATTERN = /嚴重等級\\*\\*:(.+)/;\n// ...\nreviewCalls[0].comments.map(c => c.body.match(SEVERITY_PATTERN)?.[1]),\n```", - "is_new": true - }, - { - "level": "warning", - "role": "Leo", - "location": "app/comments.test.js:212", - "problem": "測試中直接硬編碼了嚴重等級的顯示文字 `['🔴 嚴重', '🟡 警告', '🔵 建議']`。這些字串很可能是應用程式中定義的常數,直接複製貼上會導致未來當這些等級的顯示文字需要調整時,必須同時修改應用程式程式碼和測試程式碼,增加維護負擔,也容易造成不一致。", - "suggestion": "如果這些嚴重等級的顯示文字在應用程式中已有定義(例如:`SEVERITY_LEVELS.CRITICAL.DISPLAY_TEXT`),建議在測試中引入並使用這些常數,確保測試與實際邏輯保持同步,避免重複定義的技術債。", - "is_new": true - }, - { - "level": "warning", - "role": "Leo", - "location": "app/comments.test.js:211", - "problem": "測試中用來解析 `c.body` 的正規表達式 `(/嚴重等級\\*\\*:(.+)/)` 過於依賴特定的字串格式「嚴重等級**:」。如果未來這個前綴文字有任何微小的變動(例如:多一個空格、換個標點符號、或改用其他詞彙),即使語義不變,測試也會立刻失效,導致不必要的維護工作,增加了測試的脆弱性。", - "suggestion": "考慮讓正規表達式更具彈性,例如使用 `\\s*` 匹配空白,或將這個字串格式的解析邏輯封裝成一個輔助函式,並在該函式中處理可能存在的格式變動彈性。如果 `c.body` 的內容是從結構化資料組裝而來,更理想的做法是測試該結構化資料本身,而非其最終的字串呈現。", - "is_new": true - }, - { - "level": "warning", - "role": "Mage", - "location": "app/comments.test.js:211", - "problem": "此處的正規表達式 `嚴重等級\\*\\*:(.+)` 使用了貪婪匹配 `.+`。這可能導致它捕獲到「嚴重等級**:」後方,直到字串結尾的所有內容,而非僅僅是預期的嚴重等級文字。若 `c.body` 中在嚴重等級後方還有其他文字(例如:「嚴重等級**:🔴 嚴重。請注意此問題。」),此測試將會因為提取到錯誤的內容而失敗,或在實際情況與預期不符時,無法正確驗證。", - "suggestion": "為了確保只捕獲到預期的嚴重等級文字,應使用更精確的正規表達式。考量到測試的預期值是固定的 `['🔴 嚴重', '🟡 警告', '🔵 建議']`,最穩健的作法是直接匹配這些值,例如:\n`c.body.match(/嚴重等級\\*\\*:(🔴 嚴重|🟡 警告|🔵 建議)/)?.[1]`\n或者,如果嚴重等級後方可能跟隨其他文字但有明確分隔符(如句點、換行符或字串結尾),可以使用非貪婪匹配並指定邊界,例如:\n`c.body.match(/嚴重等級\\*\\*:(.+?)(?:\\s|$|\\.|\\n)/)?.[1]`", - "is_new": true - }, - { - "level": "warning", - "role": "Maya", - "location": "app/comments.test.js:211", - "problem": "這個新增的斷言只驗證了評論內文(`c.body`)中存在且格式正確的嚴重等級資訊。然而,它沒有涵蓋到評論內文可能沒有嚴重等級資訊,或是嚴重等級格式不符預期的情境,這讓程式碼的行為在這些邊界條件下缺乏驗證。", - "suggestion": "建議新增測試案例,或擴充現有案例的測試資料,以涵蓋以下情境:\n1. 評論內文完全沒有「嚴重等級**:」這個模式時,應驗證程式碼的行為(例如,是否正確提取到 `undefined` 或預設值)。\n2. 評論內文有「嚴重等級**:」但後面沒有內容,或內容格式不符預期時,應驗證程式碼的行為(例如,是否正確處理為 `undefined` 或拋出錯誤)。", + "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 } ] diff --git a/app/comments.test.js b/app/comments.test.js index f413765..319c55e 100644 --- a/app/comments.test.js +++ b/app/comments.test.js @@ -187,6 +187,31 @@ describe('postNewCriticalComments', () => { }); describe('postFindingsReview', () => { + const REVIEW_SEVERITY_LABELS = ['🔴 嚴重', '🟡 警告', '🔵 建議']; + const REVIEW_SEVERITY_PATTERN = new RegExp(`\\*\\*嚴重等級\\*\\*:(${REVIEW_SEVERITY_LABELS.join('|')})(?:\\n|$)`); + + function reviewSeverityLabel(comment) { + return comment?.body?.match(REVIEW_SEVERITY_PATTERN)?.[1]; + } + + it('handles missing review severity bodies gracefully', () => { + assert.equal(reviewSeverityLabel(null), undefined); + assert.equal(reviewSeverityLabel(undefined), undefined); + assert.equal(reviewSeverityLabel({}), undefined); + assert.equal(reviewSeverityLabel({ body: null }), undefined); + assert.equal(reviewSeverityLabel({ body: undefined }), undefined); + }); + + it('extracts review severity labels only when the format is valid', () => { + assert.equal( + reviewSeverityLabel({ body: '**嚴重等級**:🔴 嚴重\n**審查員**:Rex' }), + '🔴 嚴重', + ); + assert.equal(reviewSeverityLabel({ body: '**審查員**:Rex' }), undefined); + assert.equal(reviewSeverityLabel({ body: '**嚴重等級**:' }), undefined); + assert.equal(reviewSeverityLabel({ body: '**嚴重等級**:高風險' }), undefined); + }); + it('posts one review with statistics and sorted line comments', async () => { const reviewCalls = []; const findings = [ @@ -209,8 +234,8 @@ describe('postFindingsReview', () => { ['app/a.js', 'app/b.js', 'app/c.js'], ); assert.deepEqual( - reviewCalls[0].comments.map(c => c.body.match(/嚴重等級\*\*:(.+)/)?.[1]), - ['🔴 嚴重', '🟡 警告', '🔵 建議'], + reviewCalls[0].comments.map(reviewSeverityLabel), + REVIEW_SEVERITY_LABELS, ); assert.deepEqual( reviewCalls[0].comments.map(c => c.new_position),