fix(odoo-spo): #523 review follow-up — multi-emitter deep-reads + _inherit-only models#525
Merged
Merged
Conversation
…t-only Post-merge follow-up to #523 (merged at 69a3b0a before these landed). Both codex findings were real correctness gaps in the enrichment: 1. P1 multi-emitter deep-reads: the field→emitter index kept only the LAST emitted_by method, so a field emitted by several methods (e.g. stock_move.quantity ← _compute_quantity AND _onchange_product_uom_qty) lifted the deep reads_field to only one — the _compute_* lost its recompute-ordering edge. Now field → sorted set of emitters; deep read emitted per emitter (self-loop drop preserved). 2. P2/Major _inherit-only model extensions: _scan_file bound fields only when _name was present, dropping the COMMON Odoo extension form (_inherit="x" / ["a","b"] with no _name) — those relational fields never got target/inverse_name and deep hops through them were skipped. Now model_names binds from _name OR _inherit (string/list), mapping local fields onto each. Corpus regenerated: target 618→842, inverse_name 102→144, reads_field 2831→3030 (the previously-dropped _inherit fields + per-emitter deep reads). Doc nits: AGENT_LOG scope widened (Python extractor + tests, not "lance-graph only"); EPIPHANIES "verified locally" claim now cites an in-repo fixture test (test_spo_enrich) instead of the discarded worktree. Tests: extractor 14→20 (6 new fixture tests for both cases), lance-graph odoo_ontology 9→11, fmt clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcpLeEC3XK8Eye53GKBVvi
|
Warning Review limit reached
More reviews will be available in 52 minutes and 56 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughTwo behavioral fixes are applied to Changesspo_enrich enrichment fixes and test updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: baa019e02c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
PR #525's `test_to_sql` job failed with `ld terminated with signal 7 [Bus error], core dumped` while LINKING the test binary — a runner resource/toolchain flake (OOM / disk mmap truncation on the largest link unit: lance+datafusion+arrow+aws). NOT a regression: this PR adds only a Python module, ~465 corpus lines, and Rust count-assertions — no new dependency, no link-graph change, so it cannot cause a linker SIGBUS. Empty commit to re-run the workflow (API rerun-failed-jobs is 403). Dropped on rebase/merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcpLeEC3XK8Eye53GKBVvi
A no-_name Odoo class reopens ONE existing model in place; Odoo binds it to _inherit[0] and treats further entries as mixins. The prior fix bound local relational fields to the whole _inherit list, so build_relation_map() could emit bogus target/reads_field triples for secondary parents (e.g. _inherit = ['account.move', 'mail.thread']). Scope to inherit_models[:1], mirroring parsers/classes.py. Corpus byte-identical: regenerated from base 1ec76f5 (22245) with the fixed enrich -> out=24166, target=842, inverse_name=144 (diff clean). No real scanned-addons class triggers the secondary-mixin path scoped to a corpus-declared model, so odoo_ontology.rs counts are unchanged. Test: test_inherit_list_binds_field_to_first_model_only asserts only inherit[0] is bound. 20/20 python tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcpLeEC3XK8Eye53GKBVvi
AdaWorldAPI
pushed a commit
that referenced
this pull request
Jun 18, 2026
… + extend constraint binding to _inherit-only classes All three codex findings are real correctness issues. Fixing them tightens the validation_kind classifier so a regenerated corpus does not pollute downstream queries with false-positive kinds, and lifts the Odoo "extend-in-place" idiom (the same shape #525 fixed for relational fields). # codex #1 — `_inherit`-only classes' constraints attach to the bases The original block flushed `local_constrains` inside `if model_name is not None`, silently dropping `@api.constrains` methods on the common Odoo extension form (`_inherit='account.move'` without `_name`). Now `model_targets = [_name]` if set, else `list(_inherit)` underscored, and constraints attach to *each* target. Same shape #525 took for relational fields — kept consistent. # codex #2 — `search_count(...) > 1` is uniqueness ONLY, not also range The unconditional `Compare` walker added `range` for any `< | > | <= | >=`, including the canonical uniqueness pattern `<rs>.search_count(...) > 1`. That polluted every uniqueness method with a stray `validation_kind=range`. Added `_is_uniqueness_call(left)` guard so range is skipped when the LHS is a `search_count` call. Direct field-vs- bound checks (`self.amount < 0`, `self.amount > 1000000`) still classify as range. # codex #3 — `not re.match(...)` is format ONLY, not also presence The broad `UnaryOp(Not)` rule added `presence` for `if not re.match(...):` and `if not <rs>.search(...):` — the negated-call wrap of format / lookup checks, not presence checks. Restricted presence to truthiness of a *value-like* operand (`Name` / `Attribute` / `Subscript`); negated calls are skipped. Direct field truthiness (`if not self.partner:`) still classifies as presence. # Tests 8 new regression tests under `TestCodexFindings` (32 → 40 OK): - `test_constraints_bind_to_inherit_only_extension` - `test_constraints_bind_to_each_inherit_when_list` - `test_constraints_with_name_skip_inherit_targets` (specificity) - `test_search_count_gt_is_only_uniqueness` - `test_field_range_check_still_classifies_range` (specificity) - `test_negated_re_match_is_only_format` - `test_negated_search_is_only_lookup` - `test_direct_field_truthiness_is_still_presence` (specificity) Each kind has a paired specificity test ensuring the fix did not break the canonical pattern. python3 -m unittest tests.test_spo_enrich : 40/40 OK (was 32)
AdaWorldAPI
pushed a commit
that referenced
this pull request
Jun 18, 2026
…f#21) Rebased on top of #525 (multi-emitter deep-reads + `_inherit`-only field binding). All three codex P2 findings from the pre-rebase iteration of #526 are baked in: `range` skipped when LHS is `search_count(...)`, `presence` restricted to `not <Name|Attribute|Subscript>`, and constraint binding mirrors #525's `model_names` decision (no mixin broadcast). # inherits_from (P1b, ruff#19 ratified shape) Wire shape identical to C++ / Rails: `(odoo:<this>, inherits_from, odoo:<base>)`. Sources: - `_inherit = 'mail.thread'` (single string) - `_inherit = ['mail.thread', 'mail.activity.mixin']` (list) - `_inherits = {'mail.thread': 'thread_id'}` (delegation; dict keys) Drops at scan time: - `_inherit == _name` ("extend-in-place" idiom) - `_inherit`-only classes (no `_name`) — the inheritance edge already exists at the original declaration site; extending in place does not create a new edge. Bases not in the corpus's `ogit:ObjectType` set are skipped at emission time + counted (`inherits_skip_unknown_base`). Truth `(0.95, 0.90)` — class-level declaration is authoritative. # validation_kind (P2, ruff#21 ratified shape) Wire shape mirrors ruff#21's per-attribute keying, subject flipped to the method IRI (Odoo's `@api.constrains` is method-level). Recognised kinds: - `format` — `re.match` / `re.fullmatch` / `re.search` - `uniqueness` — `<rs>.search_count(...)` - `range` — `< | > | <= | >=` where LHS is NOT a `search_count` call (codex P2 fix from pre-rebase #526) - `lookup` — `<rs>.search(...)` / `<rs>.browse(...)` - `presence` — `not <Name|Attribute|Subscript>` (codex P2 fix: `not <Call>` is the negated-call wrap for format/lookup, NOT presence) / `<expr> is None` Truth `(0.85, 0.75)` — AST classification is inferred. Constraint binding follows #525's `model_names`: - `_name = 'X'` → bind to `X` only - `_inherit = 'X'` (no `_name`) → bind to `X` only - `_inherit = ['X','Y']` (no `_name`) → bind to `X` only (inherit[0]) No mixin broadcast — the same conservative rule #525 codex review arrived at for fields. # Corpus regen pending The shipped `odoo_ontology.spo.ndjson` does not yet carry the new triples — regenerating requires running the script against a live Odoo source tree (`/home/user/odoo/addons`), which is not present on this host. The Rust loader's predicate-histogram match arm gained `inherits_from` + `validation_kind` so a future regenerated corpus drops into the harness without code changes. The `parses_all_triples` count assertion stays at 23 701 from #525; re-locks the moment a session with the source re-runs enrichment. # Files tools/odoo-blueprint-extractor/odoo_blueprint_extractor/spo_enrich.py: +helpers (_is_uniqueness_call, _is_api_constrains, _classify_constrains_body), +`inherits`/`constrains` kwargs on _scan_file + build_all_facts, +`_inherits` dict-key extraction, P1b + P2 emission loops, CLI status print extended. tools/odoo-blueprint-extractor/tests/test_spo_enrich.py: +18 tests over 5 classes: TestP1bInheritsFrom (8), TestP2ValidationKind (4), TestAstClassifier (6 — codex P2 specificity locks paired with canonical patterns), TestConstrainsScan (3 — model_names binding). crates/lance-graph/src/graph/spo/odoo_ontology.rs: +doc table row for `inherits_from` + `validation_kind`; +match arm in predicate_histogram_matches_extraction. .claude/board/EPIPHANIES.md: prepended E-ODOO-SPO-INHERITS-VALKIND. # Tests python3 -m unittest tests.test_spo_enrich : 41/41 OK (was 23 post-#525) cargo test -p lance-graph --lib odoo_ontology : 11/11 OK
AdaWorldAPI
pushed a commit
that referenced
this pull request
Jun 18, 2026
…aph test link) No code change. The linux-build job hit the same intermittent linker bus error seen on #525 — rust-lld crashing in llvm::parallelFor during the final link of the heavy lance-graph test binaries (collect2: ld terminated with signal 7). #528 is doc + board only and does not touch lance-graph code, so this is a runner resource flake, not a regression. API rerun-failed-jobs is 403 for this token; empty commit re-triggers the workflow. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcpLeEC3XK8Eye53GKBVvi
AdaWorldAPI
pushed a commit
that referenced
this pull request
Jun 18, 2026
…dation_kind Runs the deferred spo_enrich corpus regen (flagged by E-ODOO-SPO-INHERITS-VALKIND as "corpus regen pending Odoo source") against /home/user/odoo/addons. Completes the odoo-rs UPSTREAM_WISHLIST: P0 deep-reads (#525) was already materialized; this lands the P1b/P2 layer #526 added to the extractor but had not yet emitted. Corpus odoo_ontology.spo.ndjson: +413 triples, 24 166 -> 24 579, purely additive (0 deletions): - +166 inherits_from (ruff#19 cross-language inheritance predicate; _inherit / _inherits bases) - +247 validation_kind (ruff#21 per-@api.constrains kind) Idempotent on the P0 layer: 0 new target/inverse_name/deep reads_field. odoo_ontology.rs: count assert 24_166 -> 24_579; histogram test gains inherits_from=166 + validation_kind=247 asserts (now locked); provenance doc updated. Python extractor 41/41 green. Counts verified against the regenerated corpus histogram. Board: LATEST_STATE narrative entry; EPIPHANIES E-ODOO-SPO-INHERITS-VALKIND Status flipped (per-corpus CONJECTURE -> FINDING). Co-Authored-By: Claude <noreply@anthropic.com> https://claude.ai/code/session_016b33swuXE23hKtqxsHu9p1
3 tasks
AdaWorldAPI
pushed a commit
that referenced
this pull request
Jun 18, 2026
Adds a fifth enrichment pass to spo_enrich.py: `fields.Selection` declarations with a statically-resolvable list of 2-tuples emit one `(odoo:<model>.<field>, selection_value, "<key>")` triple per enum key. # Shape `_extract_selection_values(call)` pulls the first element of each 2-tuple from the Selection list — positional arg 0 OR `selection=` kwarg — preserving source order, de-duplicating (first occurrence wins). Skipped (values not statically knowable): - `selection='_compute_states'` (compute-method ref, a str) - `selection=STATE_CONSTANT` (bare Name) - `related=...` / any non-list/tuple selection arg - individual entries that aren't 2-tuples Truth `(0.95, 0.90)` — read straight from the field decorator, authoritative. Scoped to corpus-declared ObjectTypes (the additive boundary, same as P1); Selection fields bind to the same `model_names` as relational fields (`_name`, else `_inherit[0]`, per #525's decision). # Consumer use Lets odoo-rs lower a Selection field to `DEFINE FIELD state … ASSERT $value IN ['draft','posted','cancel']` — the UPSTREAM_WISHLIST P3 ask. The five-pass enrichment is now: P1 (target/inverse_name) + P0 (deep reads_field) + P1b (inherits_from) + P2 (validation_kind) + P3 (selection_value). # Corpus regen pending The shipped corpus does not yet carry selection_value triples — regenerating requires running the script against a live Odoo source tree (`/home/user/odoo/addons`), not present on this host. The Rust loader's predicate-histogram match arm gained `selection_value` so a future regenerated corpus drops into the harness without code changes. `parses_all_triples` count assertion stays at 24 579; re-locks the moment a session with the source re-runs enrichment. # Files spo_enrich.py: +`_extract_selection_values` helper, +`SELECTION_VALUE_TRUTH`, +`selections` param threaded through `_scan_file` / `build_all_facts` / `enrich`, +Selection branch in the field loop, +P3 emission loop, +CLI status field. test_spo_enrich.py: +12 tests (6 extraction edge cases + 3 scan-binding + 3 emission). odoo_ontology.rs: +doc table row, +histogram match arm for `selection_value`. EPIPHANIES.md: prepended E-ODOO-SPO-SELECTION-VALUE. # Tests python3 -m unittest tests.test_spo_enrich : 53/53 OK (was 41) cargo test -p lance-graph --lib odoo_ontology : 13/13 OK
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Post-merge follow-up to #523 (merged at
69a3b0a3before these review-fixes landed). Both codex findings were real correctness gaps in the SPO enrichment — fixing forward.Fixes
emitted_bymethod, so a field emitted by several methods (e.g.stock_move.quantity←_compute_quantityand_onchange_product_uom_qty) lifted the deepreads_fieldto only one — the_compute_*lost its recompute-ordering edge. Nowfield → sorted set of emitters, deep read emitted per emitter (self-loop drop preserved)._inherit-only model extensions._scan_filebound fields only when_namewas present, dropping the common Odoo extension form (_inherit="x"/["a","b"]with no_name) — those relational fields never gottarget/inverse_name, and deep hops through them were skipped. Nowmodel_namesbinds from_nameor_inherit(string/list), mapping local fields onto each.Corpus impact (regenerated)
target618 → 842,inverse_name102 → 144,reads_field2,831 → 3,030 — the previously-dropped_inheritextension fields + per-emitter deep reads now captured.Doc nits (CodeRabbit)
AGENT_LOG.mdscope widened — "lance-graph corpus + the stdlib Python extractor tooling," not "lance-graph only."EPIPHANIES.md"verified locally" claim now cites an in-repo fixture test (test_spo_enrich) instead of the discarded worktree → re-runnable.Tests
python3 -m unittest14 → 20 (6 new fixtures: thestock_move.quantitymulti-emitter case + an_inherit-only extension case, asserting the expectedtarget/inverse_name+ per-emitter deepreads_fieldtriples).cargo test -p lance-graph --lib odoo_ontology9 → 11;cargo fmtclean. Additive, stdlib-only Python, Core-first (no ClassView/new trait).Resolves the codex P1, codex/CodeRabbit P2/Major, and both CodeRabbit doc threads from #523.
🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests