Skip to content

Commit 431979e

Browse files
os-trumpclaude
andauthored
fix(cli): give every FieldType member a generate.ts vocabulary entry, and make the next gap loud (#14834)
* fix(cli): map every FieldType member in generate.ts, and make the next gap loud Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza * test(cli): make the vocabulary pin name the cause when the satisfies guard is removed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza * chore(runtime): classify the CLI field-type `code` entry as a foreign vocabulary Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 79af704 commit 431979e

4 files changed

Lines changed: 330 additions & 19 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
fix(cli): `os generate` now has an answer for every field type, instead of silently guessing
6+
7+
Three hand-kept vocabularies in `generate.ts` decide what `os generate types`
8+
and `os generate migration` emit for a field: the TypeScript type, the SQL
9+
column type, and the knex builder call. None of them was ever checked against
10+
the `FieldType` enum they describe, and measured against the 49 members on
11+
`main`, **21 real members had no entry in either lookup table and 24 had no arm
12+
in the migration switch**.
13+
14+
An unmapped member did not fail — it fell to the default. So a `secret` field
15+
scaffolded as TypeScript `unknown` and a `TEXT` column, a `location` as
16+
`unknown` and `TEXT`, and `address` / `composite` / `repeater` / `record` — all
17+
four stored as JSON on the parent row — as scalar `TEXT` columns. The output
18+
looked plausible and nothing said otherwise, which is what made this worth
19+
fixing rather than tidying.
20+
21+
All 49 members now have an entry in all three, and the values are read off the
22+
platform rather than invented: the spec's ADR-0104 D1 value classes
23+
(`STRING_VALUE_TYPES`, `NUMERIC_VALUE_TYPES`, `STRUCTURED_JSON_TYPES`, …) decide
24+
the class, and `driver-sql`'s own DDL emitter — which creates the real columns —
25+
decides the shape. `location` becomes a JSON column, not a `POINT`: that is what
26+
the driver does, `POINT` is not portable to SQLite, and the spec's own value
27+
contract for it is `{lat, lng, altitude?, accuracy?}`. `location` and `address`
28+
now emit the spec's exported `Data.LocationValue` / `Data.AddressValue` types,
29+
so the generated interface cannot drift from the value contract.
30+
31+
The gap can no longer reopen quietly. Both lookup tables are
32+
`satisfies Record<FieldType, string>`, so a field type added to the spec is a
33+
named compile error here; the switch — whose scrutinee is a plain string off an
34+
unvalidated config and so cannot carry one — is held by
35+
`generate-field-type-vocabulary.pin.test.ts`, which walks the real enum and
36+
names any member left unmapped.
37+
38+
The runtime fallbacks (`|| 'unknown'`, `|| 'TEXT'`, `default:`) are unchanged
39+
and still reachable: they answer a `type` string that is not a field type at
40+
all, which the unvalidated authoring door can still deliver.

packages/cli/src/commands/generate-field-type-vocabulary.pin.test.ts

Lines changed: 112 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,42 @@
4343
* accepts, and an AI or a human reading this switch to learn the field types
4444
* would learn four that do not exist.
4545
*
46-
* ## What this pin asserts, and what it deliberately does NOT
46+
* ## What this pin asserts
4747
*
48-
* FORWARD ONLY: every token the three vocabularies key on is a `FieldType`
49-
* member. The converse is NOT asserted — plenty of real members (`secret`,
50-
* `address`, `location`, `code`, `tags`, …) have no entry and fall to the
51-
* `default` arm / the `|| fallback`, and that fallback is deliberate. Demanding
52-
* total coverage would be a different card with a different decision behind it
53-
* (what column type each unmapped member deserves), and this pin is written so
54-
* it does not prejudge that.
48+
* BOTH DIRECTIONS, since #14657.
49+
*
50+
* FORWARD (#13871): every token the three vocabularies key on is a `FieldType`
51+
* member.
52+
*
53+
* BACKWARD (#14657): every `FieldType` member is keyed on by all three. #13871
54+
* deliberately did not assert this, because "what column type does each
55+
* unmapped member deserve" was an open question; #14657 answered it member by
56+
* member and this half became assertable. It matters because the gap was
57+
* SILENT: 21 real members had no entry in either map (24 in the switch), and
58+
* every one of them generated a plausible-looking wrong schema — TS `unknown`,
59+
* a `TEXT` / `table.text` column — with nothing to tell the author. `secret`
60+
* and `location` were among them.
61+
*
62+
* The two lookup tables carry the same rule a second time as
63+
* `satisfies Record<FieldType, string>`, which makes a missing member a named
64+
* `tsc` error (`packages/cli` type-checks `src/**`) as well as a red test. That
65+
* annotation is itself pinned below: the extractor here REQUIRES it as each
66+
* table's terminator, so deleting it cannot quietly demote the type-level half
67+
* to nothing. The `switch` cannot carry a `satisfies` — its scrutinee is a
68+
* plain `string` off an unvalidated config — so for that vocabulary this file
69+
* is the only mechanism, which is why the totality assertion lives here rather
70+
* than being left to the compiler.
71+
*
72+
* ⚠️ What is NOT asserted, and why the difference is the point: that a mapping
73+
* is CORRECT. This pin measures presence, not the value — a wrong-but-present
74+
* entry is a different defect (`autonumber: 'SERIAL'` against a runtime that
75+
* writes a rendered string, `formula` given a column the runtime never
76+
* creates), filed separately rather than pinned here on a guess.
77+
*
78+
* The runtime fallbacks (`|| 'unknown'`, `|| 'TEXT'`, `default:`) stay and are
79+
* NOT dead: they answer a `type` string that is not a `FieldType` at all, which
80+
* the UNVALIDATED authoring door still delivers. Totality is over the enum, not
81+
* over every string that can reach the generator.
5582
*
5683
* The `FieldType` side is imported, never transcribed: a list written out here
5784
* would just relocate the drift into this file. And the vocabularies are read
@@ -88,13 +115,36 @@ function lookupTableNames(): string[] {
88115
return [...SOURCE.matchAll(LOOKUP_TABLE_DECL)].map((m) => m[1]);
89116
}
90117

118+
/**
119+
* The terminator every lookup table must carry — the type-level half of the
120+
* #14657 totality rule. Required rather than tolerated: if someone deletes the
121+
* annotation, extraction fails loudly here instead of the compiler silently
122+
* stopping to check.
123+
*/
124+
const TABLE_TERMINATOR = '} satisfies Record<FieldType, string>;';
125+
91126
/** The keys of one top-level `Record<string, string>` table, in source order. */
92127
function lookupTableKeys(name: string): string[] {
93128
const declaration = `const ${name}: Record<string, string> = {`;
94129
const start = SOURCE.indexOf(declaration);
95130
if (start < 0) throw new Error(`lookup table not found in generate.ts: ${name}`);
96-
const end = SOURCE.indexOf('\n};', start);
97-
if (end < 0) throw new Error(`unterminated lookup table in generate.ts: ${name}`);
131+
// Bound the table at ITS OWN closing line — the first line starting with `}`
132+
// after the declaration — and then require that line to be the terminator.
133+
// Searching for the terminator directly would silently run past a table
134+
// whose annotation was deleted and swallow the NEXT table's body, turning a
135+
// removed guard into a wrong measurement instead of a named failure.
136+
const closing = SOURCE.slice(start).search(/\n\}/);
137+
if (closing < 0) throw new Error(`unterminated lookup table in generate.ts: ${name}`);
138+
const end = start + closing;
139+
const closingLine = SOURCE.slice(end + 1, SOURCE.indexOf('\n', end + 1));
140+
if (closingLine !== TABLE_TERMINATOR) {
141+
throw new Error(
142+
`${name} in generate.ts must be closed by \`${TABLE_TERMINATOR}\`, but it is closed by ` +
143+
`\`${closingLine}\`. That annotation is the type-level half of the #14657 rule that every ` +
144+
'FieldType member has an entry: without it, adding a field type to the spec stops being a ' +
145+
'compile error here and goes back to silently generating `unknown` / a TEXT column.',
146+
);
147+
}
98148
const body = SOURCE.slice(start + declaration.length, end);
99149
return [...body.matchAll(/^ {2}([A-Za-z_][\w]*):/gm)].map((m) => m[1]);
100150
}
@@ -147,4 +197,56 @@ describe('generate.ts field-type vocabularies (#13871)', () => {
147197
const ghosts = labels.filter((l) => !REAL_FIELD_TYPES.has(l));
148198
expect(ghosts, 'the field-type switch cases on types that are not FieldType members').toEqual([]);
149199
});
200+
201+
// ── The #14657 half: no real member may go unmapped ──────────────────────
202+
//
203+
// Read this as one rule stated three times, not three rules: the authority is
204+
// `FieldType`, and each vocabulary is measured against it. A member added to
205+
// the spec with no answer here used to produce TS `unknown` and a `TEXT`
206+
// column in silence; it now names itself in a failing assertion.
207+
208+
const VOCABULARIES: ReadonlyArray<readonly [string, () => string[]]> = [
209+
['FIELD_TYPE_MAP (os generate types)', () => lookupTableKeys('FIELD_TYPE_MAP')],
210+
['FIELD_TYPE_SQL_MAP (os generate migration --format sql)', () => lookupTableKeys('FIELD_TYPE_SQL_MAP')],
211+
['the migration switch (os generate migration, typescript)', migrationSwitchLabels],
212+
];
213+
214+
for (const [label, read] of VOCABULARIES) {
215+
it(`${label} covers every FieldType member`, () => {
216+
const covered = new Set(read());
217+
// Non-vacuity: the same control the forward assertions buy. An extractor
218+
// that returned nothing would make "everything is missing" the finding,
219+
// not a silent pass — but state it anyway so the failure is legible.
220+
expect(covered.size).toBeGreaterThan(20);
221+
222+
const unmapped = [...REAL_FIELD_TYPES].filter((t) => !covered.has(t));
223+
expect(
224+
unmapped,
225+
`${label} has no entry for these real FieldType members, so each one silently ` +
226+
'takes the generator default (TS `unknown` / a TEXT column). Add an entry — or, ' +
227+
'if the default is genuinely the right answer for it, say so with an explicit ' +
228+
'entry that spells the default out, so the decision is written down rather than ' +
229+
'left as an absence.',
230+
).toEqual([]);
231+
});
232+
}
233+
234+
it('the two lookup tables carry the type-level totality annotation', () => {
235+
// The runtime half above and the compile-time half must both be present:
236+
// `tsc` names a missing member at build time, this file names it in CI even
237+
// if the annotation is loosened. `lookupTableKeys` throws without it, so
238+
// this assertion is the readable statement of a rule already enforced.
239+
for (const table of ['FIELD_TYPE_MAP', 'FIELD_TYPE_SQL_MAP'] as const) {
240+
expect(
241+
SOURCE.includes(`const ${table}: Record<string, string> = {`),
242+
`${table} declaration moved`,
243+
).toBe(true);
244+
expect(() => lookupTableKeys(table)).not.toThrow();
245+
}
246+
expect(
247+
SOURCE.match(/^\} satisfies Record<FieldType, string>;$/gm),
248+
'both FIELD_TYPE_MAP and FIELD_TYPE_SQL_MAP must close with the satisfies annotation ' +
249+
'that makes an unmapped FieldType member a compile error',
250+
).toHaveLength(2);
251+
});
150252
});

0 commit comments

Comments
 (0)