review fixes (#164 P1/P2): DO-arm carries private hook bodies; zip guard 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.

Author: claude

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

@@ -532,9 +532,19 @@ pub fn project_canonical_roles(class: &Class) -> std::collections::HashSet<&'sta
 /// downstream at registration, not in the producer.
 #[must_use]
 pub fn lift_actions(model: &Model) -> Vec<ActionDef> {
+    // Public actions AND non-public helpers (AT-CARRY-1b, review on #164):
+    // since ruff #45 the Ruby walker splits private/protected defs into
+    // `Model::helpers` — and Rails lifecycle hook targets are conventionally
+    // private (Redmine measurement: 67/84 hook targets live there). The W3.3
+    // delete-blocking rows ARE those hook bodies, so the DO-arm must carry
+    // both. Consistent with this fn's contract: the producer emits effect
+    // annotations; routability / guard / RBAC enrichment happens downstream
+    // at registration (a registrar can re-join `Model::callbacks` by name to
+    // tell hook targets from routable actions).
     model
         .functions
         .iter()
+        .chain(model.helpers.iter())
         .map(|f| {
             let mut a = ActionDef::new(
                 format!("{}::action_def::{}", model.name, f.name),
@@ -1812,4 +1822,32 @@ mod tests {
         // Mismatched / partial — pass-through.
         assert_eq!(strip_ruby_literal_markers("\"User"), "\"User");
     }
+
+    #[test]
+    fn lift_actions_carries_private_helper_hook_bodies() {
+        // AT-CARRY-1b (review on #164): Rails lifecycle hook targets are
+        // conventionally private -> `Model::helpers` (ruff #45). The DO-arm
+        // must carry their body facts, or the W3.3 delete-blocking rows
+        // never reach a consumer.
+        let mut m = Model::new("Issue");
+        m.functions.push(ruff_spo_triplet::Function {
+            name: "public_action".to_string(),
+            ..Default::default()
+        });
+        m.helpers.push(ruff_spo_triplet::Function {
+            name: "update_closed_on".to_string(),
+            writes: vec!["closed_on".to_string()],
+            reads: vec!["updated_on".to_string()],
+            ..Default::default()
+        });
+        let actions = lift_actions(&m);
+        assert_eq!(actions.len(), 2, "public action + private hook body");
+        let hook = actions
+            .iter()
+            .find(|a| a.predicate == "update_closed_on")
+            .expect("private hook body must arrive in the DO-arm");
+        assert_eq!(hook.identity, "Issue::action_def::update_closed_on");
+        assert_eq!(hook.writes, vec!["closed_on".to_string()]);
+        assert_eq!(hook.reads, vec!["updated_on".to_string()]);
+    }
 }

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

@@ -81,7 +81,12 @@ pub fn compile_graph_python<P: PortSpec>(graph: &ModelGraph) -> Vec<CompiledClas
     // The lift maps `graph.models` 1:1 in declaration order (member-level
     // filter_maps only), so zipping recovers each class's source `Model` —
     // the carrier `lift_actions` needs (AT-CARRY-1).
-    debug_assert_eq!(classes.len(), graph.models.len());
+    assert_eq!(
+        classes.len(),
+        graph.models.len(),
+        "lift must map graph.models 1:1 in declaration order — a model-level \
+         filter would silently mis-zip actions onto the wrong class"
+    );
     classes
         .into_iter()
         .zip(&graph.models)
@@ -134,7 +139,12 @@ pub fn compile_graph_sqlalchemy<P: PortSpec>(graph: &ModelGraph) -> Vec<Compiled
     let mint = mint_graph::<P>(graph);
     let classes = lift_model_graph_sqlalchemy(graph);
     // 1:1 with `graph.models` in declaration order (see compile_graph_python).
-    debug_assert_eq!(classes.len(), graph.models.len());
+    assert_eq!(
+        classes.len(),
+        graph.models.len(),
+        "lift must map graph.models 1:1 in declaration order — a model-level \
+         filter would silently mis-zip actions onto the wrong class"
+    );
     classes
         .into_iter()
         .zip(&graph.models)
@@ -164,7 +174,12 @@ pub fn compile_graph_ruby<P: PortSpec>(graph: &ModelGraph) -> Vec<CompiledClass>
     let mint = mint_graph::<P>(graph);
     let classes = lift_model_graph(graph);
     // 1:1 with `graph.models` in declaration order (see compile_graph_python).
-    debug_assert_eq!(classes.len(), graph.models.len());
+    assert_eq!(
+        classes.len(),
+        graph.models.len(),
+        "lift must map graph.models 1:1 in declaration order — a model-level \
+         filter would silently mis-zip actions onto the wrong class"
+    );
     classes
         .into_iter()
         .zip(&graph.models)