Skip to content

Commit e07566b

Browse files
fix(lint,spec): field-no-consumers reads a subform entry's child keys against the child, and credits a derived inline grid through deriveInlineGridColumns (#21089)
Fixes #20951 Clause-②: yes (widening) ## What changes `field-no-consumers` (`packages/lint/src/validate-field-consumers.ts`) called two kinds of in-use child field "inert". Both are corrected here, and the second goes through one new derivation the spec owns. **Site 1: a `subforms` entry's child-field keys are read per key.** `amountField` ("Numeric child column summed for the running total") is now read against the entry's `childObject`. `totalField` ("Parent field to receive the rolled-up sum") stays on the parent, which is the context the walk already carries. The read uses the child resolution that PR #20950 added for `subforms[].columns`: `strName(rec.childObject)` in the `CHILD_COLLECTION_KEYS` branch of `walk`. There is no second child-object lookup. The generic walk now skips these keys on an entry, so a same-named parent field is no longer credited in the child's place. - `relationshipField` ("FK on the child pointing back to the parent") gets the same per-key read. This is a bounded in-place fix: the same defect class as `amountField`, in the same file, with the same gate family. Evidence is in the probe table below (`pr_quote_line.quote`). The renderer loads the child rows with `$filter` on this key and stamps it on save, so the field is read. **Site 2: a derived inline grid credits the columns it draws.** The new `deriveInlineGridColumns` lives in `packages/spec/src/data/inline-grid-columns.ts`, beside `deriveFieldGroupLayout`, and is exported through the `@objectstack/spec/data` barrel. The lint credits exactly what it returns, `defaultHidden` overflow included, because those columns are collapsed into the column chooser and never dropped. Two carriers trigger it: - a relationship field with `inlineEdit` (`true`, `'grid'` or `'form'`; both modes pass the same `columns` to the grid), `type` `master_detail` or `lookup`, a target that resolves, and no authored `inlineColumns` (absent or empty). These are the conditions objectui's `attachInlineSubforms` checks; - a `subforms` entry with no `columns` (absent or empty). The spec documents this second carrier with the same words, "derived from the child object when omitted", and the renderer uses the same derivation for it. ## The spec function and objectui's rule (triage's ⛔, PM hypothesis H2) Signature: `deriveInlineGridColumns(def: unknown, opts?: { relationshipField?: string; exclude?: readonly string[]; maxColumns?: number }): DerivedInlineGridColumn[]`. `DerivedInlineGridColumn` is `{ name: string; defaultHidden?: true }`, which is a valid identity-only `inlineColumns` entry. `DEFAULT_MAX_INLINE_GRID_COLUMNS` is `6`. Import path: `@objectstack/spec/data`. The input discipline matches `deriveFieldGroupLayout`: it takes the child object's definition and tolerates un-parsed input. **The rule, as measured** in objectui `main` at `be5211522412` (`packages/plugin-form/src/deriveMasterDetail.ts`, `deriveColumns` plus `curateColumns`, read over REST): - Every child field is a candidate, in the field map's order. - A field is skipped when: - its name is an identity, audit, tenancy or ownership column (`id`, `_id`, `recordId`, `created_at`/`updated_at`/`created_by`/`updated_by` and their camelCase forms, `organization_id`, `tenant_id`, `space`, `owner`); - its name is a sort-position name (`position`, `sort_order`, `sequence`, `line_no`, `line_number`, `sort`); - it is the relationship field, or a name in `exclude`; - it is flagged `system`, `readonly` or `hidden` (a truthy value is enough); - its type cannot be edited in a cell: `formula`, `summary`, `rollup`, `autonumber`, `auto_number`, `json`, `object`, `grid`, `table`, `location`, `vector`, `html`, `markdown` or `richtext`. - Visible budget: 6. The first name-like column is kept visible (or the first column, when none is name-like), and so is every required column. A computed column is never required. The remaining slots go by cell type: select first, then currency and number, then lookup, then date, datetime and time, then text, and file last. Ties keep field order. Columns past the budget are marked `defaultHidden`. `maxColumns` of `0` or less marks no column hidden. **The differential: 80,004 cases and 0 mismatches.** I ran the spec function against objectui's `deriveColumns`, imported from that `main` file. The cases were objectui's 4 own fixtures, 50,000 random definitions and 30,000 wide definitions built to exercise the budget (29,075 of them produced `defaultHidden` columns). The random inputs included null field definitions, array-shaped `fields`, non-spec type names, truthy and falsy flag values, CEL-envelope expressions, and `NaN`, negative and absent `maxColumns`. Names, order and `defaultHidden` matched in every case. So the spec function reproduces the rule with no behaviour change. objectui's renderer is not touched here. **For objectui's switch (the coordination child; not in this PR):** `hydrateColumns(deriveInlineGridColumns(schema, opts), schema)` equals `deriveColumns(schema, opts)` in every case but one kind. When a derived field's own definition is falsy (`null`), `hydrateColumns` returns the bare `{ name }` where `deriveColumns` builds a text column labelled with the name. That covered 3,889 of the cases, all of that kind. A served schema never carries a null field definition. Still, keeping objectui's own per-column builder over the returned names makes the switch exact by construction. ## Evidence **The door: `os validate --json`, CLI from this branch's source, before vs after.** The probe stack is a `defineStack` app with four parent/child pairs. "Before" rebuilt `@objectstack/lint` from the base commit's source; `ablation-dist-preflight --absent` confirmed the change was gone from `dist/`. Both runs exit 0 with `valid: true`. | field | before | after | why | |:--|:--|:--|:--| | `pr_invoice_line.line_total` | inert | not reported | site 1: `amountField` | | `pr_invoice.line_total` (unused parent twin) | not reported | inert | it was credited in the child's place | | `pr_invoice_line.total` (unused child twin) | inert | inert | control: `totalField` stays on the parent | | `pr_invoice_line.memo` | inert | inert | control: nothing reads it | | `pr_quote_line.quote` (a `lookup` FK) | inert | not reported | `relationshipField`, per key | | `pr_order_item.sku`, `.quantity` | inert | not reported | site 2: derived grid columns | | `pr_ticket_note.body` | inert | not reported | site 2, on a `lookup` relationship | | `pr_order_item.secret` (`hidden`) | inert | inert | lit control: the derivation leaves it out | **A real producer: `examples/app-showcase`, the same door, before vs after.** The finding count went from 57 to 54, and no finding was added. The three removed findings are `showcase_expense_line.category`, `.incurred_at` and `.incurred_on`, which were `carrier-only` before. `showcase_expense_line.expense_report` sets `inlineEdit: 'grid'` with no `inlineColumns`. `examples/app-crm` `opportunity_line_item.opportunity` has the same shape (I read it; I did not run it). **Tests** (final HEAD `513570747`): - `pnpm --filter @objectstack/lint exec vitest run`: 118 files, 5,467 tests passed. This includes the new `[#20951]` block in `validate-field-consumers.test.ts` (15 tests) and the unchanged `[#20929]` block. - `pnpm --filter @objectstack/spec exec vitest run --project local`: 585 files, 17,222 passed and 1 todo. That run was at `bf01c7979`; the only later commit is a one-line lint change, and the new `inline-grid-columns.test.ts` (11 tests) was re-run at `513570747`. - `pnpm --filter @objectstack/spec --filter @objectstack/lint run typecheck`: both exit 0, and `check:test-typecheck` is OK for both. - The lint import of `deriveInlineGridColumns` compiles only against the rebuilt `.d.ts`, because the name does not exist in the base build. - `pnpm --filter @objectstack/cli exec vitest run --project unit`: 238 files and 3,391 tests passed. 2 files (10 tests) are NOT MEASURED; see below. **Reverse verification.** The fix was committed first. Then `validate-field-consumers.ts` was restored to the base blob `4c109d4ef`. With that source, 11 of the 15 new tests fail, and the 4 baselines and controls pass. The restore went through `git checkout HEAD --` and was checked by blob hash (`2ef0118a7`, then equal to HEAD); `git status` was clean afterwards. **Gates.** `node scripts/pm/dispatch-gates.mjs --commands` was derived from this diff (8 paths, 86 commands) and every command was run at `513570747`. `--ran` reports "86 derived, 84 run, 2 NOT-MEASURED, 0 UNRUN". 83 exited 0, including `check:generated` (all 15 artefacts up to date after `gen:api-surface` and `gen:export-origins`), `check:api-surface`, `check:export-origins`, `check:entry-nameability`, `check:dual-source-exports`, `check:spec-changes` (inside `check:generated`), `check:nul-bytes` and `check:engine-double-contract`. The rest are NOT MEASURED, listed below. **NOT MEASURED** (none of these are a verdict on this diff): - `check:dts-closure` exited 1. It names 55 packages with missing `.d.ts`. This tree built those packages with `OS_SKIP_DTS=1`, only so that `os validate` could run from source. `spec`, `lint`, `formula` and `sdui-parser` had full builds and are not named. - `check:dual-build-cjs-loads` and `check:type-check-debt` exited 3 with `PREREQUISITE NOT MET`: they need the whole workspace built with declarations. - In the CLI unit tier, `published-subpath-console.pin.test.ts` and `published-subpath-hook-body.pin.test.ts` (10 tests) fail with `ENOENT` on `packages/cli/dist/*.d.ts`. That is the same JS-only build. - The CLI `integration` tier and `packages/qa/dogfood` (an importer of `field-group-layout`, whose bytes do not change) are left to CI. - I did not merge `main`. Since the base, 14 commits have landed there, and none of them touches the 8 paths in this diff. ## Acceptance notes - **Exports.** The claim said one new export. There are three: the function, its element type and the budget constant. The constant lets objectui re-export one value instead of keeping a second `6`. No accept set moves. - **Landing sites.** Everything lands at the expected sites. The derived-grid credit also covers `subforms` entries with no `columns`, and `relationshipField` is read per key. Both are named above. - **Boundary: a `subforms` entry that names no `relationshipField`.** The renderer detects the FK itself. The lint keeps no copy of that detection, so the derived list it credits includes the FK. The FK is read anyway, as the join key, so the verdict is unchanged. - **Boundary: an explicit override.** When a parent form has an explicit `form.subforms` entry for the same child, objectui draws that entry instead of the field-derived grid. The lint still credits the field-derived grid, which matches how PR #20950 already treats `inlineColumns`. This is an over-credit in that case only. - **Not filed; same family, measured at the door after this PR, handed to the seat:** - A `lookup` relationship field that sets `inlineEdit` is the inline grid's join key, yet `pr_ticket_note.ticket` is still reported `inert`. `master_detail` is exempt; `lookup` is not. - Fields drawn only in the per-row expand form (objectui `deriveFormFields`: rich text, JSON, `readonly`) are still reported. `pr_order_item.spec_sheet`, a `richtext` field, is reported `inert`, while the grid offers the expand form because the child has more form fields than grid columns. --- _Generated by [Claude Code](https://claude.ai/code/session_017VaLJnYwhPsanVCe9dMCJU)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d19da5d commit e07566b

8 files changed

Lines changed: 690 additions & 5 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/lint': patch
4+
---
5+
6+
`deriveInlineGridColumns` (`@objectstack/spec/data`) derives the default columns of an inline master-detail grid, and `field-no-consumers` stops calling two kinds of in-use child field "inert" (#20951).
7+
8+
Clause-②: yes (widening)
9+
10+
- **`@objectstack/spec`.** New exports from `@objectstack/spec/data`: `deriveInlineGridColumns(def, { relationshipField?, exclude?, maxColumns? })`, its element type `DerivedInlineGridColumn`, and `DEFAULT_MAX_INLINE_GRID_COLUMNS` (`6`). The function answers which child fields an inline grid draws when its author listed no columns: a relationship field with `inlineEdit` and no `inlineColumns`, or a `subforms` entry with no `columns`. It returns identity-only entries (`{ name }`, plus `defaultHidden: true` on columns past the visible budget, which collapse into the column chooser and are never dropped), in the child's field order, skipping system, audit, tenancy, ownership and sort-position names, the relationship field, `system` / `readonly` / `hidden` fields and the types a grid cell cannot edit. It is the renderer's current rule, reproduced exactly; the renderer hydrates each column from the child field. No schema accepts anything new or refuses anything new.
11+
- **`@objectstack/lint`.** `field-no-consumers` now reads a `subforms` entry's `amountField` and `relationshipField` against the entry's `childObject`, and keeps `totalField` on the parent. It also credits the columns of a derived inline grid through `deriveInlineGridColumns`. Before, `os validate` warned that the child's summed amount column, the subform's relationship field and every derived grid column were inert, and credited a same-named parent field in the amount column's place. A field the derivation leaves out (for example a `hidden` one) is still reported.

‎packages/lint/src/validate-field-consumers.test.ts‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -669,3 +669,169 @@ describe('[#20929] validateFieldConsumers — an inline grid column names a fiel
669669
expect(verdicts(stack({}, {}, { datasets }))).toEqual({ 'inv.qty': 'inert', 'line.qty': 'inert', 'line.memo': 'inert' });
670670
});
671671
});
672+
673+
/**
674+
* [#20951] Two more positions where an inline child collection names a CHILD
675+
* field without the walk crediting it there.
676+
*
677+
* Site 1 — a `subforms` entry's child-field keys, read per key. `amountField`
678+
* ("Numeric child column summed for the running total") and
679+
* `relationshipField` ("FK on the child pointing back to the parent") name
680+
* fields of the entry's `childObject`; `totalField` ("Parent field to receive
681+
* the rolled-up sum") names a field of the PARENT. The walk carried the
682+
* parent's context into the entry, so the child's amount column read as inert
683+
* while a same-named parent field was credited in its place.
684+
*
685+
* Site 2 — a DERIVED inline grid. A relationship field with `inlineEdit` and
686+
* no `inlineColumns`, or a `subforms` entry with no `columns`, draws the
687+
* columns `deriveInlineGridColumns` (`@objectstack/spec/data`) derives from
688+
* the child object. The rule credits exactly those, and nothing the derivation
689+
* leaves out.
690+
*
691+
* `inv` is the parent, `line` the child. Each declares a field the other's
692+
* key names, so a key read against the wrong object shows up as the wrong
693+
* one of the pair going quiet.
694+
*/
695+
describe('[#20951] validateFieldConsumers — a child collection credits its child fields per key, and a derived grid its derived columns', () => {
696+
const data = { provider: 'object', object: 'inv' };
697+
const MASTER_DETAIL = { type: 'master_detail', reference: 'inv' };
698+
699+
const stack = (lineFields: AnyRec, view: AnyRec = {}, invoice: AnyRec = MASTER_DETAIL): AnyRec => ({
700+
objects: [
701+
{ name: 'inv', fields: { name: { type: 'text' }, total: { type: 'number' }, line_total: { type: 'number' } } },
702+
{ name: 'line', fields: { name: { type: 'text' }, invoice, ...lineFields } },
703+
],
704+
views: [{ list: { type: 'grid', data, columns: [{ field: 'name' }] }, ...view }],
705+
});
706+
707+
/** `object.field` → verdict, for every field the rule reports. */
708+
const verdicts = (s: AnyRec): Record<string, string> =>
709+
Object.fromEntries(validateFieldConsumers(s).map((f) => [`${f.object}.${f.field}`, f.verdict]));
710+
711+
describe('site 1: `amountField` and `relationshipField` on the child, `totalField` on the parent', () => {
712+
const lineFields = { qty: { type: 'number' }, line_total: { type: 'number' }, total: { type: 'number' }, memo: { type: 'text' } };
713+
const entry = (extra: AnyRec): AnyRec => ({ childObject: 'line', columns: [{ name: 'qty' }], ...extra });
714+
715+
it('baseline: with neither key, every field but the grid column is reported', () => {
716+
const form = { type: 'simple', data, subforms: [entry({})] };
717+
expect(verdicts(stack(lineFields, { form }))).toEqual({
718+
'inv.total': 'inert',
719+
'inv.line_total': 'inert',
720+
'line.line_total': 'inert',
721+
'line.total': 'inert',
722+
'line.memo': 'inert',
723+
});
724+
});
725+
726+
it("`amountField` credits the CHILD's field, `totalField` the PARENT's — each same-named counterpart stays reported", () => {
727+
const form = { type: 'simple', data, subforms: [entry({ amountField: 'line_total', totalField: 'total' })] };
728+
expect(verdicts(stack(lineFields, { form }))).toEqual({
729+
'inv.line_total': 'inert',
730+
'line.total': 'inert',
731+
'line.memo': 'inert',
732+
});
733+
});
734+
735+
it('each `formViews` entry, the same way', () => {
736+
const edit = { type: 'simple', data, subforms: [entry({ amountField: 'line_total', totalField: 'total' })] };
737+
expect(verdicts(stack(lineFields, { formViews: { edit } }))).toEqual({
738+
'inv.line_total': 'inert',
739+
'line.total': 'inert',
740+
'line.memo': 'inert',
741+
});
742+
});
743+
744+
it('`relationshipField` credits the child FK the rows are loaded and saved by', () => {
745+
const link = { type: 'lookup', reference: 'inv' };
746+
const without = { type: 'simple', data, subforms: [entry({ amountField: 'line_total', totalField: 'total' })] };
747+
const withKey = { type: 'simple', data, subforms: [entry({ amountField: 'line_total', totalField: 'total', relationshipField: 'link' })] };
748+
expect(verdicts(stack({ ...lineFields, link }, { form: without }))['line.link']).toBe('inert');
749+
expect(verdicts(stack({ ...lineFields, link }, { form: withKey }))['line.link']).toBeUndefined();
750+
});
751+
752+
it('control: each key is read against its own object, so swapped keys credit the swapped pair', () => {
753+
const form = { type: 'simple', data, subforms: [entry({ amountField: 'total', totalField: 'line_total' })] };
754+
// `amountField: 'total'` names the CHILD's `total`, `totalField:
755+
// 'line_total'` the PARENT's `line_total` — what the keys literally say.
756+
expect(verdicts(stack(lineFields, { form }))).toEqual({
757+
'inv.total': 'inert',
758+
'line.line_total': 'inert',
759+
'line.memo': 'inert',
760+
});
761+
});
762+
});
763+
764+
describe('site 2: a grid with no authored columns draws the derived ones', () => {
765+
/** Three the derivation draws, and three it leaves out: JSON, `hidden`, `readonly`. */
766+
const lineFields = {
767+
qty: { type: 'number' },
768+
line_total: { type: 'currency' },
769+
memo: { type: 'text' },
770+
blob: { type: 'json' },
771+
secret: { type: 'text', hidden: true },
772+
frozen: { type: 'number', readonly: true },
773+
};
774+
/** The parent's two fields (nothing reads them) and the three the derivation leaves out. */
775+
const DERIVED = {
776+
'inv.total': 'inert',
777+
'inv.line_total': 'inert',
778+
'line.blob': 'inert',
779+
'line.secret': 'inert',
780+
'line.frozen': 'inert',
781+
};
782+
783+
it('baseline: without `inlineEdit` no grid is drawn, and every child field is reported', () => {
784+
expect(verdicts(stack(lineFields))).toEqual({
785+
...DERIVED,
786+
'line.qty': 'inert',
787+
'line.line_total': 'inert',
788+
'line.memo': 'inert',
789+
});
790+
});
791+
792+
it.each([['grid'], ['form'], [true]])('`inlineEdit: %s` with no `inlineColumns` credits the derived columns, and only them', (inlineEdit) => {
793+
expect(verdicts(stack(lineFields, {}, { ...MASTER_DETAIL, inlineEdit }))).toEqual(DERIVED);
794+
});
795+
796+
it('an empty `inlineColumns` is no authored list: the grid is derived', () => {
797+
expect(verdicts(stack(lineFields, {}, { ...MASTER_DETAIL, inlineEdit: 'grid', inlineColumns: [] }))).toEqual(DERIVED);
798+
});
799+
800+
it('an authored `inlineColumns` replaces the derivation: only the named column is credited', () => {
801+
expect(verdicts(stack(lineFields, {}, { ...MASTER_DETAIL, inlineEdit: 'grid', inlineColumns: [{ name: 'qty' }] }))).toEqual({
802+
...DERIVED,
803+
'line.line_total': 'inert',
804+
'line.memo': 'inert',
805+
});
806+
});
807+
808+
it('`inlineEdit` on a field that is not a relationship draws no grid', () => {
809+
expect(verdicts(stack({ ...lineFields, tag: { type: 'text', inlineEdit: 'grid' } }))).toEqual({
810+
...DERIVED,
811+
'line.qty': 'inert',
812+
'line.line_total': 'inert',
813+
'line.memo': 'inert',
814+
'line.tag': 'inert',
815+
});
816+
});
817+
818+
it('a column the budget collapses into the chooser is credited too: it is drawn on demand, never dropped', () => {
819+
const wide = Object.fromEntries(Array.from({ length: 9 }, (_, i) => [`f${i}`, { type: 'text' }]));
820+
const findings = verdicts(stack(wide, {}, { ...MASTER_DETAIL, inlineEdit: 'grid' }));
821+
expect(findings).toEqual({ 'inv.total': 'inert', 'inv.line_total': 'inert' });
822+
});
823+
824+
it('a `subforms` entry with no `columns` credits the derived columns of its `childObject`', () => {
825+
const form = { type: 'simple', data, subforms: [{ childObject: 'line' }] };
826+
expect(verdicts(stack(lineFields, { form }))).toEqual(DERIVED);
827+
});
828+
829+
it("a `subforms` entry's derivation excludes the `relationshipField` it names, which that key credits instead", () => {
830+
const link = { type: 'lookup', reference: 'inv' };
831+
const form = { type: 'simple', data, subforms: [{ childObject: 'line', relationshipField: 'link' }] };
832+
const findings = validateFieldConsumers(stack({ ...lineFields, link }, { form }));
833+
expect(findings.find((f) => f.object === 'line' && f.field === 'link')).toBeUndefined();
834+
expect(Object.fromEntries(findings.map((f) => [`${f.object}.${f.field}`, f.verdict]))).toEqual(DERIVED);
835+
});
836+
});
837+
});

0 commit comments

Comments
 (0)