fix(flows): move quote pricing and forecast accumulators to CEL value envelopes

.changeset/flow-values-cel.md

@@ -0,0 +1,11 @@
+---
+'hotcrm': patch
+---
+
+Quote pricing and the nightly forecast snapshot now compute their amounts with CEL, the
+expression language the platform declares for flow values, instead of the older `{…}`
+template form. Nothing you see changes: a quote prices exactly as before (whole cents,
+and a cleared discount still means no discount), and the forecast's pipeline, best case,
+commit and closed-won totals are the same sums. An opportunity with no amount still
+counts as 0, and an amount with cents keeps its cents. `objectstack validate --strict`
+no longer reports these six expressions.

src/revenue/flows/quote-generation.flow.ts

@@ -1,10 +1,17 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
-import { P } from '@objectstack/spec';
+import { P, expression } from '@objectstack/spec';
 import type * as Automation from '@objectstack/spec/automation';
 import { QUOTE_DISCOUNT_CEILING } from '../../sales/objects/_thresholds';
 type Flow = Automation.Flow;
 
+/**
+ * The screen's discount as a CEL double, 0 when the rep cleared the box. A
+ * source FRAGMENT, spliced into both pricing envelopes with `expression()`:
+ * the `P` tag JSON-quotes an interpolated string, so it cannot splice one.
+ */
+const DISCOUNT = '(!has(vars.discount) || isBlank(vars.discount) ? 0.0 : double(vars.discount))';
+
 /** Quote Generation — screen flow to create a quote from an opportunity */
 export const QuoteGenerationFlow: Flow = {
   name: 'quote_generation',
@@ -91,20 +98,20 @@ export const QuoteGenerationFlow: Flow = {
           // write is now ACCEPTED and the tail would be stored silently. The
           // rounding keeps `discount_amount` / `total_price` whole-cent amounts.
           //
-          // `round()` is the CEL stdlib's, mirrored 1:1 into flow value
-          // expressions from service-automation 17.3.0. It is INTEGER-ONLY and
-          // single-argument, so N-decimal rounding is spelled `round(x * 100) /
-          // 100` — the platform's own arity diagnostic names this exact pattern.
-          // ⛔ Not `round(x, 2)`: there is no precision form, and it now fails
-          // loudly. ⛔ Never an operator trick like `(x * 100 + 0.5 | 0) / 100`
-          // either — `|0` is an int32 coercion that SILENTLY overflows above
-          // ~21.5M, which on a money field is worse than the defect it dodges;
-          // `round()` refuses loudly past `Number.MAX_SAFE_INTEGER` instead.
+          // Both are CEL value envelopes. CEL's `round()` is INTEGER-ONLY and
+          // single-argument, and it returns an INT — so the divisor MUST be
+          // the decimal `100.0`. ⛔ Never `/ 100`: in CEL int / int is integer
+          // division, which silently drops the cents (1,234.56 at 10% would
+          // store 123, not 123.46; `test/flow-quote.test.ts` pins it). ⛔ Not
+          // `round(x, 2)` either: there is no precision form, and it fails
+          // loudly. `double()` types the amount, which some drivers return as
+          // a string. A cleared discount (null / absent / "") prices as 0%, as
+          // it did before; the `has()` guard is what keeps that TOTAL.
           //
           // ⭐ This shape applies ANYWHERE a flow multiplies a currency by a
           // percentage. Write the rounding, not the bare product.
-          discount_amount: '{round(oppRecord.amount * (discount / 100) * 100) / 100}',
-          total_price: '{round(oppRecord.amount * (1 - discount / 100) * 100) / 100}',
+          discount_amount: expression(`round(double(oppRecord.amount) * (${DISCOUNT} / 100.0) * 100.0) / 100.0`, 'cel'),
+          total_price: expression(`round(double(oppRecord.amount) * (1.0 - ${DISCOUNT} / 100.0) * 100.0) / 100.0`, 'cel'),
           payment_terms: 'net_30',
         },
         outputVariable: 'quoteId',

src/sales/flows/forecast-snapshot.flow.ts

@@ -1,6 +1,6 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
-import { P } from '@objectstack/spec';
+import { P, expression } from '@objectstack/spec';
 import type * as Automation from '@objectstack/spec/automation';
 import { guarded } from './_guarded-iteration';
 type Flow = Automation.Flow;
@@ -51,9 +51,8 @@ type Flow = Automation.Flow;
  *    current period", owned by the object.
  * 2. Sums come from four separate `get_record` + `loop` pairs rather than one
  *    query with in-loop branching. A filter is plain metadata, so the bucket
- *    definitions stay declarative and greppable; the accumulator templates
- *    stay pure arithmetic (`* 1` coerces a currency that arrives as a string
- *    and maps null to 0).
+ *    definitions stay declarative and greppable; the accumulators stay pure
+ *    CEL arithmetic (see `sumBucket`).
  *
  * Idempotency: the row is keyed by (owner, period, containing window), so a
  * re-run finds it and OVERWRITES the amounts instead of inserting a second
@@ -182,12 +181,16 @@ const inPeriod = (extra: Record<string, unknown>) => ({
 /**
  * One `get_record` + `loop` pair that sums `amount` into `total`.
  *
- * `amount * 1` rather than a bare `amount`: the accumulator runs through the
- * template evaluator, which stringifies non-numeric values — a currency that
- * comes back from the driver as `"90000"` would CONCATENATE instead of add.
- * `* 1` coerces it, and maps a null/absent amount to 0.
+ * The accumulator is a CEL value envelope, so its operands are TYPED:
+ * `double()` turns a currency some drivers return as a string (`"90000"`) into
+ * a number, and a decimal amount keeps its cents. ⛔ Never a bare
+ * `double(amount)`: it ERRORS on null — `found no matching overload for
+ * 'double(null)'` — and one opportunity without an amount would fail the
+ * owner's whole sweep. The `has()` / `isBlank()` guard sums a null or absent
+ * amount as 0. `expression()` rather than `P`: the source splices the node's
+ * variable names, which the tag would JSON-quote.
  */
-const sumBucket = (key: string, label: string, filter: Record<string, unknown>, total: string) => ({
+const sumBucket = (key: string, label: string, filter: Record<string, unknown>, total: string, item = `current_${key}`) => ({
   find: {
     id: `find_${key}`,
     type: 'get_record' as const,
@@ -205,14 +208,18 @@ const sumBucket = (key: string, label: string, filter: Record<string, unknown>,
     label: `Sum ${label}`,
     config: {
       collection: `{${key}Opps}`,
-      iteratorVariable: `current_${key}`,
+      iteratorVariable: item,
       body: {
         nodes: [
           {
             id: `add_${key}`,
             type: 'assignment' as const,
             label: `Add to ${label}`,
-            config: { assignments: { [total]: `{${total} + current_${key}.amount * 1}` } },
+            config: {
+              assignments: {
+                [total]: expression(`${total} + (!has(${item}.amount) || isBlank(${item}.amount) ? 0.0 : double(${item}.amount))`, 'cel'),
+              },
+            },
           },
         ],
         edges: [],

test/flow-quote.test.ts

@@ -120,3 +120,41 @@ describe('quote_generation flow — runtime', () => {
     expect(q.crm_contact == null, 'contact left empty').toBe(true);
   });
 });
+
+/**
+ * The two pricing expressions are CEL value envelopes (#1984), and CEL divides
+ * an int by an int as INTEGERS. `round()` returns an int, so `round(x * 100) /
+ * 100` silently drops the cents there; only the decimal divisor `/ 100.0`
+ * keeps them. The pins above cannot see that: every amount they use prices to
+ * whole units. 1,234.56 at 10% is 123.456 → 123.46 with the decimal divisor
+ * and 123 with an integer one, and the total 1,111.104 → 1,111.10 vs 1,111 —
+ * so these go red if either divisor loses its `.0`.
+ */
+describe('quote_generation flow — CEL pricing (#1984)', () => {
+  const quoteAt = async (amount: unknown, screen: Rec) => {
+    const h = makeQuote({
+      crm_opportunity: [{
+        id: 'opp_4', name: 'Cents Deal', amount,
+        crm_account: 'acc_4', primary_contact: null, stage: 'qualification',
+      }],
+    });
+    await runQuote(h, 'opp_4', { quoteName: 'Q-4', expirationDays: 30, ...screen });
+    expect(h.store.crm_quote?.length, 'quote created').toBe(1);
+    return h.store.crm_quote[0];
+  };
+
+  it('keeps the cents: both divisors are decimal, not integer division', async () => {
+    const q = await quoteAt(1234.56, { discount: 10 });
+    expect(q.discount_amount).toBe(123.46);
+    expect(q.total_price).toBe(1111.1);
+  });
+
+  // The template dialect read a cleared discount as 0. A bare `double(discount)`
+  // ERRORS on null, which would fail the quote instead — the guard keeps the
+  // old pricing: no discount, full price.
+  it.each([null, ''])('prices a cleared discount (%j) as no discount', async (discount) => {
+    const q = await quoteAt(180000, { discount });
+    expect(q.discount_amount).toBe(0);
+    expect(q.total_price).toBe(180000);
+  });
+});

test/forecast-snapshot-amounts.test.ts

@@ -0,0 +1,96 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { describe, it, expect } from 'vitest';
+import { declaredRow, makeFlowHarness, type Rec } from './helpers/flow-harness';
+import { ForecastSnapshotFlow } from '../src/sales/flows/forecast-snapshot.flow';
+import forecastDerive from '../src/sales/objects/forecast.hook';
+
+/**
+ * `forecast_snapshot`'s four bucket accumulators are CEL value envelopes
+ * (#1984), run here through the REAL `AutomationEngine`: each one is an
+ * `assignment` inside a `loop` body inside the owner loop's `try_catch`, so
+ * this is also the proof that a value envelope in that position is evaluated
+ * at all rather than written into the variable verbatim.
+ *
+ * Two operand shapes the old `{… * 1}` template absorbed and CEL types
+ * instead:
+ *
+ *  - a NULL amount. `double(null)` errors, so an unguarded accumulator would
+ *    throw, the owner's `try_catch` would swallow the iteration, and the row
+ *    would keep whatever it held before. That is why the row here starts with
+ *    STALE amounts: a sweep that died on the null deal leaves them in place,
+ *    one that summed it as 0 overwrites them. A freshly opened row could not
+ *    tell the two apart — it is born with zeros either way.
+ *  - a DECIMAL amount. The cents must survive the sum. 1,000.25 and 0.5 are
+ *    exact in binary, so the expected totals carry no floating-point tail.
+ *
+ * (An ABSENT key — the sparse-driver shape — is not reachable here: the harness
+ * materialises every declared column as `null`. The guard is `has()`-first,
+ * which reads absent and null alike.)
+ */
+
+const pad = (n: number) => String(n).padStart(2, '0');
+const isoUtc = (d: Date) => `${d.getUTCFullYear()}-${pad(d.getUTCMonth() + 1)}-${pad(d.getUTCDate())}`;
+const nowUtc = new Date();
+const qStart = new Date(Date.UTC(nowUtc.getUTCFullYear(), Math.floor(nowUtc.getUTCMonth() / 3) * 3, 1));
+const qEnd = new Date(Date.UTC(qStart.getUTCFullYear(), qStart.getUTCMonth() + 3, 0));
+const inPeriod = isoUtc(qStart);
+
+const STALE = 9_999_999;
+
+const opp = (id: string, over: Rec): Rec => ({
+  id, owner_id: 'rep1', close_date: inPeriod, ...over,
+});
+
+const sweep = async (opps: Rec[]) => {
+  const h = makeFlowHarness(
+    { forecast_snapshot: ForecastSnapshotFlow },
+    {
+      sys_user: [{ id: 'rep1', name: 'Rep One' }],
+      crm_opportunity: opps,
+      crm_forecast: [declaredRow('crm_forecast', {
+        id: 'f_rep1', owner_id: 'rep1', period: 'quarter',
+        period_start: inPeriod, period_end: isoUtc(qEnd),
+        snapshot_date: '2026-01-02', source: 'scheduled',
+        pipeline_amount: STALE, best_case_amount: STALE,
+        commit_amount: STALE, closed_amount: STALE,
+      })],
+    },
+    { hooks: [forecastDerive] },
+  );
+  await h.run('forecast_snapshot', {}, { event: 'schedule' });
+  expect(h.store.crm_forecast, 'the sweep opened a second row').toHaveLength(1);
+  return h.store.crm_forecast[0];
+};
+
+describe('forecast_snapshot — CEL accumulators (#1984)', () => {
+  it('sums a null amount as 0 instead of failing the owner\'s sweep', async () => {
+    const row = await sweep([
+      opp('o1', { stage: 'negotiation', forecast_category: 'commit', amount: null }),
+      opp('o2', { stage: 'negotiation', forecast_category: 'commit', amount: 30_000 }),
+      opp('o3', { stage: 'closed_won', forecast_category: 'closed', amount: null }),
+    ]);
+
+    // Every bucket was rewritten — including the two whose only or first deal
+    // has no amount — and the snapshot was restamped, so the write ran.
+    expect(row.pipeline_amount).toBe(30_000);
+    expect(row.best_case_amount).toBe(30_000);
+    expect(row.commit_amount).toBe(30_000);
+    expect(row.closed_amount).toBe(0);
+    expect(row.snapshot_date).toBe(isoUtc(nowUtc));
+  });
+
+  it('keeps the decimals of a non-integer amount', async () => {
+    const row = await sweep([
+      opp('o1', { stage: 'qualification', forecast_category: 'pipeline', amount: 1_000.25 }),
+      opp('o2', { stage: 'negotiation', forecast_category: 'commit', amount: 0.5 }),
+      opp('o3', { stage: 'closed_won', forecast_category: 'closed', amount: 70_000.75 }),
+    ]);
+
+    expect(row.pipeline_amount).toBe(1_000.75);
+    expect(row.best_case_amount).toBe(0.5);
+    expect(row.commit_amount).toBe(0.5);
+    expect(row.closed_amount).toBe(70_000.75);
+    expect(typeof row.pipeline_amount, 'the accumulator produced a non-number').toBe('number');
+  });
+});