From 342fbf0ed901967efb8fd15d59ce4cb21275170f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 20:34:02 +0000 Subject: [PATCH 1/2] fix(data-objectstack): lower rule-shaped filter arrays on the aggregate path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `find()` has translated `[{ field, operator, value }, ...]` into the server's filter AST since `convertQueryParams` was written. `aggregate()` did not: the analytics path assigned `payload.where = params.filter` verbatim and posted it to `/analytics/query`. The two doors are not equally forgiving. `lowerAnalyticsWhere` in `@objectstack/service-analytics` — shared by BOTH aggregation strategies, so no deployment gets a lenient reading — accepts AST tuples and throws on an array of rule objects ("received a 'where' array that is not a filter"). The spec's own `isFilterAST` gate agrees about the same value. So a stored `ViewFilterRule[]` that a LIST renders correctly rendered `element:number` into its error state on every analytics-capable deployment, which is the default one: the CLI always loads analytics. Reuse, not a second lowering: an array filter goes through the same `translateFilterArray` the `find()` path runs, so the two paths cannot drift the way the two `find()` routes once did. The new tests assert that as cross-path parity on the wire, not just as a shape. Non-array filters stay untouched — the MongoDB-style object this branch was written for is what `/analytics/query` already accepts, and translating it would be a semantic change this card does not make. Already-AST arrays, record-shaped filters, the empty array and the no-filter case are byte-unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- .changeset/6302-aggregate-filter-lowering.md | 27 ++ .../src/aggregate-filter-lowering.test.ts | 261 ++++++++++++++++++ packages/data-objectstack/src/index.ts | 25 +- 3 files changed, 312 insertions(+), 1 deletion(-) create mode 100644 .changeset/6302-aggregate-filter-lowering.md create mode 100644 packages/data-objectstack/src/aggregate-filter-lowering.test.ts diff --git a/.changeset/6302-aggregate-filter-lowering.md b/.changeset/6302-aggregate-filter-lowering.md new file mode 100644 index 0000000000..c10dcd9616 --- /dev/null +++ b/.changeset/6302-aggregate-filter-lowering.md @@ -0,0 +1,27 @@ +--- +'@object-ui/data-objectstack': patch +--- + +`ObjectStackAdapter.aggregate()` lowers rule-shaped filter arrays before the +analytics wire, reusing the lowering `find()` already runs (objectui#6302). + +`find()` has translated `[{ field, operator, value }, ...]` into the server's +filter AST for as long as `convertQueryParams` has existed. The analytics path +did not: `aggregate()` assigned `payload.where = params.filter` verbatim and +posted it to `/analytics/query`. + +The two doors are not equally forgiving, so the gap had a user-visible end. +`lowerAnalyticsWhere` in `@objectstack/service-analytics` — shared by both +aggregation strategies — accepts AST tuples and throws on an array of rule +objects. A stored `ViewFilterRule[]` that a LIST renders correctly therefore +rendered `element:number` into its error state on every analytics-capable +deployment, which is the default one because the CLI always loads analytics. + +An array filter now goes through the same `translateFilterArray` the `find()` +path uses — one lowering, so the two paths cannot disagree about one stored +filter. Rules spread into a logical node (`['and', ...rules, ...tuples]`, the +commonest composite there is) are lowered at depth, as they already were on +`find()`. Non-array filters are untouched: the MongoDB-style object this branch +was written for is what `/analytics/query` already accepts, and translating it +would be a semantic change this fix does not make. Already-AST arrays, +record-shaped filters, and the no-filter case are byte-unchanged. diff --git a/packages/data-objectstack/src/aggregate-filter-lowering.test.ts b/packages/data-objectstack/src/aggregate-filter-lowering.test.ts new file mode 100644 index 0000000000..adfbc41dd4 --- /dev/null +++ b/packages/data-objectstack/src/aggregate-filter-lowering.test.ts @@ -0,0 +1,261 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `aggregate()` lowers rule-shaped filter arrays before the analytics wire. + * + * WHY THIS FILE EXISTS (objectui#6302). `find()` has translated + * `[{ field, operator, value }, ...]` into the server's filter AST since the + * day `convertQueryParams` learned to — see `filter-entry-translation.test.ts`, + * which runs every shape down both `find()` routes. The analytics path did not: + * `aggregate()` assigned `payload.where = params.filter` verbatim and posted it + * to `/analytics/query`. + * + * The two doors are not equally forgiving, which is why the gap had a + * user-visible end. `lowerAnalyticsWhere` in `@objectstack/service-analytics` + * — shared by BOTH aggregation strategies, so there is no deployment where the + * lenient reading applies — accepts AST tuples and THROWS on an array of rule + * objects ("[analytics] received a 'where' array that is not a filter"). The + * spec's own `isFilterAST` gate says the same thing about the same value, and + * the tests below assert on it directly so the refusal is pinned by the + * contract rather than by a message string: + * + * isFilterAST([{ field: 'stage', operator: 'equals', value: 'won' }]) // false + * isFilterAST(['stage', '=', 'won']) // true + * + * Net effect before the fix: a stored `ViewFilterRule[]` that a LIST renders + * correctly rendered `element:number` into its error state on every + * analytics-capable deployment — and analytics is the default one, because the + * CLI always loads it. + * + * The fix reuses `translateFilterArray` rather than adding a second lowering. + * That is load-bearing and is asserted as such below: the cross-path parity + * block requires `aggregate()`'s `where` and `find()`'s `filter=` to be the + * SAME value for the same input, so the two paths cannot drift the way the two + * `find()` routes once did. Non-array filters are deliberately untouched — the + * MongoDB-style object this branch was written for is already what the + * analytics endpoint accepts, and translating it would be a semantic change + * this fix does not make. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { isFilterAST } from '@objectstack/spec/data'; +import { ObjectStackAdapter, clearSharedDiscoveryCache, isMalformedFilterError } from './index'; + +/** Rows that carry the requested measure, so nothing degrades to the fallback. */ +const ANALYTICS_ROWS = { rows: [{ amount_sum: 150 }] }; + +function makeAdapter() { + /** Every parsed `/analytics/query` request body, in order. */ + const analyticsBodies: any[] = []; + const urls: string[] = []; + const fetchImpl = vi.fn(async (url: any, init?: any) => { + const u = String(url); + urls.push(u); + if (u.includes('/api/v1/discovery')) { + return { + ok: true, status: 200, statusText: 'OK', + json: async () => ({ success: true, data: { version: 'v1', routes: {} } }), + } as any; + } + if (u.includes('/api/v1/analytics/query')) { + analyticsBodies.push(init?.body ? JSON.parse(String(init.body)) : undefined); + return { ok: true, status: 200, statusText: 'OK', json: async () => ANALYTICS_ROWS } as any; + } + return { + ok: true, status: 200, statusText: 'OK', + json: async () => ({ success: true, data: { object: 'opportunity', records: [], total: 0 } }), + } as any; + }); + const adapter = new ObjectStackAdapter({ + baseUrl: 'http://localhost:3000', token: 't', autoReconnect: false, fetch: fetchImpl as any, + }); + return { adapter, analyticsBodies, urls }; +} + +const SUM_BY_STAGE = { function: 'sum', field: 'amount', groupBy: '_all' }; + +/** + * The `where` this filter put on the analytics wire. + * + * `HAS_NO_WHERE` distinguishes "the key was absent" from "the key was present + * and undefined" — the empty-filter cases below turn on exactly that. + */ +const HAS_NO_WHERE = Symbol('no where key'); + +async function whereOnWire(filter: unknown): Promise { + const { adapter, analyticsBodies } = makeAdapter(); + await adapter.aggregate('opportunity', { ...SUM_BY_STAGE, filter }); + expect(analyticsBodies).toHaveLength(1); + const body = analyticsBodies[0]; + return 'where' in body ? body.where : HAS_NO_WHERE; +} + +/** The `filter=` the SAME value produces on the plain `find()` route. */ +async function filterOnFindWire(filter: unknown): Promise { + const { adapter, urls } = makeAdapter(); + await adapter.find('opportunity', { $filter: filter } as any); + const dataCall = urls.filter((u) => u.includes('/data/opportunity')).pop(); + const raw = dataCall ? new URL(dataCall).searchParams.get('filter') : null; + return raw === null ? HAS_NO_WHERE : JSON.parse(raw); +} + +describe('aggregate() lowers a rule-shaped filter array before `client.analytics.query`', () => { + beforeEach(() => clearSharedDiscoveryCache()); + + it('translates a single rule into an AST tuple', async () => { + const where = await whereOnWire([{ field: 'stage', operator: 'equals', value: 'won' }]); + expect(where).toEqual(['stage', '=', 'won']); + }); + + it('the lowered value passes the AST gate the raw one fails', async () => { + const rules = [{ field: 'stage', operator: 'equals', value: 'won' }]; + // Negative control: this is what used to reach the wire, and it is exactly + // the value `lowerAnalyticsWhere` refuses. Without this line the test above + // could pass against a lowering that produced some OTHER non-AST shape. + expect(isFilterAST(rules)).toBe(false); + expect(isFilterAST(await whereOnWire(rules) as any)).toBe(true); + }); + + it('maps operator aliases the way the find() path does', async () => { + expect(await whereOnWire([{ field: 'amount', operator: 'greater_than_or_equal', value: 3 }])) + .toEqual(['amount', '>=', 3]); + }); + + it('joins several rules with `and`', async () => { + expect(await whereOnWire([ + { field: 'stage', operator: 'eq', value: 'won' }, + { field: 'amount', operator: 'gt', value: 100 }, + ])).toEqual(['and', ['stage', '=', 'won'], ['amount', '>', 100]]); + }); + + it('lowers rules SPREAD into a logical node, not just top-level ones', async () => { + // The commonest composite there is: a view's stored filter plus one the + // user added in the panel. The head is the string `and`, so a top-level-only + // check would call the whole thing "already AST" and ship the rule raw. + const composite = ['and', { field: 'stage', operator: 'eq', value: 'won' }, ['amount', '>', 100]]; + expect(isFilterAST(composite as any)).toBe(false); + const where = await whereOnWire(composite); + expect(where).toEqual(['and', ['stage', '=', 'won'], ['amount', '>', 100]]); + expect(isFilterAST(where as any)).toBe(true); + }); + + it('produces the SAME `where` as the AST-tuple equivalent (the acceptance criterion)', async () => { + const fromRules = await whereOnWire([{ field: 'stage', operator: 'equals', value: 'won' }]); + const fromTuple = await whereOnWire(['stage', '=', 'won']); + expect(fromRules).toEqual(fromTuple); + }); +}); + +describe('aggregate() leaves every already-correct filter shape byte-unchanged', () => { + beforeEach(() => clearSharedDiscoveryCache()); + + it('an AST tuple passes through untouched', async () => { + expect(await whereOnWire(['stage', '=', 'won'])).toEqual(['stage', '=', 'won']); + }); + + it('a logical AST node passes through untouched', async () => { + expect(await whereOnWire(['or', ['stage', '=', 'won'], ['stage', '=', 'lost']])) + .toEqual(['or', ['stage', '=', 'won'], ['stage', '=', 'lost']]); + }); + + it('a legacy nested array of nodes passes through untouched', async () => { + expect(await whereOnWire([['stage', '=', 'won'], ['amount', '>', 100]])) + .toEqual([['stage', '=', 'won'], ['amount', '>', 100]]); + }); + + it('a record-shaped (MongoDB-style) filter is NOT translated', async () => { + // The shape this branch was written for. `/analytics/query` accepts it, so + // lowering it here would be a semantic change, not a fix. + expect(await whereOnWire({ stage: 'won' })).toEqual({ stage: 'won' }); + }); + + it('a record-shaped filter with an operator object is NOT translated either', async () => { + expect(await whereOnWire({ amount: { $gt: 100 } })).toEqual({ amount: { $gt: 100 } }); + }); + + it('an empty array still reaches the wire as an empty array', async () => { + // Unchanged on purpose: `if (params.filter)` is truthy for `[]`, and this + // fix moves no boundary it did not have to move. + expect(await whereOnWire([])).toEqual([]); + }); + + it('no filter means no `where` key at all', async () => { + const { adapter, analyticsBodies } = makeAdapter(); + await adapter.aggregate('opportunity', SUM_BY_STAGE); + expect(analyticsBodies[0]).not.toHaveProperty('where'); + }); +}); + +describe('the find() path is unchanged, and the two paths share one lowering', () => { + beforeEach(() => clearSharedDiscoveryCache()); + + // Each row is one input. Both sides are measured on the wire, so a change to + // either path that the other does not make turns this red. + const SHARED_CASES: Array<[string, unknown]> = [ + ['a single rule', [{ field: 'stage', operator: 'equals', value: 'won' }]], + ['an aliased operator', [{ field: 'amount', operator: 'greater_than_or_equal', value: 3 }]], + ['several rules', [ + { field: 'stage', operator: 'eq', value: 'won' }, + { field: 'amount', operator: 'gt', value: 100 }, + ]], + ['rules spread into a logical node', ['and', { field: 'stage', operator: 'eq', value: 'won' }, ['amount', '>', 100]]], + ['an AST tuple', ['stage', '=', 'won']], + ['a logical AST node', ['or', ['stage', '=', 'won'], ['stage', '=', 'lost']]], + ]; + + for (const [name, filter] of SHARED_CASES) { + it(`aggregate() and find() agree on ${name}`, async () => { + const viaFind = await filterOnFindWire(filter); + expect(viaFind).not.toBe(HAS_NO_WHERE); + expect(await whereOnWire(filter)).toEqual(viaFind); + }); + } + + it('find() still lowers a single rule exactly as it did before', async () => { + // The `find()` half of the card's acceptance, stated independently of + // `aggregate()` so a regression there cannot hide behind the parity rows. + expect(await filterOnFindWire([{ field: 'stage', operator: 'equals', value: 'won' }])) + .toEqual(['stage', '=', 'won']); + }); + + it('find() still sends no filter for an empty array', async () => { + // The one place the two paths legitimately differ: `convertQueryParams` + // drops an empty filter, the analytics payload keeps `[]`. Recorded, not + // reconciled — reconciling it is a behaviour change this card does not make. + expect(await filterOnFindWire([])).toBe(HAS_NO_WHERE); + expect(await whereOnWire([])).toEqual([]); + }); +}); + +describe('a rule the adapter cannot translate refuses on the aggregate path too', () => { + beforeEach(() => clearSharedDiscoveryCache()); + + it('throws the same malformed-filter refusal `find()` raises, without inventing numbers', async () => { + // Dropping the untranslatable entry would WIDEN the result set and report + // success — the silent over-fetch `MalformedFilterError` exists to stop. + // Sharing the lowering means the analytics path inherits that refusal. + const { adapter, analyticsBodies, urls } = makeAdapter(); + const err = await adapter + .aggregate('opportunity', { + ...SUM_BY_STAGE, + filter: [ + { field: 'stage', operator: 'eq', value: 'won' }, + { operator: 'eq', value: 'no field here' }, + ], + }) + .then(() => null, (e) => e); + + expect(err).toBeInstanceOf(Error); + expect(isMalformedFilterError(err)).toBe(true); + // Nothing was posted to analytics, and no plausible-looking number came + // back from the fallback instead. + expect(analyticsBodies).toHaveLength(0); + expect(urls.some((u) => u.includes('/data/opportunity'))).toBe(false); + }); +}); diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index fd389f5c97..f4fb56f86a 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -4598,7 +4598,30 @@ export class ObjectStackAdapter implements DataSource { // spec/ui/dashboard.zod.ts). Send via the canonical `where` // field of the analytics endpoint, matching the unified Query // DSL (spec/data/query.zod.ts). - payload.where = params.filter; + // + // An ARRAY filter goes through the same `translateFilterArray` the + // `find()` path runs in `convertQueryParams`, because an authored + // `ViewFilterRule[]` reaches this method exactly as it reaches that + // one. It used to ship RAW from here, and the analytics door is + // stricter than the data door: `lowerAnalyticsWhere` + // (`@objectstack/service-analytics`, shared by both aggregation + // strategies) THROWS "[analytics] received a 'where' array that is + // not a filter" on an array of rule objects, while accepting AST + // tuples. So a stored filter that a list renders correctly rendered + // `element:number` into its error state on every analytics-capable + // deployment — and analytics is the default one, since the CLI always + // loads it (objectui#6302). + // + // One lowering, not two: the same function, so the analytics path and + // the `find()` path cannot disagree about one stored filter — which is + // the whole reason `translateFilterArray` was made a single definition + // (see its header). Non-array filters keep passing through untouched: + // the MongoDB-style object this branch was written for is what + // `/analytics/query` already accepts, and translating it here would be + // a semantic change this fix is expressly not making. + payload.where = Array.isArray(params.filter) + ? translateFilterArray(params.filter) + : params.filter; } const data = await this.client.analytics.query(payload); From 0d848bae55b7ba0c4f38b8de01c04f6a0c8be6aa Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 20:42:04 +0000 Subject: [PATCH 2/2] docs(data-objectstack): document the rule-shaped filter lowering both read paths share MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The README's "Filter Conversion" section documented the MongoDB-style object `find()` accepts and never mentioned the array shape server-driven view configs actually store (`ViewFilterRule[]`) — the shape objectui#6302 is about. Record it once, on both read paths, together with the properties the tests pin: alias mapping, lowering at depth inside a logical node, `MalformedFilterError` rather than a dropped condition, and non-array filters passing through untouched on the aggregate path. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- packages/data-objectstack/README.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/packages/data-objectstack/README.md b/packages/data-objectstack/README.md index 99c9d28dde..bd3b07e4af 100644 --- a/packages/data-objectstack/README.md +++ b/packages/data-objectstack/README.md @@ -147,6 +147,34 @@ const ast = [ ]; ``` +#### Rule-shaped arrays, on `find()` **and** `aggregate()` + +Server-driven view configs store their conditions as an array of rules +(`ViewFilterRule[]`), not as a MongoDB-style object: + +```typescript +const filter = [{ field: 'stage', operator: 'equals', value: 'won' }]; +``` + +Both read paths lower that array to the same AST before it reaches the wire — +`find()` via `$filter`, and `aggregate()` via the analytics `where`. They share +one translator, so a stored filter cannot mean one thing on a list and another +on a KPI: + +```typescript +// find(): filter=["stage","=","won"] +// aggregate(): { ..., where: ["stage", "=", "won"] } +``` + +Operator aliases (`equals`, `greater_than_or_equal`, `not_in`, `before`, ...) +map to the canonical AST symbols, and rules spread into a logical node +(`['and', ...rules, ...tuples]`) are lowered at depth. A rule that cannot be +translated raises `MalformedFilterError` rather than being dropped — dropping +one condition of an `and` would widen the result set and report success. + +Non-array filters are passed through unchanged on the aggregate path: a +MongoDB-style object is already what `/analytics/query` accepts. + ### Sorting ```typescript