fix(cli): os explain query teaches where / orderBy, and the catalog sweep asserts key retention

.changeset/quiet-pugs-tickle.md

@@ -0,0 +1,24 @@
+---
+'@objectstack/cli': patch
+---
+
+`os explain query` now teaches the two keys `QuerySchema` actually declares.
+
+The entry's example and its two optional-table rows named `filters` and `sort`.
+Neither is a key of `BaseQuerySchema`, which is a plain `z.object` — so both
+were dropped silently: an author who copied the example got a query that parsed
+clean and ran with no filter and no ordering, with nothing in the output saying
+so.
+
+Both faces now read the schema's own spellings:
+
+- `where` — one condition **tree**, not a `Filter[]`. A field-keyed entry is a
+  condition on that field (a bare value is implicit equality, an object is a map
+  of `$` operators), and `$and` / `$or` / `$not` combine conditions.
+- `orderBy` — sort nodes, each `{ field, order }`. The direction key is spelled
+  `order`; `direction` is rejected by name.
+
+No schema changed, and no accept set moved: the correction is to the catalog
+entry only. The `os explain` catalog sweep also gains a key-retention assertion
+— an example must parse **and** come back with every key it declares — so the
+next entry whose schema strips a key is named instead of passing.

packages/cli/src/commands/explain.ts

@@ -238,6 +238,12 @@ export const SCHEMAS: Record<string, SchemaInfo> = {
     docsPath: 'ui/app',
   },
 
+  // `BaseQuerySchema` (`spec/src/data/query.zod.ts`) is a plain `z.object`, so an
+  // unknown top-level key here is DROPPED rather than refused — the one open top
+  // level among this catalog's bound entries (closing it is #4001's to schedule).
+  // The two keys are therefore spelled exactly as the schema declares them:
+  // `where` (not `filters`) and `orderBy` (not `sort`). Spelt wrong, a copied
+  // example parses clean and returns unfiltered, unordered rows with no signal.
   query: {
     name: 'Query',
     description: 'Declarative data retrieval definition used for fetching and filtering records from objects.',
@@ -246,16 +252,20 @@ export const SCHEMAS: Record<string, SchemaInfo> = {
     ],
     optional: [
       { name: 'fields', type: 'string[]', description: 'Fields to select' },
-      { name: 'filters', type: 'Filter[]', description: 'Where conditions' },
-      { name: 'sort', type: 'SortConfig[]', description: 'Order by configuration' },
+      { name: 'where', type: 'FilterCondition', description: 'The condition TREE the query filters by — one object, never a `Filter[]`. A field-keyed entry is a condition on that field (a bare value is implicit equality, an object is a map of `$` operators such as `$in` / `$gte` / `$contains`), and `$and` / `$or` / `$not` combine conditions.' },
+      { name: 'orderBy', type: 'SortNode[]', description: 'Sort nodes, each `{ field, order }` with `order` one of `asc` / `desc`. The direction key is spelled `order` — `direction` is rejected by name, because when it was merely dropped the sort fell back to `asc` and, with `limit`, returned a different set of rows under an ordinary success.' },
       { name: 'limit', type: 'number', description: 'Maximum records to return' },
       { name: 'offset', type: 'number', description: 'Pagination offset' },
     ],
     example: `{
   object: 'project_task',
   fields: ['title', 'status', 'assigned_to'],
-  filters: [{ field: 'status', operator: 'eq', value: 'open' }],
-  sort: [{ field: 'created_at', order: 'desc' }],
+  // \`where\` is ONE condition tree, not a \`Filter[]\`: key it by field — a bare value
+  // is implicit equality, an object is a map of \`$\` operators — and combine with
+  // \`$and\` / \`$or\` / \`$not\`.
+  where: { status: 'open', priority: { $in: ['high', 'urgent'] } },
+  // A sort node spells its direction \`order\`, never \`direction\`.
+  orderBy: [{ field: 'created_at', order: 'desc' }],
   limit: 50,
 }`,
     related: ['object', 'field', 'view'],

packages/cli/test/commands.test.ts

@@ -210,7 +210,7 @@ describe('os explain — every catalog entry swept against its spec schema (#148
     ...specAutomation,
   };
 
-  type ParseResult = { success: boolean; error?: { issues: unknown[] } };
+  type ParseResult = { success: boolean; data?: unknown; error?: { issues: unknown[] } };
   type ZodLike = { safeParse: (value: unknown) => ParseResult };
 
   // The catalog stores examples as authored source, so evaluate the literal —
@@ -295,6 +295,102 @@ describe('os explain — every catalog entry swept against its spec schema (#148
     }
   }
 
+  // ── Key RETENTION: an example must SURVIVE its parse, not merely pass it ────
+  //
+  // The sweep above asserts `safeParse(...).success === true` — and that stays
+  // green over a key the schema SILENTLY STRIPS, because a plain `z.object`
+  // drops what it does not declare and still reports success. The `query` entry
+  // shipped teaching `filters` and `sort`, neither of them a `QuerySchema` key,
+  // and every run of the sweep above was green on it (#16925). "Parses" is
+  // therefore not the property worth asserting on its own; "parses AND comes
+  // back whole" is, and only the second one can see this failure mode.
+  //
+  // ⭐ This is a RATCHET, not a patch over a large hole. All nine bound entries
+  // were read back to spec on the day it landed: eight refuse an unknown key
+  // outright — seven `strictObject`, plus `object`, whose docblock states the
+  // "No silent strip (ADR-0032 / #1535)" contract explicitly — and `query` alone
+  // had an open top level, deliberately so and already owned (`query.zod.ts`:
+  // "Deliberately NOT taken here: `BaseQuerySchema`'s own top level stays
+  // non-strict. That is #4001's to schedule."). So it is green across the whole
+  // catalog the day it lands. What it defends is the day a bound entry resolves
+  // to an open top level again: the direction of travel is *closing*, and this
+  // is what notices if that reverses — with the entry named, instead of a green
+  // sweep over a silently emptied example.
+  //
+  // ⛔ It reads the EXAMPLE face only — the one face `evaluate` reads. The
+  // `optional` / `required` tables carry key names too, and no assertion in this
+  // file has ever looked at them; a row naming a key the schema does not have is
+  // the same defect on the other face (this card's own `filters` / `sort` lived
+  // on BOTH). Covering it is not a stricter version of this assertion but a
+  // different one: a table row is prose, not a key — `view`'s required row is
+  // spelled `list | form | listViews | formViews` — so it needs a way to tell a
+  // key name from a description before it can judge anything. Declared as a gap
+  // here rather than half-built.
+  //
+  // The walk is deliberately conservative — it reports a key present in the
+  // INPUT and absent from the OUTPUT, and nothing else. Keys a schema ADDS
+  // (defaults) are not drift; a value a schema TRANSFORMS to a non-object is not
+  // a dropped key, so the walk stops rather than guessing. Arrays are matched
+  // positionally, which is what every schema in this catalog does today.
+  const isWalkable = (value: unknown): value is Record<string, unknown> =>
+    typeof value === 'object' && value !== null && !(value instanceof Date);
+
+  const droppedKeys = (
+    input: unknown,
+    output: unknown,
+    path: string[] = [],
+    out: string[] = [],
+  ): string[] => {
+    if (Array.isArray(input)) {
+      if (!Array.isArray(output)) return out;
+      input.forEach((item, i) => droppedKeys(item, output[i], [...path, String(i)], out));
+      return out;
+    }
+    if (!isWalkable(input) || Array.isArray(output) || !isWalkable(output)) return out;
+    for (const key of Object.keys(input)) {
+      if (!Object.prototype.hasOwnProperty.call(output, key)) out.push([...path, key].join('.'));
+      else droppedKeys(input[key], output[key], [...path, key], out);
+    }
+    return out;
+  };
+
+  for (const [key, bound] of Object.entries(BOUND)) {
+    if (bound.card === undefined) {
+      it(`os explain ${key} — example survives ${bound.schema} with every declared key intact`, () => {
+        const schema = specSurface[bound.schema] as ZodLike;
+        const example = evaluate(key);
+        const result = schema.safeParse(example);
+        // Retention is only a question about a parse that succeeded — stated,
+        // so a failure here reads as "the parse broke" and not as a drop.
+        expect(
+          result.success,
+          `os explain ${key}: its example must parse before retention can be judged`,
+        ).toBe(true);
+        expect(
+          droppedKeys(example, result.data),
+          `os explain ${key}: ${bound.schema} SILENTLY DROPPED key(s) its example declares. `
+            + 'The example teaches keys the schema does not have, so an author who copies it '
+            + 'gets a parse that succeeds and a value with those keys gone — no error, no '
+            + 'warning. Correct the example to the schema\'s own spellings (⛔ do not relax '
+            + 'the schema to accept them). The `parses` assertion above cannot see this: a '
+            + 'non-strict object reports success and strips.',
+        ).toEqual([]);
+      });
+    } else {
+      // Asserted, never skipped: an entry whose example does not parse cannot be
+      // judged for retention, and that reason is a property of this file rather
+      // than an omission the reader has to notice.
+      it(
+        `os explain ${key} — retention NOT judged: its example does not parse yet `
+          + `(known-broken, filed as #${bound.card})`,
+        () => {
+          const schema = specSurface[bound.schema] as ZodLike;
+          expect(schema.safeParse(evaluate(key)).success).toBe(false);
+        },
+      );
+    }
+  }
+
   it(`os explain workflow — ${UNBOUND.workflow}`, () => {
     expect('WorkflowSchema' in specSurface).toBe(false);
     expect(catalog.workflow.name).toContain('no standalone type');