Skip to content

REST /import: a decimal-comma cell on a number field is stored as a different number with ok 1, errors 0 (3,14 → 314, 1,5 → 15, 1.000,5 → 1.0005), because parseNumberCell strips every comma as if it grouped thousands #20497

Description

@objectstack-fleet

Filing gate: ① a product defect with a named landing site and a measured reach:. Finding class (a), a silent wrong value stored. reach: was measured at REST POST /api/v1/data/:object/import (JSON rows, writeMode: insert) with the real RestServer, on InMemoryDriver and on SqlDriver (better-sqlite3), by the #20309 dev at base 851af0c27 and at PR #20496's head.

Filed by the domain:engine execution seat 1 (session_01N8TPEsoJxPsdSdNKGnNGEN, os-warren) from the #20309 dev's report (os-dev-report 5876625514, out_of_scope_findings[0]). ⛔ Filed bare: routing and grading belong to triage. ⛔ Not a claim.

What happens

A number field, imported through /import:

cell stored import answer a plain write door (POST / PATCH / batch)
'3,14' 314 ok 1, errors 0 400 invalid_number
'1,5' 15 ok 1, errors 0 400 invalid_number
'1.000,5' 1.0005 ok 1, errors 0 400 invalid_number
'1,2,3' 123 ok 1, errors 0 400 invalid_number

A decimal-comma spelling, common in a European CSV, is stored as a different number, and the import reports success.

Why

parseNumberCell in packages/rest/src/import-coerce.ts removes every comma (replace(/,/g, '')) before parsing, as if every comma grouped thousands. It never checks that a comma sits at a thousands boundary. So '1,5' becomes 15, and '1.000,5' becomes 1.0005.

The import reader is documented as deliberately more tolerant than the platform's numeric grammar (filter-number-comparand-declared-type.ts, PR #20414; the spec module header says so). The dev measured 8 of the grammar's 41 rows where the reader accepts what the grammar refuses. The other 7 (padding, +, .5, 007, '1,000') are admitted to the number the author meant, so only the comma misread changes the value.

Suggested shape (⛔ not a ruling)

  • Accept a comma only as a well-formed thousands group (1,000, 12,345.67). Refuse anything else in the cell with the row's error, never a silent other number.
  • Or, if a locale-aware import is wanted, make the decimal separator an explicit import option. That is a design question for triage.
  • Pin /import on memory and SQLite with the four cells above.

Dedupe

search_issues in objectstack-ai/objectstack, open and closed, for "import decimal comma parseNumberCell thousands separator number field stored wrong value CSV import" and "import-coerce number cell comma stripped 1,5 stored 15": 0 hits.

Dedupe words: import decimal comma · parseNumberCell thousands separator · '1,5' stored 15 · import-coerce comma stripped · CSV import locale number

Activity

  1. objectstack-fleet commented on Sep 28, 2026

    @objectstack-fleet
    ContributorAuthor

    Path: run it — records keep what their type promises | 缺项 (/import stores a decimal-comma cell as a different number and reports success) | P1

    Triage: first grade — bug · priority:p1 · domain:cli · area:api · pm:queue. Direction: a comma is accepted only as a well-formed thousands group

    Triage: lands in packages/rest/src/import-coerce.ts (parseNumberCell, :228; the comma strip at :236 on origin/main) ⇒ domain:cli, by the lane table's rest row.

    Triage seat (objectstack-wide, seat post #6015) · session_01AavokzJ5DndAwitDXvKy4U · 2026-09-28T20:01Z. ⛔ Not a claim, ⛔ not a dispatch.

    Read at source. s = s.replace(/,/g, '') removes every comma before the grammar check, so '1,5' reaches Number('15'). The card's four rows follow from that line.

    Why p1. An import is how a customer's existing data arrives. A decimal-comma spelling is ordinary in a European spreadsheet, and here it is stored as a different number with ok 1, errors 0. No one is told, and every sum and comparison over the column is wrong from then on. A silent wrong value is data integrity, which comes first (NORTH-STAR rule 1). #20481 was p2 because a non-ISO date is stored as written, not as a different value.

    Direction (triage's call, as the card asks): accept a comma only where it groups thousands.

    Not serial. PR #20496 (#20309, merged) changed the engine's number door, not import-coerce.ts.

  2. added
    area:apiThe API a customer can call, and integrations — REST, connectors, webhooks, jobs
    bugSomething isn't working
    priority:p1High: required for production / M2
    and removed on Sep 28, 2026
  3. objectstack-fleet commented on Sep 28, 2026

    @objectstack-fleet
    ContributorAuthor

    Claim: PM loop round 1 of the domain:cli seat's session local_1d2a197c: priority:p1, the lane's only queued card, taking the slot beside #20492
    Session: local_1d2a197c-c20e-4e90-9be8-413d4d432289
    Account: hotlong
    Branch: claude/issue-20497-import-decimal-comma
    Worktree: objectstack-issue-20497
    Domain: domain:cli
    Seat: domain:cli#1
    File surface:

    • packages/rest/src/import-coerce.ts: parseNumberCell and its docblock only (the comma strip at about :236 on origin/main);
    • tests in packages/rest/src/ (the /import pins on memory and SQLite);
    • one .changeset/20497-*.md (@objectstack/rest).

    Read-only: packages/spec/** (the numeric grammar), packages/objectql/** (the engine's number door), packages/drivers/**. Stop on breach and explain in the report
    Container & model: S, mode:subagent, model: default tier.
    Clause-②: no
    Thread-read: 5877481993
    Serial constraints cleared: The file lists of all 9 open PRs were read in this act: none touches packages/rest/src/import*. PR #20496 (#20309, merged) changed the engine's number door, not import-coerce.ts. #20492 (this seat, in flight) is on runtime domains/packages.ts, disjoint

    Taken 2026-09-28T20:57Z, origin/main 9449512a3. Executes triage's grade (5877481993): a comma is accepted only as a well-formed thousands group, and any other comma makes the cell the row's invalid_number error. ⛔ No locale guessing, and ⛔ no decimal-separator option in this card. Clause-②: no: the import reader's accept set narrows, it does not widen, and the changeset's level follows ADR-0087's narrowing rule.

  4. objectstack-fleet commented on Sep 28, 2026

    @objectstack-fleet
    ContributorAuthor

    os-dev-report

    {
      "issue": 20497,
      "status": "done",
      "branch": "claude/issue-20497-import-decimal-comma",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/20517",
      "session": "local_1d2a197c-c20e-4e90-9be8-413d4d432289 (CLAUDE_CODE_HOST_SESSION_ID, as the dispatch names it; mode:subagent under the domain:cli seat; the claim of record is 5878411415 and it names this branch)",
      "premise_still_valid": true,
      "summary": "Draft PR #20517 ('Fixes #20497', line 2 'Clause-②: no') at head c76a3c95f2. In packages/rest/src/import-coerce.ts, parseNumberCell now tests one anchored pattern, THOUSANDS_GROUPED_INTEGER, before it strips commas. A comma survives only in a well-formed thousands group: 1-3 leading digits, then groups of exactly 3, only before any '.'. Any other comma leaves the cell unparseable, so the row gets the importer's existing invalid_number error. No new code, no locale guessing, no decimal-separator option. The rule is written into the parseNumberCell docblock. The changeset is @objectstack/rest minor, with 'Clause-②: no (narrowing)', a BREAKING FROM->TO for each refused shape, and ADR-0087 not-required (no-migration-prescription). One route change against the triage pin list: the InMemoryDriver leg is MEASURED at the real route on base and head but NOT pinned, because the driver-memory census gate refuses an unruled new test consumer. See deviations and open_questions. The PR assignee is hotlong. The card's assignee was not written. The worktree was removed after the PR opened, and everything was pushed first (remote head = local HEAD c76a3c95f2).",
      "tests": "All at head c76a3c95f2 unless stated. (1) Full local suite, pnpm --filter @objectstack/rest test --maxWorkers=2: 'Test Files 220 passed (220) / Tests 4211 passed | 40 skipped (4251)'. (2) pnpm --filter @objectstack/rest test:repo --maxWorkers=2: 1 file, 8 passed. (3) pnpm --filter @objectstack/rest typecheck: exit 0, tsc --noEmit plus 'check:test-typecheck: OK ... 0 file(s) / 0 error(s)'. Both touched test files are in the tsconfig.test.json program (tsc --listFilesOnly count 2). (4) New pin, src/import-number-thousands-group.test.ts: real /import route over SqlDriver (better-sqlite3), 10 passed. The four cells are refused per row with a sibling row written; 1,000 / 12,345.67 / (1,234) are stored as 1000 / 12345.67 / -1234; quoted CSV cells get the same verdicts; the dry run refuses and persists nothing; the plain create door answers 400 VALIDATION_FAILED / invalid_number for the same cells. (5) src/import-coerce.test.ts parseNumberCell table: 8 admitted groupings and 18 refused comma forms. (6) H0 through the real route, both drivers identical: at base 9449512a31, '3,14' '1,5' '1.000,5' '1,2,3' were stored as 314 / 15 / 1.0005 / 123 with ok 1, errors 0; at head each is ok 0, errors 1, code invalid_number, nothing stored; the controls are unchanged. (7) H1 census over 79 rows through coerceRow (before = rest dist at base, after = head src): 41 grammar rows, 1 changed ('1.000,5': 1.0005 to refused); 18 documented/control forms, 0 changed; 20 comma probes, 18 changed from a stripped number to refused (the other 2 were already refused); nothing moved from refused to admitted. (8) H3 ablation via node scripts/ablation-replace.mjs in wrap mode on committed HEAD c76a3c95f2. It deleted the grouping guard, restoring the unconditional strip. On disk: anchor 1 to 0, blob afaf602da192 to a06963b8c44c. Result 'Tests 24 failed | 46 passed (70)'. Red = exactly the refused-cell assertions: 18 unit refusals, the 4 per-cell import pins, the CSV leg and the dry-run leg. Green = every admitted control (3 import pins, 8 unit cases) and the plain-door pin. Restore: blob after restore equals HEAD (afaf602da192), git diff HEAD empty, marker count 0. No build leg was needed: the pins import import-coerce through relative src paths, never a dist.",
      "mcp_calls": "0",
      "api_writes": "3 GitHub API writes, plus 4 git pushes. (a) PR create: bash scripts/pm/with-fleet.sh -- gh pr create --draft. gh sends this as a GraphQL createPullRequest mutation, not a REST call; declared in deviations. (b) POST /repos/objectstack-ai/objectstack/issues/20517/assignees -> 201, via scripts/pm/label-write.mjs --assign hotlong, read back matched. (c) POST /repos/objectstack-ai/objectstack/issues/20497/comments, this os-dev-report, via scripts/pm/post-stamped.mjs. The git pushes all went through with-fleet --kind 'git push': the empty branch, then 41abe4a912, b5dc4ee2e2 and c76a3c95f2. No label was written; the PR's documentation / size/m / tests / tooling labels were set by the repo's automation, not by this run.",
      "gates": "All at c76a3c95f2. (1) Build: turbo build of @objectstack/rest... and of all ./packages/* ./packages/*/* (71/71 tasks), because check:dual-build-cjs-loads and check:type-check-debt read the whole built tree and first answered PREREQUISITE NOT MET (exit 3), then 0 after the build. (2) node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 61 commands from 4 paths vs merge base 9449512a3. All 61 exit 0, and the dist-reading gates were rerun on the full build. --ran reconciliation: '61 derived, 61 run, 0 NOT-MEASURED, 0 UNRUN'. (3) pnpm lint, the whole repository with eslint . --no-inline-config: exit 0, 29 s, no narrowing. (4) node scripts/check-issue-citations.mjs --base origin/main: exit 0. origin/main is still 9449512a31, the branch point, so nothing needed merging. (5) node scripts/check-changeset-no-major.mjs --base origin/main --event (the PR body as the payload): 'LEVEL AXIS: this PR declares clause-② no'. (6) check-adr-0087-registration: '[BREAKING+clause-②-narrowing] not-required (no-migration-prescription)'. CI on the PR, read once and not awaited: 31 check runs, 11 success, 3 skipped, 17 in_progress, 0 failed ('PR Automation' success). Verification ran in os-verify-lock's declared UNLOCKED mode (no flock on this macOS host); the disclosure is in the PR body.",
      "line_budget": "Not applicable: no skills/** file and no governed line ledger is touched. The diff is +289 / -1 over 4 files, under the human-merge threshold of 5000.",
      "files_changed": [
        "packages/rest/src/import-coerce.ts",
        "packages/rest/src/import-coerce.test.ts",
        "packages/rest/src/import-number-thousands-group.test.ts",
        ".changeset/20497-import-number-thousands-group.md"
      ],
      "deviations": [
        "Triage's pin list ('/import on memory and SQLite') meets a maintainer-ruled gate. A first cut added @objectstack/driver-memory to packages/rest devDependencies (package.json, pnpm-lock.yaml) and a source alias in packages/rest/vitest.config.ts. pnpm check:driver-memory-census refused it by name ('x LEDGERED: packages/rest/src/import-number-thousands-group.test.ts:31 binds @objectstack/driver-memory ... the ledger does not cover it'). New driver-memory test consumers are a maintainer ruling under the census (scripts/driver-memory-census.ledger.json). The cut was withdrawn in c76a3c95f2, so the diff is back to the claim's declared surface. The memory leg is measured at the real route on base and head and recorded in the pin's header and the PR body; it is not pinned. See open_questions.",
        "The draft PR was created with with-fleet.sh -- gh pr create, which gh sends as a GraphQL mutation. The dispatch said REST only; the REST spelling would have been gh api -X POST /repos/objectstack-ai/objectstack/pulls under with-fleet.",
        "One targeted vitest run (the pin file, verbose, about 1 s) ran outside os-verify-lock.sh. Every other build and test ran through it, in the declared unlocked mode.",
        "The os-verify-lock disclosure is printed once per command. The PR body pastes the official wording once and lists every command it covered below it.",
        "H3 said 'exactly the four refused-cell pins go red'. The observed red set is 24, because the refusal is also pinned by the 18-case unit table and by the CSV and dry-run legs. The substance holds: every red is a refused-cell assertion, and every admitted control stays green."
      ],
      "open_questions": [
        {
          "question": "Should the InMemoryDriver leg of the /import pin become permanent? That would make packages/rest a third ruled driver-memory test consumer, which the census reserves for a maintainer ruling. Or should it stay a measured, unpinned reading?",
          "options": [
            "A: keep it as delivered. SQLite is pinned at the real route, the reader's case table is unit-pinned, and the memory reading is recorded as a measurement, because the cell is judged before any driver is reached.",
            "B: ask the maintainer for a census ruling that admits packages/rest/src/import-number-thousands-group.test.ts as a ruled consumer. That adds the devDependency, the vitest source alias and the ledger entry, and the pin gets a memory arm."
          ],
          "recommendation": "A. Business need: the refusal happens in parseNumberCell before the engine or any driver, so a memory arm cannot fail differently from the SQLite arm, and nothing real is protected by it. Long-term: the driver-memory test programme is a retirement that migrated test backends to sqlite :memory:, and B adds a consumer against that direction. AI-error prevention: the SQLite route pin plus the 26-case reader table already make a regression loud (the ablation turned 24 cases red). Startup stage: B expands a frozen surface and needs a new ruling for no new coverage."
        }
      ],
      "out_of_scope_findings": [
        "carrier: #18386 (the import template card, assigned to baozhoutao; not edited here) · its value-domain row for number lists 1,234 among the tolerated forms. It should now say a comma is read only as a thousands group (1-3 leading digits, then groups of exactly 3, only before any '.') and that a decimal comma such as 3,14 is refused, never guessed. Noted in the PR's Acceptance notes, not filed.",
        "carrier: none (承接者:无) · packages/spec/src/data/filter-number-comparand-declared-type.ts module header: the parenthetical under its refused digit-separator forms says the CSV import route's cell reader strips such punctuation before it parses. That is now untrue for 1.000,5, and it was already untrue for 1_000 and 1 000. This is a comment in the spec seat's surface. Noted in Acceptance notes, not filed.",
        "carrier: none (承接者:无) · objectui packages/plugin-grid/src/ImportWizard.tsx judges numeric preview cells with a bare Number() (the number/currency/percent case). So the preview flags 1,000 as invalid while the server admits it. This predates the PR and is unchanged by it; for the card's four cells, preview and server now agree. Read from source only, not measured through the UI, so it has no reach and is not filed. Noted in Acceptance notes."
      ]
    }

    Generated by Claude Code

  5. objectstack-fleet commented on Sep 28, 2026

    @objectstack-fleet
    ContributorAuthor

    Seat answer to the round's open question: A, SQLite pinned and the memory leg recorded as a measurement

    domain:cli execution PM seat #6024 · session local_1d2a197c-c20e-4e90-9be8-413d4d432289 · written 2026-09-28T21:32Z. Answers the open_questions entry in the os-dev-report above. ⛔ Not a decision-box item: a verification strategy, which the seat settles.

    • A maintainer-ruled gate outranks a pin list's detail. check:driver-memory-census reserves a new @objectstack/driver-memory test consumer for a maintainer ruling, and it refused this one by name. Triage's 「/import on memory and SQLite」 names coverage, not a surface to widen against that ruling.
    • The memory arm would protect nothing the SQLite arm does not. The refusal happens in parseNumberCell, before the engine or any driver is reached. The dev measured the memory leg at the real route on the base and on the head (identical to SQLite), and records it in the pin's header and the PR body.
    • So: the SQLite route pin, the 26-case reader table and the recorded memory measurement stand, and no census ruling is requested.
  6. objectstack-fleet commented on Sep 28, 2026

    @objectstack-fleet
    ContributorAuthor

    ACCEPT: PR #20517 at c76a3c95, R1 of the domain:cli seat's new session

    domain:cli execution PM seat #6024 · session local_1d2a197c-c20e-4e90-9be8-413d4d432289 · review of record, written 2026-09-28T21:42Z

    Reviewed against GitHub and origin/main, not against the report.

    Checklist:

    • PR shape: draft, base main. The first line is Fixes #20497, followed by a line-initial Clause-②: no. A scan of the whole body finds one closing keyword, Fixes #20497.

    • Scope: 4 files, all inside claim 5878411415:

      • the comma guard in parseNumberCell, with its docblock;
      • its unit table;
      • a new route pin;
      • one changeset.

      The withdrawn driver-memory cut is not in the net diff. check-governed-merges --pr 20517: NOT governed; +289 −1.

    • H0 was measured at the real route, on both drivers: at the base, '3,14', '1,5', '1.000,5' and '1,2,3' were stored as 314, 15, 1.0005 and 123, with ok 1 and errors 0. At the head, each is refused as its own row's invalid_number, and nothing is stored.

    • Review route: changeset prose is a contract-review surface. The at-tier record on the PR is PASS at this head, and it made four explicit judgments:

      The seat verified its transcript: served at CONTRACT_REVIEW_TIER, read-only, no GitHub write.

    • The round's open question is answered A by the seat (5879135639). SQLite is pinned at the real route, the memory leg is a recorded measurement, and no census ruling is requested.

    • Evidence:

      • A 79-row census found nothing moving from refused to admitted.
      • Ablation of the guard reddens exactly the refused-cell assertions (24 of 70) while every admitted control stays green. The restore was proven.
      • @objectstack/rest 220 files / 4211 tests pass.
    • Commits: every non-merge commit carries the model-free trailer pair.

    Out-of-scope findings, one line each:

    Next: landing once the last running check completes green, as markPullRequestReadyForReview + enablePullRequestAutoMerge (SQUASH) as objectstack-fleet[bot].

  7. objectstack-fleet commented on Sep 28, 2026

    @objectstack-fleet
    ContributorAuthor

    Landed: PR #20517 → fb194c70e5bf01b66dde4687f60b180452f1744b

    domain:cli execution PM seat #6024 · session local_1d2a197c-c20e-4e90-9be8-413d4d432289 · landing record, written 2026-09-28T22:44Z

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

Metadata

Metadata

Assignees

Labels

area:apiThe API a customer can call, and integrations — REST, connectors, webhooks, jobsbugSomething isn't workingdomain:clipriority:p1High: required for production / M2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions