feat(ontology): virtually_overrides as a computed ClassView relation (wishlist last item)#533
Conversation
…(wishlist last item)
Closes the odoo-rs wishlist's last item — `virtually_overrides` — the
Core-correct way: a DERIVED ClassView method-resolution relation, NOT a
sixth `spo_enrich.py` harvest predicate. Consistent with the Core-first
correction (E-ODOO-CORE-FIRST-STRUCTURAL): a fact an AST pass reads lives
in the typed Core; a relation the ClassView derives lives in the resolver;
neither is a flat-ndjson predicate.
# The relation
Per the doctrine the (has_function / inherits_from / virtually_overrides)
triad is the ClassView method-resolution manifest. has_function +
inherits_from are facts; virtually_overrides is the derivation:
`M.m` overrides `B.m` iff M declares m, B is reachable up M's `_inherit`
chain, and B also declares m.
# What lands — `odoo_blueprint/mro.rs`
- `resolve_overrides(methods_of, bases_of)` — pure resolver over an
abstract manifest. Nearest-base-wins BFS up the _inherit chain,
cycle-guarded, deterministic. Equals Python C3 for the linear mixin
chains Odoo uses; documented breadth-first approximation for diamonds.
- `manifest_from_curated_core()` — wires the typed Core
(`curated_entities()` methods + `structural::INHERITS`) as the
authoritative manifest source.
- `project_virtually_overrides()` — emits
`(odoo:<child>.<m>, virtually_overrides, odoo:<base>.<m>)` for consumers
that want the corpus shape.
- 7 tests: direct override, no-false-positive (method only on child),
nearest-base-wins, transitive skip, cycle guard, projection shape,
typed-core smoke.
# Manifest source is the caller's choice; resolver is identical
The curated-core manifest is authoritative but narrow — most curated
models inherit UNCURATED mixins, so the resolved set is honestly small
today (the smoke test asserts the call is total, not that overrides
exist). A consumer holding the SPO corpus builds the same two maps from
has_function + inherits_from (388 ObjectTypes incl. mixins) and gets the
full resolution from the same resolver. Same Curated-leg / Extracted-leg
convergence the structural correction established, for the derived
relation.
# Feeds the #531 ClassView
This resolution is the method-resolution layer the contract-side
`RegistryClassView` (D-CLS-RES, #531) does not yet carry — the ClassView
resolves field-sets via FieldMask but had no MRO/method surface.
EPIPHANIES: prepend E-ODOO-VIRTUALLY-OVERRIDES-COMPUTED. Wishlist is now
fully home-correct; the predicate-bolt-on cadence is retired.
# Tests
cargo test -p lance-graph-ontology --lib mro : 7/7
cargo clippy -p lance-graph-ontology --lib : clean on mro.rs
|
Warning Review limit reached
More reviews will be available in 37 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)
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: 7eb1411a8f
ℹ️ 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".
| seen.insert(start.to_string()); | ||
| // Frontier holds (model) at increasing chain distance; a BTreeSet per level | ||
| // gives the sorted tie-break, then feeds the next level. | ||
| let mut frontier: VecDeque<String> = bases_of |
There was a problem hiding this comment.
In this new module, rustfmt --edition 2021 --check crates/lance-graph-ontology/src/odoo_blueprint/mro.rs reports diffs starting at this initializer and in several later test assertions. The repository instructions require Rust to be formatted with cargo fmt, so landing this file as-is leaves the added module failing the formatter check; please run rustfmt/cargo fmt on the new code.
Useful? React with 👍 / 👎.
…ce/extend/poison + mro rustfmt Two codex P2 findings on merged #532 were real semantic bugs (live in main), both producing a WRONG `ASSERT $value IN [...]` — the silent-data-rejection class that matters for faithful DDL — plus a rustfmt miss on merged #533. # #532 finding 1 — selection_add must not close a dynamic domain A `Selection(selection_add=[('x','X')])` extension onto a base that is DYNAMIC (`Selection(STATE_CONST)` / `selection='_compute_x'`) was emitting `['x']` as if it were the complete closed set, so the projected `ASSERT $value IN ['x']` would reject every legal base value. # #532 finding 2 — full redeclarations must replace, not union The cross-class merge unioned EVERY `fields.Selection` declaration, so an `_inherit` class that REDECLARES the whole field with `selection=[...]` (replace/reorder) left stale keys from the earlier definition in the set. # The correct three-way semantics `_extract_selection(call) -> (base_dynamic, base_keys, add_keys)`: - static `selection=` → REPLACE (a redeclaration drops earlier keys) - `selection_add=` → EXTEND (unions across classes) - provided-but-dynamic → POISON (sentinel `None`; never closed; sticky) A closed `ASSERT IN` is emitted only when the whole domain is statically known. Poison is recorded as `None` in the `selections` map and skipped at emission (`selection_skip_open_domain` stat). # #533 — mro.rs rustfmt `mro.rs` landed un-`cargo fmt`'d (codex flagged the diff). Reformatted; 7/7 mro tests still pass. # Tests python3 -m unittest tests.test_spo_enrich : 61/61 (was 57) + test_dynamic_base_plus_add_stays_poisoned (finding 1) + test_full_redeclaration_replaces_not_unions (finding 2) + test_open_domain_none_emits_nothing + rewrote extraction tests for the (dynamic, base, add) contract cargo test -p lance-graph-ontology --lib mro : 7/7 rustfmt --check mro.rs : clean EPIPHANIES: E-ODOO-SELECTION-REPLACE-EXTEND-POISON.
2 participants
Summary
Closes the odoo-rs
UPSTREAM_WISHLISTlast item —virtually_overrides— the Core-correct way: a derived ClassView method-resolution relation, not a sixthspo_enrich.pyharvest predicate. Consistent with the Core-first correction (#530,E-ODOO-CORE-FIRST-STRUCTURAL): a fact an AST pass reads lives in the typed Core; a relation the ClassView derives lives in the resolver; neither is a flat-ndjson predicate.The relation
Per the doctrine the
(has_function / inherits_from / virtually_overrides)triad is the ClassView method-resolution manifest.has_function+inherits_fromare facts;virtually_overridesis the derivation —M.moverridesB.miffMdeclaresm,Bis reachable upM's_inheritchain, andBalso declaresm.What lands —
crates/lance-graph-ontology/src/odoo_blueprint/mro.rsresolve_overrides(methods_of, bases_of)— pure resolver over an abstract manifest. Nearest-base-wins BFS up the_inheritchain, cycle-guarded, deterministic. Equals Python C3 for the linear mixin chains Odoo uses; documented breadth-first approximation for diamonds.manifest_from_curated_core()— wires the typed Core (curated_entities()methods +structural::INHERITS) as the authoritative manifest source.project_virtually_overrides()— emits(odoo:<child>.<m>, virtually_overrides, odoo:<base>.<m>)for consumers wanting the corpus shape.Manifest source is the caller's choice; the resolver is identical
The curated-core manifest is authoritative but narrow — most curated models inherit uncurated mixins (
mail.activity.mixin,sequence.mixin), so the resolved set is honestly small today (the smoke test asserts the call is total, not that overrides exist). A consumer holding the SPO corpus builds the same two maps fromhas_function+inherits_from(388 ObjectTypes incl. mixins) and gets the full resolution from the same resolver — same Curated-leg / Extracted-leg convergence the structural correction established, now for the derived relation.Feeds the #531 ClassView
This is the method-resolution layer the contract-side
RegistryClassView(D-CLS-RES, #531) does not yet carry — that ClassView resolves field-sets viaFieldMaskbut had no MRO/method surface.Wishlist status
All six items are now home-correct: P0/P1/P1b/P2/P3 as typed-Core facts + Extracted-leg breadth (#523/#526/#527/#530/#532), and
virtually_overridesas this computed ClassView relation. None is a standalone flat-ndjson harvest predicate — the predicate-bolt-on cadence is fully retired.Tests
cargo test -p lance-graph-ontology --lib mro— 7/7cargo clippy -p lance-graph-ontology --lib— clean onmro.rsEPIPHANIES.mdprependsE-ODOO-VIRTUALLY-OVERRIDES-COMPUTED.