Reviewing Odoo code before it merges — your own AI-generated patch or someone else's PR. Use after writing or generating an Odoo change and before commit, or when asked to review/audit an Odoo module, diff, or addon. Catches the Odoo-specific defects AI confidently ships that lint and "it ran for me as admin" miss: blanket sudo() / privilege bypass, missing ACL or record rules, N+1 recordset loops, incomplete @api.depends on stored computes, patching core instead of a separate addon, wrong inheritance mode / MRO layer, deprecated v≤16 syntax (attrs/states/<tree>/name_get), guessed field & method names, unwired __init__ / manifest, data-loss-on-upgrade renames, and public routes with the wrong auth. Pairs with odoo-testing (review finds it, tests prove it). Read ground truth from the running instance to confirm a suspicion — don't guess whether a field or rule exists. Targets Odoo 17/18/19.
Installation
Install with Codex or Claude Copy this prompt, paste it into Codex, Claude, or another assistant, and let it review the skill page and install it for you.
Reviewing Odoo code before it merges — your own AI-generated patch or someone else's PR. Use after writing or generating an Odoo change and before commit, or when asked to review/audit an Odoo module, diff, or addon. Catches the Odoo-specific defects AI confidently ships that lint and "it ran for me as admin" miss: blanket sudo() / privilege bypass, missing ACL or record rules, N+1 recordset loops, incomplete @api.depends on stored computes, patching core instead of a separate addon, wrong inheritance mode / MRO layer, deprecated v≤16 syntax (attrs/states/<tree>/name_get), guessed field & method names, unwired __init__ / manifest, data-loss-on-upgrade renames, and public routes with the wrong auth. Pairs with odoo-testing (review finds it, tests prove it). Read ground truth from the running instance to confirm a suspicion — don't guess whether a field or rule exists. Targets Odoo 17/18/19.
Odoo code review — the second gate
odoo-testing proves a change does what it claims; this skill catches what it
claims that it shouldn't, and what it silently broke elsewhere. AI-generated
Odoo code is the dangerous case: it's syntactically clean, it "works for admin
on my machine", and it fails later — for a non-admin user, on the second
company, on a list of records, on -u, or on a hot list view. Review against
the Odoo-specific contracts a linter can't see.
The rule: every finding is either confirmed against the running instance
(via odoo-introspect) or written down as an explicit assumption to verify —
review never adds new guesses.
Version floor: Odoo 17/18, through Odoo 19 (current LTS). Deprecation checks below assume 17/18+; for older
targets — and for the v18.1 → 19 renames — see skills/odoo-introspect/references/version-matrix.md.
How to review (in order)
Get the diff and the ground truth. Read the change; run odoo-ai all <model>
for every model it touches (fields, MRO, security, depends). Half of "review"
is just comparing the patch to what the brief says already exists.
Run the checklist below, severity-first (security → data loss → silent
correctness → performance → style).
Confirm, don't assert. "This field may not exist" → check the brief. "This
override may not run" → read the MRO / trace it. Turn each suspicion into a
fact or a flagged assumption.
Hand correctness findings to odoo-testing — a real defect deserves a
failing test, not just a comment.
The review checklist (severity-ordered)
Security (highest stakes — fails open, not loud)
No blanket sudo() — every sudo() has a one-line reason and is the
narrowest sub-call, not a method-wide bypass. A sudo() added to silence an
AccessError is almost always hiding a correct record rule (→ odoo-security).
New model ships ir.model.access.csv — all four perm_* set
deliberately; otherwise non-admins get a silent 403 while admin "works".
Record rules considered — multi-company / "own records" isolation is a
rule, not Python filtering. Confirm rules exist in the brief.
Field-level groups= on sensitive fields, not deletion/Python guards.
Public routes (@http.route) use the right auth=; no csrf=False on
a session POST; no controller-wide sudo() (→ odoo-web).
No SQL/domain injection — user input is parameterized, never f-string'd
into cr.execute or a domain.
Data loss & upgrade safety
Field/model rename has a pre-migration — a bare Python rename drops the
old column and creates an empty one (→ odoo-migration). Silent data loss.
Changed stored-compute logic schedules a recompute — existing rows keep
stale values otherwise.
New required field on a populated model is backfilled in pre- — not
post- (too late for the NOT NULL).
Edited shipped data respects noupdate — UI-editable config is
noupdate="1"; a protected record needs a migration, not a -u.
Silent correctness
Field & method names exist — checked against the brief, not memory
(account.move not account.invoice, real partner_id not invented).
Override lands at the intended MRO layer — __manifest__.depends
includes the addon that owns the method (depend on sale_stock, not just
sale, to extend a stock hook). Confirm against the brief's recommended depends.
super() is called (and in the right place) — a layer that drops
super() cuts the chain; an early return skips it. Check the MRO analysis;
trace big flows.
Right hook, not the public shell — value-building belongs in a
_prepare_* / _action_* / _get_* hook, not an override of
create/write/action_* (→ odoo-dev, odoo-domain-playbooks).
Built-in, not hand-rolled — derived value = computed/related field;
rule = @api.constrains; numbering = ir.sequence. Procedural code in
write() to compute/validate is a smell.
create override is batch-safe — @api.model_create_multi, vals is
a list; tested on a multi-record recordset.
Wired in — every new .py imported in its __init__.py; every new
data/view file listed in __manifest__['data'] in dependency order; new OWL/JS
in the correct assets bundle. Unwired = dead code, no error.
Performance
No query in a loop — no search/browse per record; search once with
in, then filtered/mapped (→ odoo-perf).
@api.depends is exhaustive on every stored compute (dotted paths
included) — audit against model_brief.fields[].depends. A missing dep silently
rots the value.
store=True is justified (search/group/report on it), not "just in
case" — stored computes add write amplification.
Batch write/create, not field-by-field in a loop.
New index only where searched/grouped, not on everything.
Version & frontend currency
No attrs= / states= in v17/18 views — direct invisible=/readonly=
Python expressions; <list> not <tree>; <chatter/> (→ odoo-views).
No name_get() — _compute_display_name; no removed ORM names
(fields_view_get→get_view, etc. — → version-matrix).
No unmarked public method that should be private (v18.2+) — public
model methods are RPC-callable by default; internal helpers want @api.private
(→ odoo-security). A privilege/exposure check, not style.
OWL reads props.record.data[props.name] / writes via .update(...);
correct import paths; template in the bundle (→ odoo-owl). Public JS uses the
right framework (publicWidget vs Interactions — → odoo-web).
Dependencies & hygiene
No hallucinated Python packages — every external_dependencies['python']
/ import is a real, maintained package at a pinned version (slopsquatting guard).
No leftover debug — breakpoint(), print(), import pdb, bare
except: pass swallowing errors.
The "looks fine, fails later" smells (quick scan)
Smell in the diff
Likely real defect
Confirm with
.sudo() with no comment
privilege bypass / hidden record rule
odoo-security dossier
for r in recs: ...search(/browse
N+1
odoo-perf, trace_flow SQL count
store=True + short @api.depends
stale stored value
model_brief depends
new model, no .csv in the diff
non-admin 403
access_rights in brief
Python field renamed, no migrations/
data loss on -u
odoo-migration
override of action_confirm/create
wrong hook / broken super()
MRO + trace_flow
attrs= / <tree> / name_get
hard parse error / no-op on 17/18
odoo-views, version-matrix
auth='public' reading user data
data exposure
odoo-web, record rules
new .py not in __init__
dead override, no error
odoo-module-scaffold
Output of a review
State, concisely: blocking issues (security, data loss, silent correctness)
vs non-blocking (perf/style); each with the file/line, why it's wrong in
Odoo terms, the fix, and — for correctness — the test that should prove it.
Don't rewrite the whole patch; point to the smallest correct change (the
odoo-dev ethos).
Run the checklist as a linter
Much of the checklist above is now executable: odoo-ai validate <path...>
(local, no DB) statically flags the high-signal, mechanical defects — attrs/
states, <tree>, name_get, type='json' on 19+, create() without
@api.model_create_multi, search()/browse() in loops, f-string cr.execute,
uncommented sudo(), self._cr/_uid/_context, fragile xpath, leftover debug.
Run it first to clear the mechanical issues, then spend your judgement on what a
linter can't see (MRO layer, security intent, data-loss, right hook) — confirmed
against the brief. It complements, never replaces, the confirm-against-the-instance
rule. Before sharing any introspection JSON with an external LLM, run
odoo-ai redact <file> / odoo-ai scan-secrets <file>.
References & related skills
This skill's references
references/review-checklist.md — the checklist expanded dimension by dimension
(security/sudo, data-loss/migration, silent-correctness/MRO+super+depends,
performance/N+1, multi-company, version currency), each line with the why in
Odoo terms and the exact odoo-ai command that confirms it.
references/ai-failure-modes.md — the specific confident-but-wrong patterns AI
ships (hallucinated names, memory-era syntax, sudo()-to-silence, batch-unsafe
create, N+1, wrong-hook override, bare rename, slopsquatting, default-company
rule), each with the odoo-ai command that catches it.
Other skills in the loop
odoo-testing — turn each correctness finding into a failing-then-passing
test; the PR checklist there is the merge gate this skill feeds.
odoo-introspect — confirm every suspicion (fields, MRO, security, depends)
against the running instance instead of asserting.
