diff --git a/packages/spec/src/ui/chart.test.ts b/packages/spec/src/ui/chart.test.ts index 85bf1abbc0..dce0a8cbf9 100644 --- a/packages/spec/src/ui/chart.test.ts +++ b/packages/spec/src/ui/chart.test.ts @@ -14,111 +14,9 @@ import { } from './chart.zod'; import { ReportChartSchema, ReportSchema } from './report.zod'; import { REACT_BLOCKS } from './react-blocks'; -import { getMetadataTypeSchema, listMetadataTypeSchemaTypes } from '../kernel/metadata-type-schemas'; -import { ObjectStackSchema } from '../stack.zod'; - -/** - * Reachability of a schema from every metadata-type root plus `defineStack`'s - * `ObjectStackSchema`, by BFS over this build's in-memory Zod graph. - * - * Mirrors `computeSurfaceReachability` in `scripts/build-schemas.ts` (the - * #4650 closure). `derived-clone` counts as reachable: `.extend()` / `.strip()` - * produce a clone that shares no identity with the original but DOES share its - * per-property schema instances, which is exactly how `ChartConfigSchema` is - * reached through `ReportChartSchema`. - * - * ⚠️ Identity-keyed, so it must see the REAL schema instances. `lazySchema` - * returns a Proxy unless `OS_EAGER_SCHEMAS=1`, and comparing a Proxy against - * the instance stored in the graph reports every root as unreachable — which - * is precisely how the first run of this measurement produced three failing - * positive controls. Hence the resolve step. - */ -function reachableFromMetadataRoots(): (schema: unknown) => boolean { - // Identity is the schema's `_zod.def` OBJECT, never the schema binding. - // `lazySchema` hands out a Proxy unless `OS_EAGER_SCHEMAS=1`, and the graph - // holds the real instances — so comparing bindings reports every root as - // unreachable. That is not hypothetical: it is what the first run of this - // assertion did, and the positive controls above are the only reason it was - // caught instead of shipping as a green that proved nothing. `def` survives - // the Proxy (the `_zod` facade delegates to the real internals), so it is - // the one stable key for both identities. - const defOf = (s: unknown): unknown => (s as { _zod?: { def?: unknown } })?._zod?.def; - - const childrenOf = (node: unknown): unknown[] => { - const out: unknown[] = []; - const seen = new Set(); - const walk = (v: unknown): void => { - // `typeof v !== 'object'` alone is WRONG here and silently halves the - // graph: `lazySchema`'s Proxy target is `function lazyZod() {}`, so every - // lazy schema is `typeof 'function'`. Skipping those made the BFS stop at - // the first lazy node and report the whole chart family unreachable — - // caught only because the positive controls above went red. - // (`build-schemas.ts`'s equivalent walk never hit this: it runs under - // `OS_EAGER_SCHEMAS=1`, where there are no proxies at all.) - if (v === null || (typeof v !== 'object' && typeof v !== 'function') || seen.has(v)) return; - seen.add(v); - if (defOf(v)) { out.push(v); return; } - if (Array.isArray(v)) { for (const x of v) walk(x); return; } - if (v instanceof Map) { for (const x of v.values()) walk(x); return; } - for (const x of Object.values(v as Record)) walk(x); - }; - walk(defOf(node)); - return out; - }; - const shapeOf = (node: unknown): Record | null => { - const def = defOf(node) as { type?: string; shape?: Record } | undefined; - return def?.type === 'object' && def.shape ? def.shape : null; - }; - - const roots: unknown[] = []; - for (const type of listMetadataTypeSchemaTypes()) { - const s = getMetadataTypeSchema(type); - if (s) roots.push(s); - } - roots.push(ObjectStackSchema); - - const visitedDefs = new Set(); - const visitedNodes: unknown[] = []; - const queue = [...roots]; - while (queue.length > 0) { - const node = queue.pop(); - const def = defOf(node); - if (!def || visitedDefs.has(def)) continue; - visitedDefs.add(def); - visitedNodes.push(node); - for (const child of childrenOf(node)) queue.push(child); - } - - // (propName → prop def) pairs of every visited object node — the bridge that - // recognises a derived clone (`.extend()` / `.strip()` share no identity with - // the original but DO share its per-property schema instances, which is how - // `ChartConfigSchema` is reached through `ReportChartSchema`). - const bridged = new Map>(); - for (const node of visitedNodes) { - const shape = shapeOf(node); - if (!shape) continue; - for (const [name, prop] of Object.entries(shape)) { - const d = defOf(prop); - if (!d) continue; - let names = bridged.get(d); - if (!names) { names = new Set(); bridged.set(d, names); } - names.add(name); - } - } - - return (schema: unknown): boolean => { - const def = defOf(schema); - if (!def) return false; - if (visitedDefs.has(def)) return true; - const shape = shapeOf(schema); - if (!shape) return false; - for (const [name, prop] of Object.entries(shape)) { - const d = defOf(prop); - if (d && bridged.get(d)?.has(name)) return true; - } - return false; - }; -} +import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas'; +import { measureDoors } from './door-reachability.testkit'; +import { z } from 'zod'; describe('ChartTypeSchema', () => { it('should accept all comparison chart types', () => { @@ -563,21 +461,46 @@ describe('#4001 批 15 — the two chart sites deliberately LEFT OPEN (measured, // closure `build-schemas.ts` uses for the #4650 deletion check — NOT a // string search over a serialized schema, which cannot see a shape at all // and would pass no matter what (the vacuous-green this campaign keeps - // paying for). The positive controls below are what prove that. - const reachable = reachableFromMetadataRoots(); - - // Positive controls, in the SAME run: the five closed sites of this file - // resolve. An instrument that says "unreachable" to everything is broken, - // not informative. - expect(reachable(ChartConfigSchema), 'positive control').toBe(true); - expect(reachable(ChartAxisSchema), 'positive control').toBe(true); - expect(reachable(ChartSeriesSchema), 'positive control').toBe(true); - expect(reachable(ChartAnnotationSchema), 'positive control').toBe(true); - expect(reachable(ChartInteractionSchema), 'positive control').toBe(true); + // paying for). The controls below are what prove that. + // + // ⚠️ #5056: this file used to carry its OWN copy of that walk, and the copy + // kept the defective `any one shared property ⇒ derived clone` bridge after + // the shared walker had been fixed to a whole-shape overlap ratio. One + // implementation now, in `door-reachability.testkit.ts`, whose own controls + // live in `door-reachability.testkit.test.ts`. + const { verdict, nodeCount, rootCount } = measureDoors(); + + // Controls FIRST, in the SAME run. An instrument that reached nothing at + // all produces the same output as a correct "no door" verdict. + expect(rootCount, 'roots must include every metadata type plus ObjectStackSchema').toBeGreaterThan(20); + expect(nodeCount, 'the graph must actually have been walked').toBeGreaterThan(1000); + + // Positive controls: the five closed sites of this file resolve. + expect(verdict(ChartConfigSchema), 'positive control').toBe('direct'); + expect(verdict(ChartAxisSchema), 'positive control').toBe('direct'); + expect(verdict(ChartSeriesSchema), 'positive control').toBe('direct'); + expect(verdict(ChartAnnotationSchema), 'positive control').toBe('direct'); + expect(verdict(ChartInteractionSchema), 'positive control').toBe('direct'); + + // Negative control: a shape this graph has never seen must stay out. + expect(verdict(z.object({ osChartProbe: z.string() })), 'negative control').toBe('unreachable'); // The measurement itself. - expect(reachable(ChartAggregateSchema), 'a carrier key would make this reachable — re-read chart.zod.ts').toBe(false); - expect(reachable(ChartGroupBySchema), 'a carrier key would make this reachable — re-read chart.zod.ts').toBe(false); + expect(verdict(ChartAggregateSchema), 'a carrier key would make this reachable — re-read chart.zod.ts').toBe('unreachable'); + expect(verdict(ChartGroupBySchema), 'a carrier key would make this reachable — re-read chart.zod.ts').toBe('unreachable'); + }); + + it('a synthetic carrier flips both — the verdict is the graph, not the walker', () => { + // The third control #5056 requires and this file never had. Without it the + // assertion above is satisfiable by a walker that finds no doors anywhere, + // which is the vacuous green 批 15 shipped once already. + const carrier = z.object({ + aggregate: ChartAggregateSchema, + groupBy: ChartGroupBySchema, + }); + const { verdict } = measureDoors([carrier]); + expect(verdict(ChartAggregateSchema), 'must become reachable once something carries it').toBe('direct'); + expect(verdict(ChartGroupBySchema), 'must become reachable once something carries it').toBe('direct'); }); }); diff --git a/packages/spec/src/ui/door-reachability.testkit.test.ts b/packages/spec/src/ui/door-reachability.testkit.test.ts new file mode 100644 index 0000000000..d203b866cc --- /dev/null +++ b/packages/spec/src/ui/door-reachability.testkit.test.ts @@ -0,0 +1,194 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #5056 — the controls the door measurement itself owes. + * + * `door-reachability.testkit.ts` decides whether a #4001 batch tightens a shape + * or reclassifies it as `no door`. Its consumers (`chart.test.ts`, + * `widget.test.ts`, `i18n.test.ts`) each carry the three controls for THEIR + * shapes; nothing carried the controls for the instrument. This file does, so + * that the walker's own limbs — the `.describe()` premise the bridge rests on, + * the derived-clone bridge, and the overlap threshold that bounds it — cannot + * rot behind green consumer tests. + * + * Every number asserted here was measured on this build, not reasoned about. + */ + +import { describe, it, expect } from 'vitest'; +import { z } from 'zod'; +import { measureDoors } from './door-reachability.testkit'; +import { PageSchema } from './page.zod'; +import { ObjectListViewSchema } from './view.zod'; +import { WidgetManifestSchema } from './widget.zod'; +import { I18nLabelSchema } from './i18n.zod'; +import { SnakeCaseIdentifierSchema } from '../shared/identifiers.zod'; + +/** The walker's identity key, mirrored here so the premise tests can assert on it. */ +const defOf = (s: unknown): unknown => (s as { _zod?: { def?: unknown } })?._zod?.def; + +// ============================================================================ +// 1. The PREMISE the derived-clone bridge rests on. +// +// The bridge exists because zod builders return a clone that shares the base's +// per-property schema INSTANCES. Which builders share the `_zod.def` OBJECT +// itself, and which do not, is what decides whether the bridge sees a real +// derivation or a coincidence. A zod upgrade that changes `clone()` semantics +// changes the bridge's meaning, and that must be LOUD rather than silent. +// ============================================================================ +describe('#5056 premise — which zod builders share the `_zod.def` object', () => { + it('`.describe()` shares the def of the instance it was called on', () => { + // THE mechanism behind #5056. `clone(inst)` without an explicit def reuses + // `inst._zod.def`, so a described clone is def-IDENTICAL to its receiver. + const base = z.string(); + expect(defOf(base.describe('x')), '.describe() clone vs its receiver').toBe(defOf(base)); + expect(defOf(base.describe('x')), 'two different descriptions of ONE instance').toBe(defOf(base.describe('y'))); + }); + + it('but two independently constructed schemas do NOT share a def', () => { + // #5056's issue body spells the fact as + // `defOf(z.string().describe('x')) === defOf(z.string())` → true + // and that spelling is FALSE on this build: two separate `z.string()` calls + // build two separate defs, so there is nothing to share. Measured, not + // assumed — the corrected statement is the one above: sharing follows the + // RECEIVER INSTANCE, not the shape. That distinction is the whole reason + // the bug bites this repo: the spec funnels dozens of shapes through a + // handful of SHARED leaf instances, and every `.describe()` of one of them + // is def-identical to every other. + expect(defOf(z.string().describe('x'))).not.toBe(defOf(z.string())); + expect(defOf(z.string())).not.toBe(defOf(z.string())); + }); + + it('the repo\'s shared leaves are therefore def-identical across every site that describes them', () => { + // `SnakeCaseIdentifierSchema` and `I18nLabelSchema` are the two the campaign + // actually tripped over: `name:` and `label:` are on nearly every authorable + // shape in this repo, described in place at each site. + expect(defOf(SnakeCaseIdentifierSchema.describe('a'))).toBe(defOf(SnakeCaseIdentifierSchema.describe('b'))); + expect(defOf(I18nLabelSchema.describe('a'))).toBe(defOf(I18nLabelSchema.describe('b'))); + }); + + it('`.optional()` / `.extend()` / `.strip()` each build a NEW def', () => { + // The other half of the premise: these are the builders whose clones the + // bridge must still recognise, and they can only be recognised through + // shared PROPERTY instances, because the def itself is new. + const base = z.string(); + expect(defOf(base.optional())).not.toBe(defOf(base)); + const obj = z.object({ a: z.string() }); + expect(defOf(obj.extend({ b: z.number() }))).not.toBe(defOf(obj)); + expect(defOf(obj.strip())).not.toBe(defOf(obj)); + }); +}); + +// ============================================================================ +// 2. The three controls, on the instrument itself. +// ============================================================================ +describe('#5056 controls — the walker finds doors, and only real ones', () => { + it('positive: live authoring roots resolve, and the graph is really walked', () => { + const { verdict, nodeCount, rootCount } = measureDoors(); + expect(rootCount, 'every metadata type plus ObjectStackSchema').toBeGreaterThan(20); + expect(nodeCount, 'the graph must actually have been walked').toBeGreaterThan(1000); + expect(verdict(PageSchema), 'positive control').toBe('direct'); + expect(verdict(ObjectListViewSchema), 'positive control').toBe('direct'); + }); + + it('positive: a GENUINE derived clone is recognised — this is the bridge limb', () => { + // The limb #5056 narrowed. Nothing else in the repo exercises it: every + // shape the consumers assert as reachable measures `direct`, so a bridge + // that had been narrowed all the way to "never fires" would leave all + // three consumer files green. `.extend()` and `.strip()` are exactly the + // builders the bridge was written for. + const { verdict, cloneOverlap } = measureDoors(); + + const extended = PageSchema.extend({ osDoorProbeExtra: z.string() }); + expect(verdict(extended), 'PageSchema.extend(...) is a real derivation').toBe('derived-clone'); + expect(cloneOverlap(extended), 'one added key out of 25 kept').toBeGreaterThan(0.9); + + const stripped = ObjectListViewSchema.strip(); + expect(verdict(stripped), 'ObjectListViewSchema.strip() is a real derivation').toBe('derived-clone'); + expect(cloneOverlap(stripped), 'strip() keeps the whole shape').toBe(1); + }); + + it('negative: a look-alike that shares no property instance stays out', () => { + // #5056's own negative control, verbatim: deliberately spelled to look like + // every authorable shape in the repo, but built from fresh `z.string()` + // instances, so it shares nothing with the graph. + const { verdict, cloneOverlap } = measureDoors(); + const lookAlike = z.object({ name: z.string(), label: z.string() }); + expect(verdict(lookAlike), 'negative control').toBe('unreachable'); + expect(cloneOverlap(lookAlike)).toBe(0); + }); + + it('flip: injecting a synthetic carrier turns an unreachable shape reachable', () => { + // Without this, "unreachable" and "the walker is broken" are the same + // output. `extraRoots` exists for exactly this control. + const orphan = z.object({ osDoorProbeOrphanKey: z.string() }); + expect(measureDoors().verdict(orphan), 'before').toBe('unreachable'); + const carrier = z.object({ orphan }); + expect(measureDoors([carrier]).verdict(orphan), 'after — a carrier makes the door').toBe('direct'); + }); +}); + +// ============================================================================ +// 3. The #5056 regression boundary itself. +// ============================================================================ +describe('#5056 regression — the any-one-shared-property bridge stays dead', () => { + it('WidgetManifestSchema shares leaves with the live graph but is NOT derived from it', () => { + // The reverse verification, standing rather than one-shot. + // + // The OLD bridge fired when ANY one property of the candidate was def-equal + // to a same-named property of ANY visited object node. `cloneOverlap` is + // the max shared-property count over visited shapes, normalised — so + // `cloneOverlap(x) > 0` is EXACTLY the condition under which the old bridge + // fired. Asserting both halves here pins the fix without keeping a second, + // defective copy of the walker alive to demonstrate it: + // + // > 0 ⇒ the old bridge WOULD have called this file reachable, and did + // < 0.5 ⇒ the fixed bridge correctly does not. + // + // 2 shared keys (`name`, `label`, both described shared LEAVES) out of 19. + // A coincidence, not a derivation — and a false door here would have spent + // a v17 breaking to tighten a file nothing imports (#4583's "precisely + // validated dead slot"). + const { verdict, cloneOverlap } = measureDoors(); + const overlap = cloneOverlap(WidgetManifestSchema); + expect(overlap, 'it DOES share leaves — the old bridge fired on exactly this').toBeGreaterThan(0); + expect(overlap, 'but nothing structural: measured 2/19').toBeLessThan(0.2); + expect(verdict(WidgetManifestSchema)).toBe('unreachable'); + }); + + it('the threshold discriminates by SHARE OF SHAPE, not by count of shared keys', () => { + // Why 0.5 and not "at least 2 shared keys": the same two shared leaves + // decide opposite verdicts depending on how much of the shape they are. + const { cloneOverlap } = measureDoors(); + const shared = { + name: SnakeCaseIdentifierSchema.describe('Machine name'), + label: I18nLabelSchema.describe('Display label'), + }; + const withFiller = z.object({ ...shared, a: z.string(), b: z.string(), c: z.string() }); + expect(cloneOverlap(withFiller), '2 shared of 5 keys').toBeCloseTo(0.4, 5); + }); + + it('KNOWN BOUND — a shape made ENTIRELY of shared leaves still bridges, whatever the threshold', () => { + // Measured limitation, pinned deliberately rather than left latent for a + // later batch to rediscover as a second false door. + // + // The ratio is taken over the CANDIDATE's own keys, so a 2-key shape whose + // both keys are shared leaves scores 1.0 and is judged `derived-clone` — + // no threshold in (0, 1] excludes it, because a genuine `.strip()` of a + // live 2-key schema scores 1.0 too and must stay reachable. The instrument + // cannot separate those two by overlap alone. + // + // It costs nothing today: this is a synthetic shape, and the smallest real + // `no door` shapes the campaign has measured sit far below the threshold + // (WidgetManifestSchema at 2/19). It becomes a real hazard the moment a + // batch measures a SMALL shape (roughly 4 keys or fewer) whose keys are all + // shared leaves — measure `cloneOverlap` and read this pin before trusting + // a `derived-clone` verdict on one. Filed for the campaign as #5828. + const { verdict, cloneOverlap } = measureDoors(); + const allSharedLeaves = z.object({ + name: SnakeCaseIdentifierSchema.describe('Machine name'), + label: I18nLabelSchema.describe('Display label'), + }); + expect(cloneOverlap(allSharedLeaves), '2 shared of 2 keys').toBe(1); + expect(verdict(allSharedLeaves), 'the residual false-reachable case — see #5828').toBe('derived-clone'); + }); +}); diff --git a/packages/spec/src/ui/door-reachability.testkit.ts b/packages/spec/src/ui/door-reachability.testkit.ts index 7937162125..b5db5416ce 100644 --- a/packages/spec/src/ui/door-reachability.testkit.ts +++ b/packages/spec/src/ui/door-reachability.testkit.ts @@ -28,18 +28,34 @@ * no identity with its base but DOES share the base's per-property schema * instances, so a bridge over shared property defs is genuinely needed. The * bridge as first written fired when **any one** property matched under the - * same name — and zod's `.describe()` returns a clone that shares the - * original `_zod.def` OBJECT, which makes every described - * `SnakeCaseIdentifierSchema` / `I18nLabelSchema` def-identical across the - * whole spec. Two unrelated shapes that both declare `name` and `label` (i.e. - * almost every authorable shape here) therefore bridged, and - * `WidgetManifestSchema` — a file nothing imports — measured as REACHABLE. - * The error is one-directional: it can only produce a false door, i.e. it can - * only cause a batch to tighten something dead. + * same name — and zod's `.describe()` returns a clone that reuses the + * `_zod.def` OBJECT **of the instance it was called on**. Because this repo + * funnels dozens of shapes through a handful of SHARED leaf instances, every + * `SnakeCaseIdentifierSchema.describe(…)` in the spec is def-identical to + * every other one, and likewise `I18nLabelSchema`. Two unrelated shapes that + * both declare `name` and `label` (i.e. almost every authorable shape here) + * therefore bridged, and `WidgetManifestSchema` — a file nothing imports — + * measured as REACHABLE. The error is one-directional: it can only produce a + * false door, i.e. it can only cause a batch to tighten something dead. + * + * ⚠️ Sharing follows the **receiver instance**, not the shape: two separate + * `z.string()` calls build two separate defs and share nothing. #5056's issue + * body spelled the fact the other way; the corrected, measured statement and + * every builder's verdict are pinned in `door-reachability.testkit.test.ts`, + * so a zod upgrade that changes `clone()` semantics goes red rather than + * silently changing what this bridge means. * * The fix is to ask how much of the shape is shared rather than whether * anything is: a real derived clone carries nearly all of its base's * properties, while a coincidence carries one or two out of twenty. + * + * **Residual bound (#5828), measured and pinned, not latent:** the ratio is + * taken over the CANDIDATE's own keys, so a shape whose keys are *all* shared + * leaves scores 1.0 however few they are, and still bridges. No threshold in + * (0, 1] excludes it, because a genuine `.strip()` scores 1.0 too. It costs + * nothing today (the real `no door` shapes measured so far sit far below the + * threshold — `WidgetManifestSchema` at 2/19), but read `cloneOverlap` and + * the key count before trusting a `derived-clone` verdict on a SMALL shape. */ import { getMetadataTypeSchema, listMetadataTypeSchemaTypes } from '../kernel/metadata-type-schemas'; @@ -97,11 +113,19 @@ export interface DoorMeasurement { * its own shape, by property def identity under the same name, with one visited * object node. * - * Chosen against both ends of the measured range rather than by taste: 批 15's - * real derivation (`ChartConfigSchema` reached through `ReportChartSchema`, - * which re-narrows two of its keys) sits far above it, and 批 16's false - * positive (`WidgetManifestSchema`, 2 shared keys of 20 — `name` and `label`, - * both shared LEAVES rather than shared structure) sits far below. + * Chosen against both ends of the measured range rather than by taste: real + * derivations sit far above it (`PageSchema.extend(…)` at 0.96, + * `ObjectListViewSchema.strip()` at 1.0 — both pinned as the bridge's positive + * control), and 批 16's false positive sits far below (`WidgetManifestSchema`, + * 2 shared keys of 19 — `name` and `label`, both shared LEAVES rather than + * shared structure, measured 0.105). + * + * Note the chart family this bridge was originally written for measures + * `direct`, not `derived-clone`: `ChartConfigSchema` is in the graph outright. + * So `chart.test.ts` does not exercise this limb at all, and neither does any + * other consumer — which is why the testkit's own test file owns the positive + * control for it. Without that, narrowing the bridge to "never fires" would + * leave every consumer green. */ const DERIVED_CLONE_MIN_OVERLAP = 0.5;