Skip to content

Commit ecf90b2

Browse files
fix(spec): the top-level report alias table routes the ten block spellings it claimed to mirror (#19855)
Fixes #19822 Clause-②: no `ReportSchema`'s alias table in `packages/spec/src/ui/report.zod.ts` says it is "kept deliberately parallel" to `JoinedReportBlockSchema`'s table in the same file. It was not: ten of the block's entries were missing at the top level, even though every target (`values`, `rows`, `order`, `dataset`) is a key `ReportSchema` declares. This PR adds those ten entries. After it, the top-level table carries all 17 block entries, in the block's order and wording. ## Measured first: what the author saw on `origin/main` `71ef221988` Setup: a minimal valid top-level report (`name`, `label`, `type: 'summary'`, `dataset: 'sales'`, `rows: ['stage']`, `values: ['revenue']`) plus one extra key, run through `ReportSchema.safeParse`. Each parse gave exactly one issue, `unrecognized_keys`. Each message then ended with the same `history` sentence ("Until this shape was closed these were dropped silently — …"), which is left out of the table. | key | message on `71ef221988` (before) | suggestion before | after this PR | | :-- | :-- | :-- | :-- | | `measures` | `` Unrecognized key(s) on this report: `measures`. `` | none | `` Did you mean `measures` → `values`? `` | | `metrics` | `` Unrecognized key(s) on this report: `metrics`. `` | none | `` Did you mean `metrics` → `values`? `` | | `dimensions` | `` Unrecognized key(s) on this report: `dimensions`. `` | none | `` Did you mean `dimensions` → `rows`? `` | | `groupBy` | `` Unrecognized key(s) on this report: `groupBy`. `` | none | `` Did you mean `groupBy` → `rows`? `` | | `groupings` | `` Unrecognized key(s) on this report: `groupings`. `` | none | `` Did you mean `groupings` → `rows`? `` | | `sort` | `` Unrecognized key(s) on this report: `sort`. `` | none | `` Did you mean `sort` → `order`? `` | | `orderBy` | `` Unrecognized key(s) on this report: `orderBy`. Did you mean `orderBy` → `order`? `` | `order` (edit distance), **correct** | `` Did you mean `orderBy` → `order`? `` (now from the table) | | `sortBy` | `` Unrecognized key(s) on this report: `sortBy`. `` | none | `` Did you mean `sortBy` → `order`? `` | | `objectName` | `` Unrecognized key(s) on this report: `objectName`. `` | none | `` Did you mean `objectName` → `dataset`? `` | | `object` | `` Unrecognized key(s) on this report: `object`. `` | none | `` Did you mean `object` → `dataset`? `` | **Wrong suggestions: none.** No key drew a suggestion pointing at the wrong key. In particular, `objectName` did **not** suggest `name`. Nine of the ten got no suggestion at all. `orderBy` alone reached its target, by edit distance, and that target was correct. As a control, the same ten keys on a `JoinedReportBlockSchema` in the same run each got the block table's prescription. ## What changed - **`report.zod.ts`: `ReportSchema`'s `aliases`.** Added `objectName`/`object` → `dataset`, `measures`/`metrics` → `values`, `groupings`/`groupBy`/`dimensions` → `rows`, and `sort`/`orderBy`/`sortBy` → `order`. They sit in the block table's order, under the block's comments, adapted from "block" to "report". - **The history note stays true.** The "kept deliberately parallel" paragraph now says what parallel means: every block entry whose target this schema declares is carried here to the same target. A key that must not route at the top level gets a `guidance` line. The note also names where the two tables still differ, and why: the `guidance` tables. The block's three guidance entries point at keys this schema declares, and `drillDown` is report-only. The existing note about the three removed entries (`columns`, `chart`, `chartConfig`) is unchanged and still accurate. A new paragraph records that the ten were missing and that only `orderBy` reached its target by distance. - **No `guidance` entry was needed.** None of the ten has a reason not to route at the top level. Each target is declared on `ReportSchema`, and none of the ten keys is declared there. One path was checked end to end: on a `type: 'joined'` report, `sort:` is told `order`, and writing `order` there is refused by the existing refinement ("a `joined` report orders per block — move `order` onto `blocks[]`"). So that chain ends with a prescription, never with silence. The comment above the ordering entries says so. ## Tests `packages/spec/src/ui/report.test.ts` has a new `describe`, "ReportSchema — routes the block vocabulary the way a block does": - **Per key, ×10: what the author sees.** The parse fails, the one `unrecognized_keys` issue names exactly that key, and the message contains `` Did you mean `KEY` → `TARGET`? ``. - **Per key, ×10: taking the advice parses.** A full green `safeParse` of the valid report with the target key. These are preservation pins and are green before and after this change. - **Parity, derived at runtime.** The pin reads both tables from `strictObjectDeclarations()`, the same registry `alias-integrity.test.ts` uses, looked up by `surface`, with exactly one declaration asserted per surface. Every block alias whose target is in `ReportSchema`'s shape must be routed at the top level to the same target, or be answered by a top-level `guidance` entry. A non-vacuity floor asserts that the derived set contains all ten keys this card names. **Firing control.** Run at `8a688435cc`: the alias region was swapped back to `71ef221988`'s with `node scripts/ablation-replace.mjs`, which wraps the run and restores the file. The on-disk blob during the mutation was `fe9d289f63ba`, which is exactly `71ef221988`'s blob for `report.zod.ts`, so the test ran against the file as it is on main. Result: `Tests 10 failed | 40 passed (50)`. - The per-key pin went red for 9 of the 10 keys. - The per-key pin for `orderBy` stayed green, because edit distance already answered `order`, as measured above. Its table entry is held by the parity pin. - The parity pin went red with exactly the ten rows, for example `measures → values (top level: absent)` and `orderBy → order (top level: absent)`. Restore was proven: the blob after restore equals `HEAD`'s `452f138f054d`, and `git diff HEAD` is empty. **Runs.** - Head `926cff321d`: `@objectstack/spec` `report.test.ts` + `alias-integrity.test.ts` + `strictness-batch14.test.ts` gave `3 passed (3)`, `91 passed (91)`. At `c78242fe05` the same set plus `strict-object.test.ts` gave `4 passed (4)`, `123 passed (123)`. - Head `926cff321d`: `pnpm --filter @objectstack/platform-objects test`, run after rebuilding spec and its closure, gave `54 passed (54)` files and `883 passed (883)` tests. `pnpm --filter @objectstack/platform-objects typecheck` exits 0. - Full `@objectstack/spec` `local` project at `8a688435cc`: `523 passed (523)` files, `15434 passed | 1 todo` tests. `packages/` is byte-identical between `8a688435cc` and `c78242fe05`: `git diff --quiet` exit 0. - `pnpm --filter @objectstack/spec typecheck`: exit 0. `check:test-typecheck` reports OK and holds the ledger unchanged, and `tsc -p tsconfig.test.json --listFilesOnly` includes `src/ui/report.test.ts`. **`alias-integrity.test.ts`** is not edited; PR #19789 holds that file. Run as-is: `20 passed (20)` at `8a688435cc` and again at `c78242fe05`, inside the 123 above. Its three claims hold for the new entries. No key is declared, every target is accepted, and no two keys in the table collapse onto one probe. **Reading pin reversed in `@objectstack/platform-objects`.** `packages/platform-objects/src/apps/translations/report-form-echo-decisions.test.ts` pinned the gap this PR closes as a reading: it asserted that `measures` on a top-level report drew no rename suggestion. That turned `Test Core (4/6)` red on `c78242fe05`. The test is reversed in place, not deleted. Under a retitled `it`, it now asserts that the top-level report refuses `measures` AND names `` `measures` → `values` ``; the block half is unchanged. The file header bullet that described the asymmetry is rewritten to match. No other test in that file changed. ## Generated artifacts `pnpm --filter @objectstack/spec build` runs `gen:schema`. After it, `git status --porcelain` was empty: no generated file moved, as the claim predicted. Alias keys are not part of `authorable-surface/`. `pnpm --filter @objectstack/spec check:generated` then reported `All 15 generated artifacts are up to date`, measured against the dist that build produced (declaration stamp match). ## Changeset reading The change ships, so it carries a `patch` changeset (`.changeset/19822-report-top-level-aliases.md`), with `Clause-②: no`. - `@objectstack/spec`'s `files[]` includes `dist` and `src/**/*.zod.ts`, so `report.zod.ts` is published both as source and compiled into `dist/ui/`. - Positive control: `measures: 'values'` occurs once in `71ef221988`'s `report.zod.ts` (block only) and twice at head, and the built `dist/ui/index.js` and `index.mjs` each carry it twice. - The change is a behaviour change in a released package (`17.4.0`), namely the text of a rejection. AGENTS.md makes that a `patch`, and ⛔ never `skip-changeset`. - It is not breaking, since every accept/reject verdict is unchanged. Only the prescription inside an existing `unrecognized_keys` message is new. `check-adr-0087-registration` reads it as `1 non-breaking changeset(s) seen`. ## Gates Derived with `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` from this worktree: 85 commands for the 4-path change set, the first round's 83 plus `check:i18n` and `check:i18n-stale-fill`. All 85 were run at head `926cff321d` after a full package build. `dispatch-gates --ran` read: `85 derived, 85 run, 0 NOT-MEASURED, 0 UNRUN`, with every recorded exit code 0. That union covers `check:*` families only. Package test suites run in CI's `Test Core`, which the tool lists as a path-scheduled CI job outside its derived total. - Three families needed built packages that a fresh worktree lacks, and first answered exit 3: `check:dual-build-cjs-loads`, `check:type-check-debt` and `check:doc-formula-expressions`. - The package closure was then built, with declarations, under the shared verify lock, and all three were re-run on the same head, each exiting 0. The `type-check-debt` verdict line was "4 ledger entr(ies) re-measured … none above its recorded number". Lint, as a proven narrowing rather than a repo scan: - The three changed source files were linted with `eslint --no-inline-config --format json` at `926cff321d`: 3 files, 0 errors, 0 warnings. - The config applied to them (`--print-config`) has no `parserOptions.project`, and `eslint.config.mjs` states that it never enables type-aware linting. So this diff cannot move the lint verdict of any untouched file. - The changeset is not an eslint input. ## Acceptance notes - **Out of scope, reported for a separate card, not touched here.** A `type: 'joined'` report accepts top-level `dataset`, `rows`, `columns` and `values`. For example, `{ type: 'joined', blocks: [...], dataset: 'sales', rows: ['stage'], columns: ['region'], values: ['revenue'] }` parses clean at head. But the report's own contract says "For a `joined` report, the data lives on `blocks`", and the renderer's joined branch returns before it reads any of them (objectui `0cf2d66`, `packages/plugin-report/src/DatasetReportRenderer.tsx`, the `presentation === 'joined'` branch). So those keys are silently dropped. `order` is the only top-level selection key the joined refinement refuses. This predates this PR: the existing `fields → values` and `dataSet → dataset` entries already pointed there, and so do the new `measures`, `dimensions` and `objectName` routes on a joined report. The loud fix is to extend the joined arm of the refinement the way `order` already is. That narrows acceptance, so it needs its own card and changeset. --- _Generated by [Claude Code](https://claude.ai/code/session_013RDBh5DqXd2xnLwvHLgLFr)_ --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 77f54bf commit ecf90b2

4 files changed

Lines changed: 155 additions & 14 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
A top-level report now corrects the same ten misspelled keys a joined-report block already corrects. Before this change, `measures:` on a plain report was rejected with no suggestion, while the same key on a block was told to use `values`.
6+
7+
Clause-②: no
8+
9+
`ReportSchema`'s alias table says it is kept parallel to `JoinedReportBlockSchema`'s, but ten of the block's entries were missing from it. The rejection now names the target key, and every target is a key the report declares:
10+
11+
| authored key on a report | before | now |
12+
| :--- | :--- | :--- |
13+
| `measures`, `metrics` | no suggestion | ``Did you mean `measures` → `values`?`` |
14+
| `dimensions`, `groupBy`, `groupings` | no suggestion | ``Did you mean `dimensions` → `rows`?`` |
15+
| `sort`, `sortBy` | no suggestion | ``Did you mean `sort` → `order`?`` |
16+
| `orderBy` | `order`, found by edit distance | `order`, from the alias table |
17+
| `objectName`, `object` | no suggestion | ``Did you mean `objectName` → `dataset`?`` |
18+
19+
**Every accept/reject verdict is unchanged.** The same reports are refused, with the same `unrecognized_keys` issue. Only the prescription in that issue's message is new. Nothing authorable is added, removed or renamed.

‎packages/platform-objects/src/apps/translations/report-form-echo-decisions.test.ts‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,12 @@
114114
// with `rows`, in the rejection message, and the `order` refinement names
115115
// both halves in one live sentence ("name a `rows`/`columns` dimension or a
116116
// `values` measure").
117-
// • ⭐ AND THE TWO VOCABULARIES ARE ASYMMETRIC, which is why the sentence has
118-
// to exist: the alias table that converts them lives on the BLOCK and not
119-
// on the top-level report. That asymmetry is asserted as a reading, ⛔ not
120-
// repaired here — it is a spec-side edit outside this round's file surface.
117+
// • ⭐ AND THE TWO VOCABULARIES ARE DISTINCT, which is why the sentence has to
118+
// exist: the schema converts one into the other only in a rejection
119+
// message. That alias table lives on the block AND on the top-level report.
120+
// This round recorded it on the block only, as a reading it did not
121+
// repair; #19822 closed the gap in the spec, and the reading is reversed in
122+
// place below.
121123
//
122124
// ## ⚠️ The renderings depart from a literal mapping, and the row says why
123125
//
@@ -629,15 +631,17 @@ describe('#19403 round 9 — the semantic layer, asserted AT THE SCHEMA', () =>
629631
expect(issueMessages(order)).toContain('name a `rows`/`columns` dimension or a `values` measure');
630632
});
631633

632-
it('⚠️ A READING, ⛔ NOT A REPAIR — the alias table is on the BLOCK and not on the top-level report', () => {
633-
// The same two keys, the same two target keys, one level apart: the block
634-
// answers with the rename, the report answers with a bare unrecognised-key
635-
// error. Recorded as a finding in this round's PR, ⛔ not fixed here — it is
636-
// an edit to `packages/spec/src/ui/report.zod.ts`, outside this round's file
637-
// surface, and it does not discharge any row above.
634+
it('⭐ the alias table answers at BOTH levels — the top-level report renames `measures` onto `values` like the block', () => {
635+
// The same key, the same target key, one level apart, and now the same
636+
// answer: the block and the top-level report both refuse `measures` AND name
637+
// `values` in the rejection. This pin used to record the opposite — the
638+
// report answered with a bare unrecognised-key error — as a reading this
639+
// round did not repair. #19822 closed that gap in
640+
// `packages/spec/src/ui/report.zod.ts`, so the reading is reversed in place
641+
// rather than deleted. It still discharges no row above.
638642
const onReport = ReportSchema.safeParse({ ...MINIMAL_REPORT, measures: ['revenue'] });
639643
expect(onReport.success).toBe(false);
640-
expect(issueMessages(onReport)).not.toContain('Did you mean');
644+
expect(issueMessages(onReport)).toContain('`measures` → `values`');
641645
const onBlock = ReportSchema.safeParse({
642646
name: 'joined_report',
643647
label: 'Joined',

‎packages/spec/src/ui/report.test.ts‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
JoinedReportBlockSchema,
1111
reportSelectionOrder,
1212
} from './report.zod';
13+
import { strictObjectDeclarations } from '../shared/strict-object';
1314

1415
/**
1516
* ADR-0021 single-form: a report binds a `dataset` and selects `rows`
@@ -248,6 +249,92 @@ describe('ReportSchema — scope-filter aliases point at `runtimeFilter` (#5013)
248249
});
249250
});
250251

252+
/**
253+
* The selection and ordering spellings a block already corrects, pinned by
254+
* PARSE on the top-level report — the surface where they were missing.
255+
*
256+
* `ReportSchema`'s table says it is kept parallel to the block's, and ten of
257+
* the block's entries were absent from it: `measures:` on a plain report was
258+
* refused with no suggestion at all, while the same key one level down was
259+
* told `values`. Measured before the fix, nine of the ten got no suggestion;
260+
* `orderBy` alone reached `order` by edit distance.
261+
*
262+
* Two halves, as in the scope-filter block above: what the author SEES (the
263+
* rejection names the prescribed key), and that taking the advice parses.
264+
*/
265+
describe('ReportSchema — routes the block vocabulary the way a block does', () => {
266+
const VALID = {
267+
name: 'pipeline', label: 'Pipeline', type: 'summary',
268+
dataset: 'sales', rows: ['stage'], values: ['revenue'],
269+
} as const;
270+
271+
/** A value the TARGET key accepts on `VALID`, so the advice can be taken verbatim. */
272+
const VALUE_FOR: Record<string, unknown> = {
273+
values: ['revenue'],
274+
rows: ['stage'],
275+
order: [{ by: 'revenue', direction: 'desc' }],
276+
dataset: 'sales',
277+
};
278+
279+
const ROUTES: ReadonlyArray<readonly [string, string]> = [
280+
['measures', 'values'],
281+
['metrics', 'values'],
282+
['dimensions', 'rows'],
283+
['groupBy', 'rows'],
284+
['groupings', 'rows'],
285+
['sort', 'order'],
286+
['orderBy', 'order'],
287+
['sortBy', 'order'],
288+
['objectName', 'dataset'],
289+
['object', 'dataset'],
290+
];
291+
292+
it.each(ROUTES)('`%s` on a top-level report is renamed onto `%s`', (key, target) => {
293+
const r = ReportSchema.safeParse({ ...VALID, [key]: VALUE_FOR[target] });
294+
expect(r.success, `\`${key}\` must still be rejected — it is not a declared key`).toBe(false);
295+
const issue = r.error!.issues.find((i) => i.code === 'unrecognized_keys');
296+
expect(issue, `\`${key}\` must be refused as an unknown key`).toBeDefined();
297+
expect((issue as { keys?: readonly string[] }).keys).toEqual([key]);
298+
expect(issue!.message).toContain(`Did you mean \`${key}\` → \`${target}\`?`);
299+
});
300+
301+
it.each(ROUTES)('taking the advice for `%s` parses — `%s` is a key this report accepts', (_key, target) => {
302+
const r = ReportSchema.safeParse({ ...VALID, [target]: VALUE_FOR[target] });
303+
expect(r.success, JSON.stringify(r.error?.issues ?? [])).toBe(true);
304+
});
305+
306+
/**
307+
* The parity claim itself, derived from the two tables at runtime rather
308+
* than from a list: every key the block routes to a target this schema also
309+
* declares is routed here, to the same target — or, where a key must not
310+
* route at the top level, answered by a `guidance` entry that says why.
311+
* Never silence.
312+
*/
313+
it('every block alias whose target the report declares is routed here too, to the same target', () => {
314+
// Force both lazy schemas so their declarations are registered.
315+
ReportSchema.safeParse(VALID);
316+
JoinedReportBlockSchema.safeParse({ ...VALID, type: 'tabular' });
317+
const declarationOf = (surface: string) => {
318+
const found = strictObjectDeclarations().filter((d) => d.options.surface === surface);
319+
expect(found, `exactly one declaration answers to "${surface}"`).toHaveLength(1);
320+
return found[0]!;
321+
};
322+
const block = declarationOf('this joined report block');
323+
const top = declarationOf('this report');
324+
325+
const judged = Object.entries(block.options.aliases ?? {}).filter(([, target]) => target in top.shape);
326+
// Not vacuous: the derivation reaches every key this pin was written for.
327+
expect(judged.map(([key]) => key)).toEqual(expect.arrayContaining(ROUTES.map(([key]) => key)));
328+
329+
const topAliases = top.options.aliases ?? {};
330+
const topGuidance = top.options.guidance ?? {};
331+
const silentOrDivergent = judged
332+
.filter(([key, target]) => topAliases[key] !== target && !(key in topGuidance))
333+
.map(([key, target]) => `${key} → ${target} (top level: ${topAliases[key] ?? 'absent'})`);
334+
expect(silentOrDivergent).toEqual([]);
335+
});
336+
});
337+
251338
describe('ReportChartSchema', () => {
252339
it('requires xAxis + yAxis', () => {
253340
expect(ReportChartSchema.parse({ type: 'bar', xAxis: 'stage', yAxis: 'revenue' }).type).toBe('bar');

‎packages/spec/src/ui/report.zod.ts‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,8 +251,13 @@ export const ReportSchema = lazySchema(() => strictObject({
251251
'Until this shape was closed these were dropped silently — the item still registered, minus whatever the key was meant to configure.',
252252
// Kept deliberately parallel to `JoinedReportBlockSchema` above: a block is a
253253
// sub-report, so an author who learns one vocabulary must not be corrected
254-
// differently on the other. The scope-filter entries are that table's,
255-
// verbatim.
254+
// differently on the other. Every block entry whose target this schema also
255+
// declares is carried here, to the same target — today, every one of them —
256+
// and `report.test.ts` derives that parity from the two tables at runtime. A
257+
// key that must NOT route here gets a `guidance` line saying why, never
258+
// silence. The `guidance` tables do differ, on purpose: the block's three are
259+
// wrong-layer pointers to keys this schema declares, and `drillDown` below
260+
// disambiguates this schema's own `drilldown` boolean, which a block lacks.
256261
//
257262
// #5013 — `filter` used to point at `filters`, a key `ReportSchema` does not
258263
// declare either, so taking the advice earned a SECOND rejection and that one
@@ -262,15 +267,41 @@ export const ReportSchema = lazySchema(() => strictObject({
262267
// only from the `unrecognized_keys` path — and `chartConfig` was not a key
263268
// either. `alias-integrity.test.ts` now proves both halves for every table in
264269
// the package.
270+
//
271+
// The parallel claim above was not true when it was first written: the ten
272+
// selection and ordering spellings below existed only on the block, so
273+
// `measures:` on a plain report was refused with no suggestion while the
274+
// same key one level down was told `values`. Of the ten, only `orderBy`
275+
// reached its target by edit distance alone.
265276
aliases: {
266-
dataSet: 'dataset', source: 'dataset',
277+
// ADR-0021 single-form: the legacy inline query was removed in the cutover.
278+
// These are the spellings that cutover retired, aimed at their successors.
279+
objectName: 'dataset',
280+
object: 'dataset',
281+
dataSet: 'dataset',
282+
source: 'dataset',
283+
// A report selects measures by name; `columns` is a real key here (the
284+
// matrix across-axis), so the value list cannot borrow it — hence the
285+
// explicit map.
267286
fields: 'values',
287+
measures: 'values',
288+
metrics: 'values',
289+
groupings: 'rows',
290+
groupBy: 'rows',
291+
dimensions: 'rows',
268292
// Scope filter. `runtimeFilter` is camelCase, so the edit-distance fallback
269293
// under-reaches every one of these (#4990) — same as on a block.
270294
filter: 'runtimeFilter',
271295
filters: 'runtimeFilter',
272296
where: 'runtimeFilter',
273297
criteria: 'runtimeFilter',
298+
// Ordering, spelled as the objectql/dashboard surfaces spell it. On a
299+
// `joined` report the prescribed `order` is itself refused, with the
300+
// pointer onto `blocks[]` — the same answer the author would get for
301+
// writing `order` directly.
302+
sort: 'order',
303+
orderBy: 'order',
304+
sortBy: 'order',
274305
},
275306
guidance: {
276307
// #5022 — the reverse half of a two-way disambiguation. The forward half

0 commit comments

Comments
 (0)