From 0a1178dccba98a2101b39fd76c938cc75f2cc07f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 21:17:17 +0000 Subject: [PATCH 1/2] fix(flows): move quote pricing and forecast accumulators to CEL value envelopes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The six flow value expressions `objectstack validate --strict` flagged as `{…}` template dialect now use `{ dialect: 'cel', source }` envelopes: `quote_generation`'s `discount_amount` / `total_price` and the four `forecast_snapshot` bucket accumulators. The quote divides `round(...)` by the decimal `100.0`, since CEL divides two ints as ints. A cleared discount and a null opportunity amount are guarded with `has()` / `isBlank()` so they still read as 0, as the template did; a bare `double(null)` errors. Tests pin both, plus the decimals, through the real AutomationEngine. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01WXp8E7s1gzSBje9ZwwqVux --- src/revenue/flows/quote-generation.flow.ts | 31 ++++--- src/sales/flows/forecast-snapshot.flow.ts | 29 ++++--- test/flow-quote.test.ts | 38 +++++++++ test/forecast-snapshot-amounts.test.ts | 96 ++++++++++++++++++++++ 4 files changed, 171 insertions(+), 23 deletions(-) create mode 100644 test/forecast-snapshot-amounts.test.ts diff --git a/src/revenue/flows/quote-generation.flow.ts b/src/revenue/flows/quote-generation.flow.ts index a233a7e68..24d6b0955 100644 --- a/src/revenue/flows/quote-generation.flow.ts +++ b/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', diff --git a/src/sales/flows/forecast-snapshot.flow.ts b/src/sales/flows/forecast-snapshot.flow.ts index e7a433078..96f675977 100644 --- a/src/sales/flows/forecast-snapshot.flow.ts +++ b/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) => ({ /** * 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, total: string) => ({ +const sumBucket = (key: string, label: string, filter: Record, 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, 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: [], diff --git a/test/flow-quote.test.ts b/test/flow-quote.test.ts index 38033786f..ad2c50a08 100644 --- a/test/flow-quote.test.ts +++ b/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 0%%', async (discount) => { + const q = await quoteAt(180000, { discount }); + expect(q.discount_amount).toBe(0); + expect(q.total_price).toBe(180000); + }); +}); diff --git a/test/forecast-snapshot-amounts.test.ts b/test/forecast-snapshot-amounts.test.ts new file mode 100644 index 000000000..9776f1f3a --- /dev/null +++ b/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'); + }); +}); From 151392a4250cc57c99c433754453bc4fe86c1819 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 21:18:37 +0000 Subject: [PATCH 2/2] chore(changeset): note the CEL move of quote pricing and forecast totals Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01WXp8E7s1gzSBje9ZwwqVux --- .changeset/flow-values-cel.md | 11 +++++++++++ test/flow-quote.test.ts | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 .changeset/flow-values-cel.md diff --git a/.changeset/flow-values-cel.md b/.changeset/flow-values-cel.md new file mode 100644 index 000000000..5f6515d35 --- /dev/null +++ b/.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. diff --git a/test/flow-quote.test.ts b/test/flow-quote.test.ts index ad2c50a08..50c13df6a 100644 --- a/test/flow-quote.test.ts +++ b/test/flow-quote.test.ts @@ -152,7 +152,7 @@ describe('quote_generation flow — CEL pricing (#1984)', () => { // 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 0%%', async (discount) => { + 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);