Skip to content

fix(objectql): apply a formula field's declared scale where the value is produced - #10515

Merged
os-elon merged 4 commits into
mainfrom
claude/issue-10280-formula-scale-at-producer
Aug 21, 2026
Merged

fix(objectql): apply a formula field's declared scale where the value is produced#10515
os-elon merged 4 commits into
mainfrom
claude/issue-10280-formula-scale-at-producer

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#10280

Field.formula({ scale: 2 }) was accepted and then ignored. applyFormulaPlan
assigned the evaluator's raw double with no scale read anywhere on the path, so
HotCRM's crm_campaign.response_ratereturned41.666666666666664 for
num_sent: 12 / num_responses: 5 and the record page printed all fifteen digits.
scale is the only declarative rounding a formula author has; inert, it made every
division-shaped formula a coin flip on how it prints.

The fix — rounding at the producer

packages/objectql/src/engine.ts

  • FormulaPlanEntry gains scale?: number, read in planFormulaProjection by
    formulaRoundingScale(def).
  • applyFormulaPlan passes the evaluated value through roundFormulaValue
    before assigning it.

planFormulaProjection feeds all three plan-entry call sites, so one producer
covers the whole surface: find, findOne, and hydrateWriteFormulas (the write
response, itself reached from both the single-record and the batch write paths). A
fix that rounded on read but not on the write response would have shipped the
defect on half the surface; each of the three is asserted separately below.

⚠️ One clause of the issue is FALSE — a formula value is never stored

The card's title says "stores and returns". Only "returns" is true. A formula field
has no SQL column (driver-sql's fieldHasColumn returns false for formula;
sql-driver refuses it as a cross-field referent — "virtual, no column"), and
driver-memory returns shallow copies, so applyFormulaPlan's in-place mutation
cannot reach any store. The DECIMAL(10,2) hazard the card describes is real but
strictly downstream — an app copying the formula result into a stored money
field — and producer-side rounding is exactly what makes that copy writable. The
changeset is worded accordingly and claims no persistence path.

Rounding semantics

Number(v.toFixed(scale)), matching objectui's computeRow (GridField.tsx) so a
grid's client-side computed column and the engine's server-side formula round the
same way. Per ECMA-262 toFixed extracts the sign first and then picks the larger
n on a tie — round-half-away from zero on the magnitude. Measured on this
container rather than assumed, because the opposite (half-to-even) was believed at
dispatch time and is false:

expressionresulthalf-even would give
(2.5).toFixed(0)32
(1.25).toFixed(1)1.31.2
(0.125).toFixed(2)0.130.12
(-2.5).toFixed(0)-3-2

⚠️Negatives round away from zero (-1.5-2), not toward +Infinity.

Edge cases, each pinned by a test

casebehaviourwhy
non-number result (string / boolean / null)returned untouched'hello'.toFixed does not exist — without the typeof guard one scale-declaring string formula throws and takes down every read of that object (verified by ablation, below)
NaN / Infinityreturned untouched, still numberNaN.toFixed(2) yields the STRING 'NaN'. Both round-trip unchanged through Number(...), so the Number.isFinite guard alters no value; it is kept so the helper is total by construction rather than by a coincidence of two library behaviours
-0 (e.g. -0.001 at scale: 2)normalized to +0Number((-0.001).toFixed(2)) is -0. It JSON-serializes as 0, but new Intl.NumberFormat('en-US', { minimumFractionDigits: 2 }).format(-0) is "-0.00" — a brand-new display wart on the very record page this card is about. One comparison at the producer beats a rule in each renderer
magnitude at or above 1e21unchangedtoFixed returns exponential form ('1e+21') which Number() round-trips losslessly — degrades to a no-op instead of corrupting the value
scale above 100rounding skipped, value returned wholetoFixed throws RangeError past 100 fraction digits and FieldSchema.scale declares no upper bound. A display-precision declaration must never be the reason a read fails, and asking for more decimals than a double carries is a no-op request anyway
malformed scale (2.5, -1)left unenforcedthe same Number.isInteger(s) && s >= 0 test the write path uses, so one malformed declaration cannot mean two things one layer apart. The producer already refuses it (z.number().int().min(0))

⛔ Why NOT record-validator.ts — and the control that proves it

validateFieldValue's scale branch is the package's only other .scale reader
and therefore the tempting seam. It is the wrong one: that branch enforces scale
by rejection (max_scale) for caller-supplied values under the maintainer's
2026-08-11 ruling — "scale — enforced by REJECTION, never rounding" — because a
value someone sent has an author to refuse, and rounding it would be the silent
data alteration the ruling forbids. A formula result is platform-computed: there is
nobody to refuse.

Because plan entries are type === 'formula'only, rounding inside
applyFormulaPlan leaves the #7501 path structurally untouched — and nothing was
carved out of #7501 to make it work: the type door at the end of validateOne
already excludes formula / summary / autonumber outputs, so this fills a hole
#7501 never covered. A comment at that branch records the boundary so the next
reader is not tempted the same way.

The three controls

All three run through the protocol surface REST calls (createData / updateData
/ createManyData / insertManyData / getData / find), not by unit-calling a
private helper. New file: packages/objectql/src/engine-formula-scale.test.ts
(24 tests).

1. Defect control — fails on origin/main

Run first on the unmodified tree (f094214b3), all 24 tests present:
13 failed | 11 passed (24).

FAIL src/engine-formula-scale.test.ts > #10280 CONTROL 1 (defect) — a formula
applies its declared `scale` > the card's own repro: 5 of 12 returns 41.67,
not 41.666666666666664
AssertionError: expected 41.666666666666664 to be 41.67 // Object.is equality
- Expected
+ Received
- 41.67
+ 41.666666666666664

After the fix: 24 passed (24). The card's own expression, evaluated by the real
ExpressionEngine, now returns 41.67 on every one of the three call sites.

2. No-scale control — full precision, unchanged

response_rate_raw declares the same expression on the same record with no
scale. It asserts 41.666666666666664 exactly, on both the write response and
the read path. It passed before the fix and passes after — which is the whole point:
without it, the change could quietly round every formula in the platform and nothing
would say so.

Blast-radius check backing it: a repo-wide scan for a formula declaration that
also declares scale finds no existing one outside the new test file (the two
Field.formula fields in examples/ declare no scale; the nearby scale: 2 in
those files is on neighbouring currency fields). The change is a no-op for every
formula that exists today.

3. ⭐ Discriminating control — Field.number({ scale }) still REJECTS

fs_campaign.plain_rate is a plain number carrying the same scale: 2, on the
same object as the rounded formula. Writing 41.666666666666664 into it is refused,
with the envelope rather than a bare throw — code: 'VALIDATION_FAILED', field
entry { field: 'plain_rate', code: 'max_scale', constraint: { scale: 2, actual: 15 } },
recognised by the platform's own validationFailureDetails (the status side: an
objectql ValidationError deliberately carries no .status, so the test reads the
recogniser and VALIDATION_FAILED_STATUS instead of re-spelling 400) — and
nothing is written.

It can fail, and here is the run where it did. Ablation: the tempting wrong seam
implemented — the validator's scale branch stops refusing and rounds the caller's
value in place instead:

FAIL ... CONTROL 3 ⭐ (discriminating) > an over-scale caller-supplied number is
refused, not silently rounded
AssertionError: promise resolved "{ object: 'fs_campaign', …(2) }" instead of rejecting
+ "record": {
+ "plain_rate": 41.67, ← the caller's number, silently altered
+ "response_rate": 41.67,
+ "response_rate_raw": 41.666666666666664,
+ },

3 failed | 21 passed (24) — all three CONTROL 3 tests red, CONTROL 1 and CONTROL 2
untouched. That is the discrimination: the two paths are independent, and the control
is red exactly when the rounding is put in the wrong place. Ablation reverted with
git checkout HEAD -- … (the fix was committed first); restore leg re-run: 24 passed (24).

Second ablation — the typeof guard is load-bearing

Guard removed from roundFormulaValue: 8 failed | 16 passed (24), and the engine
log shows the mechanism rather than an assertion diff:

ERROR Insert operation failed {"object":"fs_edge","error":{"message":"value.toFixed is not a function",
"stack":"TypeError: value.toFixed is not a function
at roundFormulaValue (packages/objectql/src/engine.ts:1317:32)
at applyFormulaPlan (packages/objectql/src/engine.ts:1335:29)
at hydrateWriteFormulas (packages/objectql/src/engine.ts:1388:3) …

One scale-declaring string formula would have taken down every read and every write
of that object. Guard restored; restore leg green.

Verification

Both ablations state their restore leg. No dist resolution is involved in either:
the test imports ./engine.js and the validator relatively, from src, so vitest
compiles the mutated source directly — the only cross-package imports
(@objectstack/metadata-protocol, @objectstack/formula, @objectstack/types)
were untouched by both ablations and were rebuilt before the runs regardless.

pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 # on the MERGED tree
Test Files 225 passed (225)
Tests 3969 passed (3969)
pnpm --filter @objectstack/objectql typecheck → tsc --noEmit, exit 0
(script name echoed, so not a zero-match filter)
pnpm --filter @objectstack/rest exec vitest run src/rest-write-response-formula.test.ts
Test Files 1 passed (1) (run against a rebuilt @objectstack/objectql dist)

Also proved, because the fix is worthless if the authored declaration never reaches
def.scale: Field.formula({ …, scale: 2 })FieldSchema.parse keeps
scale = 2 on a type: 'formula' field.

Gates

Set re-derived after the final commit with node scripts/pm/dispatch-gates.mjs
(no path arguments — the script takes the merge-base change set itself), which added
five convention-triggered gates the dispatch list did not name, all because this PR
adds a test file: check:query-options-erasure, check:type-check-coverage,
check:type-check-debt, check:engine-double-contract, check:where-matcher.

All run at 3929e68d0, on the merged tree.

gateexitverdict line it printed
pnpm check:changeset-gate-self-tests0✓ check-changeset-no-major --self-test: 116 assertions … (plus the 118- and 212-assertion self-tests)
pnpm check:cross-package-test-inputs0OK: 12 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
pnpm check:durability-log-level0✓ durability-degradation log levels: 30 durability-critical catch seam(s), all loud, rethrowing or propagating to the caller (5 propagating, declared).
pnpm check:error-code-casing0✓ no lowercase error codes in 4328 scanned file(s) (ADR-0112).
pnpm check:objectui-changeset0✓ objectui-range --self-test: all checks passed
pnpm check:slot-lookup0✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new … baseline key set verified against 5c3faa7: no files added.
pnpm check:stack-collection-maps0✓ check-stack-collection-maps: 7 enumerations reconciled against 32 declared collections (16 waiver rows, each with a reason).
pnpm check:nul-bytes0check-nul-bytes: OK (scanned 6145 text file(s) …; no raw ASCII control bytes).
pnpm check:query-options-erasure0✓ query-options-erasure ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new … baseline key set verified against 5c3faa7: no files added.
pnpm check:engine-double-contract0check-engine-double-contract: OK — 340 pinned, 133 in the DEBT ledger, 2 exempt.
pnpm check:where-matcher0✓ where-matcher conformance holds: 267 matcher(s) discovered, 267 answer the combinator battery correctly or refuse it loudly (161 refuse). 0 silently-wrong …
pnpm check:type-check-coverage0check-type-check-coverage: OK — 64/77 workspace packages type-checked (plus the root), 13 in the DEBT ledger …
pnpm check:type-check-debt0--re-measure: OK — 33 ledger entr(ies) re-measured in 535.9s, 1924 raw tsc error(s) total, none above its recorded number.red first, see below
node scripts/check-adr-0087-registration.mjs0✓ check-adr-0087-registration: this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen).
node scripts/check-changeset-no-major.mjs0✓ This diff introduces no major bump.
node scripts/check-cross-package-test-inputs.mjs0OK: 12 package(s) read outside themselves, all declared …
node scripts/check-empty-changeset.mjs0✓ No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added).
node scripts/docs-audit/check-affected-docs.mjs0✓ affected-docs self-test: 262 cases pass.

node scripts/check-engine-split-ratio.mjs is report-only (always exits 0; the
ADR-0076 OQ#5 threshold is deliberately unset). It reports engine-core commits 30 · also cross-package 28 · ratio 93.3% — unchanged by this PR's arithmetic in any way
a threshold could read.

check:type-check-debt was RED on the first run, and it was mine. The new test
file contributed one raw tsc error to @objectstack/objectql's TEST_DEBT ledger:

• @objectstack/objectql: TEST_DEBT records 355 raw tsc error(s), `tsc --noEmit`
now reports 356 (+1).
src/engine-formula-scale.test.ts(270,55): error TS2554: Expected 2-5 arguments, but got 1.

Fixed at the author's end, which is the ratchet's own prescription — the call now
passes the required packageId (registerObject(obj as never, 'test'), the
spelling every other test in this package uses). ⛔ The ledger was not raised.
Re-measured: 355, with zero errors attributed to the new file.

Serialization against PR #10455 (issue #10172), re-checked at push time

Condition from the dispatch, discharged rather than assumed. git merge origin/main
merged clean (no conflicts, no deferred regeneration). #10455 re-read at push time:
still 1 commit, 1 file, +13/−3, head b54f1a44d, still exactly one hunk
@@ -1357,9 +1357,19 @@ export interface HookEntry { — additive doc prose inside
the HookEntry docblock. It has not grown into applyFormulaPlan's region.
This PR's edits sit at FormulaPlanEntry / planFormulaProjection and
roundFormulaValue / applyFormulaPlan, all above it, in different declarations,
with zero textual overlap.

One observation, recorded and not acted on: #10455 now reads draft: false,
where the dispatch described it as a draft. State on someone else's PR that I did not
set belongs to another actor — noted for the PM, not "corrected".

Deliberately not done


Generated by Claude Code

…ue is produced
`Field.formula({ scale: 2 })` was inert: `applyFormulaPlan` assigned the
evaluator's raw double, so a percentage formula returned
`41.666666666666664` and every consumer — including the record page —
inherited all fifteen digits.
`scale` now reaches `applyFormulaPlan` on the plan entry
`planFormulaProjection` builds, and the value is rounded with
`Number(v.toFixed(scale))` (round-half-away-from-zero, matching objectui's
`computeRow`). All three plan-entry call sites — `find`, `findOne`, and the
write-response hydration — inherit it from the one producer.
The rounding is deliberately NOT in `validateFieldValue`'s `scale` branch:
that branch enforces `scale` by REJECTION for caller-supplied values
(#7501, maintainer ruling 2026-08-11), and rounding there would convert a
refusal into the silent data alteration the ruling forbids. A comment at
that branch records the boundary.
Fixes#10280
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
…the scale suite
`check:type-check-debt --re-measure` caught the new test file adding one raw
tsc error to @objectstack/objectql's TEST_DEBT ledger (355 -> 356):
src/engine-formula-scale.test.ts(270,55): error TS2554: Expected 2-5
arguments, but got 1.
Fixed at the author's end, per the ratchet's own prescription — the ledger is
shrink-only and was NOT raised. Re-measured at 355 with zero errors from this
file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

7 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 14 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 99f1f91b06a233d70e873a83025420c16cf83b62packageMentionDocs.

Which tree this was computed on

This run read content/docs from a0b898aa6cdf3ab138bb3d9e886618824706422d — the merge of head 3929e68d0e5dc6588eca784d7a3d6d075aac18fe into base 99f1f91b06a233d70e873a83025420c16cf83b62, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a0b898aa6cdf3ab138bb3d9e886618824706422d && git checkout a0b898aa6cdf3ab138bb3d9e886618824706422d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 99f1f91b06a233d70e873a83025420c16cf83b62 3929e68d0e5dc6588eca784d7a3d6d075aac18fe && git checkout -B drift-repro 99f1f91b06a233d70e873a83025420c16cf83b62 && git merge --no-ff 3929e68d0e5dc6588eca784d7a3d6d075aac18fe
node scripts/docs-audit/affected-docs.mjs --json 99f1f91b06a233d70e873a83025420c16cf83b62

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 21, 2026
@os-elon
os-elon marked this pull request as ready for review August 21, 2026 01:54
@os-elon
os-elon added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit f3a8134Aug 21, 2026
32 checks passed
@os-elon
os-elon deleted the claude/issue-10280-formula-scale-at-producer branch August 21, 2026 02:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Field.formula({ scale: 2 }) is not applied: a percentage formula stores and returns 41.666666666666664, and the record page prints all 15 digits

2 participants

@os-elon@claude