Skip to content

Commit d9508d1

Browse files
os-zhuangclaude
andauthored
fix(driver-sql): self-heal numeric type fidelity on legacy TEXT columns + extend dogfood matrix (#2028)
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 <noreply@anthropic.com>
1 parent 311bcc6 commit d9508d1

4 files changed

Lines changed: 143 additions & 0 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@objectstack/driver-sql": patch
3+
---
4+
5+
fix(driver-sql): make numeric-scalar type fidelity self-heal on legacy SQLite columns
6+
7+
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.
8+
9+
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`.

packages/dogfood/test/field-zoo-roundtrip.dogfood.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,15 @@ const MATRIX: FieldCase[] = [
6262
{ field: 'f_multiselect', type: 'multiselect', check: { kind: 'setEqual', write: ['red', 'blue'] } },
6363
{ field: 'f_checkboxes', type: 'checkboxes', check: { kind: 'setEqual', write: ['email', 'push'] } },
6464
{ field: 'f_tags', type: 'tags', check: { kind: 'setEqual', write: ['alpha', 'beta', 'gamma'] } },
65+
// numeric scalar — same fidelity class as rating/slider (was TEXT-affinity)
66+
{ field: 'f_progress', type: 'progress', check: { kind: 'equal', write: 60 } },
6567
// structured JSON
6668
{ field: 'f_json', type: 'json', check: { kind: 'equal', write: { a: 1, b: [2, 3] } } },
6769
{ field: 'f_color', type: 'color', check: { kind: 'equal', write: '#FF8800' } },
70+
// object-valued types that must store/parse as JSON, not stringify to TEXT
71+
{ field: 'f_record', type: 'record', check: { kind: 'equal', write: { home: '+1', work: '+2' } } },
72+
{ field: 'f_video', type: 'video', check: { kind: 'equal', write: { url: 'https://cdn/v.mp4', duration: 12 } } },
73+
{ field: 'f_audio', type: 'audio', check: { kind: 'equal', write: { url: 'https://cdn/a.mp3', duration: 30 } } },
6874
// computed / system — not written, must materialize
6975
{ field: 'f_autonumber', type: 'autonumber', check: { kind: 'present' } },
7076
// f_number(42) * f_percent(75) / 100 = 31.5

packages/plugins/driver-sql/src/sql-driver-numeric-fidelity.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,3 +128,84 @@ describe('SqlDriver scalar type fidelity (rating/slider/toggle/progress)', () =>
128128
expect(row.f_audio).toEqual({ url: 'https://cdn/a.mp3', duration: 30 });
129129
});
130130
});
131+
132+
/**
133+
* The column-affinity fix only governs NEWLY created columns; SQLite never
134+
* alters a column's type in place, and the schema reconciler only adds missing
135+
* columns. So a `rating`/`slider`/`progress` column created BEFORE the fix keeps
136+
* its TEXT affinity and would still hand back '4' forever. The `numericFields`
137+
* read coercion is what makes the fix retroactive — this reproduces the legacy
138+
* column (pre-create it as TEXT, then initObjects, which must NOT alter it) and
139+
* proves the stored string still reads back as a number.
140+
*/
141+
describe('SqlDriver numeric read coercion repairs legacy TEXT columns', () => {
142+
let driver: SqlDriver;
143+
144+
beforeEach(async () => {
145+
driver = new SqlDriver({
146+
client: 'better-sqlite3',
147+
connection: { filename: ':memory:' },
148+
useNullAsDefault: true,
149+
});
150+
151+
// Simulate a pre-fix database: the table already exists with TEXT-affinity
152+
// columns for the numeric scalars (what the old `default → table.string`
153+
// mapping produced).
154+
const knex = (driver as unknown as { knex: import('knex').Knex }).knex;
155+
await knex.schema.createTable('legacy', (t) => {
156+
t.string('id').primary();
157+
t.text('name');
158+
t.text('f_rating'); // legacy TEXT affinity
159+
t.text('f_slider');
160+
t.text('f_progress');
161+
});
162+
163+
// initObjects sees the existing table and must leave the columns as TEXT
164+
// (it only adds missing columns) — but it still registers numericFields.
165+
await driver.initObjects([
166+
{
167+
name: 'legacy',
168+
fields: {
169+
name: { type: 'string' },
170+
f_rating: { type: 'rating', max: 5 },
171+
f_slider: { type: 'slider', min: 0, max: 100 },
172+
f_progress: { type: 'progress', min: 0, max: 100 },
173+
},
174+
},
175+
]);
176+
});
177+
178+
afterEach(async () => {
179+
await driver.disconnect();
180+
});
181+
182+
it('coerces stored strings back to numbers on read', async () => {
183+
// Write through the driver — on a TEXT column SQLite stores the number as
184+
// the string '4'/'25'/'60' (exactly the pre-fix leak).
185+
await driver.create(
186+
'legacy',
187+
{ id: 'L1', name: 'old-row', f_rating: 4, f_slider: 25, f_progress: 60 },
188+
{ bypassTenantAudit: true },
189+
);
190+
const row = await driver.findOne('legacy', 'L1', { bypassTenantAudit: true });
191+
192+
expect(typeof row.f_rating).toBe('number');
193+
expect(row.f_rating).toBe(4);
194+
expect(typeof row.f_slider).toBe('number');
195+
expect(row.f_slider).toBe(25);
196+
expect(typeof row.f_progress).toBe('number');
197+
expect(row.f_progress).toBe(60);
198+
});
199+
200+
it('leaves null and genuinely non-numeric legacy values intact', async () => {
201+
const knex = (driver as unknown as { knex: import('knex').Knex }).knex;
202+
// Hand-write a row with a null and a non-numeric string straight to the
203+
// TEXT columns, bypassing the driver, to model messy legacy data.
204+
await knex('legacy').insert({ id: 'L2', name: 'messy', f_rating: null, f_slider: 'n/a', f_progress: '60' });
205+
const row = await driver.findOne('legacy', 'L2', { bypassTenantAudit: true });
206+
207+
expect(row.f_rating).toBeNull(); // null stays null, not 0
208+
expect(row.f_slider).toBe('n/a'); // non-numeric junk is preserved, not NaN
209+
expect(row.f_progress).toBe(60); // numeric-looking string is repaired
210+
});
211+
});

packages/plugins/driver-sql/src/sql-driver.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,27 @@ const JSON_COLUMN_TYPES = new Set<string>([
5252
'multiselect', 'checkboxes', 'tags', 'repeater', 'vector',
5353
]);
5454

55+
/**
56+
* Field types whose value is a numeric scalar. SINGLE SOURCE for the DDL
57+
* column-type switch (these map to INTEGER/REAL columns) and the read-side
58+
* coercion registry (`numericFields`).
59+
*
60+
* The read coercion exists so the fix is robust on SQLite even when the column
61+
* predates it: a `rating`/`slider`/`progress` column created before #2025 has
62+
* TEXT affinity and returns '4' not 4, and SQLite never alters a column's type
63+
* in-place (the reconciler only ADDS columns). Coercing numeric-looking strings
64+
* back to numbers on read transparently repairs those legacy rows — mirroring
65+
* how `dateFields` repairs legacy timestamp-typed `Field.date` rows — so the
66+
* type fidelity no longer depends on column affinity alone. `toggle`/`record`
67+
* already self-heal this way via `booleanFields`/`jsonFields`; this closes the
68+
* gap for the numeric scalars.
69+
*/
70+
const NUMERIC_SCALAR_TYPES = new Set<string>([
71+
'integer', 'int',
72+
'float', 'number', 'currency', 'percent', 'summary',
73+
'rating', 'slider', 'progress',
74+
]);
75+
5576
// ── Introspection Types ──────────────────────────────────────────────────────
5677

5778
export interface IntrospectedColumn {
@@ -172,6 +193,7 @@ export class SqlDriver implements IDataDriver {
172193
protected config: Knex.Config;
173194
protected jsonFields: Record<string, string[]> = {};
174195
protected booleanFields: Record<string, string[]> = {};
196+
protected numericFields: Record<string, string[]> = {};
175197
protected dateFields: Record<string, Set<string>> = {};
176198
protected datetimeFields: Record<string, Set<string>> = {};
177199
protected tablesWithTimestamps: Set<string> = new Set();
@@ -1090,6 +1112,7 @@ export class SqlDriver implements IDataDriver {
10901112

10911113
const jsonCols: string[] = [];
10921114
const booleanCols: string[] = [];
1115+
const numericCols: string[] = [];
10931116
const autoNumberCols: Array<{ name: string; format: string; prefix: string; padWidth: number; tenantField: string | null }> = [];
10941117
// Auto-detect tenant field. Convention: the field named
10951118
// `organization_id` (matching tenantPolicy default) scopes the
@@ -1122,6 +1145,12 @@ export class SqlDriver implements IDataDriver {
11221145
if (type === 'boolean' || type === 'toggle') {
11231146
booleanCols.push(name);
11241147
}
1148+
// Numeric scalars are coerced back to JS numbers on read so legacy
1149+
// TEXT-affinity columns (created before they were mapped to a numeric
1150+
// column) still return numbers, not strings — see NUMERIC_SCALAR_TYPES.
1151+
if (NUMERIC_SCALAR_TYPES.has(type) && !field.multiple) {
1152+
numericCols.push(name);
1153+
}
11251154
if (type === 'date') {
11261155
(this.dateFields[tableName] ??= new Set()).add(name);
11271156
}
@@ -1144,6 +1173,7 @@ export class SqlDriver implements IDataDriver {
11441173
}
11451174
this.jsonFields[tableName] = jsonCols;
11461175
this.booleanFields[tableName] = booleanCols;
1176+
this.numericFields[tableName] = numericCols;
11471177
this.autoNumberFields[tableName] = autoNumberCols;
11481178
this.tenantFieldByTable[tableName] = tenantField;
11491179

@@ -2102,6 +2132,23 @@ export class SqlDriver implements IDataDriver {
21022132
}
21032133
}
21042134
}
2135+
2136+
// Numeric scalars stored on a legacy TEXT-affinity column come back as
2137+
// strings ('4'); coerce numeric-looking strings back to numbers so the
2138+
// declared type wins regardless of when the column was created. Only
2139+
// touch strings — a fresh REAL/INTEGER column already yields a number,
2140+
// and a genuinely non-numeric value (junk legacy data) is left intact
2141+
// rather than turned into NaN. See NUMERIC_SCALAR_TYPES.
2142+
const numericFields = this.numericFields[object];
2143+
if (numericFields && numericFields.length > 0) {
2144+
for (const field of numericFields) {
2145+
const v = data[field];
2146+
if (typeof v === 'string' && v.trim() !== '') {
2147+
const n = Number(v);
2148+
if (!Number.isNaN(n)) data[field] = n;
2149+
}
2150+
}
2151+
}
21052152
}
21062153

21072154
// ADR-0053 Phase 1: present `Field.date` as a timezone-naive `YYYY-MM-DD`

0 commit comments

Comments
 (0)