From d46f8aaed94bf977586e159d28a54ed5687f02ae Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Mon, 10 Aug 2026 10:54:08 +0000 Subject: [PATCH] fix(objectql): the autonumber fallback reads the declared `{0000}` default, not the empty string (#7262) Execution half 2/3 -- the last one -- of the maintainer's route-3 ruling on #6555. `applyAutonumbers` resolved the format by hand (`autonumberFormat ?? format`, then `typeof fmt === 'string' ? fmt : ''`), so a format-LESS field parsed the EMPTY string and rendered `renderAutonumber`'s no-slot branch as a bare `1`, `2`, .... driver-sql substituted its own `'{0000}'` and issued `0001`. One metadata document, two number shapes, decided by which driver served it. It is now `resolveAutonumberFormat(def)` -- the resolver landed by #7265 and already read by driver-sql since #7263. Unlike the driver half this MOVES behaviour, two ways, both in the changeset: - a format-less field on the engine fallback path issues `0001` where it issued `1` (engine-fallback deployments only; stored driver-sql data is undisturbed, and #6468's counter continuity is unaffected -- `{0000}` renders prefix '' / suffix '', so the seeding scan stays on its unanchored legacy reading); - `??` -> truthiness means `autonumberFormat: ''` / `format: ''` resolve to the default instead of rendering bare, and an empty canonical key no longer masks a declared `format` shorthand. Tests. New `engine-autonumber-default-format.test.ts` covers the empty-string inputs no suite on either side declared before (the gap the drivers seat measured on this card), plus the format-less renders and the controls that must NOT move. `autonumber-seed-cross-side-parity.integration.test.ts` gains the bug report's own reproduction -- no format, stored 1/2/10, both sides `0011` -- which is what lets that file assert a shared RENDERING and not only a shared counter. Five pre-existing cases asserting bare `'1'`/`'2'`/`'8'`/`'11'` on format-less fields were re-pinned to the padded form, each with a #6555 comment. Closes #7262. Part of #6555. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2 --- .../autonumber-format-defallback-engine.md | 54 ++++ .../src/sql-driver-autonumber-suffix.test.ts | 14 +- .../engine-autonumber-default-format.test.ts | 292 ++++++++++++++++++ .../src/engine-autonumber-seed-scan.test.ts | 23 +- .../src/engine-autonumber-seed-suffix.test.ts | 22 +- .../engine-insert-runtime-owned-strip.test.ts | 30 +- packages/objectql/src/engine.ts | 45 ++- ...seed-cross-side-parity.integration.test.ts | 51 ++- 8 files changed, 492 insertions(+), 39 deletions(-) create mode 100644 .changeset/autonumber-format-defallback-engine.md create mode 100644 packages/objectql/src/engine-autonumber-default-format.test.ts diff --git a/.changeset/autonumber-format-defallback-engine.md b/.changeset/autonumber-format-defallback-engine.md new file mode 100644 index 0000000000..6c2d5901c9 --- /dev/null +++ b/.changeset/autonumber-format-defallback-engine.md @@ -0,0 +1,54 @@ +--- +"@objectstack/objectql": minor +--- + +fix(objectql): the engine's autonumber fallback reads the declared `{0000}` default instead of parsing the empty string (#7262) + +Execution half 2/3 — the last one — of the maintainer's route-3 ruling on #6555. +`{0000}` became a declared contract default in `@objectstack/spec/data` +(`DEFAULT_AUTONUMBER_FORMAT` / `resolveAutonumberFormat`); `driver-sql` stopped +writing its own copy down, and the engine now stops too. + +`applyAutonumbers` resolved the format by hand: + +```ts +const fmt = (def as any).autonumberFormat ?? (def as any).format; +const tokens = parseAutonumberFormat(typeof fmt === 'string' ? fmt : ''); +``` + +An undeclared format therefore parsed the EMPTY string, whose empty token list +`renderAutonumber` renders through its no-slot branch as a bare counter. It is +now `resolveAutonumberFormat(def)` — one resolver, shared with the SQL driver. + +**⚠ Unlike the driver half, this one MOVES behaviour — two ways.** + +1. **A format-less field on the engine's fallback path issues `0001` where it + issued `1`.** The path is taken whenever the driver does not advertise + `supports.autonumber` — `driver-memory`, `driver-mongodb`, any driver without + the capability. Per the ruling: *choosing {0000} keeps stored driver-sql data + undisturbed; engine-fallback deployments flip from bare 1 to 0001 for newly + issued numbers. Counter continuity itself is unaffected (#6468 pinned it).* + The counter is genuinely untouched: `{0000}` renders an empty prefix and an + empty suffix, so the seeding scan stays on its unanchored legacy reading and + goes on reading already-stored bare values (`1`, `2`, `10` → next is 11, + rendered `0011`). Only the width of newly issued numbers changes, and only on + this path. + +2. **An EMPTY declared format is now "undeclared".** The engine read the key with + `??`, which respects an empty string, so `autonumberFormat: ''` reached + `parseAutonumberFormat` as `''` and rendered bare. `resolveAutonumberFormat` + counts anything that is not a non-empty string as undeclared — the SQL + driver's long-standing truthiness rule, which is what makes the two sides + agree — so `autonumberFormat: ''` and `format: ''` now resolve to `{0000}` + too. One further consequence of the same rule: an empty canonical key no + longer masks a declared shorthand, so + `{ autonumberFormat: '', format: 'D-{0000}' }` renders `D-0001` where it used + to render a bare `1`. + +**To keep a bare, unpadded counter**, declare a format with no `{0..0}` slot — +`autonumberFormat: 'PRE-'` renders `PRE-1`. `autonumberFormat: ''` is NOT that +spelling. **To keep the `0001` shape** that SQL deployments already store, and +that a format-less field now mints everywhere, change nothing. + +With this, #6555 is closed: one metadata document mints one number shape, +whichever driver serves it. diff --git a/packages/drivers/driver-sql/src/sql-driver-autonumber-suffix.test.ts b/packages/drivers/driver-sql/src/sql-driver-autonumber-suffix.test.ts index b859a8f547..346c0a95bf 100644 --- a/packages/drivers/driver-sql/src/sql-driver-autonumber-suffix.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-autonumber-suffix.test.ts @@ -164,13 +164,13 @@ describe('SqlDriver autonumber seeding — the counter is located by the declare // byte-for-byte: `'10'` wins over `'2'` — a numeric max, never a // lexicographic one — so the counter continues at 11. // - // The RENDERING of a format-less field is a separate, pre-existing matter - // this fix does not touch: a format-less field resolves to the contract - // default `{0000}` (`resolveAutonumberFormat`, #6555), so 11 renders - // `0011` here — while the engine's fallback still emits the bare `11` - // until #7262 lands the other half. That divergence is in the render - // default, not in the seeding parse #6468 is about, so the cross-side - // parity test uses explicitly-formatted fields. + // The RENDERING of a format-less field is a separate matter this fix does + // not touch: a format-less field resolves to the contract default + // `{0000}` (`resolveAutonumberFormat`, #6555), so 11 renders `0011` here. + // The engine's fallback rendered a bare `11` until #7262 landed the other + // half of that ruling; both sides now read the declared default, and + // `autonumber-seed-cross-side-parity.integration.test.ts` asserts these + // very rows against each other. await initRec(); await seedRows(['1', '2', '10']); diff --git a/packages/objectql/src/engine-autonumber-default-format.test.ts b/packages/objectql/src/engine-autonumber-default-format.test.ts new file mode 100644 index 0000000000..cfaa18b0a0 --- /dev/null +++ b/packages/objectql/src/engine-autonumber-default-format.test.ts @@ -0,0 +1,292 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #6555 (half 2/3, #7262) — a format-LESS autonumber field renders through the + * contract default `{0000}`, not through the empty string. + * + * `applyAutonumbers` used to read the format by hand — `autonumberFormat ?? + * format`, then `typeof fmt === 'string' ? fmt : ''` — so a field declaring no + * format handed `parseAutonumberFormat` the EMPTY string. An empty token list + * renders through `renderAutonumber`'s no-slot branch as a bare counter: `1`, + * `2`, …. `driver-sql` answered the same question with its own hardcoded + * `|| '{0000}'` and issued `0001`, `0002`, …. One metadata document therefore + * minted differently-shaped numbers depending on which driver served it, and a + * suite asserting `'1'` against the memory driver did not hold in production on + * SQL. The counter VALUE always agreed — #6468 pinned that — so the fork was + * rendering width alone. + * + * The maintainer's route-3 ruling on #6555 (2026-08-08) moved the default into + * the contract: `DEFAULT_AUTONUMBER_FORMAT` / `resolveAutonumberFormat` in + * `@objectstack/spec/data` (#7265), read by `driver-sql` (#7263) and, here, by + * the engine. This file is the engine-side pin for the two behaviour moves that + * lands with. + * + * ## Why this file exists at all — a measured coverage gap + * + * The drivers seat measured, while landing #7263, that NOT ONE test on either + * side declared an empty-string format: `git grep "format: ''\|autonumberFormat: + * ''"` returned nothing across all 8 driver-sql autonumber suites and all 7 + * `engine-autonumber-*.test.ts` suites. `''` is precisely the input this half + * moves (`??` respects an empty string, the resolver's truthiness rule does + * not), so a green run of the pre-existing suites is not evidence about it in + * EITHER direction. Every `''` case below was written for that gap. + * + * The counterpart pins live in + * `packages/drivers/driver-sql/src/sql-driver-autonumber-suffix.test.ts` (the SQL + * arm, `0011` since #7263) and + * `packages/runtime/src/autonumber-seed-cross-side-parity.integration.test.ts` + * (the two arms asserted against each other over one dataset). + * + * These tests drive a fake DRIVER (not a fake engine) whose `supports = {}`, so + * the engine's own fallback owns the counter — the path the whole card is about. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { ObjectQL } from './engine'; +import { SchemaRegistry } from './registry'; +import type { IDataDriver } from '@objectstack/spec/contracts'; + +vi.mock('./registry', () => { + const instance: any = { + getObject: vi.fn(), + resolveObject: vi.fn((n: string) => instance.getObject(n)), + registerObject: vi.fn(), + getObjectOwner: vi.fn(), + registerNamespace: vi.fn(), + registerKind: vi.fn(), + registerItem: vi.fn(), + registerApp: vi.fn(), + installPackage: vi.fn(), + reset: vi.fn(), + metadata: { get: vi.fn(() => new Map()) }, + }; + function SchemaRegistry() { + return instance; + } + Object.assign(SchemaRegistry, instance); + return { + SchemaRegistry, + computeFQN: (_ns: string | undefined, name: string) => name, + parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }), + RESERVED_NAMESPACES: new Set(['base', 'system']), + }; +}); + +/** Date tokens render from the wall clock, so the clock is pinned. Only `Date`. */ +const FIXED_NOW = new Date('2026-06-15T09:00:00Z'); + +/** + * Evaluate the operators the seeding walk actually emits. Anything else throws + * rather than being tolerated: silently ignoring an unknown operator would let a + * bad query pass as a good one. + */ +function matches(row: Record, where: any): boolean { + if (where == null) return true; + for (const [key, cond] of Object.entries(where)) { + if (key === '$and') { + if (!(cond as any[]).every((w) => matches(row, w))) return false; + continue; + } + if (key.startsWith('$')) throw new Error(`fake driver: unsupported logical operator ${key}`); + const v = row[key]; + if (cond !== null && typeof cond === 'object' && !Array.isArray(cond)) { + for (const [op, operand] of Object.entries(cond as Record)) { + if (op === '$startsWith') { + if (typeof v !== 'string' || !v.startsWith(String(operand))) return false; + } else if (op === '$gt') { + if (!(String(v) > String(operand))) return false; + } else if (op === '$eq') { + if (v !== operand) return false; + } else { + throw new Error(`fake driver: unsupported operator ${op}`); + } + } + } else if (v !== cond) { + return false; + } + } + return true; +} + +function makeDriver(rows: Array>): IDataDriver { + const driver: any = { + name: 'memory', + version: '0.0.0', + // No `autonumber` support — this is exactly the engine fallback path. + supports: {}, + connect: vi.fn().mockResolvedValue(undefined), + disconnect: vi.fn().mockResolvedValue(undefined), + checkHealth: vi.fn().mockResolvedValue(true), + execute: vi.fn(), + find: vi.fn(async (_obj: string, ast: any) => { + let out = rows.filter((r) => matches(r, ast?.where)); + const orderBy = ast?.orderBy; + if (Array.isArray(orderBy) && orderBy.length > 0) { + const { field, order } = orderBy[0]; + out = [...out].sort((a, b) => { + const av = String(a[field] ?? ''); + const bv = String(b[field] ?? ''); + const cmp = av < bv ? -1 : av > bv ? 1 : 0; + return order === 'desc' ? -cmp : cmp; + }); + } + if (typeof ast?.limit === 'number') out = out.slice(0, ast.limit); + return out.map((r) => ({ ...r })); + }), + findOne: vi.fn(), + create: vi.fn(async (_obj: string, row: any) => ({ id: 'new1', ...row })), + update: vi.fn(), + delete: vi.fn(), + count: vi.fn(), + }; + return driver as IDataDriver; +} + +const rowId = (n: number) => `r${String(n).padStart(6, '0')}`; + +/** + * A schema whose single autonumber field carries EXACTLY the given keys — the + * point of most cases below is a key that is present and empty, which a + * `format?: string` parameter cannot express. + */ +function schemaWith(declaration: Record) { + return { + name: 'rec', + fields: { + title: { type: 'text' }, + rec_no: { type: 'autonumber', required: true, ...declaration }, + }, + }; +} + +/** Stored rows carrying pre-existing record numbers, in insertion order. */ +const storedRows = (values: string[]) => + values.map((v, i) => ({ id: rowId(i + 1), rec_no: v })); + +async function issueOne(schema: any, rows: Array> = []): Promise { + vi.mocked(SchemaRegistry.getObject).mockReturnValue(schema as any); + const engine = new ObjectQL(); + engine.registerDriver(makeDriver(rows) as any, true); + await engine.init(); + const result: any = await engine.insert('rec', { title: 'next' }); + return result.rec_no; +} + +describe('ObjectQL applyAutonumbers — the contract default for a format-less field (#6555)', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers({ toFake: ['Date'] }); + vi.setSystemTime(FIXED_NOW); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + // ----------------------------------------- (1) the primary behaviour move -- + + describe('an undeclared format renders `{0000}`, not the bare counter', () => { + /** The bug report's own metadata: `{ rec_no: { type: 'autonumber' } }`. */ + it('issues `0001` on an empty store', async () => { + expect(await issueOne(schemaWith({}))).toBe('0001'); + }); + + it('issues `0011` after stored `1` / `2` / `10` — the bug report verbatim', async () => { + // The reproduction from #6555. Two facts in one assertion: the counter + // still reads the stored BARE values (seeding is untouched — `{0000}` + // renders prefix '' and suffix '', so the unanchored legacy reading still + // applies and `'10'` beats `'2'` numerically), and the number it issues is + // now RENDERED padded. `driver-sql` answers `0011` over the same rows. + expect(await issueOne(schemaWith({}), storedRows(['1', '2', '10']))).toBe('0011'); + }); + + it('the counter continues across calls, each rendered padded', async () => { + vi.mocked(SchemaRegistry.getObject).mockReturnValue(schemaWith({}) as any); + const engine = new ObjectQL(); + engine.registerDriver(makeDriver([]) as any, true); + await engine.init(); + + const a: any = await engine.insert('rec', { title: 'a' }); + const b: any = await engine.insert('rec', { title: 'b' }); + + expect([a.rec_no, b.rec_no]).toEqual(['0001', '0002']); + }); + }); + + // ------------------------------- (2) the second, smaller move: `''` inputs -- + + /** + * The gap the drivers seat measured (#7262, comment 5237739551): no suite on + * either side declared an empty-string format, and `''` is the one input whose + * behaviour this half moves. The engine read the key with `??`, which respects + * an empty string; `resolveAutonumberFormat` counts anything that is not a + * NON-EMPTY string as undeclared — driver-sql's long-standing truthiness rule, + * adopted deliberately so the two sides agree. + */ + describe('an EMPTY declared format is undeclared, and resolves to the default', () => { + it("`autonumberFormat: ''` renders `0001`, not a bare `1`", async () => { + expect(await issueOne(schemaWith({ autonumberFormat: '' }))).toBe('0001'); + }); + + it("`format: '' ` renders `0001`, not a bare `1`", async () => { + expect(await issueOne(schemaWith({ format: '' }))).toBe('0001'); + }); + + it("an empty canonical key no longer MASKS a declared `format` shorthand", async () => { + // The sharpest edge of `??` → truthiness, and the only case where the two + // rules disagree on something other than the default: `'' ?? 'D-{0000}'` + // is `''` (nullish coalescing does not fall through an empty string), so + // the engine used to render bare and ignore the shorthand entirely. The + // resolver falls through to it. + expect(await issueOne(schemaWith({ autonumberFormat: '', format: 'D-{0000}' }))).toBe('D-0001'); + }); + + it('a key holding a non-string is undeclared too', async () => { + // Unreachable through a parsed `FieldSchema`, reachable through the + // unvalidated field documents both generators actually hold. The old code + // fell to `''` here (`typeof fmt === 'string' ? fmt : ''`) and rendered + // bare; the resolver answers the declared default, same as driver-sql. + expect(await issueOne(schemaWith({ autonumberFormat: 42 }))).toBe('0001'); + expect(await issueOne(schemaWith({ format: null }))).toBe('0001'); + }); + }); + + // -------------------------------------------------- (3) controls — UNMOVED -- + + /** + * Drift guards for the surface this change must NOT touch. Stated plainly: + * these cannot go red when the fix is reverted, so they are not evidence for + * the moving leg above — they exist to catch a future edit that overreaches. + */ + describe('a DECLARED format is honoured exactly as written', () => { + it('`D-{0000}` is unchanged', async () => { + expect(await issueOne(schemaWith({ format: 'D-{0000}' }), storedRows(['D-0001', 'D-0002']))).toBe('D-0003'); + }); + + it('the spec-canonical key still wins over the shorthand (#1603)', async () => { + expect(await issueOne(schemaWith({ autonumberFormat: 'A-{000}', format: 'B-{000}' }))).toBe('A-001'); + }); + + it('a slot-less format still renders a BARE counter — the escape hatch', async () => { + // The documented way to keep an unpadded number after this change: declare + // a format with no `{0..0}` slot. `autonumberFormat: ''` is NOT that + // spelling (see above), which is the whole reason the changeset spells + // this out for anyone who was relying on the engine's bare rendering. + expect(await issueOne(schemaWith({ format: 'PRE-' }))).toBe('PRE-1'); + }); + + it('a driver that owns autonumber is untouched — the engine fills nothing', async () => { + vi.mocked(SchemaRegistry.getObject).mockReturnValue(schemaWith({}) as any); + const driver: any = makeDriver([]); + driver.supports = { autonumber: true }; + const engine = new ObjectQL(); + engine.registerDriver(driver, true); + await engine.init(); + + await engine.insert('rec', { title: 'next' }); + + // The driver's own sequence answers; the engine hands it an empty slot. + expect(driver.create.mock.calls[0][1].rec_no).toBeUndefined(); + }); + }); +}); diff --git a/packages/objectql/src/engine-autonumber-seed-scan.test.ts b/packages/objectql/src/engine-autonumber-seed-scan.test.ts index 76e7a6b770..ee29560f84 100644 --- a/packages/objectql/src/engine-autonumber-seed-scan.test.ts +++ b/packages/objectql/src/engine-autonumber-seed-scan.test.ts @@ -90,10 +90,15 @@ const TICKET_SCHEMA = { }; /** - * Legacy path: NO format at all, so there is no `{0..0}` slot and no prefix. - * `renderAutonumber` emits the bare counter (`'1'`, `'2'`, … `'10'`), and the - * seed reads the LAST digit run of the whole stored value. There is no - * lexicographic reading of these values that equals the numeric one. + * Legacy path: NO format at all, so the seed reads the LAST digit run of the + * whole stored value. There is no lexicographic reading of these values that + * equals the numeric one. + * + * The stored values below are bare (`'7'`, `'8'`, … `'10'`) — what a deployment + * on this path had already issued. What the engine now ISSUES is rendered + * through the contract default `{0000}` (#6555/#7262), so the next number is + * `'0011'`, not `'11'`. The reading is unaffected: `{0000}` renders prefix `''` + * and suffix `''`, which is the unanchored branch these rows have always taken. */ const LEGACY_SCHEMA = { name: 'legacy', @@ -388,9 +393,13 @@ describe('ObjectQL seedAutonumber — the scan must cover every row in scope (#6 const result = await engine.insert('legacy', { title: 'Next' }); - expect(result.ref_no).toBe('11'); - // `'9'` is the lexicographic max; seeding from it re-issues `'10'`. - expect(result.ref_no).not.toBe('10'); + // ⚠️ `0011`, not a bare `11`, since #6555/#7262 — a format-less field + // renders through the declared default `{0000}`. Do not "fix" this back: + // the padding is what makes this engine path and `driver-sql` mint the + // same shape. The counter (11) is what this case is really about. + expect(result.ref_no).toBe('0011'); + // `'9'` is the lexicographic max; seeding from it re-issues counter 10. + expect(result.ref_no).not.toBe('0010'); }); it('scans past one page with no prefix to push down', async () => { diff --git a/packages/objectql/src/engine-autonumber-seed-suffix.test.ts b/packages/objectql/src/engine-autonumber-seed-suffix.test.ts index 6143c81412..a6a0857f2f 100644 --- a/packages/objectql/src/engine-autonumber-seed-suffix.test.ts +++ b/packages/objectql/src/engine-autonumber-seed-suffix.test.ts @@ -289,20 +289,30 @@ describe('ObjectQL seedAutonumber — the counter is located by the declared suf describe('a format declaring neither prefix nor suffix keeps the legacy reading', () => { it('reads the LAST digit run of the whole value', async () => { - // No format at all: `renderAutonumber` emits the bare counter, and the - // stored values have no anchor. `'10'` must win over `'2'` — this is a - // numeric max, never a lexicographic one. + // No format at all, so the stored values have no anchor: `'10'` must win + // over `'2'` — this is a numeric max, never a lexicographic one. That + // READING is what #6468 is about here and it is unchanged. + // + // ⚠️ The `0011` is the RENDERING, and it is deliberate — do not "fix" it + // back to a bare `'11'`. A format-less field resolves to the contract + // default `{0000}` since #6555/#7262 (`resolveAutonumberFormat`), which is + // what makes the engine and `driver-sql` mint the same shape over this + // exact dataset; the SQL arm pins `0011` too, in + // `sql-driver-autonumber-suffix.test.ts`. `{0000}` renders prefix '' and + // suffix '', so the unanchored reading below is untouched by it. const { result } = await insertOne(schemaWith('ref_no'), storedRows('ref_no', ['1', '2', '10'])); - expect(result.ref_no).toBe('11'); + expect(result.ref_no).toBe('0011'); }); it('reads past leading text that no format describes', async () => { // Values written before any format existed: the unanchored reading takes - // the trailing run, which is what it has always done. + // the trailing run, which is what it has always done. Rendered through the + // `{0000}` default (#6555/#7262) — see the note above before re-pinning + // this to a bare `'8'`. const { result } = await insertOne(schemaWith('ref_no'), storedRows('ref_no', ['SO-2024-0007'])); - expect(result.ref_no).toBe('8'); + expect(result.ref_no).toBe('0008'); }); }); }); diff --git a/packages/objectql/src/engine-insert-runtime-owned-strip.test.ts b/packages/objectql/src/engine-insert-runtime-owned-strip.test.ts index 136938cc69..44f9f0af7e 100644 --- a/packages/objectql/src/engine-insert-runtime-owned-strip.test.ts +++ b/packages/objectql/src/engine-insert-runtime-owned-strip.test.ts @@ -118,12 +118,17 @@ describe('insert strip acts on CALLER-submitted values (#6339)', () => { }); it('B (THE REPORT): a code the hook OVERWROTE lands, even though the caller sent the key', async () => { - // Identical to A except that the caller's payload also carries `code`. On - // `origin/main` this committed "1" — the sequence value, because the strip - // deleted the hook's write. Stated as the value it must NOT be, then as the - // value it must be. + // Identical to A except that the caller's payload also carries `code`. + // Before #6339 this committed the SEQUENCE value, because the strip deleted + // the hook's write. Stated as the value it must NOT be, then as the value it + // must be. + // + // ⚠️ The sequence value is spelled `'0001'`, not `'1'`, since #6555/#7262: + // this field declares no format, so it renders through the contract default + // `{0000}`. Re-pinned rather than left at `'1'` because `'1'` stopped being + // the value this guard means to exclude the moment the render moved. const row: any = await engine.insert('probe_num2', { title: 'B', code: 'CALLER-FORGED' }); - expect(row.code).not.toBe('1'); + expect(row.code).not.toBe('0001'); expect(row.code).not.toBe('CALLER-FORGED'); expect(row.code).toBe('HOOK-B'); }); @@ -142,9 +147,15 @@ describe('insert strip acts on CALLER-submitted values (#6339)', () => { // `title: 'no-hook'` makes the hook return without writing, so the caller's // value is the value on the key — and it goes, exactly as before. The // sequence issues the number instead. + // + // ⚠️ The sequence's first number is `'0001'`, not `'1'`, since #6555/#7262: + // `code` declares no format, so it renders through the contract default + // `{0000}`. Do not "fix" this back to a bare `'1'` — that spelling was the + // engine-vs-driver-sql fork the ruling closed. What this case is about is + // the STRIP, not the width. const row: any = await engine.insert('probe_num2', { title: 'no-hook', code: 'CALLER-FORGED' }); expect(row.code).not.toBe('CALLER-FORGED'); - expect(row.code).toBe('1'); + expect(row.code).toBe('0001'); expect(warns).toHaveLength(1); // The contract of the text, not its wording (#5503's own pin discipline). expect(warns[0]).toContain("Field 'code' on 'probe_num2'"); @@ -211,8 +222,11 @@ describe('insert strip acts on CALLER-submitted values (#6339)', () => { ]); expect(rows[0].code).toBe('HOOK-r1'); expect(rows[1].code).toBe('HOOK-r2'); - expect(rows[2].code).toBe('1'); - expect(rows[3].code).toBe('2'); + // ⚠️ `'0001'` / `'0002'`, not bare — the format-less contract default + // `{0000}` (#6555/#7262). The counter, which is what this case pins, still + // runs 1 then 2 across the two sequence-issued rows. + expect(rows[2].code).toBe('0001'); + expect(rows[3].code).toBe('0002'); // Exactly one row was stripped, so exactly one warning. expect(warns).toHaveLength(1); expect(warns[0]).toContain("Field 'code'"); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index f43be49db7..8802688e81 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -25,7 +25,7 @@ import type { WriteObservabilityOptions } from '@objectstack/spec/contracts'; // engine is what `metadata-protocol.validateData` returns, so letting the two // drift would put a translation layer between a verdict and its contract. import type { ValidateDataIssue, ValidateDataResponse } from '@objectstack/spec/api'; -import { parseAutonumberFormat, renderAutonumber, readAutonumberCounter, missingFieldValues, isTenancyDisabled, FILE_REFERENCE_TYPES, REFERENCE_VALUE_TYPES, referenceTargetOf, isFileIdToken, RAW_FILE_VALUES_CONTEXT_KEY, isCurrentUserDefaultToken, isNowDefaultToken } from '@objectstack/spec/data'; +import { parseAutonumberFormat, renderAutonumber, resolveAutonumberFormat, readAutonumberCounter, missingFieldValues, isTenancyDisabled, FILE_REFERENCE_TYPES, REFERENCE_VALUE_TYPES, referenceTargetOf, isFileIdToken, RAW_FILE_VALUES_CONTEXT_KEY, isCurrentUserDefaultToken, isNowDefaultToken } from '@objectstack/spec/data'; // [#5158] Door 2's lowering sink — the SAME pair the protocol face (Door 1) // runs, so `FilterArray` has exactly one lowering in the product. import { isFilterAST, parseFilterAST, VALID_AST_OPERATORS } from '@objectstack/spec/data'; @@ -2601,6 +2601,37 @@ export class ObjectQL implements IObjectQLEngine { * to the SQL driver's persistent sequence (#1603). NOTE: this in-memory seeding * is single-instance. * + * # A format-LESS field renders through the contract default, not bare (#6555) + * + * "Identically to the SQL driver" was true of every DECLARED format and false + * of the one case nobody declares: with no format at all this method used to + * hand `parseAutonumberFormat` the empty string, whose empty token list + * `renderAutonumber` renders through its no-slot branch as a BARE counter — + * `1`, `2`, …. The SQL driver substituted `'{0000}'` in the same case and + * issued `0001`, `0002`, …. One metadata document, two number shapes, decided + * by which driver happened to serve it; the counter VALUE always agreed + * (#6468 pinned that), so what forked was rendering width alone. + * + * The maintainer ruling of 2026-08-08 (#6555, route 3) puts the default in the + * contract instead of in either fallback, and this method now reads it: + * {@link resolveAutonumberFormat} answers the canonical `autonumberFormat`, + * then the `format` shorthand (#1603), then `DEFAULT_AUTONUMBER_FORMAT` + * (`{0000}`). Two consequences, both deliberate: + * + * - A format-less field on this path now issues `0001` where it issued `1`. + * `{0000}` was chosen because it is the shape SQL deployments already + * store, so the flip lands on engine-fallback deployments only. + * - "Declared" is now a NON-EMPTY string (the SQL driver's long-standing + * truthiness rule) rather than the `??` this method used, so + * `autonumberFormat: ''` resolves to the default too instead of rendering + * bare — and an empty canonical key no longer masks a declared `format` + * shorthand. The bare counter is still reachable, by declaring a format + * with no `{0..0}` slot (`'PRE-'` → `PRE-1`). + * + * Seeding is untouched by all of this: `{0000}` renders prefix `''` and suffix + * `''`, so {@link readStoredAutonumberCounter} stays on its UNANCHORED legacy + * branch and reads already-stored bare values exactly as before. + * * # Keeping the seeded counter in sync (#6806) * * Seeding "once per counter key" is only the truth while the engine is the @@ -2649,10 +2680,14 @@ export class ObjectQL implements IObjectQLEngine { const issued: IssuedAutonumber[] = []; for (const [name, def] of Object.entries(fields)) { if ((def as any)?.type !== 'autonumber') continue; - // Honor either the spec-canonical `autonumberFormat` or the shorthand - // `format` (both appear in metadata; the driver reads both too) — #1603. - const fmt = (def as any).autonumberFormat ?? (def as any).format; - const tokens = parseAutonumberFormat(typeof fmt === 'string' ? fmt : ''); + // The contract answers "which format?" — canonical `autonumberFormat`, + // then the `format` shorthand (#1603), then the declared default + // `{0000}`. One resolver, shared with the SQL driver, so a format-less + // field cannot render two shapes again (#6555; see the docstring above). + // `fmt` is kept only for the diagnostic below, which now names the format + // actually rendered with rather than `undefined`. + const fmt = resolveAutonumberFormat(def as never); + const tokens = parseAutonumberFormat(fmt); const current = record[name]; // Respect an explicit value — reachable only for an EXEMPT writer now // (isSystem / preserveAudit / a hook stamp): #5503's strip removed every diff --git a/packages/runtime/src/autonumber-seed-cross-side-parity.integration.test.ts b/packages/runtime/src/autonumber-seed-cross-side-parity.integration.test.ts index 44f6ab7020..e306050068 100644 --- a/packages/runtime/src/autonumber-seed-cross-side-parity.integration.test.ts +++ b/packages/runtime/src/autonumber-seed-cross-side-parity.integration.test.ts @@ -24,6 +24,20 @@ * answer. `packages/objectql` and `packages/drivers/driver-sql` each pin their * own half; only this package can see both at once. * + * ## The format-LESS fixture (#6555) — a second fork, in RENDERING + * + * #6468's four fixtures all DECLARE a format, so the two arms agreed on the + * number's width by construction and only the counter was ever at stake. The + * fifth fixture declares none, which is where the sides forked a second time: + * the engine parsed the empty string and rendered `renderAutonumber`'s no-slot + * branch (`11`), `driver-sql` substituted its own `'{0000}'` (`0011`). The + * maintainer's route-3 ruling on #6555 put that default in the contract + * (`DEFAULT_AUTONUMBER_FORMAT` / `resolveAutonumberFormat`, #7265) and both + * generators now read it — driver-sql in #7263, the engine in #7262. That + * fixture is therefore what makes THIS file assert a shared rendering rather + * than only a shared counter, and it is the only case here whose two arms + * disagreed on shape. + * * The engine half runs on the REAL `InMemoryDriver` (`supports = {}`, so the * engine's fallback owns the counter — the shape memory/mongo deployments run) * and the driver half on a REAL `SqlDriver` over better-sqlite3, each holding @@ -83,13 +97,18 @@ import { SqlDriver } from '@objectstack/driver-sql'; /** Date tokens render from the wall clock, so the clock is pinned. Only `Date`. */ const FIXED_NOW = new Date('2026-06-15T09:00:00Z'); -/** One object, one text field, one autonumber field carrying `format`. */ -function recSchema(format: string) { +/** + * One object, one text field, one autonumber field. `format` is omitted from the + * declaration entirely when it is `undefined` — a field carrying `format: + * undefined` is not the same document as a field with no `format` key, and the + * format-LESS case below is exactly what #6555 is about. + */ +function recSchema(format?: string) { return { name: 'rec', fields: { title: { type: 'text' }, - rec_no: { type: 'autonumber', format }, + rec_no: format === undefined ? { type: 'autonumber' } : { type: 'autonumber', format }, }, } as any; } @@ -98,7 +117,7 @@ const legacyRows = (stored: string[]) => stored.map((v, i) => ({ id: `l${i + 1}`, rec_no: v, title: `legacy ${i + 1}` })); /** The number the ENGINE's fallback seeding issues after `stored`. */ -async function engineIssues(format: string, stored: string[]): Promise { +async function engineIssues(format: string | undefined, stored: string[]): Promise { const driver = new InMemoryDriver(); const engine = new ObjectQL(); engine.registerDriver(driver as any, true); @@ -114,7 +133,7 @@ async function engineIssues(format: string, stored: string[]): Promise { } /** The number the SQL DRIVER's sequence bootstrap issues after `stored`. */ -async function sqlDriverIssues(format: string, stored: string[]): Promise { +async function sqlDriverIssues(format: string | undefined, stored: string[]): Promise { const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, @@ -130,7 +149,8 @@ async function sqlDriverIssues(format: string, stored: string[]): Promise {