Skip to content

Commit 5553ee4

Browse files
committed
test(spec): widen the readers of conversions/registry.ts to its per-entry modules (#29987)
The ADR-0087 registration gate, the affected-docs self-test, the ADR anchor and the major-18 merge test read the split registry with its entries. Claude-Session: https://claude.ai/code/session_01WGufzg1QFj9ukYzafVoAqg Co-authored-by: Claude <noreply@anthropic.com>
1 parent f1f2ed0 commit 5553ee4

6 files changed

Lines changed: 171 additions & 34 deletions

‎packages/spec/scripts/conversions-major18-merge.test.ts‎

Lines changed: 52 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,14 @@
2121
// end-appended list, the tail-appended definitions and the old array's tail are
2222
// the lit controls — each MUST conflict, or a clean result would be equally
2323
// explained by a harness that cannot see a conflict at all.
24+
//
25+
// [#29987 codemod] Since the codemod split (`scripts/codemod-split-conversions-registry.mjs`)
26+
// each conversion is DEFINED in a module of its own, `src/conversions/entries/18/<id>.ts`,
27+
// which a retirement adds as a new file and which therefore never conflicts. What a
28+
// retirement still edits in `registry.ts` is two lines: the import of its conversion and
29+
// its entry in the list. The imports stand in the order the definitions stood, so the
30+
// placement rule reads them where it read the definitions: an import is a definition's
31+
// place in this file, and the module it names is read for the definition itself.
2432

2533
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
2634
import { spawnSync } from 'node:child_process';
@@ -46,10 +54,14 @@ const CLOSE = '];\n';
4654
const ENTRY = / \{ conversion: ([A-Za-z_$][\w$]*), order: (\d+(?:\.\d+)?) \},\n/y;
4755
/** A blank line — which is what a comment between entries is once `maskComments` has blanked it; it belongs to the entry below it. */
4856
const BLANK = /[ \t]*\n/y;
49-
/** A conversion definition and its id, which every definition in the file spells on the next line. */
50-
const DEFINITION = /^(?:export )?const ([A-Za-z_$][\w$]*): MetadataConversion = \{\n id: '([^'\n]+)',$/gm;
51-
/** Where a conversion that sorts last is defined: directly above this declaration's doc comment. */
52-
const TAIL_ANCHOR = '\ninterface OrderedConversion {\n';
57+
/**
58+
* A conversion's place in the file: its import, from the module named for its id
59+
* (`./entries/<toMajor>/<id>.js`). The module itself is read for the definition
60+
* ({@link definitionInModule}).
61+
*/
62+
const DEFINITION = /^import \{ ([A-Za-z_$][\w$]*) \} from '\.\/entries\/18\/([^'\n]+)\.js';$/gm;
63+
/** The module a conversion is defined in, from its id. */
64+
const MODULE_PATH = (id: string): string => path.resolve(HERE, `../src/conversions/entries/18/${id}.ts`);
5365
/** How `CONVERSIONS_BY_MAJOR` reads the entries (the annotation: see that table's docblock). */
5466
const WIRING = ' 18: /* @__PURE__ */ inApplicationOrder(MAJOR_18_CONVERSIONS),\n';
5567

@@ -128,11 +140,26 @@ function entriesOf(source: string): { entries: Entry[]; end: number } {
128140
return { entries, end: at };
129141
}
130142

131-
/** Every conversion definition in the file, in file order — a commented-out one is not a definition. */
143+
/** Every conversion's import in the file, in file order — a commented-out one is not an import. */
132144
function definitionsOf(source: string): { ident: string; id: string; at: number }[] {
133145
return [...maskOnce(source).matchAll(DEFINITION)].map((m) => ({ ident: m[1]!, id: m[2]!, at: m.index! }));
134146
}
135147

148+
/** Where a conversion that sorts last is imported: directly after every other conversion's import. */
149+
function definitionsTail(source: string): number {
150+
const defs = definitionsOf(source);
151+
expect(defs.length, 'the conversions\' imports').toBeGreaterThan(0);
152+
const last = defs.at(-1)!;
153+
return source.indexOf('\n', last.at) + 1;
154+
}
155+
156+
/** The definition in a conversion's module: `const IDENT: MetadataConversion = {` with its id on the next line, exported by name. */
157+
function definitionInModule(ident: string, id: string): boolean {
158+
const text = maskComments(readFileSync(MODULE_PATH(id), 'utf8'));
159+
return text.includes(`\nconst ${ident}: MetadataConversion = {\n id: '${id}',\n toMajor: 18,\n`)
160+
&& text.endsWith(`\nexport { ${ident} };\n`);
161+
}
162+
136163
/** Rule 1, as findings: the entries are sorted strictly by identifier. */
137164
function sortFindings(source: string): string[] {
138165
const idents = entriesOf(source).entries.map((e) => e.ident);
@@ -147,13 +174,13 @@ function placementFindings(source: string): string[] {
147174
return entries.flatMap((e, i) => {
148175
if (PLACED_BEFORE_THE_RULE.has(e.ident)) return [];
149176
const at = index.get(e.ident);
150-
if (at === undefined) return [`${e.ident}: no \`const ${e.ident}: MetadataConversion\` definition in the file`];
177+
if (at === undefined) return [`${e.ident}: no import of \`${e.ident}\` from its module in the file`];
151178
const want = entries[i + 1]?.ident;
152179
const next = defs[at + 1]?.ident;
153180
if (next === want) return [];
154181
return [
155-
`${e.ident} is defined above ${next ?? 'nothing (it is the last conversion defined)'}; define it directly above `
156-
+ (want ? `${want}'s definition, the entry that follows it` : 'OrderedConversion, after every other conversion'),
182+
`${e.ident} is imported above ${next ?? 'nothing (it is the last conversion imported)'}; import it directly above `
183+
+ (want ? `${want}'s import, the entry that follows it` : 'nothing, after every other conversion\'s import'),
157184
];
158185
});
159186
}
@@ -165,17 +192,20 @@ function docStart(source: string, offset: number): number {
165192
}
166193

167194
const kebab = (ident: string): string => `synthetic-${ident.replace(/[A-Z]/g, (c) => `-${c.toLowerCase()}`)}`;
168-
const definition = (ident: string): string => `/** A synthetic major-18 retirement. */\nconst ${ident}: MetadataConversion = {\n`
195+
/** A retirement's line in `registry.ts`: the import of its conversion (the module is a new file, so it never conflicts). */
196+
const definition = (ident: string): string => `import { ${ident} } from './entries/18/${kebab(ident)}.js';\n`;
197+
/** The pre-split shape, for the old-shape control only: the definition itself, in the one file. */
198+
const legacyDefinition = (ident: string): string => `/** A synthetic major-18 retirement. */\nconst ${ident}: MetadataConversion = {\n`
169199
+ ` id: '${kebab(ident)}',\n toMajor: 18,\n surface: 'synthetic.key',\n summary: 'synthetic',\n`
170200
+ ' apply: (stack) => stack,\n fixture: { before: {}, after: {}, expectedNotices: 0 },\n};\n\n';
171201
const entry = (ident: string, order: number): string => ` { conversion: ${ident}, order: ${order} },\n`;
172202

173-
/** Offset of the definition the doc comment asks for: above the next entry's, or above `OrderedConversion`. */
203+
/** Offset of the import the doc comment asks for: above the next entry's, or after every other conversion's. */
174204
function definitionSite(source: string, ident: string): number {
175205
const next = entriesOf(source).entries.find((e) => e.ident > ident);
176-
if (!next) return docStart(source, source.indexOf(TAIL_ANCHOR) + 1);
206+
if (!next) return definitionsTail(source);
177207
const def = definitionsOf(source).find((d) => d.ident === next.ident);
178-
expect(def, `${next.ident}'s definition`).toBeDefined();
208+
expect(def, `${next.ident}'s import`).toBeDefined();
179209
return docStart(source, def!.at);
180210
}
181211

@@ -198,9 +228,9 @@ function retireAtListEnd(source: string, ident: string, order: number): string {
198228
return insert(withDefinition, entriesOf(withDefinition).end, entry(ident, order));
199229
}
200230

201-
/** The definition half done the old way: defined after every other conversion (the entry still sorted). */
231+
/** The definition half done the old way: imported after every other conversion (the entry still sorted). */
202232
function retireAtDefinitionsEnd(source: string, ident: string, order: number): string {
203-
const tail = docStart(source, source.indexOf(TAIL_ANCHOR) + 1);
233+
const tail = definitionsTail(source);
204234
return sortedEntry(insert(source, tail, definition(ident)), ident, order);
205235
}
206236

@@ -302,9 +332,10 @@ describe('major 18 in the conversions registry — the shape the merge needs (#2
302332
).toEqual([]);
303333
});
304334

305-
it('each entry is a major-18 conversion defined in this file, and the list is all of them', () => {
335+
it('each entry is a major-18 conversion imported from the module that defines it, and the list is all of them', () => {
306336
const ids = new Map(definitionsOf(SOURCE).map((d) => [d.ident, d.id]));
307337
expect(IDENTS.filter((i) => !ids.has(i))).toEqual([]);
338+
expect(IDENTS.filter((i) => !definitionInModule(i, ids.get(i)!)), 'each module defines its conversion under its id').toEqual([]);
308339
expect(new Set(IDENTS.map((i) => ids.get(i)))).toEqual(new Set(APPLIED));
309340
expect(APPLIED).toHaveLength(IDENTS.length);
310341
expect(CONVERSIONS_BY_MAJOR[18]!.every((c) => c.toMajor === 18)).toBe(true);
@@ -321,7 +352,7 @@ describe('major 18 in the conversions registry — the shape the merge needs (#2
321352
expect(MIGRATIONS_BY_MAJOR[18]!.conversionIds).toEqual(order);
322353
});
323354

324-
it('a conversion added after the rule is defined directly above the entry that follows it', () => {
355+
it('a conversion added after the rule is imported directly above the entry that follows it', () => {
325356
expect(PLACED_BEFORE_THE_RULE.size, '⛔ the exemption is closed: place a new conversion by the rule').toBe(46);
326357
expect(placementFindings(SOURCE)).toEqual([]);
327358
});
@@ -376,9 +407,9 @@ describe('major 18 in the conversions registry — two retirements, no merge dri
376407
expect(placementFindings(retireAtDefinitionsEnd(SOURCE, A, NEXT_ORDER))).toEqual([
377408
...(PLACED_BEFORE_THE_RULE.has(neighbour)
378409
? []
379-
: [`${neighbour} is defined above ${IDENTS[k]}; define it directly above ${A}'s definition, the entry that follows it`]),
380-
`${A} is defined above nothing (it is the last conversion defined); define it directly above ${IDENTS[k]}'s `
381-
+ 'definition, the entry that follows it',
410+
: [`${neighbour} is imported above ${IDENTS[k]}; import it directly above ${A}'s import, the entry that follows it`]),
411+
`${A} is imported above nothing (it is the last conversion imported); import it directly above ${IDENTS[k]}'s `
412+
+ 'import, the entry that follows it',
382413
]);
383414
});
384415

@@ -408,7 +439,7 @@ describe('major 18 in the conversions registry — two retirements, no merge dri
408439
expect(m.conflicted).toEqual([REL]);
409440
});
410441

411-
it('the same two conversions defined after every other conversion conflict — the definitions have a tail too', () => {
442+
it('the same two conversions imported after every other conversion conflict — the imports have a tail too', () => {
412443
const m = repo.merge('definitions-end-a', 'definitions-end-b');
413444
expect(m.status).toBe(1);
414445
expect(m.conflicted).toEqual([REL]);
@@ -419,7 +450,7 @@ describe('major 18 in the conversions registry — two retirements, no merge dri
419450
+ " id: 'view-list-tabs-removed',\n};\n\n"
420451
+ 'export const CONVERSIONS_BY_MAJOR = {\n 18: [\n viewListTabsRemoved,\n ],\n};\n';
421452
const append = (ident: string) => old
422-
.replace('export const CONVERSIONS_BY_MAJOR', `${definition(ident)}export const CONVERSIONS_BY_MAJOR`)
453+
.replace('export const CONVERSIONS_BY_MAJOR', `${legacyDefinition(ident)}export const CONVERSIONS_BY_MAJOR`)
423454
.replace(' viewListTabsRemoved,\n', ` viewListTabsRemoved,\n ${ident},\n`);
424455
const legacy = new FileRepo(old);
425456
try {
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"file": "packages/spec/src/conversions/entries/18/declared-index-unique-scope.ts",
3+
"adrs": [
4+
"ADR-0120"
5+
],
6+
"invariant": "`declared-index-unique-scope` (ADR-0120 D2, toMajor 18) rewrites a DECLARED index's bare `unique: true` to `'global'` — and nothing else: field-level `unique: true` is never converted (D1: it means per-organization and stays valid), and `false` / `'global'` / `'organization'` pass through untouched. It is lossless only because `'global'` IS the verbatim column list bare `true` built — the S4/S5 nine-key corpus is pinned byte-identical before and after in driver-sql's `sql-driver-unique-tenancy.test.ts`. ⛔ Never re-target it at `'organization'` (the #4986 trap in mirror image, a silent physical change) and never take it off `retiredFromLoadPath`: the authoring funnel must REFUSE the bare spelling so the author states the scope, while data-at-rest seams replay it."
7+
}

‎scripts/adr-anchors/packages__spec__src__conversions__registry.ts.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
{
22
"file": "packages/spec/src/conversions/registry.ts",
33
"adrs": [
4-
"ADR-0120",
54
"ADR-0087"
65
],
76
"invariant": "`declared-index-unique-scope` (ADR-0120 D2, toMajor 18) rewrites a DECLARED index's bare `unique: true` to `'global'` — and nothing else: field-level `unique: true` is never converted (D1: it means per-organization and stays valid), and `false` / `'global'` / `'organization'` pass through untouched. It is lossless only because `'global'` IS the verbatim column list bare `true` built — the S4/S5 nine-key corpus is pinned byte-identical before and after in driver-sql's `sql-driver-unique-tenancy.test.ts`. ⛔ Never re-target it at `'organization'` (the #4986 trap in mirror image, a silent physical change) and never take it off `retiredFromLoadPath`: the authoring funnel must REFUSE the bare spelling so the author states the scope, while data-at-rest seams replay it."

‎scripts/check-adr-0087-registration.mjs‎

Lines changed: 81 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,7 @@ const SELF_TEST_BATTERIES = Object.freeze({
475475
'V3: the real repository is the case that matters -- this gate\'s own vocabulary': 10,
476476
'W1-W6 (#22485): the parser-rot witness is GENERATED at head, never read from a committed copy': 9,
477477
'GR1-GR7 (#22554): the migration ledger is GENERATED at build, read from its template and entry files': 13,
478+
'CR1-CR3 (#29987): the conversions ledger is split per entry, read from registry.ts and its modules': 3,
478479
'Unit pins on the two pattern-shaped judgements': 29,
479480
'the floors: what the new vocabulary must refuse': 9,
480481
'the floors: labels that are NOT mentions, and mentions that ARE evidenced': 9,
@@ -497,7 +498,7 @@ const SELF_TEST_BATTERIES = Object.freeze({
497498

498499
// DELETING an entry silences that battery's floor exactly as effectively as
499500
// zeroing it, so the roster's own size is pinned too.
500-
const SELF_TEST_BATTERY_FLOOR = 59;
501+
const SELF_TEST_BATTERY_FLOOR = 60;
501502

502503
// The key an assertion is filed under when no battery is open. It is not a
503504
// declared battery, so it reds by the same set difference rather than silently
@@ -531,6 +532,16 @@ export const MIGRATION_LEDGER_ENTRY_DIRS = [
531532
'packages/spec/src/migrations/entries/retired-defs',
532533
];
533534

535+
/**
536+
* Where `LEDGER_SOURCES[1]`'s conversions live since the #29987 codemod split
537+
* (`scripts/codemod-split-conversions-registry.mjs`): one module per conversion
538+
* under this directory, plus the shared helpers. `registry.ts` itself still
539+
* exists and lists them, but no conversion's `id:` line is in it any more. A rev
540+
* that tracks no `.ts` file here is a rev from before the split, and the
541+
* registry is the whole text. See `conversionLedgerTextAt`.
542+
*/
543+
export const CONVERSION_LEDGER_ENTRY_DIR = 'packages/spec/src/conversions/entries';
544+
534545
/**
535546
* The generator whose output proves the source parser still matches (#4690), and
536547
* the inputs it reads, relative to the tree root. Since #22485 there is no
@@ -2341,7 +2352,7 @@ export function ledgerAt(rev, cwd) {
23412352
const bySource = {};
23422353
const missingSources = [];
23432354
for (const path of LEDGER_SOURCES) {
2344-
const src = path === LEDGER_SOURCES[0] ? migrationLedgerTextAt(rev, cwd) : showOrNull(rev, path, cwd);
2355+
const src = path === LEDGER_SOURCES[0] ? migrationLedgerTextAt(rev, cwd) : conversionLedgerTextAt(rev, cwd);
23452356
if (src === null) { missingSources.push(path); continue; }
23462357
const found = extractIds(src);
23472358
bySource[path] = found.length;
@@ -2381,6 +2392,33 @@ export function migrationLedgerTextAt(rev, cwd) {
23812392
return [template, ...entries.map((p) => texts.get(p))].join('\n');
23822393
}
23832394

2395+
/**
2396+
* The text of the conversions ledger at a rev: `registry.ts` followed by every
2397+
* `.ts` module under `CONVERSION_LEDGER_ENTRY_DIR` (sorted), which `extractIds`
2398+
* reads as a set. Before the #29987 split no module exists there and the
2399+
* registry is the whole text; after it, the registry's own text carries no id.
2400+
* `null` when the rev has no registry: a red in `assertInputs`.
2401+
*
2402+
* Exported for the self-test's CR battery.
2403+
*
2404+
* @returns {string | null}
2405+
*/
2406+
export function conversionLedgerTextAt(rev, cwd) {
2407+
const registry = showOrNull(rev, LEDGER_SOURCES[1], cwd);
2408+
if (registry === null) return null;
2409+
let listed;
2410+
try {
2411+
listed = git(['ls-tree', '-r', '--name-only', rev, '--', CONVERSION_LEDGER_ENTRY_DIR], cwd);
2412+
} catch {
2413+
return null;
2414+
}
2415+
const entries = listed.split('\n').map((p) => p.trim()).filter((p) => p.endsWith('.ts')).sort();
2416+
if (entries.length === 0) return registry;
2417+
const texts = showManyOrNull(rev, entries, cwd);
2418+
if (texts.size !== entries.length) return null;
2419+
return [registry, ...entries.map((p) => texts.get(p))].join('\n');
2420+
}
2421+
23842422
// ---------------------------------------------------------------------------
23852423
// Input assertions -- absence is never a pass (#4690)
23862424
// ---------------------------------------------------------------------------
@@ -5837,7 +5875,7 @@ function ledgerTouchingCommits(path, head, cwd) {
58375875
try { names = git(['show', '--name-only', '--format=', sha], cwd); } catch { continue; }
58385876
// The generated era (#22554): a registration commit adds an entry file and
58395877
// touches no registry, so the ledger's two committed sources count too.
5840-
if ([...LEDGER_SOURCES, MIGRATION_LEDGER_TEMPLATE, ...MIGRATION_LEDGER_ENTRY_DIRS].some((src) => names.includes(src))) hits.push(sha);
5878+
if ([...LEDGER_SOURCES, MIGRATION_LEDGER_TEMPLATE, ...MIGRATION_LEDGER_ENTRY_DIRS, CONVERSION_LEDGER_ENTRY_DIR].some((src) => names.includes(src))) hits.push(sha);
58415879
}
58425880
return { available: true, shas: hits };
58435881
}
@@ -8871,6 +8909,46 @@ function selfTest() {
88718909
red('GR7 transition: an entry carried over from the committed era is not new', transition('alpha'), [/none of those ids is NEW in this diff/]);
88728910
}
88738911

8912+
// ---- CR1-CR3 (#29987): the conversions ledger is split per entry ----------
8913+
//
8914+
// After the codemod split, `registry.ts` imports and lists the conversions and
8915+
// each conversion's `id:` line lives in its own module under
8916+
// `CONVERSION_LEDGER_ENTRY_DIR`. A ledger read off `registry.ts` alone would
8917+
// find ZERO ids there -- every `registered` claim on a conversion would fail.
8918+
battery('CR1-CR3 (#29987): the conversions ledger is split per entry, read from registry.ts and its modules');
8919+
{
8920+
const splitRepo = (files) => {
8921+
const dir = mkdtempSync(join(tmpdir(), 'adr0087-conversions-split-'));
8922+
cleanup.push(dir);
8923+
const w = (rel, text) => { mkdirSync(dirname(join(dir, rel)), { recursive: true }); writeFileSync(join(dir, rel), text); };
8924+
git(['init', '-q', '-b', 'main'], dir);
8925+
git(['config', 'user.email', 't@t'], dir);
8926+
git(['config', 'user.name', 't'], dir);
8927+
for (const [rel, text] of Object.entries(files)) w(rel, text);
8928+
git(['add', '-A'], dir);
8929+
git(['commit', '-qm', 'split'], dir);
8930+
return dir;
8931+
};
8932+
const HOST = "import { a } from './entries/18/conv-a.js';\nexport const ALL = [a];\n";
8933+
const MODULE = (id) => `const a = {\n id: '${id}',\n toMajor: 18,\n};\n\nexport { a };\n`;
8934+
const c1 = splitRepo({
8935+
[LEDGER_SOURCES[1]]: HOST,
8936+
[`${CONVERSION_LEDGER_ENTRY_DIR}/18/conv-a.ts`]: MODULE('conv-a'),
8937+
[`${CONVERSION_LEDGER_ENTRY_DIR}/README.md`]: " id: 'readme-example',\n",
8938+
});
8939+
// CR1 control: the host alone carries no id, so a green below is the modules'.
8940+
assert(extractIds(showOrNull('HEAD', LEDGER_SOURCES[1], c1)).length === 0, 'CR1 control: the split host must carry no id line');
8941+
const t1 = conversionLedgerTextAt('HEAD', c1);
8942+
assert(t1 !== null && extractIds(t1).join() === 'conv-a', `CR1: the split ledger reads the module's id -- got ${JSON.stringify(t1 && extractIds(t1))}`);
8943+
const l1 = ledgerAt('HEAD', c1);
8944+
assert(l1.ids.has('conv-a') && l1.bySource[LEDGER_SOURCES[1]] === 1, `CR1: ledgerAt reads the split ledger -- got ${JSON.stringify(l1.bySource)}`);
8945+
// CR2: a non-`.ts` file under the entries directory is not read.
8946+
assert(!extractIds(t1 ?? '').includes('readme-example'), 'CR2: an id-shaped line in a non-.ts file under entries/ must not be read');
8947+
// CR3: before the split (no module under entries/) the registry is the whole text.
8948+
const c3 = splitRepo({ [LEDGER_SOURCES[1]]: MODULE('pre-split') });
8949+
assert(conversionLedgerTextAt('HEAD', c3) === MODULE('pre-split'), 'CR3: an unsplit rev reads registry.ts alone');
8950+
}
8951+
88748952
// ---- Unit pins on the two pattern-shaped judgements ----------------------
88758953
//
88768954
// P1-P3 are the LABEL branch, unchanged since #6148 -- the template placeholder

0 commit comments

Comments
 (0)