spring-code-review-rubric

Spring code review rubric

Severity ladder

• blocker — must fix before commit. (Security hole, broken behavior, gate-bypass.)
• major — must fix before commit OR document as ADR + waiver.
• minor — should fix; leave a note if not.
• nit — taste; mention once, don't insist.

Ten sections

1. Traceability

• Every diff hunk maps to an AC-NNN and a T-NNN.
• Files-in-scope honored (no edits outside the task's declared paths).
• Any new public API has a test referencing the AC.

2. Architecture

• Module boundaries respected (no internal cross-imports).
• ArchUnit rules pass (see archunit-rules).
• Layering correct (no controller→repository skip).
• No new circular dependency.

3. Spring idioms

• Constructor injection only (spring-boot-4-conventions).
• Package layout is by feature/domain, not by layer (no top-level controller/service/repository/model packages).
• @HttpExchange / RestClient (no new RestTemplate).
• @MockitoBean, not @MockBean.
• No @Autowired on fields or constructors.
• No @SpringBootTest where a slice would do.
• No Lombok anywhere in the diff (any lombok.* import is a must-fix).
• No fully-qualified type names (FQNs) inline in code — method parameters, return types, throws clauses, and bodies must use simple names backed by proper import statements at the top of the file. E.g. jakarta.servlet.http.HttpServletResponse response as a parameter is a must-fix; it must be HttpServletResponse response with import jakarta.servlet.http.HttpServletResponse;.

4. Error handling

• Exceptions translated at the controller boundary, not in the service.
• Single error envelope shape, documented in OpenAPI.
• No catch (Exception e) { throw new RuntimeException(e); }.
• Domain exceptions are checked or sealed, not raw RuntimeException.

5. Data access

• No N+1 queries (look for entity navigation in a loop).
• All list endpoints paginated.
• Migration script forward-only OR rollback documented.
• No raw SQL with string concatenation.
• Transactions on services, not controllers.