Code Review – LGTM (1)

Linus Torvalds, cha đẻ của Linux, đã thẳng thừng reject PR của 1 Google Engineer, và nói đó là rác.

Hẳn là với mỗi developer, thì điều đó là thật tệ.

Nhất là trong kỷ nguyên GenAI, 1 prompt có thể sinh ra cả ngàn dòng code, nếu không thực sự kiểm soát được chất lượng, thì rất có thể bạn đang mang “rác” đi cho cả team để review.

Để với mỗi dòng code do AI tạo ra, chúng ta đều đánh giá được nó có thực sự đúng, phù hợp và an toàn hay không thì có thể đọc các setup review code của các agent, như của Anthropics (cty cung cấp các model Claude mà ae đang sử dụng) là 1 cách.
Ví dụ điển hình là vấn đề Silent Failure, nghĩa là có lỗi nhưng xảy ra trong im lặng, đội vận hành ko biết, khách hàng không rõ nhưng khi phát hiện ra đã quá muộn, mất mát dữ liệu, số liệu ko chính xác, thất thoát doanh thu cho khách hàng. Incident level 3 !!! 😱

Trong PR Review Tool kit của Claude Code có riêng 1 agent để kiểm tra việc này Slient Failure Hunter, dưới đây là bộ nguyên lý cốt lõi

1. Silent failures are unacceptable – Coi việc lỗi trong im lặng là defect/bug và ko thể chấp nhận được.
2. Users deserve actionable feedback – Mọi thông báo lỗi đến user cần chỉ rõ cái gì bị lỗi và họ có thể làm gì tiếp theo
3. Fallbacks must be explicit and justified – Cơ chế fallback mà thiếu đi sự nhận thức của user là ẩn dấu vấn đề
4. Catch blocks must be specific – Bắt lỗi quá rộng (broad exception catching) dẫn đến việc các lỗi ko liên quan trực tiếp bị ẩn dấu và rất khó để debug
5. Mock/fake implementations belong only in tests – Production code mà fallback về mocks thì là vấn đề của kiến trúc

Vậy cụ thể 1 process review của agent này gồm những bước nào?

Bước đầu tiên, agent sẽ đi tìm tất cả case “error handling”, bao gồm

• try-catch blocks (scala/java), hoặc try-except (python) hoặc tương tự ở các ngôn ngữ khác
• errors callback hoặc event handlers
• fallback hoặc default value khi failure xảy ra
• xem xét log khi fail
• xem xét các function nối tiếp nhau, hoặc có nguy cơ trả ra null

Bước thứ 2, với mỗi trường hợp sẽ xem xét cụ thể

• Chất lượng log thế nào, có đủ context không, có ID để trace không, có giúp ích gì nếu 6 tháng sau review lại không?
• Với lỗi này liệu user có nhận được phản hồi không, lỗi cụ thể hay chung chung, có giúp user quyết định việc gì tiếp theo không?
• Việc catch lỗi có phải chỉ bắt dc expected lỗi không, liệu có bỏ qua lỗi nào không?
• Có fallback khi xảy ra lỗi không, có được user chỉ định fallback hoặc nếu không user có confuse khi fallback xảy ra mà không phải báo lỗi không?
• Lỗi này có cần đẩy lên parent handler để báo lỗi ko, nếu xử lý lỗi ở đây thì resource management tốt hơn hay không?

Ngoài ra việc xem xét cụ thể message lỗi đối với end-user cũng rất quan trọng

• Message có được viết rõ ràng, non-technical cũng hiểu đc không?
• Có ghi rõ lỗi xảy ra ở đâu không?
• Có gợi ý về action tiếp theo không?
• Có đúng context liên quan đến phần user đang thao tác không?

Bước cuối là tổng hợp lại theo các thông tin như lỗi ở file nào, dòng bao nhiêu, ưu tiên cần fix cho những lỗi nào CRITICAL (silent failure, broad catch), HIGH (poor error message, unjustified fallback), MEDIUM (missing context, could be more specific) và có user impact ra sao…

Ngoài ra ae có thể đọc thêm có 1 số agent khác review về design type-design-analyzer, comment-analyzer để đánh giá về comment trong code, hoặc làm thế nào để code đơn giản hơn code-simplifier hoặc từ các tài liệu về code review khác.

Khi việc viết code không còn tốn công thì việc review code lại là bước quan trọng nhất để kiểm soát chất lượng, hãy tự làm việc đó, đừng chỉ vibe-coding!