Skip to content

Commit f379f57

Browse files
refactor(spec): major 18's conversions as identifier-sorted entries with an explicit application order, so two retirements merge clean (#20574) (#20685)
Fixes #20574 Clause-②: no Every major-18 retirement that carries a D2 conversion touched two tails of `packages/spec/src/conversions/registry.ts`: it appended its conversion to the end of `CONVERSIONS_BY_MAJOR[18]`, and most of them defined the conversion at the end of the definitions, directly above that table. So any two such retirement PRs in flight conflicted in GitHub's driver-free merge. This PR reshapes both spots so that two retirements no longer insert into the same gap. Nothing a consumer reads changes: the list the loader replays is value-identical. This is the sibling of #20535 (PR #20572), which reshaped `step18.rationale` in `migrations/registry.ts`. It uses the same instrument. ## What changed - **`CONVERSIONS_BY_MAJOR[18]` is `inApplicationOrder(MAJOR_18_CONVERSIONS)`.** `MAJOR_18_CONVERSIONS` holds one `{ conversion, order }` entry per conversion. The list is kept **sorted by the conversion's identifier** (code-unit order). `order` is the application order: `inApplicationOrder` sorts by ascending `order`, ties broken by the conversion's `id`, and returns the plain `readonly MetadataConversion[]` it always was. Orders 1 to 46 are the 46 entries' old positions, so the replayed sequence is unchanged. - **The definitions' insertion point, the other shared tail.** I measured it. On `origin/main` `9a4b2bb38f`, 57 of the last 80 first-parent commits that touched this file added a conversion definition. Together they added 66 definitions, and 42 of them were added after every other definition, in that one gap. The list's doc comment now says where a new definition goes: directly above the definition of the entry that follows it in `MAJOR_18_CONVERSIONS`. A conversion that sorts last goes after every other conversion. Two retirements in different list gaps therefore also define their conversions in different gaps. No existing definition moved. - `OrderedConversion` and `inApplicationOrder` are module-private, and `CONVERSIONS_BY_MAJOR` keeps its explicit type annotation. `api-surface/` and `export-origins/` are unchanged, and `check:api-surface` is green. The released majors (11 to 17) stay plain arrays. The next major takes the same shape when it opens. - Consumers are untouched. `ALL_CONVERSIONS`, step 18's `conversionIds` (`CONVERSIONS_BY_MAJOR[18]!.map((c) => c.id)`, pinned verbatim by the #20572 test), `spec-changes.ts` and `build-upgrade-guide.ts` all read `CONVERSIONS_BY_MAJOR[18]` as before. No generated projection moved: `check:generated` reports 15 of 15 up to date and regenerated nothing. ## Why sorted by identifier, with an explicit order Git reports a conflict whenever two branches insert into the **same gap** between unchanged lines, whatever they insert. A list appended at its end is one gap. An explicit `order` alone would not change that: the end-append lit control below conflicts with the order key present. Kept sorted by identifier, two retirements insert into different gaps, and one existing entry between them is enough. The application order cannot depend on where an insertion lands, so it lives in `order`. Two PRs in flight may both take the next number. They then apply in `id` order, which is deterministic whatever the textual positions are. ## Verification record ### Lit controls on the parent (`c6b37cd08d`) This was a one-shot run with git 2.43.0 in a scratch repo that held the real file, with no attributes and no driver. Each side made the edit a major-18 retirement makes: | pair | `git merge-tree --write-tree` | |---|---| | two list-tail appends (a new identifier after `viewListTabsRemoved,`) | **exit 1**, CONFLICT (content) in `registry.ts` | | two definitions added directly above `export const CONVERSIONS_BY_MAJOR` | **exit 1**, CONFLICT (content) | | both edits on each side | **exit 1**, CONFLICT (content) | ### Dark control and the permanent pin The pin is `packages/spec/scripts/conversions-major18-merge.test.ts`. It is in the repo project beside `step18-rationale-merge.test.ts`, and it works against the REAL file. It is a test, not a new gate. It asserts: - **The premise.** `CONVERSIONS_BY_MAJOR` wires `18: inApplicationOrder(MAJOR_18_CONVERSIONS)`, with no `18: [` array. The entries are strictly sorted by identifier. Each entry names a major-18 conversion defined in the file, and the list is all of them. `order` is positive, and the application order parsed from the entries (by `order`, ties by `id`) equals `CONVERSIONS_BY_MAJOR[18]`, the tail of `ALL_CONVERSIONS`, and `MIGRATIONS_BY_MAJOR[18].conversionIds`. The anti-vacuity check: that order differs from key order. A conversion added after this change must be defined directly above its list successor. The 46 entries that predate the rule are exempt, and that set is closed: its size is pinned. - **The card's reproduction, now clean.** Two retirement-shaped edits, each adding a definition above its successor's and an entry where its identifier sorts, one existing entry apart, both taking the same next `order`: **exit 0**. The merged bytes equal both edits applied together. The merged file still satisfies both rules, and its application order is the old 46 followed by the two new ids in `id` order. - **Lit controls**, all **exit 1** with conflicted path `registry.ts`: - two identifiers in the same gap, the residue a key sort cannot remove; - the same two entries appended at the list's END; - the same two conversions defined after every other conversion; - a synthetic model of the old array-plus-tail shape. The pin also proves that its two rule checks fire. The end-appended side breaks the sort rule, and the tail-defined side breaks the placement rule with the expected message. ### Byte-identical replay | value | parent `c6b37cd08d` | after the restructure | |---|---|---| | `CONVERSIONS_BY_MAJOR[18]` ids | 46, sha256 `9e9666c56a5e6314…` | identical | | majors 11, 13, 14, 15, 17 ids | 4 / 3 / 1 / 2 / 57 | identical, each | | `ALL_CONVERSIONS` ids | 113, `c0dee67fbecf705e…` | identical | | every conversion's body (JSON with function source) | `fbba8d0066fb121c…` | identical | | `MIGRATIONS_BY_MAJOR[18].conversionIds` | `9e9666c56a5e6314…` | identical | | the whole `MIGRATIONS_BY_MAJOR` value as JSON | `72f0c698a5bfcc09…` | identical | The two fingerprint files, from tsx over `src` at `c6b37cd08d` and at `ed1c54db8d` (the restructure commit), are byte-identical. After merging `origin/main` `9a4b2bb38f`, which carries #20305's change to one conversion's body, I compared head `63af2cb5f6` with main's plain arrays, read from main's source text. Every major's id sequence, `ALL_CONVERSIONS` (113) and step 18's `conversionIds` are identical. The built CJS and ESM bundles (`dist/index.{js,mjs}` and `dist/browser/index.{js,mjs}`) all load with the same id hashes. This branch's delta to `registry.ts` against main has the same `git patch-id --stable` as the restructure commit's own diff. ### Ablations (one-shot, at `d316f458a6`, registry blob `f9d797be1e80`) Each ablation went through `scripts/ablation-replace.mjs`, with the anchor proven to hit (1 → 0). Each was restored to the HEAD blob with `git diff HEAD` empty. 1. Replacing the `order` sort in `inApplicationOrder` with `.slice()` (apply in list position) turned the pin **red**: 2 failed, 10 passed. The two failures were the application-order assertion and the merged-order assertion. 2. Swapping the first two entries turned the pin **red**: 3 failed, 9 passed. The failures were the sortedness assertion and the two assertions that need a sorted base. The observed direction was the usual one: the pin turns red. No build or dist leg was needed, because the pin imports `../src` directly and reads the source text. ## Tests and gates (final commit `63af2cb5f6`) - `@objectstack/spec` `local` project: 575 files, 16,946 passed, 1 todo. `repo` project: 44 files, 773 passed. `pnpm --filter @objectstack/spec typecheck`: exit 0. That covers `tsc`, `check:scripts-typecheck` (the pin is in that program) and `check:test-typecheck`. All ran through `os-verify-lock` with `--maxWorkers=2` on a shared box. - `check:generated`: 15 of 15 up to date, against the `dist` built at this head. `check:migration-registry`: exit 0. - `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` was reconciled with `--ran`: 84 derived, **80 exit 0**, 4 NOT MEASURED, 0 unrun. The 4 exited 3 with `PREREQUISITE NOT MET` because they need the whole-repo build closure, which CI builds: `check:doc-formula-expressions`, `check:dual-build-cjs-loads`, `check:lean-entry-closure` and `check:type-check-debt`. - The first gate round, at `b4f0179259`, caught `check:comment-mask-adoption` red. It flagged the pin's own line-comment regex as a private comment stripper. Commit `0000e4ab13` fixed it: the pin now reads the list and the definitions through `scripts/js-comment-mask.mjs`'s `maskComments`. - Declared to CI: - the CLI integration tier (this diff touches no CLI file); - the tests in other packages that read `ALL_CONVERSIONS` (`metadata-core`, `metadata-protocol`, `metadata`, `service-automation`). The public surface is byte-unchanged, and the values are proven identical above. - Repo-level `pnpm lint` is CI-owned. - Changeset: `@objectstack/spec` `patch`. I measured this rather than assumed it. After the build, `inApplicationOrder` appears in 6 files under `dist/` and `MAJOR_18_CONVERSIONS` in 8, with the positive control `view-list-tabs-removed` also present. So the published bundle moves, while no value it exports does. ## How a retirement adds its conversion once this lands 1. Define the conversion directly above the definition of the entry that will follow it in `MAJOR_18_CONVERSIONS`. If it sorts last, define it after every other conversion, directly above `OrderedConversion`. 2. Add ONE line, `{ conversion: IDENT, order: N }`, where `IDENT` sorts. Never add it at the end. 3. `N` is one more than the highest present. A conversion that must apply before an existing one takes a number between its neighbours' (for example `23.5`) instead of renumbering them. A branch cut before this lands meets the change once, on its next base merge: its appended `18: [` line becomes one entry at its sorted position. ## Acceptance notes - **Governed wording, routed to the skills lane (not edited here).** `.claude/skills/spec-property-retirement/SKILL.md`'s registration checklist tells a retirement author to add a `MetadataConversion` in this file. For major 18 that now means the entry-and-placement rule above. #20575 already carries the step-18 rewording of the same checklist, and its body asks to fold this sibling's wording in at the same time. - **Measured residue, not reshaped here.** 8 of the same 80 commits also changed the file's import block (its first 60 lines). Two retirements that each add an import at the same point would still conflict there. The import block is outside the two spots this card names, and the frequency is about one in ten. - **Same-gap residue.** Two identifiers that sort into the same gap still conflict. That is pinned as a lit control, as in #20572. - The pin copies the small hermetic `FileRepo` harness from `step18-rationale-merge.test.ts` rather than extracting a shared helper. That keeps the #20572 pin untouched. - The sortedness and placement checks are assertions in a repo-project test, not a `check:*` gate. They are what make an end-append, or a tail definition, fail loudly instead of quietly bringing the conflict back. --- _Generated by [Claude Code](https://claude.ai/code/session_014EJ1ED8X4MMrT18BhVx4tx)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 14f80e2 commit f379f57

4 files changed

Lines changed: 553 additions & 52 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
refactor(spec): protocol 18's conversions are authored as identifier-sorted entries with an explicit application order — no value changes (#20574)
6+
7+
Nothing a consumer reads changes. `CONVERSIONS_BY_MAJOR[18]` (46 conversions, same
8+
order), `ALL_CONVERSIONS` (113, same order) and `MIGRATIONS_BY_MAJOR[18].conversionIds`
9+
are value-identical to the previous release, so the loader and the migration chain
10+
apply the same conversions in the same sequence.
11+
12+
What changed is how the list is written, so two major-18 retirements can be in flight
13+
at once without conflicting in `packages/spec/src/conversions/registry.ts`:
14+
15+
- `CONVERSIONS_BY_MAJOR[18]` is read off `MAJOR_18_CONVERSIONS`: one
16+
`{ conversion, order }` entry per conversion, kept sorted by the conversion's
17+
identifier and applied by ascending `order` (ties by the conversion's `id`). A
18+
retirement adds ONE entry where its identifier sorts — never at the end — with
19+
`order` one more than the highest present.
20+
- A new conversion is defined directly above the definition of the entry that follows
21+
it in that list, not at the end of the definitions.

0 commit comments

Comments
 (0)