Skip to content

Commit dc1b986

Browse files
fix(spec): narrow the object designer's formula returnType options to the four declared members (#19759)
Fixes #19677 Clause-②: no ## The defect `FieldSchema.returnType` declares four members — `number`, `text`, `boolean`, `date`. The object designer's quick-add grid (`packages/spec/src/data/object.form.ts`, the `fields` repeater) declared an inline `options` list of **six**, adding `datetime` and `currency`. An author who added a formula field from the object designer and picked Datetime or Currency wrote a value the parse rejects. The failure is silent at the point of choice: the select is populated from that inline list, nothing reconciles it against the enum, and the refusal arrives later from the save door naming a key the author never typed. ## The fix Narrow the control to the four declared members. One write site, plus a regression test and a patch changeset. This is a pull-back to a spelling that already existed in this package, not a decision about what a formula may return: - **The enum is unchanged.** `packages/spec/src/data/field.zod.ts` is untouched — `returnType` accepted exactly these four before this change and accepts exactly these four after it. No authorable value is removed, because neither `datetime` nor `currency` was ever accepted; what is removed is an offer with nothing behind it. - **The sibling control already spelled it correctly.** The field designer's own `returnType` select in `packages/spec/src/data/field.form.ts` carries the same explicit four-member list, with a comment saying why it is explicit. That file is untouched too. The object designer was the lone divergent carrier; three carriers now agree, counting the published reference doc (`content/docs/references/data/field.mdx`). - **The producer side agrees with the enum.** Authoring stamps `returnType` from the inferred CEL type, and `inferExpressionType` in `@objectstack/formula` is typed `number | text | boolean | date | unknown`. There is no path by which the platform ever stamps `datetime` or `currency`, so the six-member list was never a capability that existed and got broken. The opposite exit — widening the enum to six — was considered and rejected on that last reading. It would be a Feature and a clause-② `yes`, and nothing in the producer, the sibling control or the published docs suggests the platform means to support datetime- or currency-returning formulas. ## The regression test `packages/spec/src/data/form-return-type-options.test.ts`, 13 assertions over both forms. The expected set is **derived from `FieldSchema` at runtime** — read through the same JSON-Schema view the metadata-admin renderer falls back to when a row declares no `options` at all — rather than hard-coded as four strings. A test pinned to literals rots exactly the way this defect did: the enum moves, the literals do not, and the test keeps passing over a control that has drifted again. The offer is judged at **value level**, not key level: every offered value must survive a full `safeParse` on a formula field. A key-level check would have passed on the broken grid, because `returnType` was always a recognised key. The two removed members are additionally pinned as refused-and-unoffered. That is not a duplicate of the set equality — it is the assertion that fires if the platform ever decides a formula may return a datetime or a currency, so that the widening and the two forms move in one change. ## Verification All commands run in a dedicated worktree at `593004aa`. Heavy runs went through `scripts/pm/os-verify-lock.sh`; exit codes captured before any pipe. | command | verdict | |:--|:--| | `pnpm --filter '@objectstack/spec^...' build` | exit 0 — closure empty ("No projects matched the filters"); `packages/spec` has no workspace dependencies | | `pnpm --filter @objectstack/spec build` | exit 0 — 34/34 declaration files, dist stamp `e3782a80bd03` | | `pnpm --filter @objectstack/spec exec vitest run src/data/form-return-type-options.test.ts` | exit 0 — 13 passed | | `pnpm --filter @objectstack/spec test` | exit 0 — **516 files, 15056 passed**, 1 todo | | `pnpm --filter @objectstack/spec typecheck` | exit 0 — `tsc --noEmit` + scripts + test layer | | `pnpm --filter @objectstack/spec check:generated` | exit 0 — **all 15 generated artifacts up to date**; no regeneration needed | | `pnpm lint` (repo-wide, `eslint . --no-inline-config`) | exit 0 — full population, no narrowing | | `pnpm check:type-check-coverage` | exit 0 — 77/81 packages, test layer hides 0 files | | 10 spec source audits (`check:empty-state`, `variant-docs`, `yaml-examples`, `exported-any`, `dual-source-exports`, `entry-nameability`, `browser-reachable-entries`, `llms-txt`, `duration-unit-keys`, `objectui-pin-citations`) | exit 0 each | | `check:nul-bytes`, `check:cross-package-test-inputs`, `check:test-source-alias`, `check:issue-citations`, `check:doc-authoring`, `check:published-files`, `check:spec-docblock-symbol-anchors` | exit 0 each | | `check-changeset-no-major --base origin/main`, `check-adr-0087-registration --base origin/main`, `check-empty-changeset --base origin/main` (+ self-tests), `check:objectui-changeset`, `check:changeset-gate-self-tests` | exit 0 each | | `pnpm check:type-check-debt` | **NOT MEASURED** — exit 3, `PREREQUISITE NOT MET`: `--re-measure` refuses without 30 workspace dependencies built. Nothing was measured, so this is neither a pass nor a finding. The ledgered packages are not in this diff. CI builds that closure before the step. | **Reverse verification.** The fix was committed first (`3f4c20ea`), then the six-member list was reinstated on disk through `scripts/ablation-replace.mjs` — anchor 1 occurrence, dropping to 0; blob `38a494a2c532` moving to `ee2e9994b641`, both counted by the tool itself rather than by a claim. The test went **red in the expected direction**: 4 of 13 failed — the set equality (`received` carrying `currency` and `datetime`), the value-level parse (`FieldSchema` answering `invalid_value` on `'datetime'`), the historical-member pin, and the two-designers agreement. The other 9 stayed green, which is the point: the derivation, the single-declaration check and the DSL-expressibility check are orthogonal to the mutation. Restore verified by the tool on the way out: blob back to `38a494a2c532`, equal to the blob at HEAD, and `git diff HEAD` empty. No ablation artefact remains in the tree. No `dist` preflight was needed for it: this test imports `./object.form` relative, inside its own package, so vitest resolves the source directly and no build sits between the mutation and the assertion. ## The changeset `packages/spec` publishes this file. Measured rather than assumed: the changed `helpText` string resolves in 8 files under `packages/spec/dist`, and a positive control taken from the same source file (`Add the columns this object will store`) resolves in the same 8. `dist` is in the package's `files` whitelist, so the fix ships and takes a `patch` changeset — `skip-changeset` would have been wrong. ## Acceptance notes - **No reconciliation gate.** Nothing mechanically holds a form's `options` list to its schema enum; both this grid and the sibling control are hand-maintained lists. The dispatch put that outside this round — a new required gate is the maintainer's call, not a rider — so the regression test covers this one key rather than the class. Noted, not filed. - **Repo-wide, this was the only divergent carrier.** Every `returnType` occurrence under `packages/`, `content/` and `examples/` on `origin/main` was read: the six-member list existed in `object.form.ts` alone. Every other carrier already agreed with the enum. - **Stored documents remain unmeasured**, exactly as the filer left them. Whether any authored field carries `returnType: 'datetime'` or `'currency'` today is not read here, and the fix does not depend on it either way — the parse that refuses those values is the one that already ran. - **The objectui side is unestablished.** Whether that repo's own field designer offers the same six is outside this radius and was not read. ## Scope Three files. `field.zod.ts` and `field.form.ts` were read-only this round and are untouched; taking either would have been the other exit. --- _Generated by [Claude Code](https://claude.ai/code/session_013RDBh5DqXd2xnLwvHLgLFr)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a754563 commit dc1b986

3 files changed

Lines changed: 243 additions & 2 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
The object designer's quick-add grid no longer offers two formula `returnType` members that `FieldSchema` refuses (#19677).
6+
7+
`FieldSchema.returnType` declares four members — `number`, `text`, `boolean`, `date`. The `fields` repeater in `object.form.ts` declared an inline `options` list of **six**, adding `datetime` and `currency`. An author who added a formula field from the object designer and picked Datetime or Currency wrote a value the parse rejects: the select is populated from that inline list, nothing reconciled it against the enum, and the refusal arrived later from the save door naming a key the author never typed.
8+
9+
The control is narrowed to the four declared members. This is a pull-back to a spelling that already existed in this package twice, not a decision about what a formula may return:
10+
11+
- **The enum is unchanged** — `FieldSchema.returnType` accepted exactly these four before this change and accepts exactly these four after it. No authorable value is removed, because neither `datetime` nor `currency` was ever accepted; what is removed is an offer with nothing behind it.
12+
- **The sibling control already spelled it correctly.** The field designer's own `returnType` select in `field.form.ts` carries the same explicit four-member list. The object designer was the lone divergent carrier; three now agree, counting the published reference doc.
13+
- **The producer side agrees with the enum too.** Authoring stamps `returnType` from the inferred CEL type, and that inference is typed `number | text | boolean | date | unknown`, so there is no path by which the platform stamps `datetime` or `currency`.
14+
15+
Whether any stored field carries `returnType: 'datetime'` or `'currency'` today is not measured here and is unaffected either way: the parse that refuses those values is the one that already ran.
16+
17+
The regression test derives its expected set from `FieldSchema` at runtime rather than hard-coding four strings — a test pinned to literals rots exactly the way this defect did — and judges the offer at value level: every offered value must survive a full parse on a formula field.
Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,217 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #19677 — the object designer must not OFFER a formula `returnType` the
5+
* schema REFUSES.
6+
*
7+
* `FieldSchema.returnType` has always been `z.enum(['number','text','boolean',
8+
* 'date'])`. The object designer's quick-add grid (`object.form.ts`, the
9+
* `fields` repeater) declared an inline `options` list of SIX — the four plus
10+
* `datetime` and `currency` — so an author who added a formula field there and
11+
* picked either of the extra two wrote a value the parse refuses. The failure
12+
* is silent at the point of choice: the select is populated from that inline
13+
* list, nothing reconciles it with the enum, and the refusal arrives later from
14+
* the save door naming a key the author never typed.
15+
*
16+
* The exit is a pull-back, not a decision about what a formula may return. Two
17+
* carriers in this package already spelled the accept set correctly — the enum
18+
* itself, and the field designer's own control in `field.form.ts` (four
19+
* members, explicit list). Narrowing the grid makes it three.
20+
*
21+
* ## Why the four, and not the six
22+
*
23+
* The producer side agrees with the enum, not with the grid: authoring stamps
24+
* `returnType` from the inferred CEL type, and that inference is typed
25+
* `number | text | boolean | date | unknown` (`@objectstack/formula`'s
26+
* `inferExpressionType`). There is no path by which the platform stamps
27+
* `datetime` or `currency`, so the six-member list was never a capability that
28+
* existed and got broken — it was an offer with nothing behind it.
29+
*
30+
* ## Why the expectation is DERIVED and not four literals
31+
*
32+
* A test asserting the row's `options` against a hard-coded list of four rots
33+
* exactly the way the bug did: the enum moves, the literals do not, and the
34+
* test keeps passing over a control that has drifted again. So the expected set
35+
* is read from `FieldSchema` at runtime, through the same JSON-Schema view the
36+
* metadata-admin renderer falls back to when a row declares no `options` at
37+
* all. If the enum gains or loses a member, this file fails until the forms
38+
* move with it.
39+
*
40+
* ## What the historical-roster assertions are for
41+
*
42+
* The two removed members are additionally pinned as REFUSED-and-UNOFFERED.
43+
* That is not a duplicate of the set equality: it is the pin that fires if the
44+
* platform ever decides a formula may return a datetime or a currency. Widening
45+
* the enum is a product decision with its own card, and this is where it is
46+
* made to notice that the two forms must move in the same change.
47+
*/
48+
49+
import { describe, it, expect } from 'vitest';
50+
import { z } from 'zod';
51+
52+
import { objectForm } from './object.form';
53+
import { fieldForm } from './field.form';
54+
import { FieldSchema } from './field.zod';
55+
import { FormViewSchema } from '../ui/view.zod';
56+
57+
// ────────────────────────────────────────────────────────────────────────────
58+
// The expectation, DERIVED from the schema at runtime.
59+
// ────────────────────────────────────────────────────────────────────────────
60+
61+
/**
62+
* `returnType`'s declared members, read off `FieldSchema` itself. This is also
63+
* the renderer's derived option source — what a row with no inline `options`
64+
* would offer — so a form that declares nothing and a form that declares the
65+
* full list are judged against one authority.
66+
*/
67+
function schemaReturnTypeMembers(): string[] {
68+
const js = z.toJSONSchema(FieldSchema as unknown as z.ZodType, {
69+
unrepresentable: 'any',
70+
io: 'input',
71+
}) as { properties?: Record<string, { enum?: unknown[] }> };
72+
const node = js.properties?.returnType;
73+
if (!node) throw new Error('FieldSchema no longer exposes a `returnType` JSON Schema node');
74+
if (!Array.isArray(node.enum)) {
75+
throw new Error(
76+
'`FieldSchema.returnType` no longer resolves to a closed enum in the JSON Schema view; '
77+
+ 'this test derives its expectation from that enum and cannot judge the forms without it.',
78+
);
79+
}
80+
return node.enum.map((m) => String(m));
81+
}
82+
83+
/**
84+
* The members this round removed from the grid. Deliberately literal: they are
85+
* a record of what was wrongly offered, not the expectation — the expectation
86+
* above is derived. If one of these ever becomes a declared member, the
87+
* assertions below fail and the forms are re-opened on purpose.
88+
*/
89+
const HISTORICALLY_OFFERED_NON_MEMBERS = ['datetime', 'currency'] as const;
90+
91+
// ────────────────────────────────────────────────────────────────────────────
92+
// Locating the declarations
93+
// ────────────────────────────────────────────────────────────────────────────
94+
95+
type Decl = { path: string; spec: Record<string, unknown> };
96+
97+
/** Every form-field spec named `key`, at any depth (sections, repeater rows). */
98+
function findDeclarations(node: unknown, key: string, path = '$', out: Decl[] = []): Decl[] {
99+
if (!node || typeof node !== 'object') return out;
100+
if (Array.isArray(node)) {
101+
node.forEach((n, i) => findDeclarations(n, key, `${path}[${i}]`, out));
102+
return out;
103+
}
104+
const rec = node as Record<string, unknown>;
105+
if (rec.field === key) out.push({ path, spec: rec });
106+
for (const child of ['sections', 'fields'] as const) {
107+
if (rec[child]) findDeclarations(rec[child], key, `${path}.${child}`, out);
108+
}
109+
return out;
110+
}
111+
112+
/** Declared option VALUES, or `undefined` when the spec has no inline list. */
113+
function declaredOptionValues(spec: Record<string, unknown>): string[] | undefined {
114+
const opts = spec.options;
115+
if (!Array.isArray(opts) || opts.length === 0) return undefined;
116+
return opts.map((o) => String((o as { value?: unknown }).value));
117+
}
118+
119+
/**
120+
* What a form actually offers for `returnType`: its inline list, or — when it
121+
* declares none — the derived enum the renderer falls back to. Both spellings
122+
* are a full offer and are judged against the same expected set, so neither can
123+
* drift unnoticed.
124+
*/
125+
function offeredValues(decl: Decl): string[] {
126+
return declaredOptionValues(decl.spec) ?? schemaReturnTypeMembers();
127+
}
128+
129+
const FORMS = [
130+
{ name: 'objectForm', form: objectForm as unknown },
131+
{ name: 'fieldForm', form: fieldForm as unknown },
132+
] as const;
133+
134+
/** A minimal formula field carrying one `returnType`, for a full parse. */
135+
function formulaFieldWith(returnType: string): Record<string, unknown> {
136+
return {
137+
name: 'line_total',
138+
label: 'Line Total',
139+
type: 'formula',
140+
expression: 'record.quantity * record.unit_price',
141+
returnType,
142+
};
143+
}
144+
145+
// ────────────────────────────────────────────────────────────────────────────
146+
147+
describe('#19677 — `returnType` is never offered a value the schema refuses', () => {
148+
it('derives a non-empty closed member set from FieldSchema', () => {
149+
// The floor for every assertion below: an empty or unreadable derivation
150+
// would make the set-equality checks vacuously true.
151+
const members = schemaReturnTypeMembers();
152+
expect(members.length).toBeGreaterThan(0);
153+
expect(new Set(members).size).toBe(members.length);
154+
});
155+
156+
for (const { name, form } of FORMS) {
157+
describe(name, () => {
158+
const decls = findDeclarations(form, 'returnType');
159+
160+
it('declares exactly one `returnType` control — no overlap, no gap', () => {
161+
// Two declarations of one key have two ways to go wrong that a bare
162+
// "datetime is absent" assertion would not see: both rendering (two
163+
// selects writing one key), or a narrowing applied to only one of them.
164+
expect(decls.map((d) => d.path)).toHaveLength(1);
165+
});
166+
167+
it('offers exactly the members FieldSchema declares — no more, no fewer', () => {
168+
const offered = offeredValues(decls[0]);
169+
expect([...offered].sort()).toEqual([...schemaReturnTypeMembers()].sort());
170+
});
171+
172+
// THE DEFECT, stated as the value-level judgment: every value this
173+
// control can write must survive a full parse. A key-level check would
174+
// pass on the broken grid — `returnType` was always a recognised key.
175+
it('offers only values that a formula field actually parses with', () => {
176+
for (const value of offeredValues(decls[0])) {
177+
const parsed = (FieldSchema as unknown as z.ZodType).safeParse(formulaFieldWith(value));
178+
expect(
179+
parsed.success,
180+
`${name} offers returnType '${value}', which FieldSchema refuses: `
181+
+ JSON.stringify((parsed as { error?: unknown }).error),
182+
).toBe(true);
183+
}
184+
});
185+
186+
it('offers neither of the two members the grid used to carry', () => {
187+
const offered = offeredValues(decls[0]);
188+
for (const value of HISTORICALLY_OFFERED_NON_MEMBERS) {
189+
expect(offered).not.toContain(value);
190+
}
191+
});
192+
193+
it('is expressible in the form DSL as it stands — no schema extension', () => {
194+
// `defineForm` parses at module load; re-parsing makes the claim an
195+
// assertion rather than an import side effect.
196+
const parsed = (FormViewSchema as unknown as z.ZodType).safeParse(form);
197+
expect(parsed.success, JSON.stringify((parsed as { error?: unknown }).error)).toBe(true);
198+
});
199+
});
200+
}
201+
202+
it('the two removed members are refused by the schema, which is why they went', () => {
203+
// The justification for the narrowing, asserted rather than asserted-about.
204+
// Should the platform ever declare one of these, this fails and the two
205+
// forms are re-opened in the same change that widens the enum.
206+
for (const value of HISTORICALLY_OFFERED_NON_MEMBERS) {
207+
expect(schemaReturnTypeMembers()).not.toContain(value);
208+
const parsed = (FieldSchema as unknown as z.ZodType).safeParse(formulaFieldWith(value));
209+
expect(parsed.success, `FieldSchema unexpectedly accepts returnType '${value}'`).toBe(false);
210+
}
211+
});
212+
213+
it('the object designer and the field designer offer the same list, in the same order', () => {
214+
const lists = FORMS.map(({ form }) => offeredValues(findDeclarations(form, 'returnType')[0]));
215+
expect(lists[0]).toEqual(lists[1]);
216+
});
217+
});

‎packages/spec/src/data/object.form.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,9 +288,16 @@ export const objectForm = defineForm({
288288
// `expression`, not `formula` — the key is named for what it holds,
289289
// not for the field type that uses it.
290290
{ field: 'expression', type: 'code', language: 'expression', helpText: 'CEL formula expression', visibleWhen: "data.type == 'formula'" },
291+
// The four members are what `FieldSchema.returnType` declares, and
292+
// the same explicit list the field designer's own control carries
293+
// in `field.form.ts` (#19677). This grid additionally offered
294+
// `datetime` and `currency`, so an author who added a formula field
295+
// here and picked either wrote a value the parse refuses — the
296+
// refusal arriving from the save door, naming a key they never
297+
// typed. Declared-vs-enforced, one seam before that door.
291298
{ field: 'returnType', type: 'select', helpText: 'Result type for formulas', visibleWhen: "data.type == 'formula'", options: [
292-
{ label: 'Text', value: 'text' }, { label: 'Number', value: 'number' }, { label: 'Boolean', value: 'boolean' },
293-
{ label: 'Date', value: 'date' }, { label: 'Datetime', value: 'datetime' }, { label: 'Currency', value: 'currency' },
299+
{ label: 'Text', value: 'text' }, { label: 'Number', value: 'number' },
300+
{ label: 'Boolean', value: 'boolean' }, { label: 'Date', value: 'date' },
294301
] },
295302
// A roll-up is ONE key — `summaryOperations` {object, field, function}.
296303
// The flat `summaryType` / `summaryField` pair named neither of them

0 commit comments

Comments
 (0)