Skip to content

Commit 72e3046

Browse files
committed
fix(lint)!: refuse a dataset dimension over a JSON-stored field, and count_distinct over a field declared multiple: true
The dataset-member rule gains the dimension leg the measure already had: a dataset dimension whose field is structured JSON (STRUCTURED_JSON_TYPES) or multi-value (isMultiValueField, the declaration with its multiple flag) is refused as dimension-json-stored-field-refused, the two predicates the analytics door refuses a grouped member by. The measure leg reads the declaration as well as the type: count_distinct over a multi-capable type flagged multiple: true is refused, as the compile leg and the engine's count_distinct door refuse it. Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6073bb9 commit 72e3046

2 files changed

Lines changed: 431 additions & 26 deletions

File tree

‎packages/lint/src/validate-dataset-measure-aggregates.test.ts‎

Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,16 @@ import {
1717
AGGREGATE_FIELD_TYPE_COMPATIBILITY,
1818
BOOLEAN_VALUE_TYPES,
1919
FieldType,
20+
MULTI_CAPABLE_TYPES,
2021
NUMERIC_VALUE_TYPES,
22+
STRUCTURED_JSON_TYPES,
2123
isAggregateCompatibleWithFieldType,
24+
isMultiValueField,
2225
} from '@objectstack/spec/data';
2326

2427
import { runAuthoringRules } from './authoring-rules.js';
2528
import {
29+
DIMENSION_JSON_STORED_FIELD_REFUSED,
2630
MEASURE_AGGREGATE_FIELD_TYPE_REFUSED,
2731
validateDatasetMeasureAggregates,
2832
} from './validate-dataset-measure-aggregates.js';
@@ -388,3 +392,230 @@ describe('measure-aggregate-field-type-refused — reaches the author through th
388392
).toEqual([]);
389393
});
390394
});
395+
396+
// ───────────────────────────────────────────────────────────────────────────
397+
// [#20890] The declaration half: `count_distinct` over a field declared
398+
// `multiple: true`. The table is per TYPE and accepts `select`; the declaration
399+
// makes it a list stored as JSON, which the compile leg (`400 DATASET_INVALID`)
400+
// and the engine's `count_distinct` door already refuse.
401+
// ───────────────────────────────────────────────────────────────────────────
402+
403+
/** {@link stackWith}, with the measured field declared `multiple: true`. */
404+
const flaggedStackWith = (aggregate: string, fieldType: string): Record<string, unknown> =>
405+
stackWith(aggregate, fieldType, {
406+
fields: { name: { type: 'text' }, measured: { type: fieldType, multiple: true } },
407+
});
408+
409+
describe('measure-aggregate-field-type-refused — reads the declaration, not the type alone', () => {
410+
// ⭐ The addendum's pin and its control, one pair.
411+
it('refuses count_distinct over a select declared multiple: true, and accepts it over a single select', () => {
412+
const found = findings(flaggedStackWith('count_distinct', 'select'));
413+
expect(found).toHaveLength(1);
414+
const issue = found[0];
415+
expect(issue.severity).toBe('error');
416+
expect(issue.rule).toBe(RULE);
417+
expect(issue.path).toBe('datasets[0].measures[0].aggregate');
418+
// The DECLARATION is named, flag included, or the author reads the verdict
419+
// as a claim about `select` that the table plainly does not make.
420+
expect(issue.message).toContain('`select` with `multiple: true`');
421+
expect(issue.message).toContain('none of them with `multiple: true`');
422+
// The way out is computed from the same predicate: only `count` is left.
423+
expect(issue.hint).toContain('accepts: count.');
424+
425+
expect(findings(stackWith('count_distinct', 'select'))).toEqual([]);
426+
});
427+
428+
it('refuses count_distinct over every multi-capable type flagged multiple: true, and nothing else moves', () => {
429+
expect(MULTI_CAPABLE_TYPES.size).toBeGreaterThan(0);
430+
for (const fieldType of MULTI_CAPABLE_TYPES) {
431+
expect(findings(flaggedStackWith('count_distinct', fieldType)), `count_distinct(${fieldType}, multiple)`).toHaveLength(1);
432+
expect(findings(stackWith('count_distinct', fieldType)), `count_distinct(${fieldType})`).toEqual([]);
433+
// `count` reads no value, flagged or not.
434+
expect(findings(flaggedStackWith('count', fieldType)), `count(${fieldType}, multiple)`).toEqual([]);
435+
}
436+
});
437+
438+
// The whole flagged surface against the spec's two predicates — the table's
439+
// row and `isMultiValueField` — never against a list retyped here.
440+
it('agrees with the table and isMultiValueField on every aggregate × every declared FieldType flagged multiple: true', () => {
441+
let refused = 0;
442+
let accepted = 0;
443+
for (const aggregate of AGGREGATES) {
444+
for (const fieldType of FieldType.options) {
445+
const fires = findings(flaggedStackWith(aggregate, fieldType)).length > 0;
446+
const expected =
447+
!isAggregateCompatibleWithFieldType(aggregate, fieldType) ||
448+
(aggregate === 'count_distinct' && isMultiValueField({ type: fieldType, multiple: true }));
449+
expect(fires, `${aggregate}(${fieldType}, multiple)`).toBe(expected);
450+
if (fires) refused++;
451+
else accepted++;
452+
}
453+
}
454+
expect(refused).toBeGreaterThan(50);
455+
expect(accepted).toBeGreaterThan(50);
456+
});
457+
});
458+
459+
// ───────────────────────────────────────────────────────────────────────────
460+
// [#20890] `dimension-json-stored-field-refused` — the dimension leg. The
461+
// analytics door refuses a query that groups by a JSON-stored column
462+
// (`400 INVALID_FIELD`); this is that verdict where the author is standing.
463+
// ───────────────────────────────────────────────────────────────────────────
464+
465+
const DIMENSION_RULE = DIMENSION_JSON_STORED_FIELD_REFUSED;
466+
467+
/**
468+
* One dataset over one object whose one dimension groups by `grouped`,
469+
* declared as `fieldDef`. `field` overrides the path the dimension names.
470+
*/
471+
const dimensionStack = (
472+
fieldDef: Record<string, unknown> | undefined,
473+
overrides: { field?: unknown; datasetObject?: string } = {},
474+
): Record<string, unknown> => ({
475+
name: 'analytics_probe',
476+
objects: [
477+
{
478+
name: 'crm_opportunity',
479+
sharingModel: 'private',
480+
fields: {
481+
name: { type: 'text' },
482+
grouped: fieldDef ?? { label: 'Untyped' },
483+
},
484+
},
485+
],
486+
datasets: [
487+
{
488+
name: 'opportunity_metrics',
489+
object: overrides.datasetObject ?? 'crm_opportunity',
490+
dimensions: [
491+
{ name: 'the_dimension', field: 'field' in overrides ? overrides.field : 'grouped' },
492+
],
493+
measures: [{ name: 'n', aggregate: 'count' }],
494+
},
495+
],
496+
});
497+
498+
const dimensionFindings = (stack: unknown) =>
499+
validateDatasetMeasureAggregates(stack).filter((f) => f.rule === DIMENSION_RULE);
500+
501+
describe('dimension-json-stored-field-refused — a dimension over a JSON-stored field is refused', () => {
502+
// ⭐ The card's pin and its control, one pair.
503+
it('refuses a dimension over a json field, and accepts one over a text field', () => {
504+
const found = dimensionFindings(dimensionStack({ type: 'json' }));
505+
expect(found).toHaveLength(1);
506+
const issue = found[0];
507+
expect(issue.severity).toBe('error');
508+
expect(issue.rule).toBe(DIMENSION_RULE);
509+
expect(issue.path).toBe('datasets[0].dimensions[0].field');
510+
expect(issue.where).toBe('dataset "opportunity_metrics" › dimension "the_dimension"');
511+
// The author can act without opening the analytics service: the DIMENSION,
512+
// the FIELD, its declared TYPE, the class, and what the door answers.
513+
expect(issue.message).toContain('"the_dimension"');
514+
expect(issue.message).toContain('"grouped"');
515+
expect(issue.message).toContain('`json`');
516+
expect(issue.message).toContain('structured-JSON');
517+
expect(issue.message).toContain('400 INVALID_FIELD');
518+
expect(issue.hint).toContain('scalar value');
519+
520+
expect(dimensionFindings(dimensionStack({ type: 'text' }))).toEqual([]);
521+
// And a dimension never lands under the measure rule's id.
522+
expect(findings(dimensionStack({ type: 'json' }))).toEqual([]);
523+
});
524+
525+
it('refuses a dimension over every member of STRUCTURED_JSON_TYPES', () => {
526+
expect(STRUCTURED_JSON_TYPES.size).toBeGreaterThan(0);
527+
for (const fieldType of STRUCTURED_JSON_TYPES) {
528+
expect(dimensionFindings(dimensionStack({ type: fieldType })), fieldType).toHaveLength(1);
529+
}
530+
});
531+
532+
// The analytics door's SECOND predicate, which reads the declaration: the
533+
// same JSON column whether the type is inherently multi or flagged.
534+
it('refuses a dimension over a multi-value field — inherently multi, or flagged multiple: true — and serves the same type unflagged', () => {
535+
for (const fieldType of ['multiselect', 'checkboxes', 'tags']) {
536+
expect(dimensionFindings(dimensionStack({ type: fieldType })), fieldType).toHaveLength(1);
537+
}
538+
const [flagged] = dimensionFindings(dimensionStack({ type: 'select', multiple: true }));
539+
expect(flagged?.message).toContain('`select` with `multiple: true`');
540+
expect(flagged?.message).toContain('multi-value');
541+
// The route the door names: filter by one member, never group.
542+
expect(flagged?.hint).toContain('$contains');
543+
expect(dimensionFindings(dimensionStack({ type: 'select' }))).toEqual([]);
544+
});
545+
546+
// The whole surface against the door's two predicates, read from the spec.
547+
it('agrees with STRUCTURED_JSON_TYPES and isMultiValueField on every declared FieldType, flagged and not', () => {
548+
let refused = 0;
549+
let accepted = 0;
550+
for (const fieldType of FieldType.options) {
551+
for (const multiple of [false, true]) {
552+
const def = multiple ? { type: fieldType, multiple: true } : { type: fieldType };
553+
const fires = dimensionFindings(dimensionStack(def)).length > 0;
554+
const expected = STRUCTURED_JSON_TYPES.has(fieldType) || isMultiValueField({ type: fieldType, multiple });
555+
expect(fires, `${fieldType}${multiple ? ', multiple' : ''}`).toBe(expected);
556+
if (fires) refused++;
557+
else accepted++;
558+
}
559+
}
560+
expect(refused).toBeGreaterThan(15);
561+
expect(accepted).toBeGreaterThan(50);
562+
});
563+
564+
it('judges a dimension over a joined field on the object the leaf lives on', () => {
565+
const joined = (leafType: string) => ({
566+
name: 'analytics_probe',
567+
objects: [
568+
{
569+
name: 'crm_opportunity',
570+
sharingModel: 'private',
571+
fields: { name: { type: 'text' }, account: { type: 'lookup', reference: 'crm_account' } },
572+
},
573+
{
574+
name: 'crm_account',
575+
sharingModel: 'private',
576+
fields: { name: { type: 'text' }, hq: { type: leafType } },
577+
},
578+
],
579+
datasets: [
580+
{
581+
name: 'opportunity_metrics',
582+
object: 'crm_opportunity',
583+
include: ['account'],
584+
dimensions: [{ name: 'acct_hq', field: 'account.hq' }],
585+
measures: [{ name: 'n', aggregate: 'count' }],
586+
},
587+
],
588+
});
589+
const found = dimensionFindings(joined('json'));
590+
expect(found).toHaveLength(1);
591+
expect(found[0].message).toContain('account.hq');
592+
expect(found[0].message).toContain('crm_account');
593+
expect(dimensionFindings(joined('text'))).toEqual([]);
594+
});
595+
596+
it('never hands the predicates a guess — the same skips as the measure leg', () => {
597+
// A base object this stack does not define: `validate-object-references.ts`'s.
598+
expect(dimensionFindings(dimensionStack({ type: 'json' }, { datasetObject: 'not_here' }))).toEqual([]);
599+
// A path that resolves to nothing: `dataset-field-unknown`'s.
600+
expect(dimensionFindings(dimensionStack({ type: 'json' }, { field: 'nope' }))).toEqual([]);
601+
// An untyped field, and a dimension that writes no field / a non-string one.
602+
expect(dimensionFindings(dimensionStack(undefined))).toEqual([]);
603+
expect(dimensionFindings(dimensionStack({ type: 'json' }, { field: undefined }))).toEqual([]);
604+
expect(dimensionFindings(dimensionStack({ type: 'json' }, { field: ['grouped'] }))).toEqual([]);
605+
});
606+
607+
it('fires through runAuthoringRules on all three commands, and only on the refused dimension', () => {
608+
const stack = dimensionStack({ type: 'json' });
609+
for (const command of ['validate', 'build', 'lint'] as const) {
610+
const found = runAuthoringRules(command, { normalized: stack, parsed: stack }).filter(
611+
(f) => f.rule === DIMENSION_RULE,
612+
);
613+
expect(found, command).toHaveLength(1);
614+
expect(found[0].severity, command).toBe('error');
615+
}
616+
const control = dimensionStack({ type: 'text' });
617+
expect(
618+
runAuthoringRules('lint', { normalized: control, parsed: control }).filter((f) => f.rule === DIMENSION_RULE),
619+
).toEqual([]);
620+
});
621+
});

0 commit comments

Comments
 (0)