67 lines
14 KiB
Markdown
67 lines
14 KiB
Markdown
# reviewer/2026-05-q1 — gist
|
||
distill-gen: 1
|
||
source-verbatim: .claude/agent-memory/reviewer/archive/2026-05-q1.md (7 entry [4 review thật + 3 self-review no-spawn] → 5 cụm)
|
||
pointer-style: substring
|
||
|
||
## TỔNG-HỢP (3-5 ý)
|
||
- Rủi-ro không tập trung ở phần phức-tạp mà ở phần bị coi là ĐÃ XONG: cùng lượt review Plan K vừa xác nhận wire đúng chỗ khó (UPSERT-trước-skip) vừa bắt endpoint zombie thân rỗng `Task.CompletedTask`; sentinel tạm chỉ được dùng khi có chunk dọn ĐẶT TÊN TRƯỚC.
|
||
- Vá theo báo-cáo lỗi = vá 1/5: Plan N sửa 1 site, Plan O phải cascade 4 site còn lại cùng khuôn + 3 test hồi-quy; record Controller phải soi gương record Command — lệch field là gốc của 2 ngày lỗi prod F1+F2.
|
||
- ROI spawn đo được: 1 lượt ~25K cho chunk nặng xuyên tầng thì lãi; 4 lượt polish ≈ 100K token đổi gần như 0 phát-hiện ⇒ luật: spawn cho wire BE/migration/xuyên-tầng, bỏ qua polish <30 phút. Chuỗi test giữ xanh 84 → 104 → 106/106 → 111.
|
||
- Dọn prod xong IIS recycle làm seeder chạy lại thành vòng lặp — gốc không ở lệnh xoá mà ở seeder không công-tắc; Plan T cờ `DemoSeed:Disabled` mới thật sự đóng.
|
||
- PASS ≠ hết việc: "Smart Friend guard" bật cả trong lượt PASS-0-issue, rủi-ro thấp treo lại CÓ KHAI; gotcha #47 phát-hiện nhưng treo chờ anh quyết — không tự thêm vào docs.
|
||
|
||
## Entries
|
||
|
||
**[cao] B5-A — Plan K: PASS nhưng để lại endpoint xác sống**
|
||
- VIỆC: review đối-kháng trước K3 cho Mig 31 (đổi chỗ + đổi nghĩa cờ skip-to-final) — 11 file BE, +4093/−83 LOC (Designer migration chiếm 3938 dòng), commit `eb106f2..56868bf..db66253..364aef6`.
|
||
- KẾT-LUẬN: PASS với 2 Major + 2 Minor. Wire đúng: nhánh Approver F2 đặt `:477` — SAU UPSERT opinion `:441-468`, TRƯỚC đẩy con-trỏ `:502` ⇒ chữ ký người duyệt ghi trước khi nhảy terminal; guard `skipToFinal` V1 đặt ở NGƯỜI GỌI (`:147-149` TransitionAsync) nên V1 giữ chữ ký cũ. sqlcmd Dev xác nhận 7 cột `Allow*` + `Users.AllowDrafterSkipToFinal` drop (count=0), snapshot regen sạch, `HasDefaultValue(false)` có nối, Mig 31 `Up()` đảo tay ADD→DROP đúng khuôn. Major-1: endpoint zombie `PATCH /api/users/{id}/allow-skip-final` còn sống, thân rỗng (`Task.CompletedTask`) — admin tick, BE nuốt im ⇒ đòi chunk K5 dọn. Major-2: comment cũ `ApprovalWorkflow.cs:78` vẫn nói cờ "đã move sang Users…" trong khi `:107-113` là prop mới. Minor: `PurchaseEvaluationFeatures.cs:401` còn chữ "Drafter"; `ApprovalWorkflowConfiguration.cs:22` chưa ghi Mig 31.
|
||
- BÀI-HỌC: sentinel tạm kèm lời hứa "chunk sau dọn" là bẫy — chunk sau đổi phạm-vi thì sentinel hoá xác sống. Muốn dùng phải có chunk dọn ĐẶT TÊN TRƯỚC, đứng trước cả chunk sửa test.
|
||
- BẤT-NGỜ: cùng lượt review vừa xác nhận wire ĐÚNG chỗ khó (thứ tự UPSERT-trước-skip) vừa bắt lỗi chỗ dễ (endpoint quên xoá) — rủi-ro không nằm ở phần phức-tạp mà ở phần bị coi là đã xong.
|
||
→ substring:"Task.CompletedTask"
|
||
|
||
**[cao] B5-B — Plan N→U: 8 kế-hoạch self-review, 3 phản-mẫu đắt**
|
||
- VIỆC: em-main tự review liên-tiếp 8 kế-hoạch (N sửa lookup per-NV, O cascade 4 site, P Controller body, Q CSS, R+S+T5 xoá prod, T cờ DemoSeed, U sidebar).
|
||
- KẾT-LUẬN: Plan N vá `PurchaseEvaluationFeatures.cs:765`; Plan O phát-hiện 4 site còn lại cùng khuôn (`EnsureCanRejectV2Async:201` · `ApplyReturnModeAsync:248` · `EnsureEditableForDetailsAsync:72` · `AdjustBudgetCommandHandler:311`) + 3 test hồi-quy `PurchaseEvaluationPerNvLookupRegressionTests.cs`; Plan P thêm 3 field vào `TransitionPeBody:267` — gốc của 2 ngày lỗi prod F1+F2; Plan R+S+T5 xoá ~720 row prod mà BE vẫn khoẻ nhờ giữ V1 active; Plan T thêm cờ `DemoSeed:Disabled` tắt 5 hàm seed demo. Tổng: 111 test (+7 so 104), 31 mig, 47 gotcha.
|
||
- BÀI-HỌC: vá theo báo-cáo lỗi = vá 1/5; refactor đổi hình-dạng khoá phải LIỆT KÊ TOÀN BỘ site tra-cứu cùng khuôn rồi vá một lượt. Record ở Controller phải soi gương record Command — lệch field là mất dữ-liệu câm.
|
||
- BẤT-NGỜ: dọn prod xong thì IIS recycle làm seeder chạy lại ⇒ dữ-liệu demo mọc lại thành vòng lặp; gốc không ở lệnh xoá mà ở seeder không công-tắc — Plan T mới thật sự đóng.
|
||
→ substring:"PerNvLookupRegressionTests"
|
||
|
||
**[vừa] B5-C — Plan M: PASS, hẹp có chủ-đích, còn lệch chữ xuyên module**
|
||
- VIỆC: review trước push 3 commit `c2afef2..508b17a..4dd6f9c` — sửa ca biên "đang ở Bước 1" + đổi nhãn hiển-thị Phase=TraLai.
|
||
- KẾT-LUẬN: PASS. Hot path `Service.cs:287-333`: thay "đặt TraLai + xoá con-trỏ + return" bằng "đặt con-trỏ (0,1) + summary 'không lùi được' → rơi xuống reset SLA `:364` → GIỮ ChoDuyet"; người gọi `:94-100` truyền Phase mới vào log ⇒ ContextNote khớp assert test. Drafter mode `:268-275` giữ nghĩa; Assignee `:335-360` vẫn ném nếu không khớp; F2/F3/F4 KHÔNG đụng. Build 0 err 2 warn, test 106/106 (58 Domain + 48 Infra, +2), npm build 2 app sạch TS, không `--no-verify`. 2 test mới phủ đúng 2 ca biên.
|
||
- BÀI-HỌC: phân-biệt động-từ hành-động ("Trả lại" trên nút) với nhãn kết-quả ("Cần chỉnh sửa lại" cho Phase=98) — đổi tên phải chọn đúng lớp, nhầm lớp thì UI mâu-thuẫn.
|
||
- BẤT-NGỜ: phạm-vi hẹp để lại 8 chỗ chữ "Trả lại" chưa đổi + 2 module anh em (`contracts.ts:29`, `budget.ts:20`) vẫn `98:'Trả lại'` ⇒ deploy chung thì 3 module gọi khác nhau cho cùng một trạng-thái.
|
||
→ substring:"c2afef2"
|
||
|
||
**[vừa] B5-D — Plan AA: PASS sạch + bài toán ROI của việc spawn** *(gộp 2 entry cùng Plan: pre-commit + post-wrap)*
|
||
- VIỆC: review trước commit chunk A (BE+Layout) + chunk B (FE trang ma-trận, implementer làm), rồi ghi nhận 4 vòng polish sau đó KHÔNG spawn.
|
||
- KẾT-LUẬN: PASS 0 critical / 0 major / 0 minor; wire xuyên suốt Controller `:21-26` → Handler `:114-117` (`request.IsUserSelectable is bool ius → Where(...)`) → FE `:29-33` gọi thật (không mock) → route `App.tsx:38` + `Layout.tsx resolvePath:81-86`; 0 migration (cột có từ Mig 25); seed menu idempotent (PK trên Key ⇒ ToDictionary an toàn); `[Authorize]` trần `:18` được GIỮ (gotcha #44); quyền 7 role qua `SeedPurchaseEvaluationPermissionDefaultsAsync:1573`. Rủi-ro thấp ghi nhận: non-admin truyền `isUserSelectable=false` xem được workflow chưa ghim — dữ-liệu không nhạy-cảm, không chặn commit. Plan AA tổng ~620 LOC / 7 commit `a1a910f..ee0902a`, spawn 1 lần ~25K.
|
||
- BÀI-HỌC: gom review một lượt cho chunk nặng (wire BE / migration / xuyên tầng), bỏ qua với polish <30 phút (CSS/màu/layout) — 4 lần spawn polish ≈ 100K token đổi gần như không phát-hiện.
|
||
- BẤT-NGỜ: "Smart Friend guard" khai VẪN BẬT ngay trong lượt PASS-0-issue, kèm 3 ghi-chú rủi-ro treo — PASS nghĩa là "không có gì đáng chặn", không phải "không còn gì"; bất đối-xứng cấu-trúc 2 app (fe-user có StaticLeaf "Hộp thư", fe-admin không) được CHẤP-NHẬN thay vì ép mirror.
|
||
→ substring:"a1a910f"
|
||
|
||
**[vừa] B5-E — chế-độ UAT không spawn (S21/S22): 3 phản-mẫu + 1 gotcha treo**
|
||
- VIỆC: 2 phiên em-main tự review theo `feedback_uat_skip_verify` (bỏ `dotnet test` mỗi chunk nhưng vẫn build + npm build 2 app); 11-12 commit cộng dồn.
|
||
- KẾT-LUẬN: 4 điểm xác nhận: phân quyền V2 siết theo `actor.UserId` (bỏ mệnh-đề lỏng `|| ApprovalWorkflowId != null`); helper `EnsureCanRejectV2Async` chặn request giả mạo `PATCH /transitions decision=Reject` (phòng-thủ 2 lớp FE+BE); handler chỉnh ngân-sách 3 tầng (Drafter Nháp/Trả-lại · Approver ChoDuyet · Admin) siết theo cờ `AllowApproverEditBudget`; mật-khẩu ≥12 ký-tự. Trạng-thái: 104 test / 30 mig / 46 gotcha / ~146 endpoint / 33 user.
|
||
- BÀI-HỌC: 3 phản-mẫu — (a) mở rộng quyền theo mặc-định (S22+4) phải sửa thành admin-tick-từng-slot (S22+5); (b) giả-định field tồn-tại khi render (`PeDetailBundle.changelogs`) → TS2339, xem kiểu trước; (c) script PS 5.1 có dấu tiếng Việt → parser lỗi (gotcha #30).
|
||
- BẤT-NGỜ: vai reviewer tự khai KHÔNG spawn 2 phiên liên-tiếp mà vẫn viết entry đầy-đủ; ghi nhận CICD Monitor đứng thay vai reviewer sau deploy; phát-hiện gotcha #47 (`paths-ignore` bỏ sót `agent-memory`) nhưng TREO chờ anh quyết, không tự thêm docs.
|
||
→ substring:"paths-ignore"
|
||
|
||
## Phụ-lục token (coverage)
|
||
*(chép TRỌN token-list section reviewer/2026-05-q1 của proposal curator — lưới coverage-diff)*
|
||
|
||
- **gotcha#:** `#45` (fix + 3 test hồi-quy, test-before §7) · `#44` (class-level `[Authorize]` bare — "protect", nhắc 2 lần) · `#47` ("paths-ignore agent-memory gap … pending bro decide" — **New gotcha discovered**) · `#30` (PS 5.1 Vietnamese diacritics)
|
||
- **surprise (nhãn nguyên-văn dạng Anti-pattern/Pattern caught/Discovery):** "**Pattern caught: 'Transient sentinel pattern'** — đặt sentinel + comment commit chunk khác cleanup nhưng chunk đó scope SHIFT → zombie state" · "**Anti-patterns observed:** (a) Default scope expansion mistake S22+4 → S22+5 fix (b) History display field assumption … FAIL TS2339 (c) PS 5.1 Vietnamese diacritics gotcha #30 reinforced" · "**Plan N point 9 chỉ catch 1/5 sites**" · "**Plan O caveat #1 surfaced Plan P pre-existing bug**" · "**DbInitializer auto re-seed** loop — Plan R+S clean → IIS recycle → re-seed loop" · "**Anti-pattern observed S24 polish chunks**: UI/UX iteration thuần CSS/layout KHÔNG cần Reviewer spawn mỗi chunk" · "**Discovery #3 anomaly re-tested S24** … KHÔNG reinforce docs-only anomaly hypothesis" · "**Low note S24**: BE filter `IsUserSelectable=false` non-admin leak workflow chưa ghim" · "Non-admin pass `isUserSelectable=false` BE không block → workflow chưa ghim leak risk — Low severity"
|
||
- **guard:** **"Smart Friend guard"** (bật/nhắc ở 6/7 entry; "KHÔNG lower bar") · `EnsureCanRejectV2Async` (BE helper chặn forge Reject, defense-in-depth FE+BE) · entry guard `:75-81` (đòi ChoDuyet mới Reject) · Reject guard `:81` · Admin bypass `:252-265` · anti-fiddle audit (scope drift 0%) · "No `--no-verify` bypass" (×2) · class-level `[Authorize]` bare `:18` giữ nguyên · `HasDefaultValue(false)` EF config · `MenuItemConfiguration HasKey(Key)` PK unique ⇒ ToDictionary an toàn · `PermissionGuard`/`usePermission` KHÔNG có (route guard đủ) · Cat-checklist 5 mục (Wire BE + Schema + Security + Code quality + Test) · `feedback_uat_skip_verify` (rào chế-độ) · `feedback_ef_migration_backfill_reorder` (rào ADD→DROP)
|
||
- **root-cause:** ① "Transient sentinel pattern" ⇒ orphan endpoint `PATCH /api/users/{id}/allow-skip-final` silent NoOp (`Task.CompletedTask`) ② "Plan P Controller `TransitionPeBody:267` record +3 fields mirror `TransitionPurchaseEvaluationCommand` schema — **root cause 2 ngày prod bug F1+F2 wire fail**" ③ "DbInitializer auto re-seed loop … Plan T flag fix root cause" ④ "Plan N point 9 chỉ catch 1/5 sites → Plan O cascade fix"
|
||
- **file:line:** `ApprovalWorkflow.cs:78` + `:107-113` · `PurchaseEvaluationFeatures.cs:401` · `:765` · `ApprovalWorkflowConfiguration.cs:22` · K2 F2 branch `:477` / UPSERT `:441-468` / advance `:502` / caller guard `:147-149` · test ref `:253` (K7-pending) · `Service.cs:287-333` · `:364` · `:268-275` · `:335-360` · caller `:94-100` · entry guard `:75-81` · `:81` · `:252-265` · `:105-132` · `EnsureCanRejectV2Async:201` · `ApplyReturnModeAsync:248` · `EnsureEditableForDetailsAsync:72` · `AdjustBudgetCommandHandler:311` · `TransitionPeBody:267` · Controller `:21-26` · Handler `:114-117` · `WorkflowMatrixViewPage :29-33` · `App.tsx:38` · `Layout.tsx resolvePath:81-86` · `SeedPurchaseEvaluationPermissionDefaultsAsync:1573` · `[Authorize]` bare `:18` · `PeListPanel.tsx:112` · `PeWorkflowPanel.tsx:270` / `:272` / `:315` / `:342` · `PeDetailTabs.tsx:152` / `:287` · `ApprovalWorkflowsV2Page.tsx:639` · `contracts.ts:29` · `budget.ts:20` · file test `PurchaseEvaluationPerNvLookupRegressionTests.cs`
|
||
- **định-danh khác:** commit `eb106f2..56868bf..db66253..364aef6` · `c2afef2..508b17a..4dd6f9c` · `a1a910f..ee0902a` (`da218f1`/`4d60598`/`fbbd361`/`ee0902a`) · Run #210 sha=`ac2c859` (4/4 wire + bundle 2 app rotate) · diff 11 file BE +4093/−83 (Designer 3938 dòng) · test 84 → 104 → **106/106** (58 Domain + 48 Infra) → **111** · mig 29 → 30 → 31 · gotcha 45 → 46 → 47 · ~146 endpoint · 33 user · 2 test mới `ApplyReturnMode_OneLevel_AtStep1Level1_ResetsToBuoc1Cap1_KeepsChoDuyet` + `ApplyReturnMode_OneStep_AtStep1_ResetsToBuoc1Cap1_KeepsChoDuyet` · fe-admin npm build 1926 modules / 740ms / 0 TS err · ~720 row prod bị xoá (Plan R+S+T5) · spawn-cost ~25K/lượt vs ~100K nếu spawn 4 polish
|
||
|
||
### Bổ-sung coverage @P3-gate S170 — ⚠️ LƯỚI GREP THUẦN, KHÔNG PHẢI DỮ KIỆN (đoạn trích sau mỗi token là phần ĐẦU DÒNG verbatim chứa token — dòng archive là mega-line nên đoạn trích thường KHÔNG liên-quan token; CHỈ dùng để coverage-diff grep trúng, đọc nội-dung thì tra verbatim theo token — reviewer P3 @S170)
|
||
|
||
- `Mig 27` — 8 plan consecutive em main self-review per UAT mode (`feedback_uat_skip_verify` rule — em main verify build + test + npm build mỗi chunk). Key val…
|
||
- `Mig 28` — 2. **Stale Mig 28 comment ApprovalWorkflow.cs:78** — comment cũ "F2 (Drafter skip) đã move sang Users.AllowDrafterSkipToFinal" còn nguyên dù l…
|
||
- `Mig 29` — **Anti-patterns observed:** (a) Default scope expansion mistake S22+4 → S22+5 fix — KHÔNG default expand permission scope without admin tick (per…
|
||
- `Mig 30` — S22+4 attachment view endpoint + S22+4 budget-adjust endpoint chưa qua live curl verify (defer post-deploy — bro UAT direct). S22+5 Mig 30 applied …
|
||
- `Service.cs:288` — 1. **Stale narrative comment Service.cs:288-289 + 327-328** UPDATED rõ "Plan M S23 t3 — KHÔNG fallback Drafter, phiếu giữ đang duyệt" — k…
|