fix(app-shell): give InspectorSelectField the unknown-value rule eight call sites hand-rolled (objectui#8488)

.changeset/8488-inspector-select-unknown-value.md

@@ -0,0 +1,34 @@
+---
+'@object-ui/app-shell': patch
+---
+
+`InspectorSelectField` surfaces a stored value the option roster does not offer,
+instead of painting a blank trigger (objectui#8488).
+
+Radix renders a controlled value matching no `SelectItem` as nothing at all, so a
+metadata key holding a stale or off-spec value drew an EMPTY select in the designer —
+pixel-identical to "this field is unset". The author saw an empty control, picked a
+value to fill it in, and silently overwrote a key they were never shown.
+
+The field now synthesises a selectable, flagged row for a non-empty value the roster
+does not contain, labelled `VALUE (not found)` by default. Every call site inherits it.
+
+Deliberately NOT the placeholder: drawing `—` over an unknown value would assert
+"nothing is stored" about a field that is storing something — confidently wrong rather
+than merely blank. objectui#8450 keeps owning the empty state; the two stay
+distinguishable on screen, which is pinned as its own case.
+
+Eight call sites had hand-rolled the rule and are now deleted in favour of it. Three
+had flagged their row — `ActionTargetField` (`(not found)`), the view-column field
+picker (`(not in object)`) and `FlowNodeConfigField`'s select branch (`(deprecated)`,
+framework#4278 / ADR-0090 D3). Five had made the value visible but UNFLAGGED —
+`ReportDefaultInspector`'s type, dataset and both chart axes, and
+`ViewVariantInspector`'s type — so a value the catalog had dropped read exactly like
+one it offered. Those five now flag it. Wording stays with the call site through the
+new optional `unknownValueLabel` prop, so `(deprecated)` still says the thing
+`(not found)` does not.
+
+Known boundary, unchanged by this: an async picker that has not loaded yet is
+indistinguishable from an empty roster, so a valid value briefly wears the flag while
+its options are in flight. That blind spot is inherited from the hand-rolled copies,
+not introduced here, and is tracked separately.

packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx

@@ -202,7 +202,10 @@ const TARGET_META_TYPE: Record<string, string> = { flow: 'flow', modal: 'page',
 
 /** Target binding: a reference picker for flow/modal/form (names come from the
  *  matching metadata type), else a free-text input (url/api). An out-of-catalog
- *  value is preserved as a synthesized option so it is never silently dropped. */
+ *  value is preserved as a synthesized option so it is never silently dropped —
+ *  `InspectorSelectField` owns that rule now (objectui#8488); this call site
+ *  used to hand-roll it, in the same `VALUE (not found)` wording the primitive
+ *  defaults to, so deleting the copy changes no text. */
 function ActionTargetField({ type, value, onCommit, cfg, readOnly }: {
   type: string;
   value: string;
@@ -213,13 +216,10 @@ function ActionTargetField({ type, value, onCommit, cfg, readOnly }: {
   const metaType = TARGET_META_TYPE[type] ?? null;
   const { options } = useMetaOptions(metaType);
   const usePicker = !!metaType && options.length > 0;
-  const opts = usePicker && value && !options.some((o) => o.value === value)
-    ? [{ value, label: `${value} (not found)` }, ...options]
-    : options;
   return (
     <div className="space-y-1">
       {usePicker ? (
-        <InspectorSelectField label={`${cfg.label} *`} value={value || undefined} options={opts} onCommit={onCommit} disabled={readOnly} />
+        <InspectorSelectField label={`${cfg.label} *`} value={value || undefined} options={options} onCommit={onCommit} disabled={readOnly} />
       ) : (
         <InspectorTextField label={`${cfg.label} *`} value={value} onCommit={onCommit} placeholder={cfg.placeholder} disabled={readOnly} mono />
       )}

packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeConfigField.tsx

@@ -258,15 +258,19 @@ export function FlowNodeConfigField({ field, value, onCommit, disabled, locale,
           // render, or editing a legacy node would silently blank it. Surface
           // it as selectable but flag it — it is not offered to fresh nodes.
           // Same rule as FlowObjectListField's select cells (ADR-0090 D3).
-          const shown =
-            current && !opts.some((o) => o.value === current)
-              ? [...opts, { value: current, label: `${current} (deprecated)` }]
-              : opts;
+          //
+          // objectui#8488 promoted that RULE into `InspectorSelectField`, where
+          // the other 42 call sites inherit it. What stays here is the WORDING:
+          // "(deprecated)" is sourced (framework#4278, ADR-0090 D3) and says
+          // something the generic "(not found)" does not — the value is known,
+          // it is simply no longer offered. The prop exists so this sentence
+          // survives the unification instead of being flattened by it.
           return (
             <InspectorSelectField
               label={field.label}
               value={current}
-              options={shown}
+              options={opts}
+              unknownValueLabel={(v) => `${v} (deprecated)`}
               onCommit={(v) => onCommit(v)}
               disabled={disabled}
             />

packages/app-shell/src/views/metadata-admin/inspectors/ReportDefaultInspector.test.tsx

@@ -7,6 +7,18 @@ import type { DatasetCatalogEntry } from '../previews/useDatasetCatalog';
 
 afterEach(cleanup);
 
+// One fetch double at MODULE scope, never torn down (objectui#7439 / #6640).
+// `useDatasetSemantics` hydrates a bound dataset the catalog does not carry
+// with `client.get('dataset', name)`, and happy-dom resolves that relative URL
+// to a live socket on localhost:3000. The read is fire-and-forget — no barrier
+// in the test body awaits it — so an `afterEach` teardown would restore the
+// real fetch while the tree is still mounted. Installing it here means no test
+// in this file can ever end with the real fetch back in place.
+vi.stubGlobal(
+  'fetch',
+  vi.fn(async () => new Response('null', { status: 404, headers: { 'content-type': 'application/json' } })),
+);
+
 // ADR-0021 single-form: the catalog override short-circuits the dataset
 // fetches so the inspector renders with zero context / transport dependency.
 const catalog: DatasetCatalogEntry[] = [
@@ -194,3 +206,48 @@ describe('ReportDefaultInspector — dataset binding (9.0 single form)', () => {
     expect(labelledInput('Label')).toBeDisabled();
   });
 });
+
+/**
+ * objectui#8488 — one of the 42 call sites that never hand-rolled the rule.
+ *
+ * `datasetName` is offered against `datasetOptions`, so a dataset dropped from
+ * the catalog used to leave the binding BLANK — a report that reads as "no
+ * dataset bound" while `dataset: 'retired_dataset'` sits in the draft. This
+ * inspector wrote no repair of its own; it inherits `InspectorSelectField`'s.
+ *
+ * The pin reads the trigger's text, not `datasetOptions`: the bug's whole shape
+ * was a correct `value` prop that reached no pixels.
+ */
+describe('ReportDefaultInspector — a dataset the catalog no longer lists (objectui#8488)', () => {
+  const datasetTrigger = () => screen.getByRole('combobox', { name: 'Dataset' });
+
+  it('shows the stored dataset name, flagged, instead of a blank binding', () => {
+    render(
+      <ReportDefaultInspector
+        {...baseProps}
+        draft={{ ...datasetDraft, dataset: 'retired_dataset' }}
+        onPatch={vi.fn()}
+        readOnly={false}
+      />,
+    );
+    expect(
+      datasetTrigger().textContent,
+      'the binding names what it is bound to, even though the catalog dropped it',
+    ).toBe('retired_dataset (not found)');
+  });
+
+  it('still shows a catalogued dataset by its own label — the flag is not unconditional', () => {
+    render(
+      <ReportDefaultInspector
+        {...baseProps}
+        draft={{ ...datasetDraft }}
+        onPatch={vi.fn()}
+        readOnly={false}
+      />,
+    );
+    // `datasetOptions` labels a catalogued dataset `LABEL (NAME)`; the point of
+    // the case is the absence of the flag, not the label's own spelling.
+    expect(datasetTrigger().textContent).toBe('Sales metrics (sales_metrics)');
+    expect(screen.queryByText(/not found/), 'nothing is flagged').toBeNull();
+  });
+});

packages/app-shell/src/views/metadata-admin/inspectors/ReportDefaultInspector.tsx

@@ -95,7 +95,7 @@ const TYPE_LABEL_KEYS: Record<string, string> = {
 };
 
 /** Build the Report-type <select> options from the spec `type` enum. */
-function useTypeOptions(currentType: string, locale: MetadataDefaultInspectorProps['locale']) {
+function useTypeOptions(locale: MetadataDefaultInspectorProps['locale']) {
   return React.useMemo(() => {
     const schema = getReportSchema();
     const rawEnum = schema?.properties?.type?.enum;
@@ -109,11 +109,11 @@ function useTypeOptions(currentType: string, locale: MetadataDefaultInspectorPro
       const label = key ? t(key, locale) : v;
       return { value: v, label: label === key ? v : label };
     });
-    if (!opts.some((o) => o.value === currentType) && currentType) {
-      opts.push({ value: currentType, label: currentType });
-    }
+    // objectui#8488 — a `type` outside the spec enum used to be appended here
+    // UNFLAGGED, so an off-spec report type read exactly like an offered one.
+    // `InspectorSelectField` synthesises and flags it now.
     return opts;
-  }, [currentType, locale]);
+  }, [locale]);
 }
 
 /** Read a `string[]` draft field defensively. */
@@ -236,7 +236,7 @@ export function ReportDefaultInspector({
 
   const reportType =
     typeof draft.type === 'string' ? (draft.type as string) : 'tabular';
-  const typeOptions = useTypeOptions(reportType, locale);
+  const typeOptions = useTypeOptions(locale);
 
   const labelValue = typeof draft.label === 'string' ? (draft.label as string) : '';
   const datasetName =
@@ -250,16 +250,21 @@ export function ReportDefaultInspector({
   const catalog = useDatasetCatalog(datasetCatalogOverride);
   const semantics = useDatasetSemantics(datasetName || undefined, catalog);
 
-  const datasetOptions = React.useMemo(() => {
-    const opts = catalog.datasets.map((d) => ({
-      value: d.name,
-      label: d.label && d.label !== d.name ? `${d.label} (${d.name})` : d.name,
-    }));
-    if (datasetName && !opts.some((o) => o.value === datasetName)) {
-      opts.push({ value: datasetName, label: datasetName });
-    }
-    return opts;
-  }, [catalog.datasets, datasetName]);
+  // A FOURTH hand-rolled copy of the unknown-value rule used to sit here, and
+  // it was the one that got the rule wrong: it appended the stored name with no
+  // marker at all, so a dataset the catalog had dropped rendered exactly like a
+  // dataset it offered. That is the direction objectui#8488 refused — it makes
+  // "stored" and "offered" indistinguishable on screen, trading a display
+  // defect for a semantic one. `InspectorSelectField` now synthesises the row
+  // AND flags it; the catalog is all this list owes.
+  const datasetOptions = React.useMemo(
+    () =>
+      catalog.datasets.map((d) => ({
+        value: d.name,
+        label: d.label && d.label !== d.name ? `${d.label} (${d.name})` : d.name,
+      })),
+    [catalog.datasets],
+  );
 
   const measureOptions: ObjectFieldInfo[] = React.useMemo(
     () =>
@@ -299,16 +304,17 @@ export function ReportDefaultInspector({
     const next = { ...chart, ...patch };
     onPatch({ chart: next.type ? next : undefined });
   };
-  const chartXOptions = React.useMemo(() => {
-    const opts = dimensionOptions.map((d) => ({ value: d.name, label: d.label || d.name }));
-    if (chartX && !opts.some((o) => o.value === chartX)) opts.push({ value: chartX, label: chartX });
-    return opts;
-  }, [dimensionOptions, chartX]);
-  const chartYOptions = React.useMemo(() => {
-    const opts = measureOptions.map((m) => ({ value: m.name, label: m.label || m.name }));
-    if (chartY && !opts.some((o) => o.value === chartY)) opts.push({ value: chartY, label: chartY });
-    return opts;
-  }, [measureOptions, chartY]);
+  // Both axis rosters used to append an out-of-catalog axis unflagged
+  // (objectui#8488): visible, but indistinguishable from a dimension the
+  // dataset actually offers. The flag is `InspectorSelectField`'s job now.
+  const chartXOptions = React.useMemo(
+    () => dimensionOptions.map((d) => ({ value: d.name, label: d.label || d.name })),
+    [dimensionOptions],
+  );
+  const chartYOptions = React.useMemo(
+    () => measureOptions.map((m) => ({ value: m.name, label: m.label || m.name })),
+    [measureOptions],
+  );
 
   // A `joined` report carries its data on dataset-bound `blocks` (edited via
   // the spec form's repeater) — the top-level binding only applies otherwise.

packages/app-shell/src/views/metadata-admin/inspectors/ViewColumnInspector.tsx

@@ -148,18 +148,23 @@ export function ViewColumnInspector({
   const objectName = readVariantObject(variantSchema);
   const { fields: objectFields } = useObjectFields(objectName || undefined);
   const currentFieldKey = col ? colFieldKey(col) : '';
-  const fieldOptions = React.useMemo(() => {
-    const opts = objectFields.map((f) => ({
-      value: f.name,
-      label: f.label && f.label !== f.name ? `${f.label} · ${f.name}` : f.name,
-    }));
-    // Keep the current value visible even if it isn't a known object field
-    // (computed / virtual / stale columns).
-    if (currentFieldKey && !opts.some((o) => o.value === currentFieldKey)) {
-      opts.unshift({ value: currentFieldKey, label: `${currentFieldKey} (not in object)` });
-    }
-    return opts;
-  }, [objectFields, currentFieldKey]);
+  const fieldOptions = React.useMemo(
+    () =>
+      objectFields.map((f) => ({
+        value: f.name,
+        label: f.label && f.label !== f.name ? `${f.label} · ${f.name}` : f.name,
+      })),
+    [objectFields],
+  );
+  // Keeping the current value visible when it isn't a known object field
+  // (computed / virtual / stale columns) is `InspectorSelectField`'s rule since
+  // objectui#8488 — this file only keeps the WORDING. What it does still own is
+  // a different question the synthesized row used to answer as a side effect:
+  // picker or free-text box? The old gate was `fieldOptions.length > 0` with
+  // the synthesized row already spliced in, which is true exactly when the
+  // catalog has rows OR a key is stored — spelled out here so removing the
+  // splice does not silently move that decision.
+  const showFieldPicker = fieldOptions.length > 0 || currentFieldKey !== '';
 
   if (!parsed || !col) {
     return (
@@ -266,11 +271,12 @@ export function ViewColumnInspector({
       )}
 
       <div className="border-t pt-3 space-y-3">
-        {fieldOptions.length > 0 ? (
+        {showFieldPicker ? (
           <InspectorSelectField
             label={t('engine.inspector.viewColumn.accessorKey', locale)}
             value={colFieldKey(col)}
             options={fieldOptions}
+            unknownValueLabel={(v) => `${v} (not in object)`}
             onCommit={(v) => patchIdentity({ field: v })}
             disabled={readOnly}
           />

packages/app-shell/src/views/metadata-admin/inspectors/ViewVariantInspector.tsx

@@ -181,7 +181,7 @@ function readObjectBinding(
 }
 
 /** Build the View-type <select> options from the spec `type` enum. */
-function useTypeOptions(currentType: string, locale?: string) {
+function useTypeOptions(locale?: string) {
   return React.useMemo(() => {
     const schema = getListVariantSchema();
     const rawEnum = schema?.properties?.type?.enum;
@@ -190,11 +190,13 @@ function useTypeOptions(currentType: string, locale?: string) {
         ? rawEnum.filter((v: unknown): v is string => typeof v === 'string')
         : ['grid', 'kanban', 'calendar', 'gallery', 'gantt', 'timeline'];
     const opts = values.map((v) => ({ value: v, label: typeLabel(v, locale) }));
-    if (!opts.some((o) => o.value === currentType) && currentType) {
-      opts.push({ value: currentType, label: typeLabel(currentType, locale) });
-    }
+    // objectui#8488 — a `type` the spec enum does not carry used to be appended
+    // here UNFLAGGED, so an off-spec view type read exactly like an offered
+    // one. `InspectorSelectField` synthesises and flags it now; `typeLabel`
+    // still gets its say through `unknownValueLabel` at the call site, since a
+    // retired type can still have a human name in `TYPE_LABELS`.
     return opts;
-  }, [currentType, locale]);
+  }, [locale]);
 }
 
 export function ViewVariantInspector({
@@ -216,7 +218,7 @@ export function ViewVariantInspector({
   const isFormFamily = isFormFamilyKey(familyKey ?? variantKey);
   const viewType =
     typeof variant.type === 'string' ? (variant.type as string) : 'grid';
-  const typeOptions = useTypeOptions(viewType, locale);
+  const typeOptions = useTypeOptions(locale);
   const binding = readObjectBinding(variant, draft);
 
   // Canonical label lives at the top level (`draft.label`); a list `config`
@@ -394,6 +396,7 @@ export function ViewVariantInspector({
         label={t('engine.inspector.view.type', locale)}
         value={viewType}
         options={typeOptions}
+        unknownValueLabel={(v) => `${typeLabel(v, locale)} (not found)`}
         onCommit={(v) => writeVariant({ type: v })}
         disabled={readOnly}
       />