Skip to content

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

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-1984-flow-values-cel
Oct 2, 2026
Merged

hotlong merged 2 commits into
mainfrom
claude/issue-1984-flow-values-cel

Conversation

@hotlong

@hotlong hotlong commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Fixes #1984

Description

Moves the six flow value expressions that objectstack validate --strict flags as {…} template dialect to CEL value envelopes: quote_generation → create_quote fields.discount_amount / fields.total_price, and the four forecast_snapshot bucket accumulators (add_pipeline / add_best_case / add_commit / add_won, all from sumBucket()). User-visible change: none.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Code refactoring

Related Issues

Fixes #1984
Related to #1983 (left these six on purpose)

Changes Made

  • Quote: round(double(oppRecord.amount) * (DISCOUNT / 100.0) * 100.0) / 100.0 and the 1.0 - … analogue. round() returns an int in CEL, so the divisor is the decimal 100.0 (/ 100 would integer-divide and drop the cents). DISCOUNT is one spliced fragment, (!has(vars.discount) || isBlank(vars.discount) ? 0.0 : double(vars.discount)): the template read a cleared discount (null / "") as 0, and a bare double(null) errors, which would have failed the quote. Authored with expression(src, 'cel') (precedent: event_attendee.object.ts) because the P tag JSON-quotes an interpolated string and cannot splice a fragment.
  • Forecast: TOTAL + (!has(ITEM.amount) || isBlank(ITEM.amount) ? 0.0 : double(ITEM.amount)), again via expression() because the source splices the node's variable names. The guard keeps a null/absent amount at 0 instead of failing the owner's sweep; double() coerces a string amount.
  • Comments beside both sites rewritten for the CEL form; the * 1 rationale above sumBucket and in the file header no longer describes the template path.
  • Changeset: patch.

Testing

  • New tests added: test/flow-quote.test.ts (new describe — 1,234.56 at 10% → 123.46 / 1,111.10; cleared discount null and "" → 0 / full price) and test/forecast-snapshot-amounts.test.ts (real AutomationEngine via flow-harness: a null amount sums as 0 and the stale row is still overwritten; decimal amounts keep their decimals).
  • Existing pins untouched and green, including 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's 30% / 70% of 180,000 and the string-amount forecast case.
  • pnpm verify exit 0 at 151392a4 (174 files, 3728 passed, 1 skipped; hygiene:tokens clean on main's ceilings — src/revenue business semantics ~15,665 / 16,000, src/sales ~54,323 / 55,000).
  • npx objectstack validate --strict: 14 warnings on base 39ba05e2 → 8 on this branch; the 6 removed are exactly the six sites, the 8 left are byte-identical to base (hierarchy-security capability, page:card description, six position-routed approval nodes).
  • e2e not run (starts a server; flow behaviour is covered through the real engine above).

Ablations (each from the committed state, restored with git checkout HEAD -- PATH, restoration proved by HEAD blob hash equality and empty git diff HEAD):

mutation on-disk proof result
quote divisors / 100.0 → / 100 decimal count 2 → 0, int count 0 → 2 keeps the cents red (1 failed / 5 passed)
forecast guard → bare double(ITEM.amount) guard count 1 → 0 sums a null amount as 0 red: expected 9999999 to be 30000
DISCOUNT → bare double(vars.discount) guard count 1 → 0 both cleared-discount cases red (2 failed / 4 passed)

Behaviour parity probe (same harness, template on base vs CEL on branch, identical outputs): 1,234.56@10 → 123.46 / 1,111.1; discount null / "" → 0 / 180,000; discount "30" → 54,000 / 126,000; 99.99@33 → 33 / 66.99; forecast with null + 1,000.25 + 0.1 + 0.2 → 1,000.5500000000001 in both dialects, and a null-only owner → 0.

Checklist

  • I have added a changeset (.changeset/flow-values-cel.md, patch)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings (strict: 14 → 8)
  • I have added tests that prove my fix is effective
  • New and existing unit tests pass locally with my changes

Additional Notes

Acceptance notes

  • Beyond the issue table: the quote's discount guard. The issue named the null-amount hazard for the forecast only; measuring the template baseline showed a cleared quote discount (null / "") also priced as 0%, and an unguarded double(discount) errors on it. Guarded so Acceptance §2 (pricing unchanged) holds; pinned by a test.
  • oppRecord.amount is not guarded: crm_opportunity.amount is required + notNull, so a null is unreachable, and a missing oppRecord already fails create_quote on the required crm_account (comment on edges e4a/e4b).
  • An ABSENT amount key (sparse driver) is not reachable through flow-harness, which materialises declared columns as null. Measured directly against ExpressionEngine (17.6.0) with the celScope shape: the guard returns 0 for null, absent and "".
  • The accumulator's initial value from reset_totals (0) adds to a CEL double without an overload error — measured through the real engine by the tests above.
  • test/flow-scheduled.test.ts keeps a fixture comment saying the string-amount case is caught by "the * 1 coercion in the accumulator". Left untouched because Acceptance §3 asks for the existing forecast tests to pass untouched; its assertion still holds (CEL double() coerces). A one-line wording follow-up if wanted. Carrier: none.

Generated by Claude Code

claude added 2 commits October 2, 2026 21:17
… envelopes

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WXp8E7s1gzSBje9ZwwqVux
@vercel

vercel Bot commented Oct 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
hotcrm Ignored Ignored Oct 2, 2026 9:22pm UTC

Request Review

@hotlong hotlong self-assigned this Oct 2, 2026
@github-actions github-actions Bot added ci/cd CI plumbing and the verification pipeline backend Server-side behaviour — hooks, flows, actions labels Oct 2, 2026
@hotlong
hotlong marked this pull request as ready for review October 2, 2026 21:25
@hotlong
hotlong added this pull request to the merge queue Oct 2, 2026
Merged via the queue into main with commit cf422b8 Oct 2, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Server-side behaviour — hooks, flows, actions ci/cd CI plumbing and the verification pipeline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants