fix(components): publish element:record_picker.filter, the A-class key #3808's triage dropped (#3830)

.changeset/record-picker-filter-input-3830.md

@@ -0,0 +1,41 @@
+---
+"@object-ui/components": patch
+---
+
+`element:record_picker.filter` is now discoverable from the published `inputs`
+
+The fourth A-class gap of objectui#3808's own list, and the one its three-way
+triage dropped: `filter` appears in that issue's raw key dump for this block and
+then in none of its A / B / C lists, so the change that added the repo-wide
+parity gate exempted it by name instead of declaring it. It is the same shape as
+the four #3808 fixed — `@objectstack/spec` declares
+`ElementRecordPickerProps.filter`, the renderer has read it all along
+(`composed?.filter ?? props.filter`, straight into the picker query's `$filter`),
+and the registry `inputs` never mentioned it.
+
+`element:record_picker` is not in the public tier ("record picking is a field
+widget, not a page block"), so the gap was not in `sdui.manifest.json` — it was
+in the JSX-page compiler's prop whitelist, which `renderers/layout/page.tsx`
+builds from `getKnownTypes()` plus these same `inputs`. A JSX page writing
+`filter` therefore got an `unknown-prop` warning from `sdui-parser`'s prop walk
+on the very key that decided which records the picker offered, and the designer
+panel gave an author no way to discover the key existed at all.
+
+The description is derived from what the renderer does, not from restating the
+spec's one-liner, because the one thing an author cannot read off the spec is
+which of the two places they may write a filter wins: a node-level `dataSource`
+filter (itself AND-combined with any saved `view` it names) is taken and this
+top-level `filter` is DROPPED, not merged — so this key applies only when the
+node carries no `dataSource` filter.
+
+`type` is `'object'`, taken from the spec's actual shape on the resolved pin
+rather than the `'array'` the issue's landing sketch guessed:
+`FilterConditionSchema` is `z.record(z.string(), z.unknown())` intersected with
+the `$and` / `$or` / `$not` group, so a rule array is rejected. This is the one
+key in the family where `ComponentInput`'s coarse typing costs nothing —
+`sdui-parser`'s `checkType` accepts exactly the values the spec accepts here, so
+unlike `element:text_input.defaultValue` there is no narrowing to disclose.
+
+The parity gate's explicit exemption for this key is deleted in the same change
+(its own `carries no stale unpublished-key exemption` assertion demands it), and
+the key joins #3808's four in the by-name "declared, not merely not-failing" pin.

apps/console/src/__tests__/registry-inputs-spec-parity.test.ts

@@ -511,17 +511,12 @@ const UNPUBLISHED_EXEMPTIONS: Record<string, string> = {
   'page:card.body':
     'Retired upstream by objectstack#5775 / PR objectstack#6281 (ADR-0087 D2 tombstone, converging on the `children` this renderer reads and now publishes); declaring it would publish a key the spec rejects by name — objectui#4027. Listed here only because the pinned @objectstack/spec@17.0.0-rc.5 predates the retirement. Resolves via objectui#3809, not via the pin bump.',
 
-  // ── element:record_picker.filter — a real A-class gap, out of scope here ───
-  // The renderer DOES read it (`record-picker.tsx:78`, `ds.filter ?? props.filter`,
-  // into `query.$filter` at :103) and the spec DOES declare it, so by the bar
-  // above this key should be declared, not exempted. It is exempted because
-  // objectui#3808's own three-class triage never sorted it into A, B or C — it
-  // appears in that issue's raw key dump and then in none of the three lists —
-  // so it fell outside the dispatched scope of the change that added this gate.
-  // Filed as objectui#3830 with the same evidence, rather than widened into a
-  // PR nobody reviewed for it.
-  'element:record_picker.filter':
-    'A genuine A-class gap (renderer reads it at record-picker.tsx:78 → query.$filter at :103), not a deliberate omission — it fell out of objectui#3808\'s three-class triage and so out of that PR\'s scope. Owned by objectui#3830; delete this entry when it declares the input.',
+  // `element:record_picker.filter` was the ninth entry here — a real A-class gap
+  // that fell out of objectui#3808's three-class triage, exempted only because it
+  // was outside that PR's dispatched scope. objectui#3830 declared the input, so
+  // the entry stopped describing anything and `carries no stale unpublished-key
+  // exemption` demanded its deletion. It is now pinned as DECLARED, by name,
+  // alongside #3808's four at the bottom of this file.
 
   // ── targetVariable — the spec's own "declarative hint" (2 keys) ────────────
   // Zero read points repo-wide (`grep -rn targetVariable packages/ apps/` is
@@ -676,8 +671,10 @@ describe('registry `inputs` vs `@objectstack/spec` ComponentPropsMap (repo-wide)
 
   it('every unpublished-key exemption states a reason and references a tracking issue', () => {
     // The discipline that separates "deliberately not published, and here is who
-    // owns the decision" from "we forgot". Four of the nine entries below exist
-    // only because objectui#3829 / #3830 / #3834 were opened to own them.
+    // owns the decision" from "we forgot". Four of the nine entries once here
+    // existed only because objectui#3829 / #3830 / #3834 were opened to own
+    // them, and #3830's is already gone — declaring the input is what retires an
+    // entry, which is the point of the stale check below.
     const unjustified = Object.entries(UNPUBLISHED_EXEMPTIONS)
       .filter(([, reason]) => !/#\d+/.test(reason))
       .map(([key]) => key);
@@ -699,17 +696,26 @@ describe('registry `inputs` vs `@objectstack/spec` ComponentPropsMap (repo-wide)
     expect(stale).toEqual([]);
   });
 
-  it('the four keys objectui#3808 declared are discoverable, block by block', () => {
+  it('the five A-class keys objectui#3808 / #3830 declared are discoverable, block by block', () => {
     // Named, not just covered by the derived loop above. The derived assertion
-    // would also pass if these four were added to `UNPUBLISHED_EXEMPTIONS`
+    // would also pass if these five were added to `UNPUBLISHED_EXEMPTIONS`
     // instead of declared — which is precisely the move #3808 exists to rule
     // out — so the keys it fixed are pinned by name, and pinned as DECLARED
     // rather than merely "not failing".
+    //
+    // The fifth is objectui#3830's `element:record_picker.filter`, the A-class
+    // key #3808's own triage dropped between its raw key dump and its three
+    // lists. It is listed HERE, in the same place as the other four, because it
+    // is the same fact about the same gate: the entry that used to exempt it
+    // (deleted above) is not evidence of anything once the input exists, and a
+    // future change that dropped the declaration and re-added the exemption
+    // would restore the gap while leaving every derived assertion green.
     const fixed: Array<[string, string]> = [
       ['record:details', 'hideFields'],
       ['record:related_list', 'relationshipValueField'],
       ['record:related_list', 'add'],
       ['element:text_input', 'defaultValue'],
+      ['element:record_picker', 'filter'],
     ];
     for (const [type, key] of fixed) {
       expect(specTopLevelKeys(type), `${type} spec no longer declares ${key}`).toContain(key);

packages/components/src/__tests__/record-picker-inputs-spec-parity.test.ts

@@ -0,0 +1,202 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * `element:record_picker` — the published authoring surface stays in parity with
+ * `@objectstack/spec` `ElementRecordPickerProps` for `filter` (objectui#3830).
+ *
+ * The sibling of `text-input-inputs-spec-parity.test.ts`, for the key that fell
+ * out of objectui#3808's own three-class triage: `filter` appears in that
+ * issue's raw key dump for this block and then in none of its A / B / C lists,
+ * so the change that added the repo-wide parity gate exempted it by name instead
+ * of declaring it. It is the fourth A-class gap of exactly the same shape —
+ * renderer reads it, spec declares it, `inputs` omitted it.
+ *
+ * WHY THIS BLOCK NEEDED IT. `element:record_picker` is deliberately NOT in
+ * `PUBLIC_BLOCKS` ("record picking is a field widget, not a page block",
+ * `packages/core/src/registry/public-blocks.ts`), so it never reaches
+ * `sdui.manifest.json` and the usual argument — "the manifest advertises it" —
+ * does not apply. Its `inputs` are a live prop whitelist anyway:
+ * `renderers/layout/page.tsx` builds the JSX-page compiler's whitelist from
+ * `getKnownTypes()` plus these same `inputs`, so while `filter` was undeclared,
+ * `sdui-parser/src/validate.ts` reported `unknown-prop` for it on every JSX
+ * page — a warning against a key the renderer then filtered the picker's whole
+ * candidate set by. The last test in this file is that path, end to end.
+ *
+ * Expectations are derived from the spec at runtime, not restated.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { ComponentRegistry } from '@object-ui/core';
+import { compile, manifestFromConfigs } from '@object-ui/sdui-parser';
+import { ElementRecordPickerPropsSchema } from '@objectstack/spec/ui';
+// Module scope, not a hook: the cold transform is billed to the import phase,
+// which has no test/hook timeout (AGENTS.md §测试纪律, objectui#3010).
+import '../renderers';
+
+type ShapeCarrier = { shape?: unknown; _def?: { shape?: unknown } };
+
+/** Resolve the props object's `.shape` through both spellings, lazy or plain. */
+function specTopLevelKeys(): string[] {
+  const carrier = ElementRecordPickerPropsSchema as unknown as ShapeCarrier;
+  const shape = carrier.shape ?? carrier._def?.shape;
+  const resolved = typeof shape === 'function' ? (shape as () => object)() : shape;
+  return resolved && typeof resolved === 'object' ? Object.keys(resolved) : [];
+}
+
+const TYPE = 'element:record_picker';
+const config = () => ComponentRegistry.getConfig(TYPE);
+const inputs = () => config()?.inputs ?? [];
+const inputNames = () => inputs().map((i) => i.name);
+const input = (name: string) => inputs().find((i) => i.name === name);
+const filterDescription = () => input('filter')?.description ?? '';
+
+/** A minimal spec-valid props object, so a `filter` probe fails only on `filter`. */
+const withFilter = (filter: unknown) => ({ object: 'account', displayField: 'name', filter });
+
+describe('element:record_picker — registry inputs vs @objectstack/spec', () => {
+  it('is registered with a non-empty `inputs` surface', () => {
+    expect(config()).toBeDefined();
+    expect(inputNames().length).toBeGreaterThan(0);
+  });
+
+  it('resolves a non-empty spec key set', () => {
+    // Guards the probe, not the subject: a Zod internals change would return `[]`
+    // here and make every assertion below vacuously agreeable.
+    expect(specTopLevelKeys().length).toBeGreaterThan(0);
+  });
+
+  it('publishes `filter`, which the renderer has read all along', () => {
+    // A KEY-reachability claim, so the criterion is that the key SURVIVES the
+    // parse — not that the parse succeeds. This props schema is a strip-mode
+    // `z.object`, so an UNDECLARED key parses green too and is simply absent
+    // from `data` afterwards; asserting `success` alone would prove nothing.
+    expect(specTopLevelKeys()).toContain('filter');
+    const parsed = ElementRecordPickerPropsSchema.safeParse(withFilter({ status: 'open' }));
+    expect(parsed.success).toBe(true);
+    expect(parsed.data?.filter).toEqual({ status: 'open' });
+
+    // The contrast that makes the criterion meaningful: same green parse, key
+    // gone, no diagnostic. That is what `filter` looked like to every manifest
+    // consumer before it was declared here.
+    const undeclared = ElementRecordPickerPropsSchema.safeParse({
+      object: 'account',
+      displayField: 'name',
+      notASpecKey: 1,
+    } as never);
+    expect(undeclared.success).toBe(true);
+    expect(Object.keys(undeclared.data ?? {})).not.toContain('notASpecKey');
+
+    expect(inputNames()).toContain('filter');
+    expect(filterDescription()).not.toBe('');
+  });
+
+  it('declares `object` as the type the spec actually accepts, not `array`', () => {
+    // objectui#3830's landing sketch guessed `'array'` and flagged the guess as
+    // needing checking against the resolved pin. It is wrong, and this is why:
+    // `ElementRecordPickerProps.filter` is `FilterConditionSchema`, which is
+    // `z.record(z.string(), z.unknown()).and(z.object({ $and, $or, $not }))` —
+    // an OBJECT. A rule array (an ObjectQL AST, a view's `ViewFilterRule[]`) is
+    // rejected outright.
+    expect(ElementRecordPickerPropsSchema.safeParse(withFilter({ status: 'open' })).success).toBe(true);
+    expect(ElementRecordPickerPropsSchema.safeParse(withFilter({ $and: [{ a: 1 }] })).success).toBe(true);
+    expect(ElementRecordPickerPropsSchema.safeParse(withFilter([['a', '=', 1]])).success).toBe(false);
+    expect(ElementRecordPickerPropsSchema.safeParse(withFilter('a = 1')).success).toBe(false);
+    expect(ElementRecordPickerPropsSchema.safeParse(withFilter(42)).success).toBe(false);
+
+    expect(input('filter')?.type).toBe('object');
+  });
+
+  it('the coarse `object` type costs nothing here — it accepts exactly what the spec accepts', () => {
+    // The `element:text_input.defaultValue` sibling had to name a narrowing in
+    // prose, because `ComponentInput.type` is one coarse control kind and the
+    // spec's type there is the union `string | number` (objectui#3832). This key
+    // is the case where the two agree exactly: `checkType`'s `'object'` arm in
+    // `sdui-parser/src/validate.ts` passes a non-null non-array object and warns
+    // `type-mismatch` on everything else — the same partition `safeParse` draws
+    // above. Asserted through the real validator, not by reading its source, so
+    // a future widening of either side shows up here as a disagreement.
+    const manifest = manifestFromConfigs([
+      { type: TYPE, namespace: 'element', inputs: [{ name: 'filter', type: 'object' }] },
+    ]);
+    const codesFor = (literal: string) =>
+      compile(`<${TYPE} filter={${literal}} />`, manifest).diagnostics.map((d) => d.code);
+
+    expect(codesFor('{"status":"open"}')).toEqual([]);
+    expect(codesFor('{"$and":[{"a":1}]}')).toEqual([]);
+    expect(codesFor('[["a","=",1]]')).toContain('type-mismatch');
+    expect(codesFor('42')).toContain('type-mismatch');
+    expect(codesFor('null')).toContain('type-mismatch');
+  });
+
+  it('the `filter` description says which of the two filters an author writes wins', () => {
+    // The renderer reads `composed?.filter ?? props.filter`, so a node that
+    // carries a `dataSource` filter DROPS this key rather than combining with
+    // it — while the binding's own filter AND-combines with the saved view it
+    // names. A description saying only "filter criteria" would be true and
+    // useless: an author writing both would have no way to know which one
+    // decides the candidate set, which is the one thing objectui#3830 insists
+    // this entry has to state.
+    const description = filterDescription();
+    expect(description).toMatch(/dataSource/);
+    expect(description).toMatch(/precedence/i);
+    expect(description).toMatch(/\$filter/);
+  });
+
+  it('carries no `defaultValue` on the filter entry', () => {
+    // A default here would pre-fill every picker in the designer with a filter
+    // the renderer has no opinion about — and a filter's default is not "empty
+    // object", it is "no filter at all", which `undefined` already is. The spec
+    // declares no default either.
+    //
+    // Existence asserted first: `input('filter')?.defaultValue` is also
+    // `undefined` when the input is GONE, so without this line the check would
+    // pass most loudly in the one case it is supposed to notice.
+    expect(input('filter')).toBeDefined();
+    expect(input('filter')?.defaultValue).toBeUndefined();
+    expect(
+      ElementRecordPickerPropsSchema.safeParse({ object: 'account', displayField: 'name' }).data,
+    ).not.toHaveProperty('filter');
+  });
+
+  it('a JSX page writing `filter` no longer gets `unknown-prop` from the compiler', () => {
+    // The harm objectui#3830 describes, end to end. The manifest is assembled
+    // the way `renderers/layout/page.tsx` assembles the JSX-page compiler's
+    // whitelist — `getKnownTypes()` mapped through each type's registered meta —
+    // so this runs against the LIVE registration, not a hand-written fixture
+    // that could agree with itself.
+    const manifest = manifestFromConfigs(
+      ComponentRegistry.getKnownTypes().map((t) => {
+        const meta = ComponentRegistry.getMeta(t);
+        return { type: t, namespace: meta?.namespace, isContainer: meta?.isContainer, inputs: meta?.inputs };
+      }) as unknown as Parameters<typeof manifestFromConfigs>[0],
+    );
+
+    // Non-vacuity, and the reason this test can fail for the right reason: the
+    // SAME compile call carries `searchFields`, a spec key this block
+    // deliberately does not publish (an ADR-0087 tombstone upstream — see the
+    // exemptions in `apps/console/src/__tests__/registry-inputs-spec-parity.test.ts`).
+    // It must still come back as `unknown-prop`. Without this control, "no
+    // unknown-prop for filter" would also be what a broken manifest, an
+    // unregistered tag or a silent parse failure looks like.
+    const r = compile(
+      `<${TYPE} object="account" filter={{"status":"open"}} searchFields={["name"]} />`,
+      manifest,
+    );
+
+    const unknownProps = r.diagnostics
+      .filter((d) => d.code === 'unknown-prop')
+      .map((d) => d.message);
+    expect(unknownProps.join(' | ')).toMatch(/searchFields/);
+    expect(unknownProps.join(' | ')).not.toMatch(/"filter"/);
+
+    // And the key survives into the compiled tree as itself — the whole point of
+    // publishing it is that the author's `filter` reaches the renderer, which
+    // turns it into the picker query's `$filter`.
+    expect(r.tree).toMatchObject({ type: TYPE, filter: { status: 'open' } });
+    expect(r.diagnostics.some((d) => d.severity === 'error')).toBe(false);
+  });
+});