address review: charge only calls that reach the cumulative period loop

crates/functions/src/financial.rs

@@ -13,6 +13,7 @@ mod day_count;
 mod depreciation;
 mod securities;
 mod tvm;
+pub(crate) use tvm::cumulative_args;
 
 use day_count::{coupdaybs, coupdays, coupdaysnc, coupncd, coupnum, couppcd};
 use depreciation::{amordegrc, amorlinc, db, ddb, sln, syd, vdb};

crates/functions/src/financial/tvm.rs

@@ -116,7 +116,9 @@ pub(super) fn ppmt(a: &[Arg], _c: &mut dyn Ctx) -> R<Value> {
     num_val(pmt_calc(rate, np, pv, fv, typ) - ipmt_calc(rate, per, np, pv, fv, typ))
 }
 
-fn cumulative(a: &[Arg], principal: bool) -> R<Value> {
+/// The validated (rate, periods, pv, start, end, type) of CUMIPMT/CUMPRINC: every check that
+/// refuses a call before its period loop, shared with the lifted-call work preflight.
+pub(crate) fn cumulative_args(a: &[Arg]) -> R<(f64, f64, f64, f64, f64, f64)> {
     let (rate, np, pv) = (num(a, 0)?, num(a, 1)?, num(a, 2)?);
     let (start, end, typ) = (num(a, 3)?.trunc(), num(a, 4)?.trunc(), num(a, 5)?);
     let np_i = np.trunc();
@@ -126,6 +128,11 @@ fn cumulative(a: &[Arg], principal: bool) -> R<Value> {
     if end - start > 1e7 || np_i > 1e15 {
         return Err(CellError::Num);
     }
+    Ok((rate, np_i, pv, start, end, typ))
+}
+
+fn cumulative(a: &[Arg], principal: bool) -> R<Value> {
+    let (rate, np_i, pv, start, end, typ) = cumulative_args(a)?;
     let p = pmt_calc(rate, np_i, pv, 0.0, typ);
     let mut sum = 0.0;
     let mut per = start;

crates/functions/src/lift.rs

@@ -3,7 +3,7 @@
 
 use gridcraft_core::{Array, CellError, Value};
 
-use crate::util::{MAX_CELLS, num};
+use crate::util::MAX_CELLS;
 use crate::{Arg, Ctx, FnSpec};
 
 /// Value-parameter masks (see [`FnSpec::value_args`]), observed in Excel 365: a reference to more
@@ -102,13 +102,9 @@ fn call_cost(spec: &FnSpec, cell_args: &[Arg]) -> usize {
     if !PERIOD_LOOPS.contains(&spec.name) {
         return 0;
     }
-    // The function's own coercion (text like "10000000" counts), so no loop goes uncharged.
-    let period = |i: usize| num(cell_args, i).ok().filter(|n| n.is_finite()).map(f64::trunc);
-    match (period(3), period(4)) {
-        // The function itself refuses spans past 1e7 periods; anything larger costs the cap.
-        (Some(start), Some(end)) if end >= start => ((end - start) + 1.0).min(1e8) as usize,
-        _ => 0,
-    }
+    // The function's own validation and coercion (text like "10000000" counts), so a call it
+    // refuses before its loop costs nothing and no loop goes uncharged.
+    crate::financial::cumulative_args(cell_args).map_or(0, |(.., start, end, _)| (end - start + 1.0) as usize)
 }
 
 pub(crate) fn call_lifted(spec: &FnSpec, args: &[Arg], ctx: &mut dyn Ctx) -> Value {
@@ -304,6 +300,20 @@ mod tests {
         assert!(matches!(rows_of(&out).first().and_then(|r| r.first()), Some(Value::Number(x)) if *x < 0.0));
     }
 
+    /// A call the function refuses before its loop (here: end period past the period count) is
+    /// not charged, so rejected elements can't push a valid one over the budget.
+    #[test]
+    fn lifted_period_loops_do_not_charge_rejected_calls() {
+        use gridcraft_core::Array;
+        let mut ends = vec![nv(1e7 + 1.0); 60];
+        ends.push(nv(12.0));
+        let ends = av(Array::new(ends.len(), 1, ends).unwrap_or_else(|| panic!("shape")).into());
+        let out = ev("CUMIPMT", vec![n(0.01), n(1e7), n(1000.0), n(1.0), ends, n(0.0)]);
+        let rows = rows_of(&out);
+        assert_eq!(rows.len(), 61);
+        assert!(matches!(rows.last().and_then(|r| r.first()), Some(Value::Number(x)) if *x < 0.0));
+    }
+
     /// Security: an array-producing function lifted over a computed array builds each element's
     /// whole result before keeping one cell; their cells count against the budget.
     #[test]