Repository navigation
fix(data-objectstack): lower an array analytics filter through parseFilterAST before the wire (#7752) - #7754
Merged
Conversation
…ilterAST before the wire `aggregate()`'s analytics branch posted `translateFilterArray(params.filter)` as `payload.where` — filter AST, which is still a `FilterArray`. A `FilterArray` is input-only sugar: `@objectstack/spec` lowers it to a `FilterCondition` at the single sink `parseFilterAST` the moment it arrives, and only the lowered condition travels further (objectstack#5158 ruling C). `POST /analytics/query` parses its body with `AnalyticsQueryRequestSchema` before any normalisation, and that `where` is `FilterConditionSchema`, so every array shape answered `400 Invalid AnalyticsQuery body: where: ...`. The branch's comment named `lowerAnalyticsWhere` as the wire's door. That is the in-process door (objectstack#5334) and sits one hop later, which is why objectui#6302's gate measured a function the route never reaches. Lower with `parseFilterAST` before the payload is built: - an empty array posts no `where` at all (`parseFilterAST([])` is `undefined`, and the in-process door reads `[]` the same way); - a record filter passes through — it is already a `FilterCondition`; - an array the sink cannot lower (an infix join) is refused with a new `UnlowerableAnalyticsFilterError` carrying the `INVALID_FILTER` / 400 envelope its two siblings carry, rather than posted or dropped into an unfiltered aggregate. The lowering runs before the analytics `try`, so a refusal reaches the caller as itself instead of being classified as an unknown analytics failure and answered by the client-side fallback with plausible numbers. Tests re-pointed to `parseFilterAST`'s own output rather than literals, plus the route-hop pin nobody had on either side: the posted payload is parsed with `AnalyticsQueryRequestSchema` itself. Fixes objectui#7752. Refs objectstack#15828, objectstack#5158, objectui#6302. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TezFG8ZMrNH6n5VTNpPpdH
This was referenced Sep 5, 2026
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
|
The PR adds `UnlowerableAnalyticsFilterError` to `@object-ui/data-objectstack`'s published surface (present in `dist/index.d.ts`). An additive public export is a minor under semver and under this repo's precedent for new exports, not a patch. Text unchanged; only the frontmatter bump moves. Refs objectui#7752, PR objectui#7754. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TezFG8ZMrNH6n5VTNpPpdH
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
|
os-zhuang
marked this pull request as ready for review
September 5, 2026 15:42
This was referenced Sep 5, 2026
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 free
to 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 #7752
Executes the maintainer ruling recorded on objectstack-ai/objectstack#15828 (2026-09-05, verbatim 「15828 按前端缺陷处理」): the protocol is right and this adapter was wrong. No
@objectstack/specchange, and no widening ofAnalyticsQuerySchema.where— that direction is objectstack#5158's rejected option A.What was broken
aggregate()'s analytics branch builtpayload.where = translateFilterArray(params.filter)for an array filter.translateFilterArrayonly normalises rule objects into AST tuples; the result is still aFilterArray. Per@objectstack/specdata/filter.zod.ts, theFilterArraydocblock (objectstack#5158 ruling C, verbatim):The declared doors where an array may arrive are React block props,
FilterBuilder, and the REST$filterquery string. A POST body is transport, soPOST /analytics/queryparses the body withAnalyticsQueryRequestSchema(where: FilterConditionSchema) before any normalisation and answered400 Invalid AnalyticsQuery body: where: …to every array shape. Anelement:number(array-only since objectstack#12039) or anobject-metricwith an arrayfiltertherefore rendered into its error state on any deployment served through the runtime route, and the only surviving authoring form was the MongoDB-style record — the exact form #6206 ruling B retired.The branch's own comment showed the misreading: it named
lowerAnalyticsWhere(@objectstack/service-analytics, objectstack#5334) as the wire's door. That is the in-process door, one hop later, and it is the function #6302's gate measured. The comment is rewritten to name the route hop.The change
packages/data-objectstack/src/index.ts, analytics branch only:parseFilterASTis imported beside theisFilterASTthis file already imported from@objectstack/spec/data, and a newlowerAnalyticsFilterForWiredoes the lowering: a non-array filter passes through (it is already aFilterCondition), and any other array is normalised by the sametranslateFilterArraythefind()path runs, then lowered by the sink.parseFilterAST([])isundefined, so an empty authored filter posts nowherekey at all — the same reading the in-process door gives[]("[]is no filter", objectstack#5334). It used to post a literal[].[condA, 'or', condB]above all — throws a newUnlowerableAnalyticsFilterErrorcarrying the sameINVALID_FILTER/ 400 envelopeMalformedFilterErrorandUnloweredAggregateWhereErrorcarry, soisMalformedFilterError()recognises it. Neither of those two classes' meaning is widened: this one covers only the analytics branch's authoring input. The alternative was worse than a 400 —parseFilterASTanswersundefinedfor such an array, andundefinedhere would post a body with nowhere, i.e. an aggregate over every row returned as a confident number under a filtered question. The sink's own throw (a truncated tuple such as['stage','='], whichisFilterASTaccepts) is re-dressed in the same envelope with its message kept.try. Raised inside it, a refusal would be handed toclassifyAnalyticsFailure, come backunknown, and be answered by the client-side fallback — which re-reads the same rows throughfind()and hands the caller plausible numbers.MalformedFilterErrorreaches the caller directly now too; before, it only survived becauseaggregateViaFindhappened to re-throw it.Untouched, deliberately:
translateFilterArray's body,find()'s$filterquery string (a declared door), the spec-shape branch andUnloweredAggregateWhereError(#6825, ruling 2026-08-30 option A), the record producer path, and every governed surface.Tests
aggregate-filter-lowering.test.tsre-pointed, and the route-hop pin both triage comments on objectstack#15828 asked for is added. Two properties, not one:whereisparseFilterAST's own output for the same filter, never a hand-written literal — so this file cannot drift away from the contract it pins. The cross-path block now assertsaggregate()'swhereequalsparseFilterASTapplied to the ASTfind()puts onfilter=: the two doors differ by that lowering and by nothing else.AnalyticsQueryRequestSchema.safeParse— the route's own gate, run on the real body — for a comparison tuple, a rule array, anandgroup, a bare list, both record forms, the empty array, and no filter at all. A negative control in the same block asserts the array this branch used to post is refused by that schema, so the accept rows cannot pass against a gate that accepts anything.Plus: the infix join is refused before any request (the analytics body list is empty and no
/data/…fallback call happens either), andaggregate-spec-shape-where.test.ts's one row that pinned the old analytics value is re-pointed the same way — that file's own refusal is unaffected.Gates (exit codes captured by redirect, verdict lines quoted)
pnpm exec vitest run packages/data-objectstack/Test Files 56 passed (56)·Tests 748 passed (748)pnpm exec vitest run packages/data-objectstack/ packages/plugin-charts/ packages/plugin-dashboard/ packages/components/ packages/core/Test Files 544 passed (544)·Tests 6649 passed (6649)pnpm --filter @object-ui/data-objectstack type-checktsc --noEmit, no diagnosticspnpm --filter @object-ui/data-objectstack lint✖ 427 problems (0 errors, 427 warnings)— warnings all pre-existingno-explicit-anynode scripts/check-changeset-presence.mjs✅ 3 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)node scripts/check-changeset-no-major.mjs✅ No changeset declares a major bump.pnpm check:control-bytes✅ check-control-bytes: OK (scanned 6315 tracked text file(s))pnpm check:spec-symbols✅ spec member citations: no comment cites a key its spec symbol does not declare.pnpm check:phantom-deps✅ Every in-scope import is declared by the package that publishes it.pnpm check:vi-mock-specifiers✅ check-vi-mock-specifiers: OKpnpm check:self-import/check:unreferenced-sourcesThe second row covers every in-repo
aggregate()caller's package (plugin-chartsObjectChart,plugin-dashboardObjectMetricWidget,componentselements.tsx,coreValueDataSource). The two heavy runs went through this container's shared verify lock.pnpm check:readme-exportsexits 1 in this worktree withthe population COLLAPSED — packagesRead: found 4, floor is 25: it needs every package built, and its own workflow builds first. Declared, not silently skipped — with@object-ui/data-objectstackbuilt it reports zero findings for this package, and the change only adds an export (UnlowerableAnalyticsFilterError, confirmed present indist/index.d.ts), which cannot make a README name stop resolving. The repo-wide run belongs to CI.Reverse verification
Run on the committed tree (
00acc4a9), mutation proven on disk before the run and the restore proven after:With
lowered = astin place oflowered = parseFilterAST(ast), the fourAnalyticsQueryRequestSchemaaccept rows go red (expected false to be true) alongside the value assertions, and the truncated-tuple refusal stops firing — the new pin can fail, and it fails at the hop it exists to measure. The mutation was a source-level edit read directly by vitest (the tests import./index, not the package'sdist/), so no rebuild leg applies. The script carried atrap … EXIT INT TERMrestoring fromHEADby absolute path; restoration is proven by blob equality plus an emptygit diff HEAD, not by an exit code.Changeset
.changeset/7752-analytics-where-lowering.md,@object-ui/data-objectstackpatch, written for a user.Generated by Claude Code