feat(mint): AT-CARRY-1 — CompiledClass carries the DO-arm (actions: V…#164
Conversation
…ec<ActionDef>) The W3.3 delete-gate matrix (odoo-rs docs/W3.3-DELETE-GATE-MATRIX.md, merged odoo-rs #23) found the real blocker behind the fork-delete: all 5 delete-blocking rows are behaviour-arm, and lift_actions existed (lib.rs:534, post-O-1 with reads/writes/calls) but compile_graph_* dropped its output — CompiledClass carried only {class, facet}, so no consumer ever saw the behaviour arm. CompiledClass gains actions: Vec<ActionDef>, populated in all three compile fns (python / sqlalchemy / ruby) by zipping the lifted classes with graph.models — sound because every lift maps graph.models 1:1 in declaration order (member-level filter_maps only; debug_assert pins it). THINK arm (class) and DO arm (actions) now travel together, addressed by the same facet. Acceptance (AT-CARRY-1, falsifiable): the account_move fixture's _compute_amount now arrives as an ActionDef with identity account_move::action_def::_compute_amount, reads [line_ids.balance], writes [amount_total] — asserted in the compile test. Verified: cargo test -p ogar-from-ruff 64+3 green; clippy 0 errors; ogar-adapter-surrealql + ogar-emitter check clean (no construction or exhaustive-destructuring sites outside mint.rs). Next (named, not hidden): AT-CARRY-2/3 (KausalSpec population from decorator kinds; body_source) and odoo-rs AT-CONSUME.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_a93b798b-1998-4cda-a740-af852383a8c8) |
AdaWorldAPI
left a comment
AdaWorldAPI
left a comment
There was a problem hiding this comment.
Reviewed with both load-bearing claims re-verified in source. The shape is right — the DO-arm belongs on CompiledClass, the acceptance test is falsifiable, and the effect-annotations framing matches lift_actions's own doc contract. Two findings, one latent-but-real:
P1 — CATCH-LATENT: the Rails leg's DO-arm will omit exactly the hooks the W3.3 gate cares about. lift_actions iterates model.functions only (lib.rs:534-550). Since ruff #45, the Ruby walker splits private/protected defs into Model::helpers — and Rails lifecycle callback targets are conventionally private (measured on Redmine: 67/84 hook targets were invisible until the helpers split landed). So compile_graph_ruby's actions will carry public actions but silently omit the private hook bodies — precisely the delete-blocking behaviour-arm rows that motivated AT-CARRY-1. The odoo/python leg is unaffected (no visibility split; _compute_amount sits in functions), so the acceptance test can't see this. Smallest fix, now or as a named AT-CARRY-1b: model.functions.iter().chain(model.helpers.iter()) in lift_actions (or a lift_hooks sibling if action-vs-hook must stay distinguishable — Model::callbacks carries the phase to join on). If deferred, name it in the PR's Next list so it doesn't read as covered.
P2 — the zip guard evaporates in release. The 1:1 order claim is TRUE today (verified: lift_model_graph_with_language is a straight .map over graph.models, lib.rs:100-131; sqlalchemy likewise, sqlalchemy.rs:162; member-level filter_maps only). But the pin is debug_assert_eq! — compiled out in release. If a future lift ever drops a model, zip truncates silently and every subsequent class gets the previous model's actions — cross-attributed behaviour in exactly the builds nobody tests. The check is once-per-graph and free: make it a plain assert_eq! with a message naming the invariant, or restructure to a single pass over graph.models producing (class, actions) together — structurally un-mis-zippable. Either closes it.
Verdict: LAND after deciding both — P2 is a two-character-class fix; P1 is fine as an explicitly named follow-up but must not stay implicit, because "THINK and DO now travel together" is currently true for Odoo and only ~half-true for Rails.
…ard survives release P1 (CATCH-LATENT): lift_actions now chains model.helpers — since ruff #45 the Ruby walker splits private/protected defs there, and Rails lifecycle hook targets are conventionally private (Redmine: 67/84 hook targets). Without the chain, compile_graph_ruby's actions silently omitted exactly the W3.3 delete-blocking hook bodies AT-CARRY-1 exists to carry. The producer emits effect annotations; routability enrichment stays a registration concern (join Model::callbacks by name to tell hooks from routable actions). Regression test: a private helper hook body arrives with its reads/writes intact. P2: the three zip guards are plain assert_eq! with the invariant named — debug_assert compiled out in release, where a future model-level filter would truncate the zip silently and cross-attribute every subsequent class's actions. Once-per-graph, effectively free. cargo test -p ogar-from-ruff: 65+3 green; clippy --all-targets -D warnings clean.
2 participants
…ec)
The W3.3 delete-gate matrix (odoo-rs docs/W3.3-DELETE-GATE-MATRIX.md, merged odoo-rs #23) found the real blocker behind the fork-delete: all 5 delete-blocking rows are behaviour-arm, and lift_actions existed (lib.rs:534, post-O-1 with reads/writes/calls) but compile_graph_* dropped its output — CompiledClass carried only {class, facet}, so no consumer ever saw the behaviour arm.
CompiledClass gains actions: Vec, populated in all three compile fns (python / sqlalchemy / ruby) by zipping the lifted classes with graph.models — sound because every lift maps graph.models 1:1 in declaration order (member-level filter_maps only; debug_assert pins it). THINK arm (class) and DO arm (actions) now travel together, addressed by the same facet.
Acceptance (AT-CARRY-1, falsifiable): the account_move fixture's _compute_amount now arrives as an ActionDef with identity account_move::action_def::_compute_amount, reads [line_ids.balance], writes [amount_total] — asserted in the compile test.
Verified: cargo test -p ogar-from-ruff 64+3 green; clippy 0 errors; ogar-adapter-surrealql + ogar-emitter check clean (no construction or exhaustive-destructuring sites outside mint.rs).
Next (named, not hidden): AT-CARRY-2/3 (KausalSpec population from decorator kinds; body_source) and odoo-rs AT-CONSUME.