| name | o-spreadsheet-review |
| description | Review a branch or PR in the o-spreadsheet project for correctness, bugs, performance, security, architecture, tests, and maintainability. Use when user asks to review code or mentions "review". |
Code Review for o-spreadsheet
Invocation
The user provides a PR number or branch name, or refers to the current branch ("this branch", "my branch"). If none given, ask the user.
Note: The gh CLI is not available in this sandboxed environment. Work from the local git checkout only.
Process
1. Make sure the right branch is checked out
The review runs against the currently checked-out branch.
- PR number: stop and ask the user to confirm the branch name or check it out.
- Branch name: if it differs from the current branch, check it out. If checkout fails (dirty tree, unknown branch, …), stop and ask the user.
2. Determine the review range
The dev branch is always prefixed with its base branch — e.g. base 19.0 → dev 19.0-feature-xyz. Derive <base_branch> from that prefix; if you cannot infer it, ask the user.
Then list the commits in <base_branch>..HEAD — those are the commits to review.
3. Walk each commit for hygiene
For every commit in <base_branch>..HEAD (oldest → newest), read git show <sha> (message + diff) and check:
- One concern per commit — clear, single purpose.
- Commit message follows the Odoo convention (
[FIX], [IMP], [REF], [PERF], …) and accurately describes the change.
- Commit builds and keeps tests passing on its own — no commit depends on a later one to be coherent (blocker if a commit leaves the tree broken).
- No unrelated changes or leftover debug code.
- New behavior or fix lands with its tests in the same commit.
4. Review the branch as a whole
Diff the whole branch (git diff <base_branch>...HEAD). For each changed region, read enough surrounding code at HEAD — the enclosing function, the call sites, related types/tests — to actually understand what the change does. Then apply CHECKLIST.md against the merged result.
5. Output the review
Be ruthlessly brief. A long review is an unread review — if the human has to scroll, they will skim or skip it. Aim for the shortest text that is still actionable:
- One line per finding. Two only if truly necessary.
- No restating the code, no explaining what the function does, no preamble like "I noticed that…".
- Skip findings that aren't worth the reader's time. A short list of real issues beats a long list padded with nits.
- Prefer concrete fixes ("use
history.update") over abstract advice ("consider state management").
- No closing summary, no recap of what you reviewed.
### Commit
<for commits with issues, one short line, or "All commits look clean." and nothing else.>
### Findings
#### <file>:<line> — <severity>
<very short issue description and suggested fix>
Severity levels
| Level | Meaning | Examples |
|---|
| blocker | Must fix before merge. Incorrect behavior, data loss, security hole, crash. | Wrong command handling, XSS via unsanitized input |
| warning | Should fix. Performance issue, fragile pattern, missing test coverage for key path. | O(n^2) in hot loop, no undo/redo test, direct state mutation without history.update() |
| nit | Optional. Style, naming, minor simplification. | comment typo |
Reviewer discipline
Don't flag things that are clearly intentional or match existing codebase patterns. If unsure whether something is a real issue, say so rather than invent one.
