code-review

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('.')`