Skip to content

Commit baa019e

Browse files
committed
fix(odoo-spo): #523 codex/coderabbit review — multi-emitter + _inherit-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
1 parent 69a3b0a commit baa019e

5 files changed

Lines changed: 772 additions & 33 deletions

File tree

.claude/board/AGENT_LOG.md

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,24 @@
1+
## 2026-06-17 — PR #523 review fixes: spo_enrich multi-emitter + `_inherit`-only
2+
3+
**Main thread (Opus) — single implementer**, branch `claude/odoo-spo-fk-target-deep-reads` (review-fix commit on top of the enrichment commit, no rebase). Addresses 4 valid review findings (codex P1 + codex P2/CodeRabbit Major + 2 CodeRabbit doc nits). Scope: the lance-graph SPO corpus + the stdlib Python extractor tooling under `tools/odoo-blueprint-extractor/` (no odoo-rs change).
4+
5+
**Fixed:**
6+
- **Fix 1 (codex P1, real bug):** `spo_enrich.py` deep-read lift kept only the LAST `emitted_by` method per field (`field_emitter` dict). A field with multiple emitters (confirmed `stock_move.quantity``_compute_quantity` AND `_onchange_product_uom_qty`) lifted the deep `reads_field` onto one only. Changed to a per-field sorted emitter LIST (`field_emitters`); the deep read is now emitted for EACH emitter (self-loop drop preserved per-emitter, determinism kept).
7+
- **Fix 2 (codex P2 / CodeRabbit Major):** `_scan_file` bound relational fields only when `_name` was present, dropping the `_inherit`-only extension form (`_inherit = "account.move"` / `["a","b"]` with no `_name`). Now resolves `model_names` from `_name` if present ELSE from `_inherit` (string→1, list/tuple→each, via new `_const_str_list`), mapping `local_fields` onto every resolved model — mirroring the package class parser's `_inherit` handling.
8+
- **Fix 3 (CodeRabbit doc nit):** AGENT_LOG scope reworded from "lance-graph only" to "lance-graph corpus + the stdlib Python extractor tooling" (the diff includes `spo_enrich.py` + tests).
9+
- **Fix 4 (CodeRabbit doc nit):** the EPIPHANIES "27 compute edges / no-cycle verified locally" claim used a discarded worktree. Re-anchored to a persistent in-repo fixture test (`tools/odoo-blueprint-extractor/tests/test_spo_enrich.py` `TestMultiEmitterDeepReads` + `TestInheritOnlyRelationMap`).
10+
11+
**Corpus regenerated from base (22 245):** 22 245 → **24 166** triples. `target` 618→**842** (+224, `_inherit`-only extension fields), `inverse_name` 102→**144** (+42), deep `reads_field` 736→**935** (+199, per-emitter lift); `reads_field` total 2 095→**3 030**. Unknown-hop skips 567→337 (more hops now resolve via `_inherit` targets). `rdf:type`/`depends_on`/`emitted_by`/`has_function` unchanged.
12+
13+
**Rust:** `odoo_ontology.rs` module-doc + count test (24 166), histogram (`target`=842, `inverse_name`=144, `reads_field`=3 030) updated; 2 new in-corpus tests (`enrichment_lifts_deep_reads_onto_every_emitter` on `stock_move.quantity`; `enrichment_honors_inherit_only_extension_fields` on `account_move.authorized_transaction_ids``payment.transaction`).
14+
15+
**Tests:** extractor `python3 -m unittest discover` 20/20 green (14 prior + 6 new). lance-graph `cargo test -p lance-graph --lib odoo_ontology` green (counts updated). `cargo fmt` clean; `cargo clippy -p lance-graph -- -D warnings` clean on the touched crate (pre-existing causal-edge/p64-bridge/planner `-D warnings` debt untouched). See `EPIPHANIES.md` E-ODOO-FK-DEEP-READS (Status updated with the review-fix totals).
16+
17+
---
18+
119
## 2026-06-17 — odoo SPO corpus enrichment: P1 FK-target + P0 deep-reads_field (UPSTREAM_WISHLIST)
220

3-
**Main thread (Opus) — single implementer**, branch `claude/odoo-spo-fk-target-deep-reads`. Implements the odoo-rs `UPSTREAM_WISHLIST` P1 (FK `target`/`inverse_name`) + coupled P0 (deep `reads_field`) corpus enrichment, lance-graph only.
21+
**Main thread (Opus) — single implementer**, branch `claude/odoo-spo-fk-target-deep-reads`. Implements the odoo-rs `UPSTREAM_WISHLIST` P1 (FK `target`/`inverse_name`) + coupled P0 (deep `reads_field`) corpus enrichment. Scope: the lance-graph SPO corpus + the stdlib Python extractor tooling under `tools/odoo-blueprint-extractor/` (no odoo-rs change). **Review-fix follow-up 2026-06-17 (PR #523, see entry above):** totals below (618/102/736 → 24 166) were corrected by the review pass; see the prepended entry for the new figures.
422

523
**Shipped:**
624
- `tools/odoo-blueprint-extractor/odoo_blueprint_extractor/spo_enrich.py` (new, stdlib-only): builds a `(model, field) → (comodel, inverse)` relation map from `/home/user/odoo/addons` via `ast`, then (P1) emits `target`/`inverse_name` sibling triples keyed by the relation IRI (ruff#18 shape, raw dotted comodel object) for every relational field on a corpus-declared model, and (P0) resolves each dotted `@api.depends` path through the map and lifts a deep `reads_field` onto the field's emitting method. Additive, deterministic, idempotent (`(s,p,o)` dedup); self-loops dropped; unknown-hop paths skipped + counted. CLI: `python3 -m odoo_blueprint_extractor.spo_enrich --corpus … --addons …`.

crates/lance-graph/src/graph/spo/odoo_ontology.rs

Lines changed: 73 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,15 @@
6868
//!
6969
//! Data file `odoo_ontology.spo.ndjson` carries the base extraction (388
7070
//! Object Types, 3 107 Properties, 3 328 Functions) plus the `spo_enrich`
71-
//! P1/P0 layer (618 `target` + 102 `inverse_name` + 736 deep `reads_field`),
72-
//! for 23 701 triples total. The base extraction's original generator
71+
//! P1/P0 layer (842 `target` + 144 `inverse_name` + 935 deep `reads_field`),
72+
//! for 24 166 triples total. The `target`/`inverse_name`/deep-read totals grew
73+
//! over the initial enrichment (618/102/736) once `spo_enrich` (a) honored
74+
//! `_inherit`-only extension classes — `_inherit = "account.move"` with no
75+
//! `_name`, the common Odoo extension form whose relational fields were
76+
//! previously dropped — and (b) lifted each deep `reads_field` onto EVERY
77+
//! emitter of the dependent field, not just the last (a field such as
78+
//! `stock_move.quantity` is emitted by both `_compute_quantity` and
79+
//! `_onchange_product_uom_qty`). The base extraction's original generator
7380
//! (`emit_ontology2.py` over a `methods.parquet`) is not present in this
7481
//! tree — only its output is — so the enrichment is applied over the shipped
7582
//! corpus + the Odoo source via
@@ -146,9 +153,9 @@ mod tests {
146153
#[test]
147154
fn parses_all_triples() {
148155
let triples = parse_triples(ONTOLOGY);
149-
// 22 245 base triples + 1 456 spo_enrich triples (618 target +
150-
// 102 inverse_name + 736 deep reads_field) = 23 701.
151-
assert_eq!(triples.len(), 23_701, "triple count drifted from data file");
156+
// 22 245 base triples + 1 921 spo_enrich triples (842 target +
157+
// 144 inverse_name + 935 deep reads_field) = 24 166.
158+
assert_eq!(triples.len(), 24_166, "triple count drifted from data file");
152159
}
153160

154161
#[test]
@@ -176,10 +183,13 @@ mod tests {
176183
assert_eq!(hist.get("emitted_by"), Some(&3228));
177184
assert_eq!(hist.get("rdf:type"), Some(&6823));
178185
// spo_enrich P1/P0 layer: FK target/inverse_name + deep reads_field.
179-
assert_eq!(hist.get("target"), Some(&618));
180-
assert_eq!(hist.get("inverse_name"), Some(&102));
181-
// reads_field grew from 2 095 (base) to 2 831 with 736 deep lifts.
182-
assert_eq!(hist.get("reads_field"), Some(&2831));
186+
// Totals grew from 618/102/736 once `_inherit`-only extension classes
187+
// were honored (more `target`/`inverse_name`) and deep reads were
188+
// lifted onto EVERY emitter of a field (more deep `reads_field`).
189+
assert_eq!(hist.get("target"), Some(&842));
190+
assert_eq!(hist.get("inverse_name"), Some(&144));
191+
// reads_field grew from 2 095 (base) to 3 030 with 935 deep lifts.
192+
assert_eq!(hist.get("reads_field"), Some(&3030));
183193
assert_eq!(hist.get("other"), None, "unexpected predicate kind");
184194
}
185195

@@ -369,4 +379,58 @@ mod tests {
369379
"function count drifted from module-doc claim (3 328)"
370380
);
371381
}
382+
383+
/// `spo_enrich` P0 multi-emitter — a field emitted by MORE than one method
384+
/// must have its deep `reads_field` lifted onto EVERY emitter, not just the
385+
/// last. `stock_move.quantity` is emitted by both `_compute_quantity` AND
386+
/// `_onchange_product_uom_qty`; both must carry the cross-model deep reads
387+
/// (`stock_move_line.quantity` / `stock_move_line.product_uom_id`). Before
388+
/// the multi-emitter fix the deep read landed on only one of them, dropping
389+
/// the recompute-ordering edge for the other (typically the `_compute_*`).
390+
#[test]
391+
fn enrichment_lifts_deep_reads_onto_every_emitter() {
392+
let triples = parse_triples(ONTOLOGY);
393+
let deep_on = |method: &str, obj: &str| {
394+
triples
395+
.iter()
396+
.any(|t| t.s == method && t.p == "reads_field" && t.o == obj)
397+
};
398+
// Both emitters of stock_move.quantity carry the cross-model deep read.
399+
assert!(
400+
deep_on(
401+
"odoo:stock_move._compute_quantity",
402+
"odoo:stock_move_line.quantity"
403+
),
404+
"P0 multi-emitter: _compute_quantity must read stock_move_line.quantity"
405+
);
406+
assert!(
407+
deep_on(
408+
"odoo:stock_move._onchange_product_uom_qty",
409+
"odoo:stock_move_line.quantity"
410+
),
411+
"P0 multi-emitter: _onchange_product_uom_qty must also read \
412+
stock_move_line.quantity (every emitter, not just the last)"
413+
);
414+
}
415+
416+
/// `spo_enrich` P1 `_inherit`-only — relational fields declared on an
417+
/// extension class (`_inherit = "account.move"` with no `_name`) must still
418+
/// get their `target`. `account_move.authorized_transaction_ids` is declared
419+
/// only on the `account_payment` extension of `account.move`; before the
420+
/// `_inherit` fix the class was skipped (no `_name`) and the field never got
421+
/// a target.
422+
#[test]
423+
fn enrichment_honors_inherit_only_extension_fields() {
424+
let triples = parse_triples(ONTOLOGY);
425+
let target = triples.iter().any(|t| {
426+
t.s == "odoo:account_move.authorized_transaction_ids"
427+
&& t.p == "target"
428+
&& t.o == "payment.transaction"
429+
});
430+
assert!(
431+
target,
432+
"P1 _inherit-only: authorized_transaction_ids (declared on an \
433+
_inherit='account.move' extension) must resolve to payment.transaction"
434+
);
435+
}
372436
}

0 commit comments

Comments
 (0)