← Back to list

odoo-code-review
by unclecatvn
Agent Skills Documentation
⭐ 0🍴 0📅 Jan 23, 2026
SKILL.md
name: odoo-code-review description: Review Odoo code for correctness, security, performance, and Odoo 18 standards. Use when reviewing Odoo modules, diffs, or pull requests; produce a scored report with weighted criteria.
Odoo Code Review
Objective
Review Odoo code changes against clear criteria, identify risks, and score using a weighted scale from an Odoo 18 expert perspective.
Pre-review Requirements
- Read
skills/odoo/18.0/SKILL.mdand related guides if there are changes regarding models, fields, views, controllers, security, performance. - Identify scope: module, file, and change context.
- Master Odoo 18 API changes:
<list>instead of<tree>,@api.ondelete, etc.
Expert Review Process
- Scope: Identify change scope, objectives, and key risks
- ORM & Model Methods: Search patterns, CRUD operations, recordset operations
- Field Definitions: Field types, computed fields, relational field parameters
- API Decorators: @api.depends, @api.constrains, @api.ondelete (Odoo 18!)
- Performance: N+1 detection, batch operations, field selection
- Transaction Management: Savepoints, UniqueViolation, serialization
- Views & XML: Odoo 18 tags (
<list>), inheritance, structure - Security: ACL, record rules, exceptions, sudo usage
- Controllers: Auth types, CSRF protection, routing
Odoo 18 Complete Checklist
ORM & Model Methods (30%)
- ❌ DO NOT use
search()inside loop (N+1 anti-pattern) - ✅ Use
search_read()when dict output needed - ✅ Use
read_group()for aggregate queries - ✅ Use
INdomain instead of search in loop:[('order_id', 'in', orders.ids)] - ✅ Batch
create([{...}, {...}])for multiple records - ✅ Use
recordset.write()instead of loop - ✅ Use
recordset.unlink()instead of loop - ✅ Use
mapped()instead of list comprehension - ✅ Use
filtered()before operations - ✅ Use
exists()to filter non-existing records
Field Definitions (15%)
- ✅
Many2onehasondeleteparameter (cascade,restrict,set null) - ✅
Monetaryhascurrency_fieldparameter - ✅
One2manyhasinverse_nameparameter - ❌ DO NOT use
Floatfor currency (useMonetary) - ❌ DO NOT use
<tree>in Odoo 18 (use<list>) - ✅ Computed fields have
store=Trueif searchable/groupable needed - ✅
@api.dependsincludes ALL dependencies with dotted paths
API Decorators (15%)
- ✅
@api.dependsuses dotted paths for related fields:@api.depends('partner_id.email') - ❌ DO NOT use dotted paths in
@api.constrains(only simple field names) - ✅
@api.ondelete(at_uninstall=False)instead of overridingunlink()for validation (Odoo 18!) - ✅
@api.constrainsraisesValidationError - ✅
@api.model_create_multifor batch create (Odoo 18)
Performance (20%)
- ❌ DO NOT
search()in loop - ❌ DO NOT
browse()in loop - ❌ DO NOT
create()in loop - ❌ DO NOT
write()in loop - ❌ DO NOT
unlink()in loop - ✅ Use prefetch (automatic) for related field access
- ✅ Use
search_read()to fetch specific fields - ✅ Use
bin_size=Truefor binary fields - ✅ Use advisory locks for concurrent operations
Transaction Management (10%)
- ✅ Use
with self.env.cr.savepoint():for error isolation - ❌ DO NOT continue after UniqueViolation without savepoint
- ✅ Use advisory locks to prevent serialization errors
- ✅ Group identical updates to minimize conflicts
Views & XML (5%)
- ✅ Use
<list>instead of<tree>(Odoo 18!) - ✅ Use
decoration-*for row styling - ✅ Use
xpathor shorthand withpositionfor inheritance - ✅ Proper
inherit_idreference
Security (5%)
- ✅ Has
ir.model.access.csvfile with proper permissions - ✅ Use
UserErrorfor business logic errors - ✅ Use
ValidationErrorfor constraint violations - ✅ Use
AccessErrorfor permission issues - ❌ DO NOT raise generic
Exception - ✅ Record rules defined with proper domain_force
Controllers (5%)
- ✅ Use correct
authtype (user,public,none) - ✅ Use
auth='none'for truly public endpoints (webhooks) - ✅ CSRF enabled for POST (default)
- ✅
csrf=Falseonly for external webhooks
Anti-Patterns to Detect
| Anti-Pattern | Consequence | Fix |
|---|---|---|
search() in loop | N+1 queries | Use search_read() with IN domain |
create() in loop | N INSERT statements | Batch: create([{...}, {...}]) |
write() in loop | N UPDATE statements | records.write({...}) |
unlink() in loop | N DELETE statements | records.unlink() |
Override unlink() for validation | Breaks module uninstall | Use @api.ondelete(at_uninstall=False) |
@api.depends('a') then access a.b | N queries | Add @api.depends('a.b') |
@api.constrains('a.b') | Not supported | Use only @api.constrains('a') |
<tree> in Odoo 18 | Deprecated | Use <list> |
Float for currency | Precision issues | Use Monetary |
Missing ondelete on Many2one | Orphan records | Add ondelete='cascade/restrict' |
Generic Exception | Poor UX | Use UserError, ValidationError |
| Continue after UniqueViolation without savepoint | Transaction aborted | Use with self.env.cr.savepoint(): |
Scoring Scale (Weighted)
Criteria (score 1-10):
- ORM & Model Methods (30%)
- Field Definitions (15%)
- API Decorators (15%)
- Performance (20%)
- Transaction Management (10%)
- Views & XML (5%)
- Security (5%)
- Controllers (5%)
Total calculation:
total = 0.3*orm + 0.15*fields + 0.15*decorators + 0.2*performance + 0.1*transaction + 0.05*views + 0.05*security + 0.05*controllers
Score anchors:
- 9-10: Excellent, no significant risks, follows all best practices
- 7-8: Good, minor issues or improvements possible
- 5-6: Average, clear risks to address, has anti-patterns
- 3-4: Poor, serious errors or regression-prone
- 1-2: Very poor, cannot merge, violates critical patterns
Report Format (Required)
## Quick Summary
- [1-2 sentences summarizing key points]
## Overall Score
- Total: X.X/10
- Formula: 0.3*ORM + 0.15*Fields + 0.15*Decorators + 0.2*Perf + 0.1*Trans + 0.05*Views + 0.05*Sec + 0.05*Controllers
## Score by Criteria
- ORM & Model Methods: X/10 — [brief reason, any anti-patterns?]
- Field Definitions: X/10 — [brief reason]
- API Decorators: X/10 — [brief reason, check @api.ondelete, dotted paths]
- Performance: X/10 — [brief reason, any N+1?]
- Transaction Management: X/10 — [brief reason, savepoints correct?]
- Views & XML: X/10 — [brief reason, using <list>?]
- Security: X/10 — [brief reason]
- Controllers: X/10 — [brief reason]
## Key Findings (high → low priority)
### 🔴 Critical (Must Fix)
- [Severity] Brief description + consequence + fix suggestion
- Code reference: `path/file.py:XX`
### 🟡 Major (Should Fix)
- [Severity] Brief description + consequence + fix suggestion
- Code reference: `path/file.py:XX`
### 🔵 Minor (Nice to Have)
- [Severity] Brief description + improvement suggestion
## Positive Patterns Found
- ✅ [Good pattern found] - Line XX
## Recommendations
- [Specific, clear improvements, in priority order]
## Testing
- Ran: [if any, state commands]
- Missing: [tests missing or not run, N+1 scenarios]
Response Rules
- Prioritize error and risk detection first, then suggestions
- If no significant issues, clearly state "No findings"
- Cite correct file and code when needed:
path/to/file.py:XX - State assumptions when information is missing (don't guess)
- Focus on Odoo-specific patterns, not generic Python advice
- Provide code examples for complex issues
- Reference Odoo documentation when applicable
Deep Dive Checks
When reviewing, thoroughly check:
-
Does @api.depends have complete dependencies?
- Check dotted paths:
partner_id.emailinstead of justpartner_id - Missing dependencies cause N queries
- Check dotted paths:
-
Are there N+1 queries?
- Loop with
search(),browse(),read()inside - Solution:
search_read()withINdomain orread_group()
- Loop with
-
Are there batch operations?
create(),write(),unlink()in loop- Solution: Batch operations on recordset
-
Is transaction safe?
- UniqueViolation handling without savepoint
- Concurrent updates without advisory lock
-
Are Odoo 18 patterns correct?
- Use
<list>instead of<tree> - Use
@api.ondelete()instead of overridingunlink() - Use
@api.model_create_multifor batch create
- Use
-
Are field definitions correct?
Monetarywithcurrency_fieldMany2onewithondelete- Computed field with
store=Trueif needed
-
Is exception handling correct?
UserError,ValidationError,AccessError- No generic
Exception
Score
Total Score
60/100
Based on repository quality metrics
✓SKILL.md
SKILL.mdファイルが含まれている
+20
✓LICENSE
ライセンスが設定されている
+10
○説明文
100文字以上の説明がある
0/10
○人気
GitHub Stars 100以上
0/15
○最近の活動
3ヶ月以内に更新がある
0/10
○フォーク
10回以上フォークされている
0/5
✓Issue管理
オープンIssueが50未満
+5
✓言語
プログラミング言語が設定されている
+5
○タグ
1つ以上のタグが設定されている
0/5
Reviews
💬
Reviews coming soon