Skip to content

Move the six flow value expressions os validate --strict flags from the {…} template dialect to CEL envelopes, with tests #1984

Description

@hotlong

Follow-up to #1983, which left these six findings on purpose: os validate --strict on main still reports six {…} template-dialect expressions in flow value slots. #1983's changeset says why they were not converted there: "Moving the arithmetic to CEL changes how it divides, so that is its own change with its own tests." This card is that change.

What the lint says (17.6.0, measured on main @ 39ba05e)

… is a {…} template-dialect expression. This slot also accepts a CEL value envelope — { dialect: 'cel', source: '…' } — evaluated by the engine flow conditions use, with the whole CEL stdlib; the template form keeps working unchanged. When moving arithmetic to CEL, give a division a decimal operand: CEL divides two integers as integers, so round(x * 100) / 100 drops the decimals there — write round(x * 100) / 100.0.

The six sites:

Flow Node Slot Today
quote_generation create_quote (create_record) config.fields.discount_amount {round(oppRecord.amount * (discount / 100) * 100) / 100}
quote_generation create_quote (create_record) config.fields.total_price {round(oppRecord.amount * (1 - discount / 100) * 100) / 100}
forecast_snapshot add_pipeline (assignment, in loop sum_pipeline) config.assignments.pipelineTotal {pipelineTotal + current_pipeline.amount * 1}
forecast_snapshot add_best_case config.assignments.bestCaseTotal {bestCaseTotal + current_best_case.amount * 1}
forecast_snapshot add_commit config.assignments.commitTotal {commitTotal + current_commit.amount * 1}
forecast_snapshot add_won config.assignments.closedTotal {closedTotal + current_won.amount * 1}

Sources: src/revenue/flows/quote-generation.flow.ts (the two fields lines) and src/sales/flows/forecast-snapshot.flow.ts (sumBucket(); the four sites come from one template).

Why

  • CEL is the value dialect the platform declares for these slots (FlowValueSlotSchema, the value role). The {…} template form is the legacy path, which the lint flags under --strict.
  • The template path's arithmetic has a documented trap that the forecast code already works around: "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." (comment above sumBucket). CEL types the operands instead.

Measured semantics the conversion must preserve (PM probe, ExpressionEngine from @objectstack/formula 17.6.0, scope built the way AutomationEngine.celScope builds it)

These are mechanism assumptions for the dev to re-measure, not a spec:

  • round(oppRecord.amount * (discount / 100) * 100) / 100 with amount: 180000, discount: 30 → 54000. double(round(double(oppRecord.amount) * (double(discount) / 100.0) * 100.0)) / 100.0 → 54000; the total-price analogue → 126000. Both match test/flow-quote.test.ts's existing pins (54000 / 126000).
  • double(x) accepts a number and a numeric string ("90000" → 90000).
  • ⚠️ double(null) errors (found no matching overload for 'double(null)'). The template's * 1 maps a null amount to 0, so a naive conversion would turn a null amount into a failed forecast sweep. A guard such as (has(x.amount) && x.amount != null ? double(x.amount) : 0.0) evaluated to 0 in the probe.

Acceptance

  1. All six sites use { dialect: 'cel', source: '…' } envelopes (the repo writes these with the P tag from @objectstack/spec). os validate --strict drops the six template-dialect findings: 14 → 8, with the other 8 unchanged.
  2. Quote pricing is unchanged. The existing test/flow-quote.test.ts pins pass untouched, including the whole-cent rounding cases (Generate Quote fails for most non-zero discounts: the flow writes raw IEEE-754 products into two 2-decimal money fields, and the rejection never reaches the user #1206: 30% and 70% of 180,000).
  3. Forecast totals are unchanged. Existing forecast tests pass untouched, and new tests pin, through the real AutomationEngine (the test/helpers/flow-harness.ts path):
    • a bucket whose opportunity has a null / absent amount sums as 0, and the sweep does not fail;
    • a non-integer amount keeps its decimals, so CEL does not truncate;
    • the integer-division trap is closed: a test that fails if the divisor in the quote expressions is an integer literal.
  4. Update the comments beside both sites, in particular the * 1 rationale above sumBucket, to describe the CEL form. They must no longer describe the template path.
  5. Changeset (patch), written for the release-notes reader. The expected user-visible change is none.

Out of scope


Generated by Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions