From 328671b22ba56e072e7f00edf4c4e3dd92f298d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 15:38:33 +0000 Subject: [PATCH 1/3] wip(service-analytics): resolve every reference-class dimension label through spec's arbiter Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW --- ...dataset-reference-dimension-labels.test.ts | 317 ++++++++++++++++++ .../src/__tests__/dimension-labels.test.ts | 74 +++- .../service-analytics/src/dimension-labels.ts | 72 +++- 3 files changed, 443 insertions(+), 20 deletions(-) create mode 100644 packages/services/service-analytics/src/__tests__/dataset-reference-dimension-labels.test.ts diff --git a/packages/services/service-analytics/src/__tests__/dataset-reference-dimension-labels.test.ts b/packages/services/service-analytics/src/__tests__/dataset-reference-dimension-labels.test.ts new file mode 100644 index 0000000000..2ea3baaf5c --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/dataset-reference-dimension-labels.test.ts @@ -0,0 +1,317 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #16390 — a `user` dimension renders the referenced record's display NAME, the + * same way a `lookup` dimension already does. + * + * ## The measured asymmetry + * + * ONE dataset over the reporter's object, ONE `POST /api/v1/analytics/dataset/query` + * — which is `AnalyticsService.queryDataset` one thin route away (`rest-server.ts` + * dispatches to `svc.queryDataset` and does nothing to the rows): + * + * ``` + * dimensions: ['unit'] -> {"unit":"Customer Success", "avg_score":91.5} a NAME + * dimensions: ['person'] -> {"person":"7zLYIwpX82If4Bvt…", "avg_score":124.25} an ID + * ``` + * + * Both fields carry exactly what the resolution needs, and differ in one word: + * + * ``` + * Field.user({ label: 'Person' }) -> { type: 'user', reference: 'sys_user' } + * Field.lookup('sys_business_unit', {…}) -> { type: 'lookup', reference: 'sys_business_unit' } + * ``` + * + * so the fixture below builds its field map from those BUILDERS rather than from + * hand-written literals — the premise is then re-measured by the pin on every run + * instead of being asserted once in a card. + * + * ## The class is the unit of the fix, not the one member that was reported + * + * `REFERENCE_VALUE_TYPES` (`packages/spec/src/data/field-value.zod.ts`) declares + * FOUR members — `lookup`, `master_detail`, `user`, `tree` — as one kind: "value + * points at another record … a record-id string in stored form". This service + * already treats them as one kind where it annotates measure result types + * (`measure-result-type.ts` imports that very set), while the label resolver + * hand-wrote a two-member subset of it. `user` and `tree` were BOTH outside that + * subset, so both rendered raw ids; fixing only the reported member would re-seed + * the same defect for the next reporter to find on the other one. + * + * The target object is read through spec's `referenceTargetOf`, the declared + * SINGLE arbiter of "what does this reference field point at" — which also + * supplies `sys_user` for a `user` field authored WITHOUT `reference`, a shape + * that reaches production (`packages/objectql/src/query-expression-conformance.test.ts` + * captures one) and that a `meta.reference &&` test would silently drop. + * + * ## Declaring the dimension `type: 'lookup'` is not, and never was, a way out + * + * `DatasetDimensionSchema.type` is `['string','number','date','boolean','lookup']` — + * it has no `user` member at all — and the resolver reads the OBJECT field's type, + * never the dimension's. Pinned below on both spellings, because it is the reason + * the fix belongs at the resolution site and not in an author-facing declaration. + * + * ## Reverse verification — direction predicted BEFORE running + * + * Ordinary direction, no inversion and no count movement: the change ADDS + * resolutions that were absent and narrows no rule, so restoring the two-member + * subset must turn RED exactly the cases whose dimension is `user` / `tree` / + * `master_detail`, and leave GREEN every `lookup`, `select`, unresolved-id, + * no-display-field and fail-closed case — those pin behaviour this change + * converges ON. The measured run is quoted in the PR body. + */ + +import { describe, it, expect } from 'vitest'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import { Field, referenceTargetOf } from '@objectstack/spec/data'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { AnalyticsService } from '../analytics-service.js'; +import { pickDisplayField, type FieldMetaLite } from '../dimension-labels.js'; + +// ── the reporter's object, built from the real builders ───────────────────── + +const KPI_RESULT_FIELDS: Record = { + // The card's two fields, verbatim in shape. + person: Field.user({ label: 'Person' }), + unit: Field.lookup('sys_business_unit', { label: 'Business Unit' }), + // The other two members of the same declared class. + owner_team: Field.masterDetail('kpi_team', { label: 'Team' }), + category: { type: 'tree', reference: 'kpi_category', label: 'Category' } as FieldMetaLite, + // A `user` field authored WITHOUT `reference` — the target is a constant of + // the type, so this metadata is fully specified, not under-specified. + reviewer: { type: 'user', label: 'Reviewer' } as FieldMetaLite, + score: Field.number({ label: 'Score' }), +}; + +/** `sys_user`'s real primary-title pointer is `nameField: 'name'`. */ +const SYS_USER_FIELDS: Record = { + name: { type: 'text' }, + email: { type: 'text' }, +}; +const UNIT_FIELDS: Record = { name: { type: 'text' } }; +const TEAM_FIELDS: Record = { name: { type: 'text' } }; +const CATEGORY_FIELDS: Record = { name: { type: 'text' } }; + +const FIELD_MAPS: Record> = { + kpi_result: KPI_RESULT_FIELDS, + sys_user: SYS_USER_FIELDS, + sys_business_unit: UNIT_FIELDS, + kpi_team: TEAM_FIELDS, + kpi_category: CATEGORY_FIELDS, +}; + +/** id -> display name, per referenced object. `usr_orphan` is deliberately absent. */ +const NAMES: Record> = { + sys_user: { usr_ada: 'Ada Lovelace', usr_bo: 'Bo Chen' }, + sys_business_unit: { bu_cs: 'Customer Success', bu_east: 'East China' }, + kpi_team: { team_a: 'Team Alpha' }, + kpi_category: { cat_q: 'Quality' }, +}; + +const dataset = DatasetSchema.parse({ + name: 'kpi_scores', + label: 'KPI Scores', + object: 'kpi_result', + dimensions: [ + // `lookup` is the only reference-ish spelling the dimension schema offers, + // which is what the reporter wrote for BOTH axes. + { name: 'unit', field: 'unit', type: 'lookup', label: 'Business Unit' }, + { name: 'person', field: 'person', type: 'lookup', label: 'Person' }, + { name: 'owner_team', field: 'owner_team', type: 'lookup' }, + { name: 'category', field: 'category', type: 'lookup' }, + // The same user axis declared `string` — the declaration must not decide. + { name: 'reviewer', field: 'reviewer', type: 'string' }, + ], + measures: [{ name: 'avg_score', aggregate: 'avg', field: 'score' }], +}); + +/** Rows the base aggregate returns, keyed by dimension name (raw stored ids). */ +const BASE_ROWS: Record[]> = { + 'unit,person': [ + { unit: 'bu_cs', person: 'usr_ada', avg_score: 91.5 }, + { unit: 'bu_east', person: 'usr_bo', avg_score: 106.12 }, + ], + person: [ + { person: 'usr_ada', avg_score: 124.25 }, + { person: 'usr_bo', avg_score: 128.6 }, + ], + owner_team: [{ owner_team: 'team_a', avg_score: 70 }], + category: [{ category: 'cat_q', avg_score: 80 }], + reviewer: [{ reviewer: 'usr_ada', avg_score: 60 }], +}; + +interface Wiring { + /** Referenced objects whose rows are visible; default: all of them. */ + names?: Record>; + /** Field maps override — e.g. a `sys_user` with no display field. */ + fieldMaps?: Record>; + getReadScope?: (objectName: string, context?: ExecutionContext) => unknown; + onFetch?: (targetObject: string, ids: unknown[], scope: unknown) => void; +} + +function service(w: Wiring = {}) { + const fieldMaps = w.fieldMaps ?? FIELD_MAPS; + const names = w.names ?? NAMES; + return new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + executeAggregate: async (_object: string, { groupBy }: { groupBy?: string[] }) => { + const key = (groupBy ?? []).join(','); + return BASE_ROWS[key] ?? []; + }, + ...(w.getReadScope ? { getReadScope: w.getReadScope as never } : {}), + labelResolver: { + getObjectFields: (objectName) => fieldMaps[objectName], + // Mirrors the plugin bridge: pick the target's display field, then read + // `id -> that field` for the ids in hand. Absent ids simply do not come + // back, exactly as an RLS-hidden or orphaned row does not. + fetchRecordLabels: async (targetObject, ids, scope) => { + w.onFetch?.(targetObject, ids, scope); + const map = new Map(); + if (!pickDisplayField(fieldMaps[targetObject])) return map; + const table = names[targetObject] ?? {}; + for (const id of ids) if (table[String(id)]) map.set(id, table[String(id)]); + return map; + }, + }, + }); +} + +const query = (svc: AnalyticsService, dimensions: string[]) => + svc.queryDataset(dataset, { dimensions, measures: ['avg_score'] }); + +// ── the premise, re-measured rather than recalled ─────────────────────────── + +describe('#16390 — the premise: a user field already carries its target', () => { + it('Field.user() and Field.lookup() differ in one word, and both name a target', () => { + expect(Field.user({ label: 'Person' })).toMatchObject({ type: 'user', reference: 'sys_user' }); + expect(Field.lookup('sys_business_unit', { label: 'Business Unit' })) + .toMatchObject({ type: 'lookup', reference: 'sys_business_unit' }); + // The single arbiter answers for every member of the class, and answers for + // a `user` field that omits `reference` too. + expect(referenceTargetOf(KPI_RESULT_FIELDS.person)).toBe('sys_user'); + expect(referenceTargetOf(KPI_RESULT_FIELDS.reviewer)).toBe('sys_user'); + expect(referenceTargetOf(KPI_RESULT_FIELDS.unit)).toBe('sys_business_unit'); + expect(referenceTargetOf(KPI_RESULT_FIELDS.owner_team)).toBe('kpi_team'); + expect(referenceTargetOf(KPI_RESULT_FIELDS.category)).toBe('kpi_category'); + // …and refuses the measure column, so nothing non-referential is swept in. + expect(referenceTargetOf(KPI_RESULT_FIELDS.score)).toBeUndefined(); + }); + + it("resolves sys_user through its nameField ('name'), the same convention a lookup target uses", () => { + expect(pickDisplayField(SYS_USER_FIELDS)).toBe('name'); + }); +}); + +// ── the pin that matters: ONE query, BOTH axes ────────────────────────────── + +describe('#16390 — one dataset query, a lookup dimension and a user dimension', () => { + it('renders a display name for BOTH axes', async () => { + const res = await query(service(), ['unit', 'person']); + expect(res.rows).toEqual([ + { unit: 'Customer Success', person: 'Ada Lovelace', avg_score: 91.5 }, + { unit: 'East China', person: 'Bo Chen', avg_score: 106.12 }, + ]); + }); + + it('resolves the user axis on its own, too (the reporter\'s second query)', async () => { + const res = await query(service(), ['person']); + expect(res.rows).toEqual([ + { person: 'Ada Lovelace', avg_score: 124.25 }, + { person: 'Bo Chen', avg_score: 128.6 }, + ]); + }); + + it('reads the OBJECT field type, not the dimension declaration — a `string`-declared user axis resolves identically', async () => { + const res = await query(service(), ['reviewer']); + expect(res.rows).toEqual([{ reviewer: 'Ada Lovelace', avg_score: 60 }]); + }); + + it('covers the whole declared reference class: master_detail and tree resolve as well', async () => { + const md = await query(service(), ['owner_team']); + expect(md.rows).toEqual([{ owner_team: 'Team Alpha', avg_score: 70 }]); + const tree = await query(service(), ['category']); + expect(tree.rows).toEqual([{ category: 'Quality', avg_score: 80 }]); + }); + + it('asks for the REFERENCED object, never the base object', async () => { + const targets: string[] = []; + await query(service({ onFetch: (t) => targets.push(t) }), ['unit', 'person']); + expect(new Set(targets)).toEqual(new Set(['sys_business_unit', 'sys_user'])); + }); +}); + +// ── the read scope (#3602) reaches the new members too ────────────────────── + +describe('#16390 — the label read stays scoped for every member of the class', () => { + it('resolves and forwards the referenced object read scope for a user dimension', async () => { + const asked: string[] = []; + const seen: Array<{ target: string; scope: unknown }> = []; + await query( + service({ + getReadScope: (objectName) => { + asked.push(objectName); + return objectName === 'sys_user' ? { organization_id: 'org_A' } : undefined; + }, + onFetch: (target, _ids, scope) => seen.push({ target, scope }), + }), + ['person'], + ); + // Reading a user id into a name IS a read of `sys_user`; it must carry that + // object's own RLS, exactly as a lookup target's label read does. + expect(asked).toContain('sys_user'); + expect(seen).toContainEqual({ target: 'sys_user', scope: { organization_id: 'org_A' } }); + }); + + it('fails CLOSED for a user dimension: an unresolvable scope leaves the raw id, and fetches nothing', async () => { + let fetched = false; + const res = await query( + service({ + getReadScope: (objectName) => { + // The base object stays resolvable — only the label target fails, so + // the query itself must still answer. + if (objectName === 'sys_user') throw new Error('security service unavailable'); + return undefined; + }, + onFetch: () => { fetched = true; }, + }), + ['person'], + ); + expect(fetched).toBe(false); + expect(res.rows).toEqual([ + { person: 'usr_ada', avg_score: 124.25 }, + { person: 'usr_bo', avg_score: 128.6 }, + ]); + }); +}); + +// ── C4 negative controls: degrade to the id, never to an error or a blank ─── + +describe('#16390 — an unresolvable user renders as itself, not as an error', () => { + it('leaves an orphaned / RLS-hidden user id untouched, and still answers the query', async () => { + const res = await query(service({ names: { ...NAMES, sys_user: { usr_ada: 'Ada Lovelace' } } }), ['person']); + expect(res.rows).toEqual([ + { person: 'Ada Lovelace', avg_score: 124.25 }, + { person: 'usr_bo', avg_score: 128.6 }, // raw id survives — no blank, no throw + ]); + }); + + it('leaves every id raw when sys_user carries no display field at all', async () => { + const res = await query( + service({ fieldMaps: { ...FIELD_MAPS, sys_user: { created_at: { type: 'date' } } } }), + ['person'], + ); + expect(res.rows).toEqual([ + { person: 'usr_ada', avg_score: 124.25 }, + { person: 'usr_bo', avg_score: 128.6 }, + ]); + }); + + it('leaves the id raw when the user object is unknown to the engine', async () => { + const withoutUser = { ...FIELD_MAPS }; + delete (withoutUser as Record).sys_user; + const res = await query(service({ fieldMaps: withoutUser }), ['person']); + expect(res.rows).toEqual([ + { person: 'usr_ada', avg_score: 124.25 }, + { person: 'usr_bo', avg_score: 128.6 }, + ]); + }); +}); diff --git a/packages/services/service-analytics/src/__tests__/dimension-labels.test.ts b/packages/services/service-analytics/src/__tests__/dimension-labels.test.ts index 51728a888d..5878c0f019 100644 --- a/packages/services/service-analytics/src/__tests__/dimension-labels.test.ts +++ b/packages/services/service-analytics/src/__tests__/dimension-labels.test.ts @@ -22,6 +22,11 @@ const TASK_FIELDS: Record = { ], }, account: { type: 'lookup', reference: 'crm_account' }, + // #16390 — the other two members of the same declared reference class. The + // `user` one deliberately omits `reference`: its target is a constant of the + // type, and metadata authored without it is fully specified. + assignee: { type: 'user' }, + parent: { type: 'tree', reference: 'task' }, created_at: { type: 'date' }, }; const ACCOUNT_FIELDS: Record = { @@ -34,9 +39,14 @@ function deps(overrides: Partial = {}): DimensionLabelDeps { getObjectFields: (obj) => obj === 'task' ? TASK_FIELDS : obj === 'crm_account' ? ACCOUNT_FIELDS : undefined, fetchRecordLabels: async (target, ids) => { - const names: Record = { acc1: 'Acme Corp', acc2: 'Globex' }; + const byTarget: Record> = { + crm_account: { acc1: 'Acme Corp', acc2: 'Globex' }, + sys_user: { usr_ada: 'Ada Lovelace' }, + task: { tsk_root: 'Root Task' }, + }; + const names = byTarget[target] ?? {}; const m = new Map(); - if (target === 'crm_account') for (const id of ids) if (names[String(id)]) m.set(id, names[String(id)]); + for (const id of ids) if (names[String(id)]) m.set(id, names[String(id)]); return m; }, ...overrides, @@ -68,6 +78,26 @@ describe('resolveDimensionLabels', () => { ]); }); + // ── #16390 — the same class, the same reading ───────────────────────── + it('maps a user dimension id → the referenced user\'s display name', async () => { + const rows = [{ assignee: 'usr_ada', budget_sum: 3 }]; + await resolveDimensionLabels('task', [{ name: 'assignee', field: 'assignee' }], rows, deps()); + // The field declares no `reference`; `sys_user` is the constant of the type. + expect(rows).toEqual([{ assignee: 'Ada Lovelace', budget_sum: 3 }]); + }); + + it('maps a tree dimension id → the referenced record\'s display name', async () => { + const rows = [{ parent: 'tsk_root', budget_sum: 4 }]; + await resolveDimensionLabels('task', [{ name: 'parent', field: 'parent' }], rows, deps()); + expect(rows).toEqual([{ parent: 'Root Task', budget_sum: 4 }]); + }); + + it('leaves an unresolved user id untouched, exactly as an unresolved lookup id is', async () => { + const rows = [{ assignee: 'usr_gone', budget_sum: 1 }]; + await resolveDimensionLabels('task', [{ name: 'assignee', field: 'assignee' }], rows, deps()); + expect(rows).toEqual([{ assignee: 'usr_gone', budget_sum: 1 }]); + }); + it('leaves an unresolved lookup id untouched (no blanks)', async () => { const rows = [{ account: 'orphan', budget_sum: 1 }]; await resolveDimensionLabels('task', [{ name: 'account', field: 'account' }], rows, deps()); @@ -164,10 +194,48 @@ describe('resolveDimensionLabels', () => { scopeCalls++; return undefined; }); - expect(scopeCalls).toBe(0); // scope is only resolved for lookup/master_detail dims + // A select dimension resolves from field metadata alone — it reads no + // other object, so there is no target scope to resolve. This stays true + // after #16390 widened the reference class; the positive counterpart for + // the four members that DO read another object is the next case. + expect(scopeCalls).toBe(0); expect(rows).toEqual([{ status: 'Backlog', n: 1 }]); }); + it('#16390 — a user and a tree dimension resolve the REFERENCED object scope, same as lookup', async () => { + const asked: string[] = []; + const seen: Array<{ target: string; scope: unknown }> = []; + const d = deps({ + fetchRecordLabels: async (target, ids, scope) => { + seen.push({ target, scope }); + return new Map(ids.map((id) => [id, `name-${String(id)}`])); + }, + }); + const rows = [{ account: 'acc1', assignee: 'usr_ada', parent: 'tsk_root', n: 1 }]; + await resolveDimensionLabels( + 'task', + [ + { name: 'account', field: 'account' }, + { name: 'assignee', field: 'assignee' }, + { name: 'parent', field: 'parent' }, + ], + rows, + d, + (target) => { + asked.push(target); + return { organization_id: 'org_A' }; + }, + ); + // Turning a user id into a name is a read of `sys_user`; it must carry + // that object's own RLS, not the base object's. + expect(asked).toEqual(['crm_account', 'sys_user', 'task']); + expect(seen).toEqual([ + { target: 'crm_account', scope: { organization_id: 'org_A' } }, + { target: 'sys_user', scope: { organization_id: 'org_A' } }, + { target: 'task', scope: { organization_id: 'org_A' } }, + ]); + }); + it('no resolver (no security configured) → unscoped fetch, unchanged behaviour', async () => { let seenScope: unknown = 'UNSET'; const d = deps({ diff --git a/packages/services/service-analytics/src/dimension-labels.ts b/packages/services/service-analytics/src/dimension-labels.ts index 497953a21b..352a94e60a 100644 --- a/packages/services/service-analytics/src/dimension-labels.ts +++ b/packages/services/service-analytics/src/dimension-labels.ts @@ -8,9 +8,12 @@ * * - **select** — grouped by the stored option `value` (e.g. `backlog`), but the * user-facing text is the option `label` (e.g. `Backlog`). - * - **lookup / master_detail** — grouped by the foreign-key `id` (e.g. + * - **the reference class** (`REFERENCE_VALUE_TYPES`: `lookup`, + * `master_detail`, `user`, `tree`) — grouped by the foreign-key `id` (e.g. * `8eqtuKI4G9IhUsPS`), but the user-facing text is the related record's - * display field (its name/title). + * display field (its name/title). All four store an id and all four resolve + * the same way (#16390); a `user` dimension's target is `sys_user`, whether + * the field spells `reference` out or leaves it to the type. * * `resolveDimensionLabels` post-processes the result rows IN PLACE, replacing the * raw value at `row[dimension.name]` with its display label when one is found. @@ -28,11 +31,17 @@ */ import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { referenceTargetOf } from '@objectstack/spec/data'; /** The minimal field shape this resolver needs. */ export interface FieldMetaLite { type?: string; - /** Lookup / master_detail target object name. */ + /** + * The referenced object's name, for a member of the reference class + * (`lookup` / `master_detail` / `user` / `tree`). Optional even for one of + * those: a `user` field's target is fixed by the TYPE, so `referenceTargetOf` + * supplies `sys_user` when the field omits it. + */ reference?: string; /** Select options — the value→label source. */ options?: Array<{ value: unknown; label?: string }>; @@ -110,14 +119,35 @@ export type LabelScopeResolver = ( targetObject: string, ) => Promise | null | undefined> | Record | null | undefined; -const LOOKUP_TYPES = new Set(['lookup', 'master_detail']); +/** + * The object a reference-typed dimension field points at, or `undefined` when + * the field is not one (#16390). + * + * Delegates to spec's `referenceTargetOf` — the declared SINGLE arbiter of + * "what does this field expand into" — so this module reads the reference class + * from the one place that defines it (`REFERENCE_VALUE_TYPES`: `lookup`, + * `master_detail`, `user`, `tree`) instead of restating a subset of it. Two + * things follow that a hand-written `type in {lookup, master_detail} && + * field.reference` test got wrong: + * + * - a `user` dimension (and a `tree` one) grouped by a stored FK id used to + * render that raw id where every sibling member rendered a name — the axis + * of any "by person" chart was a column of user ids; + * - a `user` field authored WITHOUT `reference` still names a target, because + * `sys_user` is a CONSTANT OF THE TYPE that `referenceTargetOf` materializes. + * Requiring the author to restate it is precisely the disagreement between + * two readers of one field that arbiter exists to end. + */ +function referenceLabelTarget(meta: FieldMetaLite | undefined): string | undefined { + return meta ? referenceTargetOf(meta) : undefined; +} /** * Sort-key label resolution for `DatasetSelection.order` (#3680). * * The executor sorts the assembled grid BEFORE `queryDataset` rewrites stored * dimension values into display labels, so an order key naming a `select` or - * `lookup`/`master_detail` dimension used to sort by the stored value / FK id — + * reference-class dimension used to sort by the stored value / FK id — * an order that presents as arbitrary once the labels render. This hook hands * the executor JUST the value→label mapping for such a dimension so it can sort * by what the user will actually read, while the rows keep their raw values @@ -128,7 +158,8 @@ const LOOKUP_TYPES = new Set(['lookup', 'master_detail']); export interface OrderLabelResolver { /** * Whether the dimension's stored value differs from the label it renders as - * (`select` options, `lookup`/`master_detail` FK ids). Synchronous — the + * (`select` options, or a reference-class FK id — `lookup`/`master_detail`/ + * `user`/`tree`). Synchronous — the * executor consults it when deciding whether the window may be pushed into * SQL, before any query runs. */ @@ -145,10 +176,10 @@ export interface OrderLabelResolver { * Build the executor's {@link OrderLabelResolver} from the dataset's dimension * list and the injected label capabilities. Mirrors the classification in * {@link resolveDimensionLabels}: a dimension is label-bearing when its field - * carries select `options` or is a lookup/master_detail with a `reference`. + * carries select `options` or belongs to the reference class and names a target. * * - `select` resolves from field metadata — no query at all. - * - `lookup`/`master_detail` costs ONE batched id→name read over the distinct + * - a reference dimension costs ONE batched id→name read over the distinct * grouped values, scoped to the REFERENCED object's own RLS (#3602). Fail * closed: an unresolvable scope degrades to sorting by the stored id rather * than fetching unscoped — consistent with the display pass, which renders @@ -171,7 +202,7 @@ export function createOrderLabelResolver( const meta = metaFor(dimension); if (!meta) return false; if (Array.isArray(meta.options) && meta.options.length > 0) return true; - return !!(meta.type && LOOKUP_TYPES.has(meta.type) && meta.reference); + return !!referenceLabelTarget(meta); }, async resolveLabels(dimension, values) { const meta = metaFor(dimension); @@ -183,16 +214,17 @@ export function createOrderLabelResolver( } return labelByValue; } - if (meta.type && LOOKUP_TYPES.has(meta.type) && meta.reference) { + const target = referenceLabelTarget(meta); + if (target) { let scope: Record | null | undefined; if (resolveScope) { try { - scope = await resolveScope(meta.reference); + scope = await resolveScope(target); } catch { return undefined; } } - return deps.fetchRecordLabels(meta.reference, values, scope ?? undefined, context); + return deps.fetchRecordLabels(target, values, scope ?? undefined, context); } return undefined; }, @@ -315,7 +347,7 @@ export function formatDateBucket(value: unknown, granularity?: DateGranularity | * @param rows - result rows, mutated in place * @param deps - injected runtime capabilities * @param resolveScope - (ADR-0021 D-C, #3602) resolves the referenced object's - * own read scope for a lookup/master_detail dimension's label fetch. When it + * own read scope for a reference-class dimension's label fetch. When it * throws, that dimension's labels are SKIPPED (fail-closed — the raw id renders * instead) rather than fetched unscoped. Omit when no read-scope provider is * configured (labels then fetch unscoped, as before — no security in play). @@ -375,8 +407,11 @@ export async function resolveDimensionLabels( continue; } - // ── lookup / master_detail: id → related record display name ─────── - if (meta.type && LOOKUP_TYPES.has(meta.type) && meta.reference) { + // ── reference class: id → related record display name ────────────── + // lookup / master_detail / user / tree — one declared class, one reading + // (#16390). `referenceLabelTarget` names the referenced object. + const target = referenceLabelTarget(meta); + if (target) { const ids = Array.from( new Set(rows.map((r) => r[dim.name]).filter((v) => v != null)), ); @@ -386,15 +421,18 @@ export async function resolveDimensionLabels( // RLS would hide (leak fires when the referenced object is stricter than // the base). Fail closed: if the scope can't be resolved, skip this // dimension's labels (raw id renders) rather than fetch unscoped. + // #16390 — this is the belt that makes resolving a `user` dimension safe: + // turning a user id into a name IS a read of `sys_user`, and it travels + // the SAME scoped path every other member of the class travels. let scope: Record | null | undefined; if (resolveScope) { try { - scope = await resolveScope(meta.reference); + scope = await resolveScope(target); } catch { continue; } } - const labelById = await deps.fetchRecordLabels(meta.reference, ids, scope ?? undefined, context); + const labelById = await deps.fetchRecordLabels(target, ids, scope ?? undefined, context); if (!labelById || labelById.size === 0) continue; for (const row of rows) { const label = labelById.get(row[dim.name]); From f4e1f0ddcb57be089674734e6e052113df1aee0f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 15:43:49 +0000 Subject: [PATCH 2/3] changeset: reference-class dimension display labels Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW --- ...tics-reference-dimension-display-labels.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 .changeset/analytics-reference-dimension-display-labels.md diff --git a/.changeset/analytics-reference-dimension-display-labels.md b/.changeset/analytics-reference-dimension-display-labels.md new file mode 100644 index 0000000000..56fff05f31 --- /dev/null +++ b/.changeset/analytics-reference-dimension-display-labels.md @@ -0,0 +1,19 @@ +--- +"@objectstack/service-analytics": patch +--- + +A dataset dimension over a `user` or `tree` field renders the referenced record's display name, the same way a `lookup` dimension already did. A "by person" chart's axis is people's names, not a column of user ids. + +`packages/spec` declares one reference class — `REFERENCE_VALUE_TYPES` = `lookup`, `master_detail`, `user`, `tree`, "value points at another record … a record-id string in stored form" — and this service already treated it as one class where it annotates measure result types (`measure-result-type.ts` imports that very set). The label resolver, one file away, hand-wrote a two-member subset of it (`lookup`, `master_detail`), so within a single dataset query one axis came back as a name and the other as a raw id, for two fields that differ in one word: + +``` +Field.user({ label: 'Person' }) -> { type: 'user', reference: 'sys_user' } +Field.lookup('sys_business_unit', { … }) -> { type: 'lookup', reference: 'sys_business_unit' } +``` + +- **The subset is gone, not extended.** The resolver now asks `referenceTargetOf` (`@objectstack/spec/data`) — the declared single arbiter of "what does this reference field point at" — at all three sites that classified a dimension: the display pass, the `#3680` sort-key hook's `isLabelBearing`, and its `resolveLabels`. Adding two literals to a private `Set` would have left the next member of the class to be re-reported by the next user. +- **A `user` field authored without `reference` resolves too.** `sys_user` is a constant of the type, which `referenceTargetOf` materializes; requiring an author to restate it is exactly the disagreement between two readers of one field that arbiter exists to end. +- **The label read stays scoped (`#3602`).** Turning a user id into a name is a read of `sys_user`, and it travels the same `LabelScopeResolver` path every other member of the class travels — the referenced object's own RLS is resolved and ANDed into the lookup, and an unresolvable scope still fails closed to the raw id rather than fetching unscoped. This is the half of the change that had to land with it, not after it. +- **Nothing degrades into an error or a blank.** An orphaned or RLS-hidden user id, a `sys_user` with no display field, and a user object unknown to the engine all leave the raw id in place and answer the query, which is the pre-existing contract for an unresolved lookup id. + +No new authorable key and no new export: `DatasetDimensionSchema` is untouched, and a dimension's own declared `type` still does not decide this — the resolver reads the object field's type, as it always has. From 44918e39daec2d24b6390cc52299303065a93088 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 15:58:04 +0000 Subject: [PATCH 3/3] test(service-analytics): record the measured ablation direction Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW --- .../dataset-reference-dimension-labels.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/services/service-analytics/src/__tests__/dataset-reference-dimension-labels.test.ts b/packages/services/service-analytics/src/__tests__/dataset-reference-dimension-labels.test.ts index 2ea3baaf5c..419fe0ffc8 100644 --- a/packages/services/service-analytics/src/__tests__/dataset-reference-dimension-labels.test.ts +++ b/packages/services/service-analytics/src/__tests__/dataset-reference-dimension-labels.test.ts @@ -54,10 +54,12 @@ * * Ordinary direction, no inversion and no count movement: the change ADDS * resolutions that were absent and narrows no rule, so restoring the two-member - * subset must turn RED exactly the cases whose dimension is `user` / `tree` / - * `master_detail`, and leave GREEN every `lookup`, `select`, unresolved-id, - * no-display-field and fail-closed case — those pin behaviour this change - * converges ON. The measured run is quoted in the PR body. + * subset must turn RED exactly the cases that depend on a `user` or `tree` axis, + * and leave GREEN every `lookup`, `master_detail`, `select`, unresolved-id, + * no-display-field and fail-closed case — `master_detail` was already inside the + * old subset, so it is a control here, not a casualty. Predicted on that reading: + * 7 red in this file and 3 in the `dimension-labels` sibling. Measured exactly + * that (10 red / 33 green over the two files); the run is quoted in the PR body. */ import { describe, it, expect } from 'vitest';