Skip to content

Commit f1640c6

Browse files
committed
ogar-from-ruff: wire constrains/onchange kausal + stored (Arm B + D)
Arm B: `lift_actions` now reads `Function::constrains` / `::onchange` (ruff main) into `KausalSpec::Constrains` / `::Onchange`, recording the matching decorator name (`api.constrains` / `api.onchange`) on `ActionDef::decorators`. Gated on `a.kausal.is_none()` so Arm A's `Depends` (a persisted recompute trigger) always outranks a validation/UI-recompute trigger on the same method; `constrains` is checked before `onchange` for the rarer case a method carries both. Arm D: `ComputedField.stored` is now wired from `Field::stored` (ruff main, `Option<bool>`) at both construction sites (`project_odoo_fields` and `project_total_schema_fields`) — `field.stored.unwrap_or(false)`, matching Odoo's own "not stored" default for computed fields. New tests cover: constrains -> Constrains, onchange -> Onchange, the Depends-wins-over-Constrains/Onchange conflict rule, the constrains-wins-over-onchange sub-case, an explicit Arm A regression, and all three `stored` literal cases (None/Some(false)/Some(true)) at both ComputedField construction sites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cg8DSUz1U4AvKqXiy9Xx2f
1 parent b291f93 commit f1640c6

1 file changed

Lines changed: 203 additions & 1 deletion

File tree

  • crates/ogar-from-ruff/src

crates/ogar-from-ruff/src/lib.rs

Lines changed: 203 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,11 @@ fn project_odoo_fields(class: &mut Class, model: &Model) {
257257
if let Some(compute_method) = &field.emitted_by {
258258
let mut computed = ComputedField::new(&field.name, compute_method);
259259
computed.depends = field.depends_on.clone();
260+
// SPEC-ATC2-OGAR Arm D: `store=` kwarg fact (ruff main,
261+
// `Field::stored: Option<bool>`). `None` (no `store=` kwarg) and
262+
// `Some(false)` both mean "not stored" (Odoo's own default for
263+
// computed fields); only `Some(true)` sets it.
264+
computed.stored = field.stored.unwrap_or(false);
260265
class.computed_fields.push(computed);
261266
}
262267
}
@@ -341,6 +346,9 @@ pub(crate) fn project_total_schema_fields(class: &mut Class, model: &Model) {
341346
if let Some(compute_method) = &field.emitted_by {
342347
let mut computed = ComputedField::new(&field.name, compute_method);
343348
computed.depends = field.depends_on.clone();
349+
// SPEC-ATC2-OGAR Arm D — same `store=` wiring as
350+
// `project_odoo_fields` above.
351+
computed.stored = field.stored.unwrap_or(false);
344352
class.computed_fields.push(computed);
345353
}
346354
}
@@ -528,7 +536,13 @@ pub fn project_canonical_roles(class: &Class) -> std::collections::HashSet<&'sta
528536
/// [`Model::fields`] compute target (`Field::emitted_by == Some(f.name)`),
529537
/// in which case `kausal` becomes `Some(KausalSpec::Depends { paths })`
530538
/// sourced from that field's `Field::depends_on` (the `@api.depends(...)`
531-
/// fact). It deliberately does **not**:
539+
/// fact). Failing that (Arm B, `ruff` main): `@api.constrains(...)` /
540+
/// `@api.onchange(...)` decorator facts (`Function::constrains` /
541+
/// `Function::onchange`) become `KausalSpec::Constrains` / `KausalSpec::
542+
/// Onchange` respectively — a validation / UI-recompute trigger, never
543+
/// conflated with the persisted `Depends` recompute trigger — and the
544+
/// matching decorator name (`"api.constrains"` / `"api.onchange"`) is
545+
/// recorded on `decorators`. It deliberately does **not**:
532546
///
533547
/// - set any execution policy (the vocab [`ActionDef`] has no `exec` slot;
534548
/// backend routing is consumer-private — see the OP-arm plan §5.2),
@@ -585,6 +599,27 @@ pub fn lift_actions(model: &Model) -> Vec<ActionDef> {
585599
if let Some(paths) = depends_by_method.get(f.name.as_str()) {
586600
a.kausal = Some(KausalSpec::depends((*paths).clone()));
587601
}
602+
// SPEC-ATC2-OGAR Arm B: `@api.constrains` / `@api.onchange`
603+
// decorator facts (`Function::constrains` / `Function::onchange`,
604+
// ruff main). Only applied when Arm A left `kausal` unset — a
605+
// persisted `@api.depends` recompute trigger outranks a
606+
// validation/UI-recompute trigger on the same method (the spec's
607+
// conflict rule; a method cannot be two kinds of trigger at once
608+
// in this sum type, and `Depends` is the stronger/persisted
609+
// claim). `constrains` is checked before `onchange` so a method
610+
// that (unusually) carries both decorators keeps the validation
611+
// reading, which is the narrower/more consequential of the two.
612+
if a.kausal.is_none() && !f.constrains.is_empty() {
613+
a.kausal = Some(KausalSpec::Constrains {
614+
paths: f.constrains.clone(),
615+
});
616+
a.decorators.push("api.constrains".to_string());
617+
} else if a.kausal.is_none() && !f.onchange.is_empty() {
618+
a.kausal = Some(KausalSpec::Onchange {
619+
paths: f.onchange.clone(),
620+
});
621+
a.decorators.push("api.onchange".to_string());
622+
}
588623
a
589624
})
590625
.collect()
@@ -1054,6 +1089,46 @@ mod tests {
10541089
assert_eq!(computed.field, "amount_total");
10551090
assert_eq!(computed.compute_method, "_compute_amount");
10561091
assert_eq!(computed.depends, vec!["line_ids.balance".to_string()]);
1092+
// `mk_odoo_model`'s `amount_total` field carries no `Field::stored`
1093+
// fact (`None`) — Arm D maps that to `false` (Odoo's own default for
1094+
// computed fields), same as the other two cases below.
1095+
assert!(!computed.stored);
1096+
}
1097+
1098+
/// SPEC-ATC2-OGAR Arm D: `Field::stored` (ruff main, `Option<bool>`)
1099+
/// wires onto `ComputedField.stored` at both construction sites
1100+
/// (`project_odoo_fields` and `project_total_schema_fields`).
1101+
/// `None` (no `store=` kwarg) and `Some(false)` both mean "not stored"
1102+
/// (Odoo's own default for computed fields); only `Some(true)` sets it.
1103+
#[test]
1104+
fn computed_field_stored_wired_from_ruff() {
1105+
for (stored, expected) in [(None, false), (Some(false), false), (Some(true), true)] {
1106+
let mut m = Model::new("account_move");
1107+
m.fields.push(Field {
1108+
name: "amount_total".to_string(),
1109+
emitted_by: Some("_compute_amount".to_string()),
1110+
depends_on: vec!["line_ids.balance".to_string()],
1111+
stored,
1112+
..Default::default()
1113+
});
1114+
1115+
// Odoo path (`project_odoo_fields`, via `lift_model_python`).
1116+
let class = lift_model_python(&m);
1117+
assert_eq!(class.computed_fields.len(), 1);
1118+
assert_eq!(
1119+
class.computed_fields[0].stored, expected,
1120+
"project_odoo_fields: stored={stored:?} must wire to {expected}"
1121+
);
1122+
1123+
// Rails/schema path (`project_total_schema_fields`).
1124+
let mut rails_class = Class::new("WorkPackage");
1125+
project_total_schema_fields(&mut rails_class, &m);
1126+
assert_eq!(rails_class.computed_fields.len(), 1);
1127+
assert_eq!(
1128+
rails_class.computed_fields[0].stored, expected,
1129+
"project_total_schema_fields: stored={stored:?} must wire to {expected}"
1130+
);
1131+
}
10571132
}
10581133

10591134
/// A Rails-shape model carrying BOTH the AR-DSL `associations` vector
@@ -1902,6 +1977,133 @@ mod tests {
19021977
assert_eq!(acts[0].raises, vec!["UserError".to_string()]);
19031978
}
19041979

1980+
/// SPEC-ATC2-OGAR Arm B: `@api.constrains('state')` (`Function::constrains`,
1981+
/// ruff main) becomes `KausalSpec::Constrains` — a validation trigger,
1982+
/// distinct from the persisted `Depends` recompute trigger — and the
1983+
/// decorator name rides `ActionDef::decorators` for extraction provenance.
1984+
#[test]
1985+
fn lift_actions_constrains_yields_kausal_constrains() {
1986+
let mut m = Model::new("SaleOrder");
1987+
m.functions.push(Function {
1988+
name: "_check_state".to_string(),
1989+
constrains: vec!["state".to_string()],
1990+
..Default::default()
1991+
});
1992+
let acts = lift_actions(&m);
1993+
assert_eq!(acts.len(), 1);
1994+
assert_eq!(
1995+
acts[0].kausal,
1996+
Some(KausalSpec::Constrains {
1997+
paths: vec!["state".to_string()]
1998+
}),
1999+
);
2000+
assert_eq!(acts[0].decorators, vec!["api.constrains".to_string()]);
2001+
}
2002+
2003+
/// SPEC-ATC2-OGAR Arm B: `@api.onchange('amount')` (`Function::onchange`,
2004+
/// ruff main) becomes `KausalSpec::Onchange` — a UI-side recompute
2005+
/// trigger, distinct from the persisted `Depends` trigger.
2006+
#[test]
2007+
fn lift_actions_onchange_yields_kausal_onchange() {
2008+
let mut m = Model::new("SaleOrder");
2009+
m.functions.push(Function {
2010+
name: "_onchange_amount".to_string(),
2011+
onchange: vec!["amount".to_string()],
2012+
..Default::default()
2013+
});
2014+
let acts = lift_actions(&m);
2015+
assert_eq!(acts.len(), 1);
2016+
assert_eq!(
2017+
acts[0].kausal,
2018+
Some(KausalSpec::Onchange {
2019+
paths: vec!["amount".to_string()]
2020+
}),
2021+
);
2022+
assert_eq!(acts[0].decorators, vec!["api.onchange".to_string()]);
2023+
}
2024+
2025+
/// SPEC-ATC2-OGAR Arm B conflict rule: a method that is BOTH a
2026+
/// `Model::fields` compute target (Arm A) AND carries `@api.constrains`
2027+
/// / `@api.onchange` decorator facts (Arm B) keeps the Arm A `Depends`
2028+
/// reading — the spec wires Arm B's checks as `if a.kausal.is_none()`,
2029+
/// i.e. strictly after Arm A, so the persisted recompute trigger always
2030+
/// outranks a validation/UI-recompute trigger on the same method.
2031+
#[test]
2032+
fn lift_actions_depends_wins_over_constrains_and_onchange() {
2033+
let mut m = Model::new("SaleOrder");
2034+
m.functions.push(Function {
2035+
name: "_compute_total".to_string(),
2036+
constrains: vec!["state".to_string()],
2037+
onchange: vec!["amount".to_string()],
2038+
..Default::default()
2039+
});
2040+
m.fields.push(Field {
2041+
name: "amount_total".to_string(),
2042+
emitted_by: Some("_compute_total".to_string()),
2043+
depends_on: vec!["qty".to_string()],
2044+
..Default::default()
2045+
});
2046+
let acts = lift_actions(&m);
2047+
assert_eq!(acts.len(), 1);
2048+
assert_eq!(
2049+
acts[0].kausal,
2050+
Some(KausalSpec::depends(vec!["qty".to_string()])),
2051+
"Arm A (Depends) must win over Arm B (Constrains/Onchange) per the spec's `kausal.is_none()` gate"
2052+
);
2053+
// Arm B's decorator push is gated by the same `kausal.is_none()`
2054+
// check, so it must not fire once Arm A has already claimed `kausal`.
2055+
assert!(acts[0].decorators.is_empty());
2056+
}
2057+
2058+
/// SPEC-ATC2-OGAR Arm B conflict rule (constrains vs onchange): when a
2059+
/// single method carries both decorator facts, `constrains` is checked
2060+
/// first, so it wins.
2061+
#[test]
2062+
fn lift_actions_constrains_wins_over_onchange() {
2063+
let mut m = Model::new("SaleOrder");
2064+
m.functions.push(Function {
2065+
name: "_check_and_onchange".to_string(),
2066+
constrains: vec!["state".to_string()],
2067+
onchange: vec!["amount".to_string()],
2068+
..Default::default()
2069+
});
2070+
let acts = lift_actions(&m);
2071+
assert_eq!(acts.len(), 1);
2072+
assert_eq!(
2073+
acts[0].kausal,
2074+
Some(KausalSpec::Constrains {
2075+
paths: vec!["state".to_string()]
2076+
}),
2077+
);
2078+
assert_eq!(acts[0].decorators, vec!["api.constrains".to_string()]);
2079+
}
2080+
2081+
/// SPEC-ATC2-OGAR Arm A regression: the pre-existing compute-depends
2082+
/// lift must keep working unchanged now that Arm B's checks ride the
2083+
/// same `.map(|f| …)` closure — a compute method with NO `constrains`/
2084+
/// `onchange` facts (the common case) still yields plain `Depends`.
2085+
#[test]
2086+
fn lift_actions_depends_arm_a_regression_unaffected_by_arm_b() {
2087+
let mut m = Model::new("SaleOrder");
2088+
m.functions.push(Function {
2089+
name: "_compute_amount".to_string(),
2090+
..Default::default()
2091+
});
2092+
m.fields.push(Field {
2093+
name: "amount_total".to_string(),
2094+
emitted_by: Some("_compute_amount".to_string()),
2095+
depends_on: vec!["line_ids.balance".to_string()],
2096+
..Default::default()
2097+
});
2098+
let acts = lift_actions(&m);
2099+
assert_eq!(acts.len(), 1);
2100+
assert_eq!(
2101+
acts[0].kausal,
2102+
Some(KausalSpec::depends(vec!["line_ids.balance".to_string()])),
2103+
);
2104+
assert!(acts[0].decorators.is_empty());
2105+
}
2106+
19052107
/// Regression: adding the effect-annotation fields must not disturb the
19062108
/// pre-existing identity / predicate / object_class shape — same
19072109
/// assertions as `lift_actions_predicate_object_class_and_identity`,

0 commit comments

Comments
 (0)