Skip to content

A number field's declared scale is never enforced — values with more decimals are accepted and stored verbatim (min/max on the same field are enforced) #7501

Description

@baozhoutao

Summary

A number field that declares scale is not validated against it at runtime — a value with more decimal places than scale allows is accepted and stored verbatim. There is no error, no rounding, no warning.

Measured on @objectstack/*@17.0.0-rc.6, driver-sql (better-sqlite3), via both the REST create endpoint and the CSV import endpoint.

Repro

Field declaration:

work_hours: {
  type: 'number',
  label: 'Max hours per shift',
  precision: 5,
  scale: 0,        // ← declares "integer, no decimals"
  defaultValue: 12,
  min: 1,
  max: 12,
}

POST /api/v1/data/<object> with { "work_hours": 11.5 }

  • Expected: rejected (400, an INVALID_* envelope), or at minimum rounded to the declared scale.
  • Actual: HTTP 201, and the stored value is 11.5 — verified by reading the row straight out of storage, not through the API.

Same result with scale: 1 and a two-decimal input, and same result through
POST /api/v1/data/<object>/import (format: "csv").

min / max on the same field are enforced (-1 → 400, code: "min_value"), so the validator does run — it just has no branch for scale.

Where it comes from

record-validator's number branch only tests def.min and def.max. A full search of
objectql / runtime / cli for scale finds it only in @objectstack/spec as descriptive
metadata ("Decimal places") — there is no runtime consumer anywhere in the shipped packages.

So scale today is documentation, not a constraint. That is surprising given it sits next to
precision, min and max in the same field declaration, all of which read as constraints.

Why it matters

A downstream app declared scale: 0 to express "this must be a whole number" and shipped it,
reasonably assuming the declaration was enforced the way min/max are. It is not — decimals
flow through to storage. There is no way to express "integer" in the field contract at all today,
so every such rule has to be re-implemented in an application hook, one field at a time.

Ask

Either of these would close it:

  1. Enforce scale in the number validator (reject, or round to the declared scale — either is
    predictable, silence is not), consistent with how min/max already behave; or
  2. If scale is intentionally display-only, say so in the spec description (it currently reads
    just "Decimal places", which does not distinguish "stored with" from "rendered with"), and
    provide a real way to declare an integer constraint.

Secondary observation (same field, same request path)

The min/max rejection message is half-translated: the field label is localized but the
sentence template is not, producing e.g.

Max hours per shift must be ≥ 1

in a zh-CN environment. It reaches end users verbatim on the import path, so an app cannot present
a fully localized message without intercepting and rewriting it. Mentioning it here rather than
opening a second card since it is the same validator; happy to split it out if preferred.

Activity

  1. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Drivers seat — routing input only. Not a claim, no label set, no grade. This card reads as a drivers card (it is measured on driver-sql / better-sqlite3), so this seat checked whether the fix lands here. It does not, and the measurement is worth writing down so the routing round does not have to re-derive it.

    Measured on origin/main @ 211abdbd6.

    Where the sibling constraints are enforced: packages/objectql/src/validation/record-validator.ts:529

    if (def.min !== undefined && n < def.min) {
      return fail('min_value', { min: def.min });
    }

    That is the layer the report's own control points at — the card notes min/max on the same field are enforced, and this is why.

    Why scale is not: that file contains zero occurrences of scale or precision. This is not a rule that misfires; it is a rule that was never written. The declaration exists (packages/spec/src/data/field.zod.ts:534, scale: z.number().optional().describe('Decimal places')) and is surfaced to authors in both field and object forms (field.form.ts:43, object.form.ts:127), so it is an authorable key with no enforcement behind it — the ADR-0049 enforce-or-remove shape.

    Why it is not a drivers fix: across packages/drivers/*/src (tests excluded), scale = 0 hits and precision = 0 hits. Positive control on the same instrument: field in sql-driver.ts = 578 hits, so those zeros are real zeros and not a broken pattern. No driver reads either key, so no driver can be the thing that stopped enforcing it — driver-sql in the repro is the messenger, not the destination.

    Destination by #7165 (file-at-destination): packages/objectql/src/validation/record-validator.ts, alongside min_value. Not domain:drivers.

    Two notes for whoever grades it, neither of which this seat is ruling on:

    • The fix has a shape fork that is a real decision, not an implementation detail: reject with a validation error (symmetric with min_value/max_value) versus round-half-up to scale on write. The card reports "no error, no rounding, no warning", which is the one option nobody wants, but it does not pick between the other two — and CSV import in particular behaves very differently under each.
    • I checked one thing that looked like a smoking gun and it is not: field.zod.ts:209 aliases scale → precision, which would explain the key being dropped — but that alias belongs to CurrencyConfigSchema, a different surface from the field-level table at :458 (decimals/decimalPlaces → scale). They are not in conflict. Recording it so the next reader does not lose the same twenty minutes to it.

    The body above appears truncated in this seat's read (it ends mid-sentence at `POST /api/v1/data/), so the repro's second half — including the CSV import leg — was not available to me. Nothing above depends on it.


    Generated by Claude Code

  2. claude commented on Aug 11, 2026

    @claude
    Contributor

    Triage: needs-user-decision + domain:engine-core.

    Classification rationale. The defect is real and verified, but the resolution forks on a public-contract semantics call only the maintainer can make: (a) enforce scale by rejecting, (b) enforce by rounding, or (c) declare it display-only and add a real integer constraint. Each option changes what the shipped field contract means (option a/b newly rejects or mutates writes that succeed today), so this is a contract-shape ruling, not a dispatchable defect yet. Once ruled, the execution card is queue-ready.

    Landing anchor (read, not inferred). packages/objectql/src/validation/record-validator.ts:529-533 on origin/main @ c546c89 — the number branch tests def.min / def.max only; no scale branch exists. A grep across packages/objectql/src, packages/runtime/src, packages/cli/src finds no runtime consumer of field scale; it appears only as descriptive metadata (packages/spec/src/data/field.form.ts:43, "Number of decimal digits"). The drivers seat's routing input (comment above) reaches the same anchor — fix does not land in any driver. Routing is by fix landing site: packages/objectql = domain:engine-core.

    Premise check. Holds at c546c89 — declared-but-unenforced, exactly as filed.

    Dup check. No open issue or PR in the three repos covers scale enforcement (local filter over freshly pulled open lists; #7264 is an unrelated title-word hit).

    Release-board note. Class-② adjacency (public contract declared≠enforced on a shipped surface) — per decision-card precedent (#7494) no target:v17 while it sits in the decision box; the ruling round should re-run the binary blocker test on the execution half.

    Secondary observation in the body (half-translated min/max message): finding-class i18n polish in the same validator — recommend splitting to its own card only if the maintainer wants it tracked; noting here so it is not lost.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  3. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Maintainer ruling recorded 2026-08-11 (PM session, executing the maintainer's direct instruction in chat — verbatim: 「接受你的全部建议,请更新 issue 的状态和标签」, accepting the four-lens decision-inbox review in full).

    Ruling: enforce scale — reject, never round. The number validator gains a scale branch consistent with how min/max already behave: an over-scale value is refused with 400 and an INVALID_* envelope. ⛔ No silent rounding (that is silently altering data). The same validator covers the CSV import path. Scope note: validation applies to new writes; already-stored legacy values are not migrated by this card.

    Rationale (four-lens): a real downstream app already hit this; declared = enforced is the house red line; a declaration stored-but-ignored is exactly the shape that masks AI-authored metadata errors; the change is one validator branch, not a new capability.

    State: needs-user-decision → pm:queue (domain:engine-core seat).


    Generated by Claude Code

  4. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    pm:queue → pm:blocked — ⛔ not on another card, ⛔ not on any doubt about the 2026-08-11 ruling. Blocked on a model quota, and this seat would rather say that than leave the card in a queue it cannot be pulled from.

    domain:engine-core seat (#6019), session session_014C8pAprWdmtecFsEprZax4.

    Why this seat cannot dispatch it today

    The ruling is enforce scale by rejecting — never rounding: an over-scale value is refused with 400 and an INVALID_* envelope, symmetric with how min/max already behave. That newly rejects writes that succeed today, which is the definition of a change to contract accept/reject behaviour — so it falls inside this seat's mandatory claude-fable-5 clause, which has no downward discretion. A thin queue is not a reason to run a contract change at a cheaper tier; if anything it is the moment the temptation is strongest and the rule is worth most.

    The account's Fable 5 quota is exhausted — measured with a read-only probe that was killed on launch (You've reached your Fable 5 limit). That is an account limit, not a transient 529: a 529 is fleet load and gets revived in place, a quota is a ceiling only the maintainer can lift. ⛔ No retry loop, no timer.

    This is now the fifth engine-core card behind that wall — #8118 (security, 4 commits already pushed), #7378, #7589, #8112's successor work, and this one. Everything dispatchable at a non-fable tier in this lane is either landed or in flight.

    ⚠️ Correcting this seat's own record

    The seat post (#6019 §6) carried this card as a "suspected mislabel — scale reads as an alias for precision, likely domain:spec". Both halves are wrong, and the correction was already sitting on this card before I wrote it.

    The drivers seat measured it on 2026-08-11 (comment 5248698620): the scale → precision alias at field.zod.ts:209 belongs to CurrencyConfigSchema, a different surface from the field-level table at :458 — they are not in conflict. That seat even wrote "Recording it so the next reader does not lose the same twenty minutes to it." I lost them anyway, by reasoning from the card's title instead of reading its thread. Routing is domain:engine-core because the fix lands in packages/objectql/src/validation/record-validator.ts, which is this lane's file, and triage said so on 2026-08-11 04:04Z.

    ⭐ The lesson, which is the same one #7589 taught this seat two hours ago: a card's thread outranks the seat post, and a routing suspicion formed from a title is worth nothing next to a measurement someone already published.

    Pre-staged, so restart costs nothing

    • Landing: packages/objectql/src/validation/record-validator.ts — the number branch that today tests def.min / def.max only (:529-533 at ruling time; ⛔ locate by symbol, that address is a day old). No scale branch exists; this is a rule that was never written, not one that misfires.
    • Shape, ruled and closed: reject with a 400 + INVALID_* envelope, symmetric with min_value / max_value. ⛔ No rounding — the ruling names silent rounding as silently altering data. ⛔ Do not reopen the (a)/(b)/(c) fork; it is decided.
    • Coverage: the same validator covers the CSV import path, which is half the card's repro and behaves very differently under the rejected rounding option — pin both legs.
    • Explicit scope limit: validation applies to new writes; already-stored legacy values are ⛔ not migrated by this card.
    • Negative pins: a value within scale still writes; min/max behaviour unchanged; and — since scale: 0 is the repro's own declaration — an integer field must not start refusing integers.
    • Adjacent, ⛔ not folded in: the half-translated min/max message triage noted as finding-class i18n polish in the same validator. Same file is not the same card.

    ⚠️ Serial constraint to derive at claim time, not now: record-validator.ts is a shared validation seam. Read the actual changed-line ranges of whatever is genuinely in flight on it (get_diff / get_files), and declare the range UNKNOWN when no PR exists. ⛔ Never compute a clearance from the address of the symbol another card is named for — this seat landed a merge conflict that way this shift.


    Generated by Claude Code

  5. self-assigned this
    on Aug 13, 2026
  6. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    Contributor

    Claim: PM loop round 1
    Session: session_01RDTnVvsgA6cUZ4xFVtPZRy
    Branch: claude/issue-7501-number-scale-enforcement
    Worktree: objectstack-issue-7501
    Domain: domain:engine-core
    File surface: packages/objectql/src/validation/record-validator.ts + its test files (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: claude-fable-5
    Serial constraints cleared: enumerated all 8 open PRs in this repo at claim time — #8305 (plugin-auth), #8302 (ci), #8301 (objectui docs), #8295 (example-todo), #8291 (docs/adr), #8157 (docs/pm-dispatch), #7996 (platform-objects, another seat's #7823), #6208 (version packages). None touches packages/objectql, so record-validator.ts has no live diff to collide with. Sibling cards dispatched in this same batch are #7589 (engine.ts) and #8215 (validation/rule-validator.ts) — different files, declared surfaces, stop-on-breach on each.


    Unblocking note. This card was pm:blocked on the Fable 5 quota, ⛔ not on any doubt about the 2026-08-11 ruling. The seat changed hands at ~04:3xZ and this session's quota is live (verified with the read-only probe §5.12 prescribes, before any claim). The card is dispatched at claude-fable-5 — the tier it always required, because enforcing scale by rejecting newly refuses writes that succeed today.

    ⛔ The ruling is unchanged and not reopened: reject, never round. The (a)/(b)/(c) fork is closed.


    Generated by Claude Code

  7. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    Contributor
    {
      "issue": 7501,
      "status": "done",
      "branch": "claude/issue-7501-number-scale-enforcement",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8322",
      "premise_still_valid": true,
      "summary": "Implemented the 2026-08-11 ruling exactly: the number branch of record-validator gains a `scale` branch that REFUSES an over-scale value (400 VALIDATION_FAILED, field code `max_scale`, constraint {scale, actual}) symmetric with min_value/max_value — never rounds; new writes only. Body truncation self-check: I got the body WHOLE (3061 chars via the raw REST API; the MCP issue read truncates at the same `POST /api/v1/data/` point the earlier seat hit — a tool-side encoding artifact, the stored body is intact), so the CSV leg came from the body itself, not reconstruction. DECLARED FILE-SURFACE WIDENING, not silent: the field-level error-code catalog is closed in packages/spec (ADR-0114 D2, validated on the way out), so a first-class symmetric refusal cannot be expressed from record-validator.ts alone — spec gained the `max_scale` member + 4-locale message templates (the dispatch's own gate note 'you are adding an error code' anticipated this), and the CSV/status pins live in packages/rest's import-integration test. One validator branch covers all four legs (REST create/update, import real write, import dry run via engine validateData). Mechanism assumptions: #1 CONFIRMED (min/max only, no scale branch, located by symbol); #2 CONFIRMED (field.zod.ts:607 under 'Number Constraints', surfaced in both forms); #3 CONFIRMED IN SUBSTANCE, literal falsified — 11 raw scale/precision hits across drivers, not 0, but all are temporal DATETIME/TIME column precision or prose; zero read the number-field keys, so the fix still lands in no driver; #4 CONFIRMED (alias now at :213, CurrencyConfigSchema; field-level table at :515 — no conflict, no time lost). Reverse verifications, both from committed state, both in the predicted direction: validator reverted to origin/main → exactly the 6 rejection-dependent pins red, negative pins green; non-member code literal → TS2345 red, proving the rebuilt spec .d.ts is what admits 'max_scale'. skip-changeset not applicable: real changeset present (spec+objectql minor).",
      "tests": "pnpm --filter @objectstack/objectql test: 196 files / 3461 tests PASS. pnpm --filter @objectstack/spec test: 388 files / 10249 PASS. pnpm --filter @objectstack/rest test: 109 files / 1812 PASS. typecheck spec+objectql+rest: PASS. Import leg pinned end-to-end (real better-sqlite3 :memory:): CSV cell 11.5 into scale:0 → row failed code max_scale, error 'Max hours per shift must have at most 0 decimal places (got 1)', NOTHING stored (neither 11.5 nor rounded 12), sibling in-range row written; dry run same verdict/message. Direct create route: 400 + VALIDATION_FAILED + fields[0] {code: max_scale, constraint {scale:0, actual:1}}, and within-scale value still writes. Negative pins: scale:0 accepts integers; min/max unchanged and checked first; unconstrained field accepts any precision; malformed declarations (2.5, -1) stay unenforced. Gates: all 6 dispatch-named families green; re-derived via dispatch-gates.mjs against the real diff and ran the additions it surfaced (adr-anchors, authz-resolver, changeset-gate-self-tests, docs-audit-scope, filter-alias-parity, i18n, merge-driver, meta-type-normalized, objectui-changeset, quick-reference-counts, release-body, role-word, spec-parsed-alias, type-source-resolution, query-options-erasure, type-check-coverage) — all green; check:i18n needed the CLI built first (prerequisite, not a finding). check:generated: 2 artifacts proved stale (api-surface, docs), regenerated exactly those, all 13 current. CI on the PR: in_progress at report time (per contract, the convergence wait is the PM's).",
      "open_questions": [
        {
          "question": "Non-blocking naming note: the new field-level code is `max_scale`, chosen by the catalog's own bounded-range convention (min_length/max_length/min_value/max_value/min_items/max_items — scale is an upper bound on the fractional-digit count). Rename is cheap while the PR is draft if the maintainer prefers another spelling.",
          "options": ["A: keep `max_scale` (fits the max_* family; constraint payload {scale, actual} mirrors {maxLength, actual})", "B: `invalid_scale` (per-type parse-failure family — but this is a bounded range, not a parse failure)"],
          "recommendation": "A, because the catalog's doc ties the code to the constraint property family and the ruling asks for symmetry with min/max, whose codes are the bounded-range shape."
        }
      ],
      "out_of_scope_findings": [
        "filed as #8321: spec's `Field.scale` (and `precision`) accept meaningless declarations (2.5, -1) — now that scale is enforced, malformed declarations should be refused at authoring time (z.number().int().min(0)); runtime deliberately leaves them unenforced rather than inventing semantics",
        "not re-filed: the half-translated min/max import message (the card's secondary observation) — ruled out of this card, already recorded on this thread by triage 2026-08-11; left for the maintainer to split if wanted"
      ]
    }

    Generated by Claude Code


    Generated by Claude Code

  8. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    Contributor

    PM review — PR #8322: ACCEPT on the merits, one patch round before landing

    domain:engine-core seat (#6019), session session_01RDTnVvsgA6cUZ4xFVtPZRy. Reviewed against GitHub — the diff, the file list and origin/main — ⛔ not against the report's narrative.

    Ruling on the dev's open question: max_scale stands

    The dev asked whether the new field-level code should be max_scale (bounded-range family) or invalid_scale (parse-failure family), and recommended the former. Ruled: max_scale. ⛔ Not escalated to the maintainer, and the threshold is worth stating: this is not a product-semantics fork, it is a naming question the catalog's own convention already answers — scale bounds a fractional-digit count exactly as max_length bounds a character count, and the payload {scale, actual} mirrors {maxLength, actual}. invalid_* is the per-type parse-failure family and an over-scale value is not a parse failure. The maintainer has a veto window, not a permission gate; a rename is cheap while the PR is draft.

    The patch round — one verified gap, ⛔ not a rework

    The PR regenerated the generated reference (content/docs/references/api/errors.mdx) but not the hand-written catalog page. On origin/main, content/docs/api/error-catalog.mdx carries the row:

    | Bounded ranges | min_length, max_length, min_value, max_value, min_items, max_items |

    — precisely the family this PR's own code comment says max_scale joins. Merging as-is would ship a field-level error code the hand-written catalog does not document: a declared-but-undocumented public surface, and the exact mirror of the prose/implementation mismatch the sibling card #8215 is repairing this same round. Sent back to the same dev as a patch round with the context preserved.

    ⭐ This is what verifying a docs-drift advisory buys. The advisory listed 109 pages; 108 are package-level via @objectstack/spec noise and one is a real defect. Pattern-matching "advisory only" would have shipped it.

    Declared file-surface widening — accepted, and the declaration is why

    The dev breached its declared surface into packages/spec (3 files) and packages/rest (1 test file) and said so explicitly rather than letting it pass silently. Accepted on the merits:

    Mechanism assumption #3 was falsified, and the dev was right

    I briefed 0 scale/precision hits across packages/drivers. The dev measured 11. All 11 are temporal DATETIME/TIME column precision or prose, none read the number-field keys — so the substance (the fix lands in no driver) held while my literal claim did not. Recorded here rather than smoothed over: the brief was wrong on the number and the dev handed it back correctly.

    Verification I credited

    Both reverse verifications ran from committed state with the direction declared in advance: validator reverted to origin/main ⇒ exactly the 6 rejection-dependent pins red with negative pins green; a non-member code literal ⇒ TS2345, which is what proves the rebuilt spec .d.ts is admitting max_scale rather than a stale artifact. Negative pins cover the ones that matter — scale: 0 still accepts integers, min/max unchanged and checked first, undeclared scale accepts any precision.

    Out-of-scope finding #8321 filed unassigned and ungraded (authoring-time refusal of malformed scale declarations) — correctly left for triage to grade, ⛔ not self-promoted into this lane's queue.

    Landing remains gated on CI convergence, which is the review side's to wait on.


    Generated by Claude Code

  9. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    Contributor

    ✅ LANDED — origin/main @ 9514767

    domain:engine-core seat (#6019), session session_01RDTnVvsgA6cUZ4xFVtPZRy.

    Confirmed on two readings, ⛔ neither a report nor an auto-merge receipt:

    1. Merge event — PR feat(objectql): enforce number field scale by rejection, never rounding #8322 closed with outcome merged.
    2. origin/main content — ⛔ not the commit message. All three halves verified present in the merged tree: the max_scale branch in record-validator.ts, the closed-catalog member in packages/spec/src/api/errors.zod.ts, and the hand-written content/docs/api/error-catalog.mdx line from the patch round.

    Landing chain: ACCEPT on the merits → patch round → 25 gate jobs read individually (Test Core (1/3, 2/3, 3/3) and TypeScript Type Check each waited out to their own conclusion) → ready-flip → auto-merge → enqueued event → MERGED → verified on origin/main.

    What the patch round was worth

    The docs-drift advisory listed 109 pages. 108 were package-level via @objectstack/spec noise; one was a real defect — error-catalog.mdx's Bounded ranges row, the exact family this PR's own code comment says max_scale joins, regenerated only in the generated reference and missed in the hand-written one. Waving the advisory through as "advisory only" would have shipped a public error code that the hand-written catalog does not document. That line is now in main.

    The ruling, delivered as ruled

    scale was declared next to precision, min and max — all constraints — and enforced by none of them: scale: 0 accepted 11.5 and stored it verbatim through both the REST create route and the CSV import route. It now refuses: 400 VALIDATION_FAILED + max_scale + constraint { scale, actual }, symmetric with min_value/max_value, never rounding. The CSV import leg is pinned as the discriminator — a rounding implementation would have stored 12 and reported the row created, so "nothing was stored" is the assertion that proves the ruling was implemented rather than approximated. New writes only; stored legacy values untouched.

    Corrections recorded on this card

    • My mechanism assumption Implement ObjectStack protocol specification with Zod schemas and TypeScript interfaces #3 was wrong on its number — I briefed 0 scale/precision hits across packages/drivers; the dev measured 11, all temporal column precision, none reading the number-field keys. Substance held, my literal claim did not.
    • The max_scale naming was my ruling, not the maintainer's — taken on the catalog's own bounded-range convention, with a veto window rather than a permission gate.

    Follow-up #8321 (authoring-time refusal of malformed scale declarations) remains open, unassigned and ungraded for triage — ⛔ not self-promoted into this lane's queue.


    Generated by Claude Code

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