| name | code-review |
| description | Use when reviewing, auditing, or assessing SAP CAP code quality: CDS models, service handlers, test files, MTA descriptors, security annotations, or any cap-related code. Performs structured checks against CAP best practices, identifies anti-patterns, flags missing security/error handling, and suggests improvements aligned with capire documentation.
|
| metadata | {"category":"cap","version":"1.0.0","keywords":["code review","CAP review","anti-pattern","quality check","N+1","security review","performance review","handler review","CDS review"],"related":{"cds-modeling":"review CDS entity definitions","service-handlers":"review service handler implementations","security-auth":"review authorization annotations","performance":"identify performance issues in code review","testing":"check test coverage in code review"}} |
Code Review — CAP Best Practices Checklist
Primary reference: Always consult https://cap.cloud.sap/docs first.
For LLM-optimised CAP documentation, fetch https://cap.cloud.sap/docs/llms.txt
or https://cap.cloud.sap/docs/llms-full.txt for deeper context.
How to conduct a CAP code review
Work through each section below. For every finding, state:
- File and approximate line
- Severity: 🔴 Critical | 🟠 Major | 🟡 Minor | 🔵 Suggestion
- Finding and the corrected version
1. CDS Model Quality
Check for:
| # | Check | Bad | Good |
|---|
| 1.1 | UUID keys defined manually | key ID : UUID @default: #uuidv4 | entity Foo : cuid { ... } |
| 1.2 | Audit fields defined manually | createdAt : Timestamp; createdBy : ... | entity Foo : managed { ... } |
| 1.3 | DB entity exposed directly in service | entity Foo as db.Foo | entity Foo as projection on db.Foo { ... } |
| 1.4 | String without length on HANA | name : String | name : String(100) |
| 1.5 | No enum for status/type fields | status : String | status : StatusType enum { ... } |
| 1.6 | Inline annotations cluttering entity | @UI.LineItem: [...] on entity | Separate annotations.cds file |
| 1.7 | Unnecessary abstraction views (ABAP VDM style) | C_MyView as select from I_MyView | Direct projection on db.* entity |
| 1.8 | Composition vs. Association confusion | address : Composition to Addresses | Use Composition only if child can't exist without parent |
| 1.9 | Missing on clause for associations in service | items : Association to many Items | items : Association to many Items on items.order = $self |
| 1.10 | Calculated fields in where / filter | where grossPrice > 100 (live calc) | Add @Capabilities.FilterRestrictions annotation |
2. Service Handler Quality
Check for:
| # | Check | Bad | Good |
|---|
| 2.1 | Missing return super.init() | async init() { this.on(...) } | async init() { ... return super.init() } |
| 2.2 | cds.connect.to() inside handler body | Per-request connect | Connect once in init(), store as this.S4 = ... |
| 2.3 | Missing return in on handler | this.on('READ', ...) with no return | Always return the result |
| 2.4 | N+1 query in after handler | for (const x of list) await SELECT.from(...) | Batch: one query with in clause |
| 2.5 | Raw req.query manipulation | req.query.SELECT.where = [...] | Use fluent CQL: SELECT.from(...).where(...) |
| 2.6 | Throwing new Error() | throw new Error('...') | req.reject(...) or throw new cds.error(...) |
| 2.7 | Mutating req.data in after phase | after('READ', ..., results => results[0].x = ...) | Understand: req.data is input, result is separate |
| 2.8 | cds.db.run() used where service API fits | Raw DB run bypassing service logic | Use this.run() or service-level queries |
| 2.9 | Direct console.log instead of cds.log | console.log(...) | const log = cds.log('myModule'); log.info(...) |
| 2.10 | Missing error handling for remote calls | Unwrapped await S4.run(...) | try/catch with specific status codes |
3. Security & Authorization
Check for:
| # | Check | Severity |
|---|
| 3.1 | Service has no @requires annotation | 🔴 Critical |
| 3.2 | @requires: 'any' used (allows unauthenticated) | 🔴 Critical (unless intentional) |
| 3.3 | Delete/write operations have no @restrict | 🔴 Critical |
| 3.4 | Instance-based checks done only client-side | 🔴 Critical |
| 3.5 | XSUAA scopes not matching @restrict role names | 🟠 Major |
| 3.6 | No where: 'createdBy = $user' on user-owned data UPDATE | 🟠 Major |
| 3.7 | xs-security.json has no role-templates | 🟠 Major |
| 3.8 | Security annotation typo (now a compiler error in CDS 9+) | 🔴 Critical |
4. Error Handling
Check for:
| # | Check | Bad | Good |
|---|
| 4.1 | No HTTP status code on reject | req.reject('Something failed') | req.reject(422, 'REASON_KEY', [args]) |
| 4.2 | Stack trace exposed to client | req.reject(500, err.stack) | Generic message; log internally |
| 4.3 | Status 500 for business errors | req.reject(500, 'Order closed') | req.reject(409, ...) |
| 4.4 | No target on field validation errors | Generic rejection | req.reject({ code: 400, target: 'price', message: '...' }) |
| 4.5 | Errors swallowed in remote calls | Empty catch {} | At minimum re-throw or return 503 |
5. Performance
Check for:
| # | Check | Why |
|---|
| 5.1 | SELECT * / no column projection | Fetches all columns including large fields unnecessarily |
| 5.2 | Missing .limit() on potentially large result sets | Can OOM or timeout under load |
| 5.3 | Remote service called without delegating req.query | Fetches all remote data, filters locally |
| 5.4 | Calculated elements used in where / filter / order | Live-calculated = no DB index; use @Capabilities annotations to restrict |
| 5.5 | Unnecessary abstraction views (extra JOINs) | Every extra view layer adds a JOIN on HANA |
| 5.6 | after READ doing N individual DB queries | Use batch fetch with in operator |
| 5.7 | Streaming query results not used for large datasets | SELECT.pipeline() / SELECT.foreach() now available in CAP Node.js |
6. Test Coverage
Check for:
| # | Check |
|---|
| 6.1 | No tests exist at all |
| 6.2 | Tests don't cover authorization (different user roles) |
| 6.3 | Tests use real HANA instead of SQLite :memory: |
| 6.4 | No test for error/rejection paths |
| 6.5 | External services not mocked (tests hit real systems) |
| 6.6 | No test for custom actions / functions |
7. Deployment & Configuration
Check for:
| # | Check |
|---|
| 7.1 | Credentials hardcoded in package.json or committed .env |
| 7.2 | hdi-container used instead of service-manager in multitenant apps |
| 7.3 | Missing connectivity service for on-premise destinations |
| 7.4 | cds build not run before mbt build (stale gen/) |
| 7.5 | xs-security.json missing $ACCEPT_GRANTED_AUTHORITIES for MTX |
| 7.6 | No .cdsrc.json / package.json profile separation ([production] vs [development]) |
8. CDS 9 / Latest Version Checks (2025+)
Check for deprecated patterns:
| # | Deprecated | Modern Replacement |
|---|
| 8.1 | @sap/cds-mtx (old MTX) | @sap/cds-mtxs |
| 8.2 | cds.User.tokenInfo | cds.User.authInfo |
| 8.3 | cds.security.draftProtection.enabled | cds.security.authorization.draftProtection.enabled |
| 8.4 | @sap/xssec < v4 | Upgrade to @sap/xssec v4 |
| 8.5 | cds 7 or lower (@sap/cds < 8) | Upgrade to @sap/cds 9 (cds 7 is EOL) |
| 8.6 | @cap-js/change-tracking without MTX multitenancy support | Version supports it since April 2025 |
| 8.7 | @protocols: [...] array syntax (Java) | @odata service Foo { } annotation syntax |
| 8.8 | annotate Books:genres with @restrict without parentheses in expressions | Now a compiler error; use ( ) |
Review output template
## CAP Code Review — <filename or PR title>
**Reviewed against**: capire (https://cap.cloud.sap/docs), CAP Node.js @sap/cds 9.x
### 🔴 Critical
- [2.6] `srv/order-service.js:45` — `throw new Error(...)` bypasses CAP error handling
Fix: `req.reject(422, 'ORDER_VALIDATION_FAILED', [orderId])`
### 🟠 Major
- [3.1] `srv/catalog-service.cds:1` — Service has no `@requires` annotation
Fix: Add `@requires: 'authenticated-user'` to CatalogService
### 🟡 Minor
- [5.1] `srv/product-service.js:22` — SELECT without column projection
Fix: `.columns('ID', 'title', 'price', 'currency_code')`
### 🔵 Suggestions
- [6.1] No test file found. Consider adding `tests/catalog.test.js` using `cds.test('.')`
