fix(fields): validate a stored location value on an edit form

.changeset/6744-location-stored-value-validation.md

@@ -0,0 +1,38 @@
+---
+'@object-ui/fields': minor
+---
+
+fields: validate a STORED `location` value on an edit form (objectui#6744)
+
+`buildValidationRules` is the producer of the host-side `error` prop that every
+field widget's published objectui#3222 slot reads, and it had no branch for
+`location`. So a coordinate that was **already in the record** and violated the
+spec's range was never validated on an edit form: the control rendered it,
+nothing marked it invalid, and submitting re-wrote it unchanged.
+
+It now compiles a `validate.location` entry that adjudicates a present value
+against `valueSchemaFor(field, 'stored')` — the platform's own value-shape
+contract (ADR-0104 D1), the same schema the engine's record validator checks a
+stored `location` against. An out-of-range stored value now marks the control
+invalid, renders the spec's own complaint, and blocks the write; a legal value
+is untouched.
+
+⛔ The bounds are not restated in objectui. A hand-copied range would be a second
+contract free to drift from the spec (AGENTS.md #0.1), so the schema is asked and
+the message is built from its issues — the same discipline `LocationField`'s own
+range refusal already follows.
+
+Deliberately unchanged:
+
+- **Input-time refusal (objectui#6714/#6716) is still the widget's.** A refusal
+  means `onChange` never fires, so the typed text never becomes a form value and
+  this rule is handed `undefined`. The two do not overlap.
+- **Absence is `required`'s business.** The spec's schema refuses `null` and
+  `undefined` outright because it describes a *present* value, so the rule asks
+  core's `isMissingForRequired` — the repo's single presence contract — rather
+  than inventing a second definition of "empty". A create form with an untouched
+  location field is unaffected.
+- **A field-authored `validate` keeps running**, composed under its own key
+  rather than replaced.
+- **Scope is `location` only.** Whether other field types have the same
+  stored-value gap is a separate question and was not surveyed here.

packages/fields/src/__tests__/LocationField.refusalDiagnostic.test.tsx

@@ -29,8 +29,8 @@
  * arms, because a refusal means `onChange` never fires, so the typed text never
  * becomes a form value at all — while the same branch fires correctly for a
  * STORED out-of-range pair. `buildValidationRules` compiles value-shaped rules;
- * a refusal has no value. It still has no `location` branch and this card does
- * not give it one.
+ * a refusal has no value. It HAS one as of objectui#6744 — for that STORED
+ * case, never for these refusal arms — and this card did not give it one.
  *
  * So the state is the widget's own, following `ObjectField`'s live precedent in
  * this same directory: a second name (`parseError` there, `refusalError` here),

packages/fields/src/index.tsx

@@ -8,7 +8,11 @@
 
 import React from 'react';
 import type { FieldMetadata, SelectOptionMetadata } from '@object-ui/types';
-import { ComponentRegistry, percentDisplayValue, getRecordDisplayName, humanizeLabel, type ComponentMeta } from '@object-ui/core';
+import { ComponentRegistry, percentDisplayValue, getRecordDisplayName, humanizeLabel, isMissingForRequired, type ComponentMeta } from '@object-ui/core';
+// The platform's own value-shape contract, asked rather than restated
+// (objectui#6744). See `locationStoredValueSchemaFor` below for why this is a
+// runtime import in the barrel and not a hand-written coordinate range.
+import { valueSchemaFor } from '@objectstack/spec/data';
 import { useLocalization, useDisplayLocale, formatDisplayNumber } from '@object-ui/i18n';
 import { Badge, Avatar, AvatarImage, AvatarFallback, Button, Checkbox, EmptyValue, cn } from '@object-ui/components';
 import { Check, X, Copy, Phone as PhoneIcon, MapPin } from 'lucide-react';
@@ -2543,6 +2547,96 @@ export function formatFileSize(bytes: number): string {
   return `${size.toFixed(unitIndex > 0 ? 1 : 0)} ${units[unitIndex]}`;
 }
 
+/**
+ * Per-field-definition cache of the spec's derived value schema
+ * (objectui#6744).
+ *
+ * `valueSchemaFor` states the requirement itself: "Pure derivation — no caching
+ * here; runtime consumers MUST cache per field definition (building a
+ * `z.object` per write is an order of magnitude costlier than parsing)." This
+ * is the same `WeakMap`-on-the-field-def idiom the platform's own write-path
+ * validator uses (`shapeSchemaFor`,
+ * `packages/objectql/src/validation/record-validator.ts`), so the hit rate is
+ * governed by the same condition there and here: whether the host hands the
+ * same field object back.
+ *
+ * ⛔ The def is passed through VERBATIM, never rebuilt from keys picked out of
+ * it. `valueSchemaFor` decides for itself what a location value is and how a
+ * field's multiplicity participates; a reconstructed def would be a second
+ * reading of the contract, free to drift from the first (AGENTS.md #0.1).
+ */
+const locationStoredValueSchemas = new WeakMap<object, ReturnType<typeof valueSchemaFor>>();
+
+function locationStoredValueSchemaFor(field: any): ReturnType<typeof valueSchemaFor> {
+  let schema = locationStoredValueSchemas.get(field);
+  if (!schema) {
+    schema = valueSchemaFor(field, 'stored');
+    locationStoredValueSchemas.set(field, schema);
+  }
+  return schema;
+}
+
+/**
+ * The STORED-value rule for a `type: 'location'` field (objectui#6744).
+ *
+ * ## What was missing
+ *
+ * `buildValidationRules` is the producer of the host-side `error` prop that
+ * every field widget's published objectui#3222 slot reads, and it had no
+ * `location` branch. So a coordinate that is ALREADY IN THE RECORD and violates
+ * the spec's range was never validated on an edit form: the control rendered
+ * it, nothing marked it invalid, and submitting re-wrote it unchanged.
+ *
+ * ⚠️ This is a different defect from objectui#6714/#6716, which are about a
+ * refusal at INPUT time — the user types something the widget will not emit. A
+ * refusal never becomes a form value, so this rule is handed `undefined` in
+ * both of those arms; the two do not overlap and neither replaces the other.
+ *
+ * ## Why the whole value is handed to the spec, not a range test
+ *
+ * ⛔ The bounds are NOT restated here as `-90`/`90` literals — the same
+ * discipline `LocationField`'s own `isSpecAcceptedLocation` follows, and for the
+ * same reason: a hand-copied range is a SECOND contract that drifts from the
+ * spec silently (AGENTS.md #0.1).
+ *
+ * Asking `valueSchemaFor(field, 'stored')` also makes this rule agree, by
+ * construction, with the platform's own write path. The engine's record
+ * validator checks a stored `location` against THIS schema (ADR-0104 D1,
+ * `record-validator.ts`), warn-first until a deployment's `os migrate
+ * value-shapes` scan certifies zero violations and rejecting afterwards. So a
+ * value this rule marks invalid is a value the platform itself already refuses
+ * or has recorded as an admitted violation — the form now surfaces that
+ * verdict where the person who can correct it is standing, rather than
+ * inventing a verdict of its own.
+ *
+ * ## Absence is `required`'s business, not this rule's
+ *
+ * The spec's schema describes a PRESENT value — it refuses `null` and
+ * `undefined` outright, and its docblock says so ("null/undefined/required
+ * handling stays with the caller"). Deciding presence HERE would be a second
+ * definition of "empty" competing with the one `required` uses, so this asks
+ * core's `isMissingForRequired` — the repo's single presence contract, the same
+ * predicate the form renderer's own `required` validator calls. That is what
+ * keeps a CREATE form with an untouched location field valid.
+ *
+ * ## The message is the spec's own complaint
+ *
+ * Built from the schema's issues, exactly as `LocationField`'s
+ * `refusedRangeMessage` builds the widget-side sentence, so the day the schema
+ * moves both sentences move with it and neither can quote a stale bound.
+ */
+function buildLocationStoredValueValidator(field: any): (value: unknown) => true | string {
+  return (value: unknown) => {
+    if (isMissingForRequired(value)) return true;
+    const parsed = locationStoredValueSchemaFor(field).safeParse(value);
+    if (parsed.success) return true;
+    const detail = parsed.error.issues
+      .map((issue: any) => `${issue.path.join('.') || 'value'}: ${issue.message}`)
+      .join('; ');
+    return `Invalid location: ${detail}`;
+  };
+}
+
 /**
  * Build validation rules from field metadata
  * @param field - Field metadata from ObjectStack
@@ -2638,6 +2732,26 @@ export function buildValidationRules(field: any): any {
     rules.validate = field.validate;
   }
 
+  // Stored-value validation for `location` (objectui#6744). See
+  // `buildLocationStoredValueValidator` for what this rule is and what it is
+  // deliberately not.
+  //
+  // Emitted in react-hook-form's OBJECT form so a field-authored `validate`
+  // keeps running under its own key instead of being replaced — the same
+  // normalisation the form renderer applies when it adds its `required` entry
+  // (`packages/components/src/renderers/form/form.tsx`), spelled the same way
+  // so the two compose rather than clobber. RHF reports an object-form failure
+  // under its key, so this surfaces as `type: 'location'`.
+  if (field.type === 'location') {
+    const authoredValidate = rules.validate;
+    rules.validate = {
+      ...(typeof authoredValidate === 'function'
+        ? { validate: authoredValidate }
+        : (authoredValidate ?? {})),
+      location: buildLocationStoredValueValidator(field),
+    };
+  }
+
   return Object.keys(rules).length > 0 ? rules : undefined;
 }
 

packages/fields/src/widgets/LocationField.tsx

@@ -353,8 +353,9 @@ export function LocationField({ value, onChange, field, readonly, error, ...prop
    * `undefined`. That was measured on this card before the route was chosen: a
    * real `location` branch installed in `buildValidationRules` saw `undefined`
    * in both refusal arms, while the same branch fired correctly for a STORED
-   * out-of-range pair. `buildValidationRules` still has no `location` branch,
-   * and this card does not give it one.
+   * out-of-range pair. `buildValidationRules` HAS a `location` branch as of
+   * objectui#6744 — for that STORED case, never for these refusal arms — and
+   * this card did not give it one.
    */
   const [refusalError, setRefusalError] = useState<string | null>(null);
 

packages/plugin-form/src/ObjectForm.locationRange.test.tsx

@@ -29,10 +29,18 @@
  * ⚠️ The pass-through assertion below is load-bearing, not decoration. The form
  * hands the widget's emission to `dataSource.create` UNCHANGED —
  * `sanitizeFormData` filters KEYS (server-managed, computed, read-only) and
- * never inspects a value, and `buildValidationRules` has no branch for
- * `location`, so its `min`/`max` rules only ever carry an author-declared bound
- * on a scalar. If a later change ever did add repair or validation on this
- * path, that assertion is what notices.
+ * never inspects a value.
+ *
+ * ⚠️ The second half of that sentence has since changed and the cases below
+ * still hold, which is worth stating rather than leaving to be re-derived.
+ * `buildValidationRules` DOES have a `location` branch now (objectui#6744): a
+ * stored value is adjudicated against `valueSchemaFor({ type: 'location' })`
+ * and an out-of-range one blocks the write. It does not touch the readings
+ * here, because every case in this file TYPES its coordinate — the widget
+ * refuses the out-of-range ones before they become form values, so the rule is
+ * handed `undefined` and has nothing to say. The measurement this file records
+ * is about the CREATE path at INPUT time; #6744's is about a value already in
+ * the record. `ObjectForm.locationStoredRange.test.tsx` pins that one.
  */
 import { describe, it, expect, vi } from 'vitest';
 import { render, waitFor, fireEvent } from '@testing-library/react';

packages/plugin-form/src/ObjectForm.locationRefusal.test.tsx

@@ -29,9 +29,16 @@
  * published objectui#3222 `error` slot. Measured: that route cannot see a
  * refusal at all — a refusal means `onChange` never fires, so the typed text
  * never becomes a form value, and a `location` branch installed there was
- * invoked with `undefined` in both arms. The last test in this file pins the
- * consequence that keeps the sibling file's docblock true: `buildValidationRules`
- * STILL has no `location` branch, and this card did not give it one.
+ * invoked with `undefined` in both arms.
+ *
+ * ⚠️ `buildValidationRules` DOES have a `location` branch now — objectui#6744
+ * added one, for the different defect of a STORED out-of-range coordinate that
+ * was never validated on an edit form. That does not weaken the reason above,
+ * it is the reason above seen from the other side: the branch adjudicates a
+ * value, and a refusal has no value to adjudicate. The last test in this file
+ * is now the pin for THAT — the rule exists and answers `true` for the
+ * `undefined` it is handed here — so the announcement stays the widget's, and
+ * this card is still not the one that installed the branch.
  */
 import { describe, it, expect, vi } from 'vitest';
 import { render, waitFor, fireEvent } from '@testing-library/react';
@@ -137,18 +144,21 @@ describe('ObjectForm shows WHY a location was refused (objectui#6716)', () => {
   });
 });
 
-describe('the fix did NOT give `buildValidationRules` a location branch (objectui#6716)', () => {
-  it('compiles no rule for a `location` field', () => {
-    // The sibling #6714 pin's docblock states this as fact; it stays true under
-    // the widget-local route, and this is what keeps the two files honest. The
-    // measured reason it is not worth changing: a refusal never becomes a form
-    // value, so a rule here is handed `undefined`.
-    // It compiles rules key by key and returns `undefined` when none applied —
-    // so `undefined` here is "no branch matched a location field", not "the
-    // helper is missing".
-    expect(buildValidationRules({ type: 'location', name: 'place' })).toBeUndefined();
+describe('the announcement is still the widget\'s, not the host rule\'s (objectui#6716)', () => {
+  it('the `location` rule objectui#6744 added is handed `undefined` by a refusal', () => {
+    // This pin used to read "compiles no rule for a `location` field". #6744
+    // answered that question — the branch exists, for STORED values — so the
+    // pin now asserts the property that actually kept #6716 widget-local, and
+    // that a future edit could still break: the host rule adjudicates a VALUE,
+    // and a refusal produces none.
+    const rules = buildValidationRules({ type: 'location', name: 'place' });
+    expect(typeof rules.validate.location).toBe('function');
+    // What the rule sees in both refusal arms above, because `onChange` never
+    // fired: nothing. A rule that answered anything but `true` here would be
+    // reporting a refusal it cannot observe.
+    expect(rules.validate.location(undefined)).toBe(true);
     // A control from the same call, so the reading above cannot be an artefact
-    // of a helper that returns `undefined` for everything.
+    // of a helper that answers the same way for everything.
     expect(buildValidationRules({ type: 'text', name: 'title', required: true })).toEqual({ required: true });
   });
 });

packages/plugin-form/src/ObjectForm.locationResidue.test.tsx

@@ -50,11 +50,12 @@
  * ⛔ Degree/hemisphere notation is NOT parsed — the ruling declines it because
  * the paste route is unmeasured. The refusal is the whole change.
  *
- * ⛔ `buildValidationRules` still has NO `location` branch, and this card did
- * not give it one — the last test is the pin that keeps
- * `ObjectForm.locationRange.test.tsx`'s docblock true. It could not have one:
- * a refusal means `onChange` never fires, so the typed text never becomes a
- * form value for a value-shaped rule to see (measured on objectui#6716).
+ * ⛔ This card did not give `buildValidationRules` a `location` branch — the
+ * last test is the pin for that. objectui#6744 later did, for the different
+ * defect of a STORED out-of-range coordinate on an edit form, and the pin was
+ * rewritten to assert the property that survives: a refusal means `onChange`
+ * never fires, so the typed text never becomes a form value, and the host rule
+ * — which adjudicates a value — is handed `undefined` (measured on #6716).
  */
 import { describe, it, expect, vi } from 'vitest';
 import { render, waitFor, fireEvent } from '@testing-library/react';
@@ -217,16 +218,18 @@ describe('ObjectForm still stores every clean numeric pair (objectui#6715)', ()
 /* The scope fence objectui#6744 owns.                                         */
 /* -------------------------------------------------------------------------- */
 
-describe('objectui#6715 did not give buildValidationRules a `location` branch', () => {
-  it('compiles no rules for a location field', () => {
-    // The fact `ObjectForm.locationRange.test.tsx`'s docblock asserts, kept
-    // true here as well. objectui#6744 owns the question of whether it should
-    // have one; this card did not answer it.
-    // It compiles rules key by key and returns `undefined` when none applied,
-    // so `undefined` here means "no branch matched a location field".
-    expect(buildValidationRules({ type: 'location', name: 'place' })).toBeUndefined();
+describe('the residue refusal is the widget\'s, not a host rule\'s (objectui#6715)', () => {
+  it('the `location` rule objectui#6744 added is handed `undefined` by a refusal', () => {
+    // This pin used to read "compiles no rules for a location field", with the
+    // note that objectui#6744 owned the question. #6744 answered it: the branch
+    // exists, and it validates a STORED value. So the pin now asserts what kept
+    // this card's refusal widget-local in the first place.
+    const rules = buildValidationRules({ type: 'location', name: 'place' });
+    expect(typeof rules.validate.location).toBe('function');
+    // A residue refusal emits nothing, so the host rule sees nothing.
+    expect(rules.validate.location(undefined)).toBe(true);
     // A control from the same call, so the reading above cannot be an artefact
-    // of a helper that returns `undefined` for everything.
+    // of a helper that answers the same way for everything.
     expect(buildValidationRules({ type: 'text', name: 'title', required: true })).toEqual({ required: true });
   });
 });