diff --git a/.changeset/9088-rest-less-tuple-identity.md b/.changeset/9088-rest-less-tuple-identity.md new file mode 100644 index 0000000000..2c8cc8457f --- /dev/null +++ b/.changeset/9088-rest-less-tuple-identity.md @@ -0,0 +1,32 @@ +--- +'@object-ui/types': patch +--- + +`stripImportedDefaults` no longer rebuilds a rest-less tuple that had nothing to +strip (objectui#9088). + +The walker documents an identity property about itself: a node is rebuilt ONLY if +the walk actually changed something beneath it, so a subtree with no `ZodDefault` +in it comes back **reference-equal** to `@objectstack/spec`'s own object. That is +decision batch #90's reversibility argument made literal — it is why option A was +taken over option B — and it was false for every tuple with no rest element. + +Zod 4 spells "no rest element" as an OWN `rest` key holding `null`, not as an +absent key. The `tuple` arm normalised that case to `undefined` before comparing, +and the comparison is `===`, so `null === undefined` was false for **every** +rest-less tuple regardless of its items and the arm always took the rebuild +branch. The arm now copies `def.rest` instead of normalising it, so `null` is +compared against `null`. + +**What changes for a consumer.** Three schema-shaped exports reachable from the +published `@objectstack/spec` surface stop being rebuilt at this package's import +boundary and are now handed back as the spec's own objects: +`@objectstack/spec/data#FieldOperatorsSchema`, +`@objectstack/spec/data#RangeOperatorSchema` and +`@objectstack/spec/ui#ListMapConfigSchema`. Nothing about what they accept or +reject moves — the rebuild was faithful, and a rest-less tuple validated +identically before and after. What moves is reference identity: code comparing +one of these against the spec's export with `===` / `toBe` now finds them equal +where it previously found them distinct. + +No accept set, no key, no check and no registry metadata changes. diff --git a/packages/types/src/__tests__/imported-defaults-describe-9034.test.ts b/packages/types/src/__tests__/imported-defaults-describe-9034.test.ts index 6448578a82..e708981d9e 100644 --- a/packages/types/src/__tests__/imported-defaults-describe-9034.test.ts +++ b/packages/types/src/__tests__/imported-defaults-describe-9034.test.ts @@ -138,18 +138,19 @@ const hasLazy = (root: z.ZodType): boolean => reaches(root, 'lazy'); /** * Does the subtree hold a tuple with no rest element? * - * ⚠️ The walker's SECOND identity-property exception, and unlike the `lazy` one - * it is not deliberate — it is a defect this file measured and objectui#9088 - * carries. Zod spells "no rest element" as `def.rest === null`, and the `tuple` - * arm compares that against the `undefined` its own `def.rest ? … : undefined` - * produces, so `null === undefined` is false and EVERY rest-less tuple is - * rebuilt whether or not anything beneath it changed. Reproduced in two lines: - * `stripImportedDefaults(z.tuple([z.number(), z.number()]))` is not identity. + * ⭐ This WAS the carve-out. objectui#9088 has landed, so it is now a POPULATION + * SELECTOR pointing the other way: these exports are the ones the fix moved, and + * the assertion below requires every one of them to come back reference-equal. * - * ⛔ Carved out here, NOT fixed here: repairing it moves the reference identity - * of published mirror bindings, which is its own contract-surface change and - * belongs in its own review. The carve-out is exact — the assertion below still - * goes red if anything OUTSIDE these two exceptions is rebuilt. + * The defect it used to excuse: zod spells "no rest element" as + * `def.rest === null`, and the `tuple` arm compared that against the `undefined` + * its own `def.rest ? … : undefined` produced, so `null === undefined` was false + * and EVERY rest-less tuple was rebuilt whether or not anything beneath it had + * changed. The arm now copies `def.rest` instead of normalising it. + * + * ⛔ Do NOT re-add the exclusion to make a future red go away. A rest-less tuple + * being rebuilt is the defect objectui#9088 closed, not a permitted exception — + * the walker has exactly ONE of those, the `lazy` arm. */ const hasRestlessTuple = (root: z.ZodType): boolean => { const seen = new Set(); @@ -433,8 +434,13 @@ describe('the carry buys nothing at the identity property\'s expense', () => { // "was anything stripped below me?" without forcing the getter, so it always // rebuilds. That exception is measured separately below rather than folded // into this set, so it can neither hide a regression nor grow unnoticed. + // + // ⭐ There used to be a SECOND exclusion here for rest-less tuples. It was a + // carve-out for objectui#9088, that card has landed, and it is deleted rather + // than narrowed — so the exports it used to excuse are back inside this + // assertion and have to satisfy it like everything else. const clean = census.roots.filter(([, s]) => !hasDefault(s)); - const plain = clean.filter(([, s]) => !hasLazy(s) && !hasRestlessTuple(s)); + const plain = clean.filter(([, s]) => !hasLazy(s)); expect(plain.length, 'no clean export free of both exceptions — this assertion is vacuous').toBeGreaterThan(50); const broken = plain.filter(([, s]) => stripImportedDefaults(s) !== s).map(([n]) => n); @@ -445,27 +451,21 @@ describe('the carry buys nothing at the identity property\'s expense', () => { ).toEqual([]); }); - it('the ONLY clean exports that are rebuilt are the two known exceptions', () => { + it('the ONLY clean exports that are rebuilt are behind a `z.lazy` — ONE exception, not two', () => { // Pins the exception's SHAPE, not just its size: every clean export that is // rebuilt reaches a `z.lazy`, and every clean export behind a `z.lazy` is // rebuilt. A future carry that started rebuilding something else would land // in the first set and go red here even if the counts happened to match. const clean = census.roots.filter(([, s]) => !hasDefault(s)); const behindLazy = clean.filter(([, s]) => hasLazy(s)); - const restlessTuple = clean.filter(([, s]) => !hasLazy(s) && hasRestlessTuple(s)); expect(behindLazy.length, 'no clean export sits behind a `z.lazy` — that exception is untested').toBeGreaterThan(0); - expect( - restlessTuple.length, - 'no clean export holds a rest-less tuple — objectui#9088\'s carve-out below is untested, and if ' + - 'that issue has landed the carve-out should be DELETED rather than left passing vacuously', - ).toBeGreaterThan(0); - const excused = new Set([...behindLazy, ...restlessTuple].map(([n]) => n)); + const excused = new Set(behindLazy.map(([n]) => n)); const rebuilt = clean.filter(([, s]) => stripImportedDefaults(s) !== s).map(([n]) => n); expect( rebuilt.filter((n) => !excused.has(n)).slice(0, 10), - 'a clean export was rebuilt that is neither behind a `z.lazy` nor holding a rest-less tuple — ' + - 'that is a NEW identity-property break, outside both known exceptions', + 'a clean export was rebuilt that is NOT behind a `z.lazy` — that is an identity-property break ' + + 'outside the walker\'s one documented exception', ).toEqual([]); expect( behindLazy.filter(([, s]) => stripImportedDefaults(s) === s).length, @@ -474,6 +474,32 @@ describe('the carry buys nothing at the identity property\'s expense', () => { ).toBe(0); }); + it('⭐ the exports that USED to be carved out are now reference-equal (objectui#9088)', () => { + // ⭐ This replaces the carve-out rather than deleting it outright. Removing + // the exclusion alone would leave the population silently untested the day + // the spec stops publishing a rest-less tuple; this asserts the population + // is non-empty AND that every member satisfies the property the carve-out + // used to suppress, which is strictly more than the carve-out ever said. + const clean = census.roots.filter(([, s]) => !hasDefault(s)); + const restless = clean.filter(([, s]) => !hasLazy(s) && hasRestlessTuple(s)); + expect( + restless.length, + 'no clean export holds a rest-less tuple — objectui#9088\'s population is empty on this spec ' + + 'version, so this assertion proves nothing and the corpus needs re-deriving', + ).toBeGreaterThan(0); + + const stillRebuilt = restless.filter(([, s]) => stripImportedDefaults(s) !== s).map(([n]) => n); + expect( + stillRebuilt.slice(0, 10), + 'a clean export holding a rest-less tuple was REBUILT — objectui#9088 has regressed: the `tuple` ' + + 'arm is normalising `def.rest` to `undefined` again, so `null === undefined` is false for every ' + + 'rest-less tuple and the identity property is broken wherever one occurs', + ).toEqual([]); + + // ⭐ PROVING REMOVAL: restore `: undefined` at the `tuple` arm's `const rest` + // in `../zod/imported-defaults.ts` and this assertion is the one that reddens. + }); + it('⭐ most visited nodes are reference-equal — the walk did not start rebuilding the world', () => { // A carry implemented by rebuilding every node on the way down would keep // every description and leave this ratio on the floor, with no other symptom. diff --git a/packages/types/src/__tests__/imported-defaults-rest-less-tuple-9088.test.ts b/packages/types/src/__tests__/imported-defaults-rest-less-tuple-9088.test.ts new file mode 100644 index 0000000000..12d0408613 --- /dev/null +++ b/packages/types/src/__tests__/imported-defaults-rest-less-tuple-9088.test.ts @@ -0,0 +1,480 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * THE IDENTITY PROPERTY HOLDS FOR A REST-LESS TUPLE (objectui#9088). + * + * `../zod/imported-defaults.ts` states the property in the walker's own + * docblock: "A node is rebuilt ONLY if the walk actually changed something + * beneath it. A subtree with no `ZodDefault` in it therefore comes back + * REFERENCE-EQUAL to the spec's own object." That is decision batch #90's + * reversibility argument made literal, and it is the argument on which option A + * was taken over option B. + * + * It was false for EVERY rest-less tuple, and the contradiction was one + * operator wide. + * + * ## The mechanism + * + * Zod 4.4.3 spells "no rest element" as `def.rest === null` — a real, OWN key + * holding `null`. Measured over the whole shipped `zod@4.4.3/v4` tree (176 files, + * 88 `.js` + 88 `.cjs`), the statement that mints it, + * `const rest = hasRest ? _paramsOrRest : null`, occurs at 6 locations — 3 + * logical sites x 2 module formats: `classic/schemas.{js,cjs}`, + * `mini/schemas.{js,cjs}` and `core/api.{js,cjs}`, the last being where zod's + * `tuple` factory lives. + * + * ⚠️ The count is pinned to zod 4.4.3 and to that corpus. An earlier revision of + * this docblock said "exactly one site" and named `classic/schemas.cjs` alone — + * a 2-file corpus that excluded `core/api.cjs`, which is precisely the file the + * `tuple` factory is in. A census is only reproducible if its corpus is stated + * AND its corpus is the one that could have contradicted it. + * + * The `tuple` arm normalised the other side of the comparison to `undefined`: + * + * const rest = def.rest ? walk(def.rest) : undefined; // the defect + * + * `unchanged` compares by `===`, and `null === undefined` is `false`. So the + * pair never matched, the arm always took the `cloneWithDef` branch, and a + * tuple with nothing to strip beneath it was rebuilt anyway. + * + * ⛔ The repair is to stop normalising — `: def.rest` — so a `null` is compared + * against a `null`. ⛔ NOT to relax `unchanged` to `==`: that would make + * `null == undefined` true for every arm at once and paper over a real zod-4 + * spelling distinction other arms may come to depend on. + * + * ## What this file pins + * + * 1. **The zod facts the fix rests on.** If a later zod spells "no rest + * element" as an absent key, or starts minting `null` into a def member the + * walker reads, that must be RED here rather than rediscovered later. + * 2. **The subject**, plus the two controls that fire the other way — a tuple + * WITH a rest element, and a schema with no tuple at all — so a green here + * cannot come from the identity property being trivially true. + * 3. **⭐ The rebuild half, for BOTH shapes.** Compare-like-with-like fixes the + * comparison; it must not change what the clone produces when the walk DID + * change something. A rest-less tuple that is legitimately rebuilt must + * still come out with `def.rest === null`, not `undefined`. + * 4. **⛔ The other direction (binding from objectui#9102).** The input graph + * is `@objectstack/spec`'s own, shared with every workspace consumer, so + * nothing here may mutate it — and a subtree with no `ZodDefault` must + * still come back reference-equal. + * 5. **The `:246`@`b8a006883d` = `:274`@head verdict**, which is the triage + * fence's census answered as a measurement rather than read off the tuple + * result. (The `def.out` site moved because the repair's own comment block + * was inserted above it; a quoted line is only a measurement against a + * stated sha.) + * + * ⭐ Every assertion below records the removal that proves it — the mutation + * that makes THIS assertion red. A proof that lives only in a pull-request body + * is one refactor away from vacuous. + */ + +import { describe, it, expect } from 'vitest'; +import { z } from 'zod'; +import { stripImportedDefaults } from '../zod/imported-defaults.js'; + +/* ── the graph reader ─────────────────────────────────────────────────────── */ + +interface ZodDef { + type: string; + shape?: Record; + options?: z.ZodType[]; + items?: z.ZodType[]; + element?: z.ZodType; + // ⚠️ `z.ZodType | null`, and the `| null` is the whole card. `WalkableDef` in + // `../zod/node-derivation.ts` declares this member `rest?: z.ZodType`, which + // does NOT admit the `null` zod actually mints — and that inaccurate + // declaration is what made `: undefined` look like the right spelling to + // write. Declared honestly HERE because this file is what measures it; the + // shared type lives outside this card's file surface. + rest?: z.ZodType | null; + valueType?: z.ZodType; + left?: z.ZodType; + right?: z.ZodType; + in?: z.ZodType; + out?: z.ZodType | null; + innerType?: z.ZodType; + getter?: () => z.ZodType; +} +const defOf = (node: z.ZodType): ZodDef => (node as unknown as { _zod: { def: ZodDef } })._zod.def; +const isZod = (v: unknown): v is z.ZodType => + v !== null && (typeof v === 'object' || typeof v === 'function') && '_zod' in (v as object); + +/** Children of a node, labelled so the stripped twin's matching child can be found. */ +const childrenOf = (s: z.ZodType): [string, z.ZodType][] => { + const d = defOf(s); + const out: [string, z.ZodType][] = []; + switch (d.type) { + case 'object': for (const [k, v] of Object.entries(d.shape ?? {})) out.push([`.${k}`, v]); break; + case 'union': (d.options ?? []).forEach((o, i) => out.push([`|${i}`, o])); break; + case 'array': if (d.element) out.push(['[]', d.element]); break; + case 'tuple': + (d.items ?? []).forEach((it, i) => out.push([`[${i}]`, it])); + if (d.rest) out.push(['[...]', d.rest]); + break; + case 'record': if (d.valueType) out.push(['{}', d.valueType]); break; + case 'intersection': + if (d.left) out.push(['&L', d.left]); + if (d.right) out.push(['&R', d.right]); + break; + case 'pipe': + if (d.in) out.push(['>in', d.in]); + if (d.out) out.push(['>out', d.out]); + break; + case 'lazy': if (d.getter) out.push(['~lazy', d.getter()]); break; + case 'default': case 'optional': case 'nullable': + case 'nonoptional': case 'readonly': case 'catch': + if (d.innerType) out.push([`(${d.type})`, d.innerType]); + break; + default: break; + } + return out; +}; + +/* ── 1. the zod facts the fix rests on ────────────────────────────────────── */ + +describe('the zod 4.4.3 facts the fix rests on (objectui#9088)', () => { + it('⭐ "no rest element" is an OWN key holding `null`, not an absent key', () => { + const d = defOf(z.tuple([z.number(), z.number()])); + + // All three halves matter and they are different claims. If a later zod + // drops the key instead of nulling it, the first goes red; if it nulls a + // different way, the second does. `: def.rest` survives BOTH — it copies + // whatever is there — which is precisely why it is the repair rather than + // an `=== null` special case. + expect(Object.prototype.hasOwnProperty.call(d, 'rest'), 'zod stopped emitting a `rest` key at all').toBe(true); + expect(d.rest, 'zod no longer spells "no rest element" as `null`').toBeNull(); + expect(d.rest === undefined, 'the pair the old arm compared is no longer mismatched').toBe(false); + + // ⭐ PROVING REMOVAL: change `toBeNull()` to `toBeUndefined()` and this test + // goes red on zod 4.4.3 — the assertion reads the real def, not a literal. + }); + + it('a tuple WITH a rest element holds the schema itself, so `: def.rest` copies it unchanged', () => { + const rest = z.string(); + const d = defOf(z.tuple([z.number()], rest)); + expect(d.rest).toBe(rest); + + // ⭐ PROVING REMOVAL: pass a different schema to `z.tuple`'s second argument + // and `toBe` fails — identity, not shape, is what is asserted. + }); + + it('⭐ `rest` is the ONLY def member the walker reads that zod ever mints as `null`', () => { + // This is the triage fence's census (`def.X ? walk(def.X) : undefined`) + // answered as a MEASUREMENT. That census had TWO hits at `b8a006883d` — the + // `tuple` arm's `def.rest` at `:223` and the `pipe` arm's `def.out` at + // `:246` — and a verdict on the second read off the first would be + // worthless. ⚠️ At this head ONE remains: the `tuple` hit is gone, removed + // by this card's own repair, and `def.out` now sits at `:274`. + // + // ⛔ The control here does NOT share the suspect part of the instrument: the + // SAME probe, applied to `rest`, returns `null`. So a green on `out` is the + // probe reporting a real absence, not the probe being unable to see a null. + const probe = (node: z.ZodType, key: 'rest' | 'out'): unknown => defOf(node)[key]; + + // every spelling zod 4.4.3 offers that produces a `pipe` def + const pipes: [string, z.ZodType][] = [ + ['transform', z.string().transform((x) => x)], + ['preprocess', z.preprocess((x) => x, z.string())], + ['explicit pipe', z.pipe(z.string().transform((s) => s.length), z.number())], + ['chained pipe', z.string().pipe(z.string().min(1))], + ]; + for (const [label, node] of pipes) { + expect(defOf(node).type, `${label} is not a pipe`).toBe('pipe'); + expect(probe(node, 'out'), `${label}: def.out is nullish — the \`pipe\` arm has the tuple defect too`).not.toBeNull(); + expect(probe(node, 'out'), `${label}: def.out is undefined`).not.toBeUndefined(); + } + + // the control leg: the identical probe DOES see a null when one is there + expect(probe(z.tuple([z.number()]), 'rest'), 'the probe cannot see a null at all — every reading above is vacuous').toBeNull(); + + // ⭐ PROVING REMOVAL: delete the control leg and the test still passes while + // proving nothing — which is exactly why the leg is in the same `it`. + }); +}); + +/* ── 2. the subject, with the two controls that fire the other way ────────── */ + +describe('⭐ the identity property holds for a rest-less tuple (objectui#9088)', () => { + it('a rest-less tuple with nothing to strip comes back REFERENCE-EQUAL', () => { + const subject = z.tuple([z.number(), z.number()]); + expect( + stripImportedDefaults(subject), + 'a rest-less tuple was REBUILT though nothing beneath it changed — the identity property in ' + + '`../zod/imported-defaults.ts` is false again, and with it batch #90\'s reversibility argument', + ).toBe(subject); + + // ⭐ PROVING REMOVAL: restore `: undefined` at the `tuple` arm's `const rest` + // and this assertion is the one that reddens. + }); + + it('CONTROL — a tuple WITH a rest element was always identity, and still is', () => { + const subject = z.tuple([z.number()], z.string()); + expect(subject).toBe(stripImportedDefaults(subject)); + + // ⭐ PROVING REMOVAL: this control is NOT vacuous — make the arm walk the + // rest into a fresh node (e.g. `walk(def.rest) ?? def.rest` replaced by + // `z.string()`) and it reddens while the subject above stays green. + }); + + it('CONTROL — a schema with no tuple at all was always identity, and still is', () => { + const subject = z.object({ a: z.string() }); + expect(subject).toBe(stripImportedDefaults(subject)); + }); + + it('CONTROL — the identity property does NOT hold where something IS stripped', () => { + // Without this leg the three above are all satisfiable by a `stripImportedDefaults` + // that is literally `(x) => x`, which would pass every identity assertion in + // this file and strip nothing. + const subject = z.tuple([z.number().default(1)]); + expect(stripImportedDefaults(subject), 'the walker returned its input though there WAS a default to strip').not.toBe(subject); + }); +}); + +/* ── 3. the rebuild half — both shapes still clone correctly ──────────────── */ + +describe('⭐ the clone still produces the right def for BOTH tuple shapes (objectui#9088)', () => { + it('a REST-LESS tuple that is legitimately rebuilt keeps `def.rest === null`', () => { + // The card's open question. `cloneWithDef` spreads the ORIGINAL def and then + // the patch, and the arm omits `rest` from the patch for a rest-less tuple — + // so `rest: null` has to survive from the spread. If it came out `undefined` + // the node would be a different shape from the one zod builds, and the NEXT + // walk of it would take a different branch. + const subject = z.tuple([z.number().default(1), z.string()]); + const stripped = stripImportedDefaults(subject); + + expect(stripped, 'nothing was rebuilt, so this test is measuring the wrong branch').not.toBe(subject); + expect(defOf(stripped).type).toBe('tuple'); + expect( + defOf(stripped).rest, + 'the rebuilt rest-less tuple lost zod\'s `null` spelling and came out `undefined` — a shape zod ' + + 'itself never builds', + ).toBeNull(); + expect(defOf(stripped).items).toHaveLength(2); + + // the strip actually happened, and only where it should have + expect(defOf(defOf(stripped).items![0]).type, 'item 0 still carries its default').not.toBe('default'); + expect(defOf(stripped).items![1], 'item 1 had nothing to strip and should be the SAME node').toBe( + defOf(subject).items![1], + ); + + // ⭐ PROVING REMOVAL, verified: replace the arm's + // `...(def.rest ? { rest: rest! } : {})` with `rest: rest ?? undefined` and + // THIS assertion is the only one in the file that reddens — the identity + // assertions above stay green, so it varies only the claim it is making. + }); + + it('a tuple WITH a rest element that is rebuilt keeps a walked rest', () => { + const subject = z.tuple([z.number().default(1)], z.string().default('x')); + const stripped = stripImportedDefaults(subject); + + expect(stripped).not.toBe(subject); + const rest = defOf(stripped).rest; + expect(rest, 'the rebuilt tuple lost its rest element entirely').toBeTruthy(); + expect(defOf(rest as z.ZodType).type, 'the rest element was not walked — its default survived').not.toBe('default'); + + // ⭐ PROVING REMOVAL: drop `rest` from the patch object and the rest element + // arrives unstripped — red on the last assertion. + }); + + it('a tuple whose rest alone carries the default is rebuilt, items shared by reference', () => { + const item = z.number(); + const subject = z.tuple([item], z.string().default('x')); + const stripped = stripImportedDefaults(subject); + + expect(stripped).not.toBe(subject); + expect(defOf(stripped).items![0], 'an item with nothing to strip was rebuilt anyway').toBe(item); + }); +}); + +/* ── 4. the other direction — binding from objectui#9102 ──────────────────── */ + +describe('⛔ the repair does not regress the property in the other direction (objectui#9102)', () => { + it('a DEEP subtree with no `ZodDefault` anywhere still comes back reference-equal', () => { + const subject = z.object({ + a: z.array(z.tuple([z.string(), z.number()])), + b: z.union([z.tuple([z.boolean()]), z.tuple([z.string()], z.number())]), + c: z.record(z.string(), z.tuple([z.tuple([z.number()])])), + }); + expect(stripImportedDefaults(subject), 'a clean subtree was rebuilt').toBe(subject); + }); + + it('⛔ the input graph is NOT mutated — it is the spec\'s own object elsewhere', () => { + const inner = z.number().default(1); + const subject = z.tuple([inner]); + const beforeType = defOf(defOf(subject).items![0]).type; + + stripImportedDefaults(subject); + + expect(defOf(subject).items![0], 'the input tuple\'s item was REPLACED in place').toBe(inner); + expect(defOf(defOf(subject).items![0]).type, 'the input tuple was stripped IN PLACE').toBe(beforeType); + expect(defOf(subject).rest, 'the input tuple\'s `rest` was rewritten in place').toBeNull(); + + // ⭐ PROVING REMOVAL: make the arm assign into `def` (e.g. `def.items = items`) + // instead of cloning and all three redden. + }); +}); + +/* ── 5. the population, re-derived over the published spec surface ────────── */ + +/** + * ⭐ THE COUNTING RULE, published beside the number, because a number without + * its rule is not a reading — and two incomparable figures for "reference + * equality on this surface" are already in this card's record. + * + * - CORPUS: every subpath in `@objectstack/spec`'s own `exports` map except + * `./package.json` and `./openapi.json`, read from the map rather than + * listed; every named export of each that answers `isZod` is a ROOT. + * - WALK: each root is paired with `stripImportedDefaults(root)` and the two + * are descended in lockstep by LABELLED children. A `before` node is + * counted ONCE, on first arrival (shared subgraphs are not double-counted). + * Depth cap 60. + * - DENOMINATOR `nodesVisited`: distinct `before` nodes reached. + * - NUMERATOR `referenceEqualNodes`: those for which `before === after`. A + * reference-equal node ENDS that branch — its subtree is equal by identity, + * so descending it would count nodes the walk never had to consider. + * + * ⚠️ That last clause is why this figure is NOT comparable to a whole-graph + * node count, and it is the same rule `imported-defaults-describe-9034.test.ts` + * uses, so the two files' figures ARE comparable to each other. + * + * ⭐ AND IT IS THE WRONG INSTRUMENT FOR THIS CARD, which is worth writing down + * because the figure moves the COUNTERINTUITIVE way. Terminating at reference + * equality means a node that BECOMES reference-equal takes its whole subtree out + * of the count — so repairing the `tuple` arm made both the numerator and the + * denominator SHRINK, and the ratio drift slightly DOWN, while strictly more of + * the surface is reference-equal than before. Read alone, that looks like a + * regression; it is the rule working as designed. + * + * ⇒ RULE B, below, is the instrument that answers this card. It walks the + * BEFORE graph exhaustively — never terminating early — so the denominator is a + * FIXED corpus of distinct spec nodes that does not move when the fix lands, and + * the numerator can only rise. Both rules are reported; neither is a summary of + * the other. + */ +interface Census { + roots: number; + // RULE A — terminates at reference equality (the `…-describe-9034.test.ts` rule) + nodesVisited: number; + referenceEqualNodes: number; + // RULE B — exhaustive walk of the BEFORE graph; fixed denominator + fixedCorpusNodes: number; + fixedCorpusReferenceEqual: number; + restlessTupleRoots: number; +} + +const buildCensus = async (): Promise => { + const pkg = (await import('@objectstack/spec/package.json', { with: { type: 'json' } })) as { + default: { exports: Record }; + }; + const subpaths = Object.keys(pkg.default.exports).filter( + (k) => k !== './package.json' && k !== './openapi.json', + ); + + const roots: z.ZodType[] = []; + for (const sp of subpaths) { + const specifier = sp === '.' ? '@objectstack/spec' : `@objectstack/spec/${sp.slice(2)}`; + let mod: Record; + try { + mod = (await import(/* @vite-ignore */ specifier)) as Record; + } catch { + continue; + } + for (const value of Object.values(mod)) if (isZod(value)) roots.push(value); + } + + let nodesVisited = 0; + let referenceEqualNodes = 0; + const seen = new Set(); + const pair = (before: z.ZodType, after: z.ZodType, depth: number): void => { + if (depth > 60 || seen.has(before)) return; + seen.add(before); + nodesVisited++; + if (before === after) { referenceEqualNodes++; return; } + const aMap = new Map(isZod(after) ? childrenOf(after) : []); + for (const [label, child] of childrenOf(before)) { + const twin = aMap.get(label); + if (twin) pair(child, twin, depth + 1); + } + }; + for (const root of roots) pair(root, stripImportedDefaults(root), 0); + + // ── RULE B: exhaustive. Never stops at a reference-equal node, so the + // denominator is a fixed corpus of distinct spec nodes and the numerator is + // the count of those the walker hands back unchanged. + let fixedCorpusNodes = 0; + let fixedCorpusReferenceEqual = 0; + const seenB = new Set(); + const pairB = (before: z.ZodType, after: z.ZodType | undefined, depth: number): void => { + if (depth > 60 || seenB.has(before)) return; + seenB.add(before); + fixedCorpusNodes++; + if (before === after) fixedCorpusReferenceEqual++; + // ⛔ `lazy` is not descended: its getter builds a fresh graph per call, so + // the seen-set cannot terminate it and the corpus would not be fixed. + if (defOf(before).type === 'lazy') return; + const aMap = new Map(isZod(after) ? childrenOf(after) : []); + for (const [label, child] of childrenOf(before)) pairB(child, aMap.get(label), depth + 1); + }; + for (const root of roots) pairB(root, stripImportedDefaults(root), 0); + + // how many roots hold a rest-less tuple at all — the population this card moves + let restlessTupleRoots = 0; + const holdsRestlessTuple = (root: z.ZodType): boolean => { + const s = new Set(); + const stack = [root]; + let budget = 20_000; + while (stack.length && budget-- > 0) { + const n = stack.pop()!; + if (s.has(n)) continue; + s.add(n); + const d = defOf(n); + if (d.type === 'tuple' && d.rest === null) return true; + if (d.type === 'lazy') continue; + for (const [, c] of childrenOf(n)) stack.push(c); + } + return false; + }; + for (const root of roots) if (holdsRestlessTuple(root)) restlessTupleRoots++; + + return { roots: roots.length, nodesVisited, referenceEqualNodes, fixedCorpusNodes, fixedCorpusReferenceEqual, restlessTupleRoots }; +}; + +const census: Census = await buildCensus(); + +describe('the published spec surface, re-derived (objectui#9088)', () => { + it('⭐ reports the reference-equality figure under the rule in this file\'s docblock', () => { + // Printed, not pinned to an exact number: the figure moves whenever the spec + // publishes a schema, and a pinned integer would make an unrelated upstream + // release look like a regression here. What IS pinned is the floor below. + console.log( + `[objectui#9088 census] roots=${census.roots}\n` + + ` RULE A (terminating) nodesVisited=${census.nodesVisited} ` + + `referenceEqual=${census.referenceEqualNodes} ` + + `notReferenceEqual=${census.nodesVisited - census.referenceEqualNodes} ` + + `ratio=${(census.referenceEqualNodes / census.nodesVisited).toFixed(4)}\n` + + ` RULE B (exhaustive) fixedCorpusNodes=${census.fixedCorpusNodes} ` + + `referenceEqual=${census.fixedCorpusReferenceEqual} ` + + `ratio=${(census.fixedCorpusReferenceEqual / census.fixedCorpusNodes).toFixed(4)}\n` + + ` restlessTupleRoots=${census.restlessTupleRoots}`, + ); + expect(census.roots, 'the spec surface loaded as zero roots — every figure here is vacuous').toBeGreaterThan(50); + expect(census.nodesVisited, 'the walk visited nothing').toBeGreaterThan(1000); + expect(census.fixedCorpusNodes, 'rule B walked nothing').toBeGreaterThan(census.nodesVisited); + }); + + it('⭐ the population this card moves is NON-EMPTY on the published surface', () => { + expect( + census.restlessTupleRoots, + 'no published spec export holds a rest-less tuple — this card\'s whole population is empty here, ' + + 'so every figure above is measuring something else', + ).toBeGreaterThan(0); + }); + + it('⭐ reference equality is the MAJORITY outcome on the published surface', () => { + // The floor, not the figure. Before this card it was already above a half; + // the card moves it up, and a regression that started rebuilding the world + // would drive it under. + expect(census.referenceEqualNodes / census.nodesVisited).toBeGreaterThan(0.5); + }); +}); diff --git a/packages/types/src/__tests__/registry-meta-carry-9102.test.ts b/packages/types/src/__tests__/registry-meta-carry-9102.test.ts index 0b392d509f..efaafba2df 100644 --- a/packages/types/src/__tests__/registry-meta-carry-9102.test.ts +++ b/packages/types/src/__tests__/registry-meta-carry-9102.test.ts @@ -194,11 +194,25 @@ const reaches = (root: z.ZodType, type: string): boolean => { }; /** - * The import boundary's SECOND identity-property exception, carried by - * objectui#9088 and deliberately not repaired here: zod spells "no rest - * element" as `def.rest === null`, the walker compares it against `undefined`, - * so every rest-less tuple is rebuilt whether or not anything changed beneath - * it. Excused below exactly as the objectui#9034 pin excuses it. + * Does the subtree hold a tuple with no rest element? + * + * ⭐ This USED to name the import boundary's SECOND identity-property + * exception, and it was excused below. objectui#9088 repaired it, so the + * walker now has exactly ONE identity-property exception — the `lazy` arm — + * and this helper selects a POPULATION the boundary is expected to get right + * rather than a set to be excused. + * + * The defect it used to describe: zod spells "no rest element" as + * `def.rest === null`, and the `tuple` arm compared that against the + * `undefined` its own `def.rest ? … : undefined` produced, so + * `null === undefined` was false and every rest-less tuple was rebuilt whether + * or not anything beneath it had changed. The arm now copies `def.rest`. + * + * ⛔ Do NOT re-add an exclusion built on this helper to make a future red go + * away. A rest-less tuple being rebuilt is the defect objectui#9088 closed, and + * the assertion below is what keeps this file from going blind to it — measured: + * while the exclusion stood, restoring the defect left this whole file at + * 34/34 green. */ const hasRestlessTuple = (root: z.ZodType): boolean => { const seen = new Set(); @@ -799,9 +813,19 @@ describe('site ① — the import boundary conveys the protocol\'s metadata', () }); it('⭐ every export with nothing to strip still comes back REFERENCE-EQUAL', () => { + // ⭐ There used to be a second exclusion here, `&& !hasRestlessTuple(s)`, a + // carve-out for objectui#9088. That card has landed and the exclusion is + // DELETED, not narrowed — so the exports it used to excuse are back inside + // this assertion and have to satisfy it like everything else. + // + // ⛔ It mattered more here than in the sibling pin, because this carve-out + // carried NO expiry assertion: nothing required the excluded set to be + // non-empty, so it could only ever come back green. Measured while it stood: + // restoring the defect in the `tuple` arm left this entire file at 34/34 + // passing, exit 0 — blind to the exact defect objectui#9088 closed. const clean = census.roots.filter(([, s]) => !reaches(s, 'default')); - const plain = clean.filter(([, s]) => !reaches(s, 'lazy') && !hasRestlessTuple(s)); - expect(plain.length, 'no clean export free of both exceptions — this assertion is vacuous').toBeGreaterThan(50); + const plain = clean.filter(([, s]) => !reaches(s, 'lazy')); + expect(plain.length, 'no clean export outside the `lazy` exception — this assertion is vacuous').toBeGreaterThan(50); const broken = plain.filter(([, s]) => stripImportedDefaults(s) !== s).map(([n]) => n); expect( broken.slice(0, 10), @@ -809,6 +833,34 @@ describe('site ① — the import boundary conveys the protocol\'s metadata', () ).toEqual([]); }); + it('⭐ the rest-less tuples this file used to excuse are reference-equal (objectui#9088)', () => { + // ⭐ The replacement for the deleted carve-out, and deliberately stronger + // than removing the exclusion alone. Dropping the filter would leave this + // population silently untested the day the spec stops publishing a rest-less + // tuple; this asserts the population is NON-EMPTY *and* that every member + // satisfies the property the carve-out used to suppress. It matches what + // `imported-defaults-describe-9034.test.ts` now asserts, so the two pins + // fail together rather than one covering for the other. + const clean = census.roots.filter(([, s]) => !reaches(s, 'default')); + const restless = clean.filter(([, s]) => !reaches(s, 'lazy') && hasRestlessTuple(s)); + expect( + restless.length, + 'no clean export holds a rest-less tuple — objectui#9088\'s population is empty on this spec version, ' + + 'so this assertion proves nothing and the corpus needs re-deriving', + ).toBeGreaterThan(0); + + const stillRebuilt = restless.filter(([, s]) => stripImportedDefaults(s) !== s).map(([n]) => n); + expect( + stillRebuilt.slice(0, 10), + 'a clean export holding a rest-less tuple was REBUILT — objectui#9088 has regressed: the `tuple` arm is ' + + 'normalising `def.rest` to `undefined` again, so `null === undefined` is false for every rest-less tuple', + ).toEqual([]); + + // ⭐ PROVING REMOVAL: restore `: undefined` at the `tuple` arm's `const rest` + // in `../zod/imported-defaults.ts` and this assertion reddens. While the + // carve-out stood, that same mutation left this file entirely green. + }); + it('⛔ the spec\'s own graph is left exactly as it was found — every node, both derivations', () => { // ⭐ A WHOLE-SURFACE DIFFERENTIAL, and the first round's version was not. // That one watched a side effect — "does the source still hold a default?" diff --git a/packages/types/src/zod/imported-defaults.ts b/packages/types/src/zod/imported-defaults.ts index f912ad9254..42765e66a0 100644 --- a/packages/types/src/zod/imported-defaults.ts +++ b/packages/types/src/zod/imported-defaults.ts @@ -218,9 +218,37 @@ const walk = (schema: z.ZodType): z.ZodType => { out = element === def.element ? schema : cloneWithDef(schema, { element }); break; } + // ⭐ `: def.rest`, ⛔ NOT `: undefined` (objectui#9088). + // + // Zod 4 spells "no rest element" as an OWN `rest` key holding `null` — not + // as an absent key — and that is the only `null` zod mints into any def + // member this walker reads. Normalising the absent case to `undefined` put + // a `null` on one side of `unchanged`'s `===` and an `undefined` on the + // other, so the pair never matched and EVERY rest-less tuple took the + // `cloneWithDef` branch whether or not anything beneath it had changed. + // Three published spec exports were rebuilt for no reason + // (`FieldOperatorsSchema`, `RangeOperatorSchema`, `ListMapConfigSchema`), + // and the identity property above — batch #90's reversibility argument made + // literal — was false wherever a rest-less tuple occurred. + // + // Copying `def.rest` compares like with like: `null` against `null`, + // `undefined` against `undefined`, a schema against its walked self. ⛔ The + // repair is NOT to relax `unchanged` to `==`; that would make + // `null == undefined` true for every arm at once and erase a real zod-4 + // spelling distinction another arm may come to depend on. + // + // ⚠️ `WalkableDef.rest` is declared `z.ZodType | undefined`, which does not + // admit the `null` zod actually mints — that inaccurate declaration is what + // made `: undefined` look correct. The value flows through untyped here; + // widening the shared type is objectui#9491. + // + // ⭐ PROVING REMOVAL: put `: undefined` back and + // `__tests__/imported-defaults-rest-less-tuple-9088.test.ts` reddens on + // "a rest-less tuple ... comes back REFERENCE-EQUAL", as does + // `__tests__/imported-defaults-describe-9034.test.ts` on the three exports. case 'tuple': { const items = (def.items ?? []).map(walk); - const rest = def.rest ? walk(def.rest) : undefined; + const rest = def.rest ? walk(def.rest) : def.rest; out = unchanged([...(def.items ?? []).map((it, i) => [it, items[i]] as const), [def.rest, rest] as const]) ? schema : cloneWithDef(schema, { items, ...(def.rest ? { rest: rest! } : {}) });