From a2a20eb395794d014d2f324dabf8d858c2f707d8 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Thu, 18 Jun 2026 14:52:20 +0800 Subject: [PATCH] fix(driver-sql): self-heal numeric type fidelity on legacy TEXT columns + extend dogfood matrix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #2025 column-affinity fix only governs newly created columns: SQLite never alters a column's type in place and the reconciler only adds missing columns, so a rating/slider/progress column created before the fix keeps TEXT affinity and still reads back '4' not 4. - add a read-side numeric coercion (numericFields registry, single- sourced from NUMERIC_SCALAR_TYPES) that coerces numeric-looking stored strings back to numbers on read — mirroring the dateFields legacy repair — so fidelity no longer depends on column affinity alone; null and non-numeric junk are preserved (not 0/NaN) - unit test: reproduce a legacy TEXT column and prove it self-heals - extend the dogfood HTTP matrix to guard progress/record/video/audio over real HTTP (previously only driver-unit-tested) Co-Authored-By: Claude Opus 4.8 --- .changeset/field-type-fidelity-legacy.md | 9 +++ .../test/field-zoo-roundtrip.dogfood.test.ts | 6 ++ .../src/sql-driver-numeric-fidelity.test.ts | 81 +++++++++++++++++++ packages/plugins/driver-sql/src/sql-driver.ts | 47 +++++++++++ 4 files changed, 143 insertions(+) create mode 100644 .changeset/field-type-fidelity-legacy.md diff --git a/.changeset/field-type-fidelity-legacy.md b/.changeset/field-type-fidelity-legacy.md new file mode 100644 index 0000000000..ed194e863e --- /dev/null +++ b/.changeset/field-type-fidelity-legacy.md @@ -0,0 +1,9 @@ +--- +"@objectstack/driver-sql": patch +--- + +fix(driver-sql): make numeric-scalar type fidelity self-heal on legacy SQLite columns + +The #2025 fix mapped `rating`/`slider`/`progress` to numeric columns, but SQLite never alters a column's type in place and the schema reconciler only adds missing columns — so a column created before that fix keeps its TEXT affinity and would still read back `'4'` instead of `4` forever. + +A read-side numeric coercion (the new `numericFields` registry, single-sourced from `NUMERIC_SCALAR_TYPES`) now coerces numeric-looking stored strings back to numbers on read, mirroring how `dateFields` already repairs legacy timestamp-typed `Field.date` rows. The fidelity no longer depends on column affinity alone; `null` and genuinely non-numeric legacy values are left intact rather than turned into `0`/`NaN`. diff --git a/packages/dogfood/test/field-zoo-roundtrip.dogfood.test.ts b/packages/dogfood/test/field-zoo-roundtrip.dogfood.test.ts index 21bf2ecb05..d1b178baac 100644 --- a/packages/dogfood/test/field-zoo-roundtrip.dogfood.test.ts +++ b/packages/dogfood/test/field-zoo-roundtrip.dogfood.test.ts @@ -62,9 +62,15 @@ const MATRIX: FieldCase[] = [ { field: 'f_multiselect', type: 'multiselect', check: { kind: 'setEqual', write: ['red', 'blue'] } }, { field: 'f_checkboxes', type: 'checkboxes', check: { kind: 'setEqual', write: ['email', 'push'] } }, { field: 'f_tags', type: 'tags', check: { kind: 'setEqual', write: ['alpha', 'beta', 'gamma'] } }, + // numeric scalar — same fidelity class as rating/slider (was TEXT-affinity) + { field: 'f_progress', type: 'progress', check: { kind: 'equal', write: 60 } }, // structured JSON { field: 'f_json', type: 'json', check: { kind: 'equal', write: { a: 1, b: [2, 3] } } }, { field: 'f_color', type: 'color', check: { kind: 'equal', write: '#FF8800' } }, + // object-valued types that must store/parse as JSON, not stringify to TEXT + { field: 'f_record', type: 'record', check: { kind: 'equal', write: { home: '+1', work: '+2' } } }, + { field: 'f_video', type: 'video', check: { kind: 'equal', write: { url: 'https://cdn/v.mp4', duration: 12 } } }, + { field: 'f_audio', type: 'audio', check: { kind: 'equal', write: { url: 'https://cdn/a.mp3', duration: 30 } } }, // computed / system — not written, must materialize { field: 'f_autonumber', type: 'autonumber', check: { kind: 'present' } }, // f_number(42) * f_percent(75) / 100 = 31.5 diff --git a/packages/plugins/driver-sql/src/sql-driver-numeric-fidelity.test.ts b/packages/plugins/driver-sql/src/sql-driver-numeric-fidelity.test.ts index fee8ead6ca..f062807260 100644 --- a/packages/plugins/driver-sql/src/sql-driver-numeric-fidelity.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-numeric-fidelity.test.ts @@ -128,3 +128,84 @@ describe('SqlDriver scalar type fidelity (rating/slider/toggle/progress)', () => expect(row.f_audio).toEqual({ url: 'https://cdn/a.mp3', duration: 30 }); }); }); + +/** + * The column-affinity fix only governs NEWLY created columns; SQLite never + * alters a column's type in place, and the schema reconciler only adds missing + * columns. So a `rating`/`slider`/`progress` column created BEFORE the fix keeps + * its TEXT affinity and would still hand back '4' forever. The `numericFields` + * read coercion is what makes the fix retroactive — this reproduces the legacy + * column (pre-create it as TEXT, then initObjects, which must NOT alter it) and + * proves the stored string still reads back as a number. + */ +describe('SqlDriver numeric read coercion repairs legacy TEXT columns', () => { + let driver: SqlDriver; + + beforeEach(async () => { + driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + + // Simulate a pre-fix database: the table already exists with TEXT-affinity + // columns for the numeric scalars (what the old `default → table.string` + // mapping produced). + const knex = (driver as unknown as { knex: import('knex').Knex }).knex; + await knex.schema.createTable('legacy', (t) => { + t.string('id').primary(); + t.text('name'); + t.text('f_rating'); // legacy TEXT affinity + t.text('f_slider'); + t.text('f_progress'); + }); + + // initObjects sees the existing table and must leave the columns as TEXT + // (it only adds missing columns) — but it still registers numericFields. + await driver.initObjects([ + { + name: 'legacy', + fields: { + name: { type: 'string' }, + f_rating: { type: 'rating', max: 5 }, + f_slider: { type: 'slider', min: 0, max: 100 }, + f_progress: { type: 'progress', min: 0, max: 100 }, + }, + }, + ]); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + it('coerces stored strings back to numbers on read', async () => { + // Write through the driver — on a TEXT column SQLite stores the number as + // the string '4'/'25'/'60' (exactly the pre-fix leak). + await driver.create( + 'legacy', + { id: 'L1', name: 'old-row', f_rating: 4, f_slider: 25, f_progress: 60 }, + { bypassTenantAudit: true }, + ); + const row = await driver.findOne('legacy', 'L1', { bypassTenantAudit: true }); + + expect(typeof row.f_rating).toBe('number'); + expect(row.f_rating).toBe(4); + expect(typeof row.f_slider).toBe('number'); + expect(row.f_slider).toBe(25); + expect(typeof row.f_progress).toBe('number'); + expect(row.f_progress).toBe(60); + }); + + it('leaves null and genuinely non-numeric legacy values intact', async () => { + const knex = (driver as unknown as { knex: import('knex').Knex }).knex; + // Hand-write a row with a null and a non-numeric string straight to the + // TEXT columns, bypassing the driver, to model messy legacy data. + await knex('legacy').insert({ id: 'L2', name: 'messy', f_rating: null, f_slider: 'n/a', f_progress: '60' }); + const row = await driver.findOne('legacy', 'L2', { bypassTenantAudit: true }); + + expect(row.f_rating).toBeNull(); // null stays null, not 0 + expect(row.f_slider).toBe('n/a'); // non-numeric junk is preserved, not NaN + expect(row.f_progress).toBe(60); // numeric-looking string is repaired + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index 5711596c5c..17f5689119 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -52,6 +52,27 @@ const JSON_COLUMN_TYPES = new Set([ 'multiselect', 'checkboxes', 'tags', 'repeater', 'vector', ]); +/** + * Field types whose value is a numeric scalar. SINGLE SOURCE for the DDL + * column-type switch (these map to INTEGER/REAL columns) and the read-side + * coercion registry (`numericFields`). + * + * The read coercion exists so the fix is robust on SQLite even when the column + * predates it: a `rating`/`slider`/`progress` column created before #2025 has + * TEXT affinity and returns '4' not 4, and SQLite never alters a column's type + * in-place (the reconciler only ADDS columns). Coercing numeric-looking strings + * back to numbers on read transparently repairs those legacy rows — mirroring + * how `dateFields` repairs legacy timestamp-typed `Field.date` rows — so the + * type fidelity no longer depends on column affinity alone. `toggle`/`record` + * already self-heal this way via `booleanFields`/`jsonFields`; this closes the + * gap for the numeric scalars. + */ +const NUMERIC_SCALAR_TYPES = new Set([ + 'integer', 'int', + 'float', 'number', 'currency', 'percent', 'summary', + 'rating', 'slider', 'progress', +]); + // ── Introspection Types ────────────────────────────────────────────────────── export interface IntrospectedColumn { @@ -172,6 +193,7 @@ export class SqlDriver implements IDataDriver { protected config: Knex.Config; protected jsonFields: Record = {}; protected booleanFields: Record = {}; + protected numericFields: Record = {}; protected dateFields: Record> = {}; protected datetimeFields: Record> = {}; protected tablesWithTimestamps: Set = new Set(); @@ -1090,6 +1112,7 @@ export class SqlDriver implements IDataDriver { const jsonCols: string[] = []; const booleanCols: string[] = []; + const numericCols: string[] = []; const autoNumberCols: Array<{ name: string; format: string; prefix: string; padWidth: number; tenantField: string | null }> = []; // Auto-detect tenant field. Convention: the field named // `organization_id` (matching tenantPolicy default) scopes the @@ -1122,6 +1145,12 @@ export class SqlDriver implements IDataDriver { if (type === 'boolean' || type === 'toggle') { booleanCols.push(name); } + // Numeric scalars are coerced back to JS numbers on read so legacy + // TEXT-affinity columns (created before they were mapped to a numeric + // column) still return numbers, not strings — see NUMERIC_SCALAR_TYPES. + if (NUMERIC_SCALAR_TYPES.has(type) && !field.multiple) { + numericCols.push(name); + } if (type === 'date') { (this.dateFields[tableName] ??= new Set()).add(name); } @@ -1144,6 +1173,7 @@ export class SqlDriver implements IDataDriver { } this.jsonFields[tableName] = jsonCols; this.booleanFields[tableName] = booleanCols; + this.numericFields[tableName] = numericCols; this.autoNumberFields[tableName] = autoNumberCols; this.tenantFieldByTable[tableName] = tenantField; @@ -2102,6 +2132,23 @@ export class SqlDriver implements IDataDriver { } } } + + // Numeric scalars stored on a legacy TEXT-affinity column come back as + // strings ('4'); coerce numeric-looking strings back to numbers so the + // declared type wins regardless of when the column was created. Only + // touch strings — a fresh REAL/INTEGER column already yields a number, + // and a genuinely non-numeric value (junk legacy data) is left intact + // rather than turned into NaN. See NUMERIC_SCALAR_TYPES. + const numericFields = this.numericFields[object]; + if (numericFields && numericFields.length > 0) { + for (const field of numericFields) { + const v = data[field]; + if (typeof v === 'string' && v.trim() !== '') { + const n = Number(v); + if (!Number.isNaN(n)) data[field] = n; + } + } + } } // ADR-0053 Phase 1: present `Field.date` as a timezone-naive `YYYY-MM-DD`