Skip to content

Commit 5f08c05

Browse files
yinlianghuiclaude
andauthored
fix(data-objectstack): type queryDataset(selection) as the spec DatasetSelection (#3753)
The adapter restated the selection contract inline, field by field, and the copy had drifted three ways from the pinned @objectstack/spec@17.0.0-rc.5: - compareTo.dimension was required. It has been optional since objectstack#5011 precisely because the EXECUTOR resolves it (exactly one time dimension carrying a dateRange is shifted; zero or several raises a loud error naming the candidates). Requiring it made the compiler demand from every typed caller the consumer-side dimension guess that change forbids, trading a loud error for a silently wrong comparison window. - timeDimensions was widened to unknown[], erasing the entry shape the executor's resolution reads, and runtimeFilter to Record<string, unknown>, erasing the $and/$or/$not vocabulary the server parses. - dateGranularity was missing entirely: the copy had stopped at whatever the contract looked like the day it was written. The parameter is now the spec type by reference, so there is nothing left to re-sync. Removing the dialect rather than correcting it is the point -- restating a contract owned elsewhere opens a second de-facto dialect of it (AGENTS.md #0/#0.1), and this issue is the instance. queryDataset.test.ts pins structural identity with DatasetSelection plus each of the three drifts individually, checked by this package's tsc --noEmit (its tsconfig includes the whole src tree). A runtime test pins that a dimension-less compareTo reaches the server untouched, so the adapter can never start guessing on the executor's behalf. The response type is deliberately untouched -- it is the REST envelope (object / dimensionFields / drillRawRows), not a restatement of AnalyticsResult. Its own fields[] drift is filed separately as #3752. Fixes #3613 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0c28a07 commit 5f08c05

3 files changed

Lines changed: 175 additions & 16 deletions

File tree

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
'@object-ui/data-objectstack': patch
3+
---
4+
5+
data-objectstack: type `queryDataset(selection)` as the spec's `DatasetSelection` instead of a hand-written copy
6+
7+
The adapter restated the selection contract inline, field by field, and the copy
8+
had drifted three ways from the pinned `@objectstack/spec@17.0.0-rc.5`:
9+
10+
- **`compareTo.dimension` was required.** It has been optional since
11+
objectstack#5011, *because the executor resolves it*: exactly one time
12+
dimension carrying a `dateRange` is the one shifted, and zero or several
13+
raises a loud error naming the candidates. Requiring it made the compiler
14+
demand from every typed caller precisely the consumer-side dimension guess
15+
that change forbids — trading a loud executor error for a silently wrong
16+
comparison window. No runtime path hit this yet (the dashboard's
17+
`DatasetWidget` passes `selection` as `unknown`), but a declaration is a live
18+
instruction to anyone calling this client from TypeScript.
19+
- **`timeDimensions` was widened to `unknown[]`**, erasing the very entry shape
20+
the executor's resolution reads (`{ dimension, granularity?, dateRange? }`),
21+
and **`runtimeFilter` to `Record<string, unknown>`**, erasing the
22+
`$and`/`$or`/`$not` vocabulary the server parses.
23+
- **`dateGranularity` was missing entirely** — the copy had simply stopped at
24+
whatever the contract looked like the day it was written, so a typed caller
25+
could not bucket a trend by month at all.
26+
27+
The parameter is now the spec type by reference, so there is nothing left to
28+
re-sync. The fix is the removal of the dialect rather than a correction to it:
29+
restating a contract owned elsewhere creates a second de-facto dialect of it, and
30+
drift is then only a matter of time (AGENTS.md #0/#0.1). `queryDataset.test.ts`
31+
pins structural identity with `DatasetSelection` plus each of the three drifts
32+
individually, checked by this package's `tsc --noEmit`; a runtime test pins that
33+
a dimension-less `compareTo` reaches the server untouched, so the adapter can
34+
never start guessing on the executor's behalf.
35+
36+
The response type is deliberately left alone — it is the REST envelope
37+
(`object` / `dimensionFields` / `drillRawRows`), not a restatement of
38+
`AnalyticsResult`.

‎packages/data-objectstack/src/index.ts‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import { ObjectStackClient, type QueryOptions as ObjectStackQueryOptions } from '@objectstack/client';
1010
import type { DroppedFieldsEvent } from '@objectstack/spec/data';
11+
import type { DatasetSelection } from '@objectstack/spec/contracts';
1112
import type {
1213
DataSource,
1314
BatchTransactionOperation,
@@ -3190,25 +3191,21 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
31903191
* silently returning wrong numbers.
31913192
*
31923193
* @param dataset - An inline dataset definition (draft) OR a saved dataset name.
3193-
* @param selection - Dimension/measure names to project + runtime directives.
3194+
* @param selection - The spec's {@link DatasetSelection} — dimension/measure
3195+
* names to project plus runtime directives. This parameter IS the spec type
3196+
* by reference, never a local restatement of it (objectui#3613): a hand
3197+
* copy of a contract is a second dialect of it, and the copy this replaced
3198+
* had already drifted three ways from `@objectstack/spec` — it required
3199+
* `compareTo.dimension` (optional since objectstack#5011, and resolved by
3200+
* the EXECUTOR, so requiring it pushed callers into exactly the
3201+
* consumer-side dimension guess AGENTS.md #0.1 forbids), it widened
3202+
* `timeDimensions` to `unknown[]` and `runtimeFilter` to
3203+
* `Record<string, unknown>`, and it had never grown `dateGranularity` at
3204+
* all. Pinned in `queryDataset.test.ts`.
31943205
*/
31953206
async queryDataset(
31963207
dataset: Record<string, unknown> | string,
3197-
selection: {
3198-
dimensions?: string[];
3199-
measures: string[];
3200-
runtimeFilter?: Record<string, unknown>;
3201-
timeDimensions?: unknown[];
3202-
compareTo?: { kind: 'previousPeriod' | 'previousYear'; dimension: string };
3203-
order?: Record<string, 'asc' | 'desc'>;
3204-
limit?: number;
3205-
offset?: number;
3206-
timezone?: string;
3207-
/** Marginal-aggregate groupings (e.g. `[rows, [colDim], []]`) — server
3208-
* computes each subtotal with the measure's TRUE aggregate (never client
3209-
* re-derived). `[]` is the grand total. */
3210-
totals?: { groupings: string[][] };
3211-
},
3208+
selection: DatasetSelection,
32123209
): Promise<{
32133210
rows: Array<Record<string, unknown>>;
32143211
/** Column metadata: a display `label` (dimensions and measures), and a

‎packages/data-objectstack/src/queryDataset.test.ts‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@
77
*/
88

99
import { describe, it, expect, beforeEach, vi } from 'vitest';
10+
import type {
11+
AnalyticsQuery as SpecAnalyticsQuery,
12+
DatasetCompareTo as SpecDatasetCompareTo,
13+
DatasetSelection as SpecDatasetSelection,
14+
} from '@objectstack/spec/contracts';
1015
import {
1116
ObjectStackAdapter,
1217
clearSharedDiscoveryCache,
@@ -130,3 +135,122 @@ describe('ObjectStackAdapter.queryDataset', () => {
130135
});
131136
});
132137
});
138+
139+
/* -------------------------------------------------------------------------- */
140+
/* objectui#3613 — the `selection` parameter IS the spec type, not a copy. */
141+
/* */
142+
/* Compile-time pins. A violation is a `tsc --noEmit` error, not a runtime */
143+
/* failure: this package's tsconfig.json includes its whole `src/**` (tests */
144+
/* included), so these are checked by `pnpm --filter @object-ui/data-objectstack*/
145+
/* type-check` with no separate tsconfig.typetests.json — the same property */
146+
/* spec-symbol-batch6.test.ts documents and relies on. */
147+
/* -------------------------------------------------------------------------- */
148+
149+
type Assert<T extends true> = T;
150+
type IsAny<T> = 0 extends 1 & T ? true : false;
151+
type Equal<A, B> = (<T>() => T extends A ? 1 : 2) extends <T>() => T extends B ? 1 : 2
152+
? true
153+
: false;
154+
type HasKey<T, K extends PropertyKey> = K extends keyof T ? true : false;
155+
156+
/** The declared type of `queryDataset`'s second parameter, read off the method. */
157+
type SelectionParam = Parameters<ObjectStackAdapter['queryDataset']>[1];
158+
159+
describe('queryDataset(selection) is @objectstack/spec DatasetSelection itself', () => {
160+
it('is pinned at compile time', () => {
161+
type _SpecNotAny = Assert<Equal<IsAny<SpecDatasetSelection>, false>>;
162+
163+
// THE pin. `Equal` rather than a two-way `extends` on purpose: a
164+
// restatement that merely *overlaps* the contract satisfies mutual
165+
// assignability in the drifted direction too (a required `dimension` is
166+
// still assignable TO an optional one). Structural identity is the only
167+
// assertion a hand copy cannot pass while drifting.
168+
type _IsTheSpecType = Assert<Equal<SelectionParam, SpecDatasetSelection>>;
169+
170+
expect(true).toBe(true);
171+
});
172+
173+
// The three drifts the deleted copy had accumulated, named individually so a
174+
// future reader learns what a restatement costs — not just that one existed.
175+
it('pins compareTo as the spec DatasetCompareTo, with dimension OPTIONAL', () => {
176+
type CompareTo = NonNullable<SelectionParam['compareTo']>;
177+
type _IsSpecCompareTo = Assert<Equal<CompareTo, SpecDatasetCompareTo>>;
178+
179+
// objectstack#5011 made `dimension` optional BECAUSE the executor resolves
180+
// it (exactly one dated time dimension → that one; zero or several → a loud
181+
// error listing the candidates). The copy declared it required, so the
182+
// compiler demanded from every typed caller precisely the consumer-side
183+
// dimension guess that change forbids — turning a loud executor error into
184+
// a silently wrong comparison window.
185+
type _DimensionOptional = Assert<Equal<CompareTo['dimension'], string | undefined>>;
186+
type _DimensionWasNotRequired = Assert<Equal<Equal<CompareTo['dimension'], string>, false>>;
187+
// …and the shape with no `dimension` at all is a legal authoring surface.
188+
type _KindOnlyIsValid = Assert<
189+
{ kind: 'previousPeriod' } extends CompareTo ? true : false
190+
>;
191+
192+
expect(true).toBe(true);
193+
});
194+
195+
it('pins timeDimensions at the spec shape, not `unknown[]`', () => {
196+
type _IsSpecTimeDimensions = Assert<
197+
Equal<SelectionParam['timeDimensions'], SpecAnalyticsQuery['timeDimensions']>
198+
>;
199+
type _NotUnknownArray = Assert<
200+
Equal<Equal<SelectionParam['timeDimensions'], unknown[] | undefined>, false>
201+
>;
202+
// The entry shape the executor's compareTo resolution reads (`dateRange`
203+
// present ⇒ shiftable) — invisible through `unknown[]`.
204+
type Entry = NonNullable<SelectionParam['timeDimensions']>[number];
205+
type _EntryHasDimension = Assert<Equal<Entry['dimension'], string>>;
206+
type _EntryHasDateRange = Assert<HasKey<Entry, 'dateRange'>>;
207+
208+
expect(true).toBe(true);
209+
});
210+
211+
it('pins runtimeFilter as a FilterCondition and carries dateGranularity', () => {
212+
// The copy widened `runtimeFilter` to `Record<string, unknown>`, which
213+
// erases the `$and`/`$or`/`$not` vocabulary the server parses.
214+
type _RuntimeFilterNarrowed = Assert<
215+
Equal<Equal<SelectionParam['runtimeFilter'], Record<string, unknown> | undefined>, false>
216+
>;
217+
type _RuntimeFilterKnowsLogicalOps = Assert<
218+
HasKey<NonNullable<SelectionParam['runtimeFilter']>, '$and'>
219+
>;
220+
221+
// `dateGranularity` (framework#3588) is the drift nobody had noticed: the
222+
// copy never grew the key, so a typed caller could not bucket a trend by
223+
// month at all. A restatement does not only drift on the fields it has — it
224+
// also stops at whatever the contract looked like the day it was written.
225+
type _HasDateGranularity = Assert<HasKey<SelectionParam, 'dateGranularity'>>;
226+
type _DateGranularityEnum = Assert<
227+
Equal<
228+
SelectionParam['dateGranularity'],
229+
'day' | 'week' | 'month' | 'quarter' | 'year' | undefined
230+
>
231+
>;
232+
233+
expect(true).toBe(true);
234+
});
235+
236+
// The runtime half of the same fact: the adapter must forward `compareTo`
237+
// VERBATIM. If it ever "helpfully" filled a `dimension` in to satisfy its own
238+
// old declaration, the executor would stop resolving and every caller's
239+
// comparison window would depend on the adapter's guess.
240+
it('forwards a dimension-less compareTo to the server untouched', async () => {
241+
const { fetchImpl, calls } = makeFetch({ ok: true, body: { rows: [], fields: [] } });
242+
const adapter = new ObjectStackAdapter({ baseUrl: 'http://localhost:3000', autoReconnect: false, fetch: fetchImpl as any });
243+
244+
await adapter.queryDataset('sales', {
245+
dimensions: ['region'],
246+
measures: ['revenue'],
247+
timeDimensions: [{ dimension: 'closedAt', granularity: 'month', dateRange: ['2026-01-01', '2026-03-31'] }],
248+
compareTo: { kind: 'previousPeriod' },
249+
});
250+
251+
const post = calls.find((c) => c.url.includes('/analytics/dataset/query'))!;
252+
const sent = JSON.parse(post.init.body);
253+
expect(sent.selection.compareTo).toEqual({ kind: 'previousPeriod' });
254+
expect(sent.selection.compareTo).not.toHaveProperty('dimension');
255+
});
256+
});

0 commit comments

Comments
 (0)