Engineering Standard · BaseUp Labs

ERP Customization Documentation Standard

What must be written down when we customize Odoo for a client on Odoo SH, and what makes a written claim acceptable. On Odoo SH we do not own the runtime — so the expensive failures are not bad code, they are undocumented assumptions about how core behaves.

Applies to
Odoo SH projects
Rules
3
Stages
Dev · Staging · Prod
Status
Proposed
Owner
ERP engineering
01

Why this exists

Odoo owns the platform, the build pipeline and the version upgrades. Every push produces a new build, and a version bump can change core behaviour underneath code nobody touched.

That moves where the risk lives. The failures that cost money are not the ones a linter catches — they are assumptions about core that were true once, were never written down, and quietly stopped being true. Two of them, both real, both from a single Odoo 19 → 20 port:

A no-op access row became a blanket grant

Odoo 20 merged ir.model.access and ir.rule into one ir.access model and inverted the meaning of an empty group. A row that had been a harmless no-op became a permission over every record of the model — and because permissions are OR-ed, it cancelled every rule that scoped the same model. A model meant to be visible only to its owner became readable by every internal user. Nothing failed. No test went red. Nothing in the diff looked like a security change.

A column silently disappeared

hr.attendance.resource_calendar_id became a related field in core. A module redeclaring that field inherits related unless it explicitly overrides it — so the stored column vanished, and writes began propagating to the employee's working schedule instead.

Neither is visible by reading our own code. Both are seconds away in core, or in a schema diff. This standard exists to make that reading mandatory, and to make its result durable.

02

Scope

Two definitions do most of the work in what follows.

TermMeans
CustomizationAnything we add to a client's Odoo SH repository — a new module, an inherited model, a view override, an access rule, a data or demo file, a scheduled action, a report, an external integration.
ClaimAny statement about how Odoo behaves, wherever it appears — a doc, a commit message, a code comment, a PR review. “Core writes validated here”, “this field is stored”, “this hook runs first” are all claims, and all fall under Rule 1.

A claim is not made safer by being informal. A comment is a claim.

03

The required document set

A customization is not done when it works. It is done when the next engineer can tell why it is shaped that way without asking the author.

ArtifactLives atRequired when
Module README<module>/README.mdEvery custom module. What it does, who for, what it depends on.
Migration notes<module>/MIGRATION_NOTES_<from>_to_<to>.mdEvery Odoo version bump.
ADRdocs/adr/NNNN-short-title.mdAny decision with more than one defensible answer, or touching a protected path.
Access rationalecomments beside the access filesAny change to ir.access or record rules.
Review receipta PR commentEvery PR. See §8.
Runbook entrydocs/runbook/Anything an on-call engineer could be paged about.
04

Rule 1 — Evidence over memory

Never document Odoo behaviour from memory, and never accept a claim that carries no evidence.

Recalled API knowledge is unreliable across versions, and confidently wrong documentation is worse than none — because it gets trusted. Every claim about core behaviour must cite one of three things:

EvidenceForm
Core sourcepath/to/file.py:LINE, from the Odoo version the branch targets
Schemaa query against a real build database
A test runquoted output, not a summary of it

Keep the core source for the target version readable locally, and read it before writing about it:

read core, do not recall it
grep -n "state = fields.Selection" -A6 \
  <odoo>/addons/hr_attendance/models/hr_attendance.py

On Odoo SH, schema evidence comes from a staging build, which runs on a neutralized copy of the production database. Comparing one table across a pre-upgrade and a post-upgrade build settles most “is this field still stored?” questions outright:

schema evidence · run on each build
SELECT column_name FROM information_schema.columns
 WHERE table_name = '<table>' ORDER BY 1;

A column present before the upgrade and absent after it is a real finding, whatever the Python says.

Label confidence

Every finding in a review or a migration note carries one of two labels:

LabelMeaning
CONFIRMEDProven, with the citation inline.
SUSPECTEDReasoned but unproven. States what would settle it.
Drop, do not soften

Anything you cannot substantiate is removed, not hedged. A short verified list is worth more than a long speculative one, because the reader can act on all of it.

05

Rule 2 — Version-scope every claim

A claim about core with no version attached is not a claim, it is folklore.

Write “in 20.0, hr.version carries a unique index on (employee_id, date_version) WHERE active” — not “contracts must have unique dates”. The first survives an upgrade by being checkable. The second does not.

This bites hardest on access control, because access mistakes fail silently and in the permissive direction. When changing access rules, write down the answers to:

QuestionWhy it matters
Which rows does this grant reach?A permission with an empty domain reaches every row.
Which existing rule does it widen?Permissions are OR-ed — the most permissive row wins.
Which roles inherit it?Group implications land a grant on more people than the row names.

State the intended blast radius in a comment next to the rule.

If the reviewer cannot derive the blast radius, the documentation has failed

— regardless of whether the code happens to be correct. Correct-but-unexplained access rules are the ones that get “tidied” into a breach six months later.

06

Rule 3 — Comments are documentation

A comment describing behaviour the code no longer has is a defect, and will be reported as one.

When a guard, a branch or an override is removed, its comment goes with it. Two failure modes, both seen in review:

  • A write() override whose comment said the method “checks” a state it no longer checked.
  • A test asserting an error the code had stopped raising, kept alive by a comment explaining the old behaviour.

Migration notes are the long form of the same duty. For each version bump, record what core renamed or moved, what we changed in response, what we deliberately did not change, and what still needs a human decision.

“Installs cleanly” is not “verified”

Items needing a human decision are listed explicitly, never left implicit in the diff. A port that installs without tracebacks has proven exactly one thing: that it installs.

07

The Odoo SH stages

Odoo SH gives three branch stages. The documentation obligation differs at each.

StageDatabaseDocument before promoting
Developmentfresh build DBThe intent: what the customization does, and which core behaviour it relies on — with citations.
Stagingneutralized copy of productionEvidence against real data shape: schema checks, upgrade results, access checks under real user records.
ProductionliveNothing new — but the runbook entry and the rollback note must already exist.

Because a staging build carries real production data shape, it is the last honest checkpoint. A claim that was only ever tested against a demo database stays labelled SUSPECTED until it has been re-checked on staging.

Staging cannot confirm everything

Staging databases are neutralized: outbound mail and scheduled actions do not behave as they do in production. Any claim about mail or cron behaviour therefore cannot be confirmed there, and must say so rather than implying coverage it does not have.

08

Review receipts

A review that exists only in a chat window is invisible to the next engineer, to the next session, and to CI — and the PR merges looking unreviewed, because as far as anything can tell, it is.

  1. Post every review as its own PR comment. Always, including a pass. Not folded into the PR description; not reported only in chat.
  2. Pin it to the commit. A review is of a commit, not of a pull request. Record the head SHA and put it in the receipt.
  3. End the body with a machine-readable receipt line, which renders invisibly and costs the reader nothing.
  4. Write the body to a file and pass --body-file. A review body is full of backticks; inlined in a double-quoted shell argument it goes through command substitution.
the receipt line · last line of every review
<!-- baseup-review kind=<kind> sha=<full-40-char-headRefOid> verdict=PASS|BLOCK -->
A wrong SHA is worse than no receipt

Because it marks unreviewed code as reviewed. Take the SHA from gh pr view <n> --json headRefOid — never from memory, and never from an earlier round of the same PR.

Do not import checklists across stacks

Odoo reviews follow the Odoo review command, which encodes §4 and §5. Gates from our application repos — console.log, bare any, Prisma, Electron nodeIntegration — do not apply to an Odoo module and must not be copied into an Odoo checklist. A checklist full of inapplicable items trains reviewers to skim.

08a

The pre-push gate

A rule that depends on remembering to follow it is a suggestion. The review in §8 is enforced before the push leaves the machine, not after it lands.

lefthook.yml binds a pre-push hook that runs the review over the outgoing diff and exits non-zero on a blocking finding, refusing the push. Install it once per clone — without this, nothing gates anything:

once per clone · nothing runs without it
brew install lefthook && lefthook install

The review runs on the Claude Code CLI already signed in on the engineer's machine. There is no API key to provision and no shared credential: if you can run claude, the gate works. The hook resolves the binary itself rather than trusting PATH, because git hooks inherit a stripped one and the IDE extensions ship it at a version-stamped path.

What blocks, and what does not

Blocks the pushDoes not
An access rule reaching more rows or roles than intendedNaming, formatting, comment style
A crash on a reachable path — bad index, undefined name, wrong arityHypothetical edge cases
cr.execute built by string formatting from non-constant inputParameterised SQL that could be prettier
New or widened sudo() with no justifying commentPre-existing sudo() you happened to read
A new model with no access row, or new XML not in the manifestMissing docstrings
Secrets, tokens, print(), pdb in application codeprint() in tests or scripts
A comment or test asserting behaviour the code no longer hasStale comment making no behavioural claim

Keeping the blocking list short is what keeps the gate alive.

A gate that blocks on nits gets bypassed within a week

— and then it protects nothing. Only CONFIRMED findings block; a SUSPECTED finding is reported as a warning and lets the push through. §4's labels are what make that split mechanical rather than a judgement call in the moment.

Failing closed

If the review cannot produce a readable verdict, the gate blocks. A gate that passes when it cannot tell is not a gate. When the reviewer itself cannot run — no CLI on the machine, not signed in — it records SKIPPED and allows the push, because a missing tool is not evidence of a defect.

A non-zero exit from the agent counts as “no review happened” only when the output also carries no verdict. If the agent reached a verdict and then exited non-zero, that verdict still stands — otherwise a stray exit code would silently turn a BLOCK into a pass.

The two bypasses are not equivalent

one is recorded · one is not
SKIP_AI_REVIEW=1 git push        # recorded
git push --no-verify             # NOT recorded

The bypass exists so a misfiring gate never blocks an emergency fix. SKIP_AI_REVIEW=1 runs the hook, which writes a BYPASS receipt and lets the push through — the skip stays visible afterwards instead of being indistinguishable from a passing one.

--no-verify leaves no trace

It tells git to skip every pre-push hook, so the script never runs and no receipt is written at all. It is the unaudited escape hatch; the only sign it was used is a pushed SHA with no receipt beside it. Prefer the recorded form.

The line that matters

A bypassed push is not a policy violation. A bypassed push that nobody can tell was bypassed is.

09

Definition of done

A customization may be promoted to production when all of the following hold.

#Check
1Every claim about core behaviour carries a citation, or is labelled SUSPECTED.
2Version-sensitive claims name the Odoo version.
3Access changes state their blast radius next to the rule.
4Comments match the code as it now stands.
5Migration notes list what still needs a human decision.
6Schema-dependent claims were re-checked on a staging build.
7A review receipt exists on the PR, pinned to the head SHA.
8Rollback is written down.
9The pre-push gate ran and passed — or the bypass is recorded and explained.
10

Anti-patterns

Each of these has cost someone a production incident or a wasted review cycle.

Anti-patternWhat it costs
“Odoo does X” with no versionSilently wrong after the next upgrade.
Comment describing removed behaviourActively misleads; outlives the code it described.
Documenting the symptom fixA workaround recorded as a fix hides the cause and blocks the repair. Record the cause, then the workaround, and say which is which.
Confidence without evidenceUnfalsifiable, so it never gets corrected.
Findings left in chatInvisible downstream; the PR merges looking unreviewed.
Scripted edits committed unreadMechanical rewrites land valid-but-wrong code. Read what the script produced.
Business change dressed as a constraint fixShifting a business date to dodge a unique index is a product decision. Escalate it; do not bury it.
Habitually bypassing the gateThe receipt records it, so the pattern is visible. A gate bypassed by reflex is worse than no gate — it buys the reassurance without the check.
11

Worked example

What the standard looks like applied to one line of a diff.

Step
Claim“This row just grants read on the model; the record rules still scope it.”
Evidenceodoo/addons/base/models/ir_access.py — _compute_kind makes a row with a group a permission and one without a restriction; _get_failed_accesses documents that permissions are OR-ed. A permission's empty domain evaluates to Domain.TRUE.
VerdictCONFIRMED FALSE. The row is a permission with an empty domain, so it reaches every record and ORs away the scoping rules for the same model and operation.
Caught by§5. Answering “which rows does this reach?” surfaces the problem before merge — with no test, and no incident.

The whole standard is an attempt to make that four-row table cheap enough that people actually fill it in.