Uh oh!
There was an error while loading. Please reload this page.
fix(data-objectstack): lower rule-shaped filter arrays on aggregate()'s analytics path - #6828
Merged
Merged
Conversation
…te path
`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49… read paths share 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This was referenced Aug 29, 2026
os-sam
marked this pull request as ready for review
August 30, 2026 06:17
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#6302
Step 1 of the maintainer's Key-2 sequencing ruling (objectstack#12039): sequence the consumer first. The spec-side convergence of
ComponentPropsMap['element:number'].filtertoz.array(ViewFilterRuleSchema)is step 2, on a follow-up inobjectstackcarryingBlocked-by:the objectui pin bump — it is NOT in this PR.The defect, confirmed on this base rather than taken from the card
Both halves of the card's premise verified on
26896c689:find()does lower.convertQueryParams(packages/data-objectstack/src/index.ts:3448) callstranslateFilterArray(params.$filter)at line 3479. The$expand/$searchroute reaches the same machinery throughtranslateFilterToAST.aggregate()did not. The analytics branch assignedpayload.where = params.filterverbatim (line 4601 pre-fix) and posted it toclient.analytics.query.translateFilterArrayis reusable as-is — a module-level function in the same file, three call sites away. Nothing had to move or be exported, so this is the reuse the ruling asked for and not a second lowering.The refusal was then measured against the real runtime door rather than inferred.
lowerAnalyticsWhere(objectstackpackages/services/service-analytics/src/strategies/filter-normalizer.ts:1508, shared by both aggregation strategies) does:and the spec's own gate answers, measured:
So an authored
ViewFilterRule[]that a LIST renders correctly threw on every analytics-capable deployment — the default one, since the CLI always loads analytics — and renderedelement:numberinto its error state.The change
packages/data-objectstack/src/index.ts— one assignment:Non-array filters stay untouched on purpose: the MongoDB-style object this branch was written for is what
/analytics/queryalready accepts, and translating it would be a semantic change the ruling excludes ("No ruled semantic changes — only ordering").One shape I checked and deliberately did NOT change: an empty array.
[]is truthy, sowhere: []still goes out — and the door above answers[]withreturn nullbefore the refusal, so it is accepted. Byte-unchanged, and now pinned as such.Tests
New:
packages/data-objectstack/src/aggregate-filter-lowering.test.ts— 22 pins.and, and rules SPREAD into a logical node (['and', ...rules, ...tuples], the commonest composite) lowered at depth.where.isFilterAST(rawRules)isfalseandisFilterAST(loweredWhere)istrue, so a lowering that produced some other non-AST shape could not pass.wherekey at all).find()path is unchanged, two ways: its own standalone expectations, plus a six-row cross-path parity table assertingaggregate()'swhereandfind()'sfilter=are the same value on the wire for the same input. That is what stops the two paths drifting the way the twofind()routes once did.convertQueryParamsdrops an empty filter, the analytics payload keeps[].MalformedFilterError), with nothing posted to analytics and no plausible-looking number from the fallback instead.No existing test moved. That is worth stating plainly, as the card asks: the analytics path had NO coverage of the request body's
where. The only filter-related aggregate test (aggregate-capability.test.ts:141) exercises the FALLBACK and asserts on the/dataGET URL, so it never looked at whatclient.analytics.querywas sent.Ablation — the pins fail without the fix
Fix committed first, then the pre-fix assignment restored on disk (anchor asserted, injected/removed strings counted, blob hash changed
f4fb56f8to4dd4ea56). The test imports the adapter by relative SOURCE path (./index), so vitest compiles the mutated file directly — nodistround-trip to invalidate.The direction is the point: the 11 lowering / parity / refusal pins go red, and the 11 byte-unchanged pins stay green — so they are not merely following the change. A representative failure:
Restore proven by observation, not by exit code:
git diff HEADempty,git statusclean, and the on-disk blob back tof4fb56f86af9ea234fb938db30596d9cf6e122bd, identical to the HEAD blob.Verification, all at
0d848bae5(final commit)The typecheck really does cover the new test file — the package's tsconfig has no test exclude, and
tsc --listFilesfinds both changed files (1 hit each), so "typecheck clean" is a statement about them and not a vacuous one.One declared narrowing.
pnpm lintisturbo run lintacross 47 packages; I ran the changed package's owneslint .instead — 0 errors, 404 warnings, all pre-existingno-explicit-anyatwarn. Three pieces of evidence that this narrowing excluded nothing: (1) the population comes from eslint's own config resolution, not my guess; (2)--format jsonreports 54 files linted, and the new test file's 11 warnings are the same rule and the same count as the siblingaggregate-capability.test.ts; (3) type-aware linting is NOT enabled —eslint.config.jsusestseslint.configs.recommendedwith noprojectorprojectService— so this diff cannot move the verdict on any file it does not contain. The other 46 packages read no file of this diff.Scope
Everything is inside
packages/data-objectstack/(adapter, its new test, its README) plus one.changeset/entry — clear of the thirteen unmerged PRs on this seat, none of which touches this package.The README addition is AGENTS.md #2: the "Filter Conversion" section documented the MongoDB-style object
find()accepts and never mentionedViewFilterRule[], the shape this whole card is about.Filed, not fixed
#6825 —
aggregate()'s OTHER branch (looksLikeSpecShape, which posts toclient.data.query) still assignswhereunlowered, andObjectChart.tsxhands the same resolved filter to both branches (lines 483 and 491). After this PR, one chart's filter is lowered and another's is not, decided by which aggregation shape it uses. Left alone deliberately: thatwhereis the spec Query DSL's, declared a FilterNode inquery.zod.ts, so lowering it there would be deciding what a filter MEANS — the thing this card's order says to stop and report on rather than do.Generated by Claude Code
Generated by Claude Code