fix(data-objectstack): aggregate()'s spec-shape branch refuses an unlowered where at the producer - #6911
Merged
os-sam merged 2 commits intoAug 30, 2026
Conversation
…owered `where` at the producer The spec-shape branch takes `where` and posts it to POST /data/:object/query verbatim; the analytics branch takes `filter` and lowers a rule-shaped array before the wire. One authored chart can reach either, decided by whether it carries `groupBy`/`aggregations` — a property with nothing to do with its filter. So the same stored rules were lowered on one branch and shipped raw on the other. Maintainer ruling 2026-08-30, option A: keep the spec-shape branch STRICT and refuse at the producer. It does not lower (option B, refused as the tolerant-consumer direction, and it would bless a params shape `AggregateParams` does not declare), and the branch is not retired (option D, refused as the riskier move — no in-repo producer can reach it, but `ObjectChart`'s props are `any` and an out-of-repo host may already send this shape). This adds no new failure, it relocates one. The gate is the spec's OWN `isFilterAST`, the same predicate the server ingress runs, so every array now refused is one the receiving engine already refused (`is not a filter`, 400 INVALID_FILTER, before the store is touched). What changes is where an author finds out: previously the predicate was lost on the wire, or dropped outright while the chart rendered confident, wrong numbers with no signal at all. `UnloweredAggregateWhereError` carries `MalformedFilterError`'s INVALID_FILTER/400 pair so `isMalformedFilterError()` recognises it and a failed widget renders "this filter is malformed" rather than "check your connection". Two carve-outs, both measured against the receiving door rather than assumed: a non-array `where` is the declared `FilterCondition` and is untouched, and `[]` is "no filter" the engine accepts, so refusing it would be a refusal nobody ruled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
…r lowers and which refuses
The filter section read "Both read paths lower that array to the same AST",
which after the spec-shape refusal is true of `find()`'s `$filter` and
`aggregate()`'s analytics `filter`, and false of `aggregate({ where })`. A
reader following it would have written the one shape that now throws.
Adds the worked accept/refuse pair, names the two deliberate carve-outs
(a FilterCondition object, and `[]`), lists `UnloweredAggregateWhereError`,
`MalformedFilterError` and `isMalformedFilterError` in the error-types block,
and records `INVALID_FILTER` in the error-codes list, where neither refusal
appeared before.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
github-actions
Bot
added
documentation
Collaborator
Review — ACCEPT, pending CI. Human floor, so no clause-② review is owed and that is a decision, not an omission.
The ruling classified this itself — "性质:已发布契约语义(生产端拒绝行为)⇒ 人工地板" — so the maintainer adjudicated the semantic directly and the ruling is its own control. Same routing as PR #6897 today. ⭐ Recording the reasoning in the PR body rather than leaving a silent absence is the right instinct; a future reader would otherwise have to guess whether the review was skipped or ruled out. ⭐ The empty-array carve-out is the review's centre of gravity — and I verified it at sourceThe PR takes the spec's own
export function isFilterAST(filter: unknown): boolean {
if (!Array.isArray(filter) || filter.length === 0) return false;⇒
⇒ A gate that simply called The first carve-out holds for the same reason and by the same method: a non-array Why importing the spec's predicate as a value is the right callThe gate is "the same predicate the server ingress runs, so the producer-side refusal and the wire-side one cannot drift." ⇒ The failure this repo keeps paying for is two copies of one rule diverging silently. Taking the value rather than restating the rule makes drift structurally impossible instead of merely unlikely. The verification I want on the record
The README repair earns its place in the diff
LandingCI at Out-of-scope exclusions all correct and all as dispatched: Generated by Claude Code |
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-sam
marked this pull request as ready for review
August 30, 2026 15:02
This was referenced Aug 30, 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 #6825
Executes the maintainer ruling of 2026-08-30 (comment 5468954321, batch #4, verbatim 「同意」) — option A:
aggregate()'s spec-shape branch stays strict and refuses an unloweredwhereat the producer.Landing path: human floor, NOT clause ②, and that is deliberate
The ruling classifies this itself: 「性质:已发布契约语义(生产端拒绝行为)⇒ 人工地板」. The maintainer adjudicated the semantic directly, so the ruling is its own control and this does not go through the
CONTRACT_REVIEW_TIERchain — that chain reviews contract accept/reject changes the maintainer has not individually adjudicated, and routing a human-floor ruling through it would add a reviewer below the authority that already decided. Same routing as PR #6897 earlier the same day. Recorded here so a future reader does not read the absent clause-② review as a skipped step.What changed
ObjectStackAdapter.aggregate()has two branches that take a filter under different names:field/function/groupByfilter/analytics/querygroupBy, arrayaggregations, or anywherekeywherePOST /data/:object/queryverbatimOne authored chart reaches either, decided by whether it carries
groupBy/aggregations— a property with nothing to do with its filter.packages/data-objectstack/src/index.tsnow gates the spec-shapewhere:The gate is the spec's own
isFilterAST, imported as a value — the same predicate the server ingress runs, so the producer-side refusal and the wire-side one cannot drift. It is scoped to arrays, with two carve-outs that are measured against the receiving door, not assumed:whereis the declared shape (QuerySchema.whereisFilterConditionSchema), so refusing{ stage: 'won' }would refuse the contract itself;engine-filter-array-lowering.test.tspinswhere: []returning every row fromfind()and3fromcount().isFilterAST([])isfalse, so a predicate-only gate would have refused a value the receiving door accepts.This adds no new failure — it relocates one
Every array now refused is one the receiving engine already refused:
engine-filter-array-lowering.test.tspinsengine.aggregate('deal', { where, groupBy, aggregations })rejecting the same shape with400 INVALID_FILTER, before the store is touched. So nothing that reaches this gate could ever have produced a correct number.What changes is the failure mode. Previously the predicate was lost on the wire — or dropped outright, leaving the chart rendering confident, wrong figures with no signal to their author. That silent pass-through is not an inference; it is measured below.
The refusal is
UnloweredAggregateWhereError(exported), carryingMalformedFilterError'scode: 'INVALID_FILTER'/httpStatus: 400pair soisMalformedFilterError()recognises it and a failed widget renders "this filter is malformed" rather than "check your connection" (#3066). Its message, in full:Four things an author can act on: the value received verbatim (so the producer is identifiable from a log), the shapes accepted and where the spec declares them, where to fix it (the producer, not this adapter), and that no numbers were invented on the way out.
Verification
All figures from
64fb1689b, the branch head. Suites run from the repo root, neverpnpm --filter PKG test.Union at the final commit —
pnpm exec vitest run --maxWorkers=2 packages/data-objectstack/:Test Files 50 passed (50),Tests 691 passed (691). New filepackages/data-objectstack/src/aggregate-spec-shape-where.test.ts: 22 tests.Which branch ran is asserted, not assumed. The two branches post to different endpoints, so the wire proves the branch:
whereland onhttp://localhost:3000/api/v1/data/opportunity/querywith zero calls to/analytics/query;filterwith legacy params lands on/analytics/queryand is still lowered to['stage','=','won']— ObjectStackAdapter.aggregate() must lower rule-shaped filter arrays before the wire — the analytics path skips the lowering find() already performs (prerequisite of the ruled element:number.filter convergence) #6302 untouched, and proof the refusal is not "we now refuse everything";wherekey to those same legacy params flips them onto the spec-shape door — the asymmetry the card reported, asserted on the wire.A refusal reaches no wire, so for the refusing inputs the branch is proven a second way:
looksLikeSpecShapeis transcribed into the test asbranchSelectors(), and the refused params are asserted to produce the same selector triple[true, true, true]as a control observed landing on the spec-shape door in that same test.Green control. A properly-lowered
wherestill reachesclient.data.querybyte-unchanged, four AST shapes plus theFilterConditionobject, the empty array, the no-wherecase andlimitforwarding.Red first, two independent ways, identical row for row:
Tests 10 failed | 12 passed (22).packages/data-objectstack/src/index.tsreverted to the merge-basedbd519450(base version proven on disk:hash-object= the base blob, zero mentions of the guard).return(mutation proven on disk: injected marker present exactly once, blob differs from the HEAD blob).Predicted vs observed, every row:
throws UnloweredAggregateWhereErrorsends NOTHINGto any of the three doorsthe message tells an author …the refused params take the SAME branch as the observed controlThe discriminating row is the fifth: it stays green under both neutralisations, which is what shows the branch-selection assertion is independent of the behaviour under test rather than riding on it.
⭐ The failure text of
sends NOTHINGis the card's silent pass-through, measured rather than argued:— with the refusal removed, the unlowered rule array lands on
client.data.queryand the call returns rows.Restore proven both ways, both legs:
git hash-objectequal to the HEAD blob, path-scopedgit diff HEADempty, zero residual markers. Both scripts carriedtrap 'RESTORE_FN' EXIT INT TERMwith absolute paths resolved fromgit rev-parse --show-toplevel. No rebuild was needed for either leg and none was skipped: the test imports the subject by relative source path (./index), nodistis in the resolution — which the ablation itself proves, since neutralising the source changed the observed result.Typecheck:
pnpm --filter @object-ui/data-objectstack type-checkclean. Confirmed it actually covers the edit —tsc --noEmit --listFileslists bothsrc/index.tsand the new test file (the package's tsconfigincludessrc/**/*with no test exclusion).Lint against the base versions of the same files, not a bare count.
eslint . --format json, 56 files both runs:src/index.ts121 warnings at base, 121 at head — delta 0; 0 errors throughout. The new test file contributes 11 warnings, all@typescript-eslint/no-explicit-anyfrom the shared fetch-mock idiom — the identical count its two siblings carry (aggregate-filter-lowering.test.ts11,aggregate-capability.test.ts11).Gates run locally, quoting each gate's own verdict line:
check:control-bytesOK (5756 files),check:doc-fencesOK (224 documents),check:changeset-presenceOK,check:changeset-no-majorOK,check:spec-symbolsOK,check:phantom-depsOK,check:self-importOK,check:vi-mock-specifiersOK,check:vi-mock-inheritOK,check:esm-specifiersOK,check:skills-pathsOK.check:spec-floorsandcheck:readme-exportsare PREREQUISITE NOT MET locally, not red: both read built artifacts and 16 of 39 packages were unbuilt here, so the gate reportsno-artifactfindings for them. The one real risk they cover — a README naming an export that does not exist — was checked directly instead:dist/index.d.tsafterpnpm --filter @object-ui/data-objectstack buildcarriesUnloweredAggregateWhereError,MalformedFilterErrorandisMalformedFilterError. CI runs both gates over a full build.Docs
packages/data-objectstack/README.mdsaid "Both read paths lower that array to the same AST", which is true offind()'s$filterandaggregate()'s analyticsfilterand false ofaggregate({ where })— a reader following it would have written the one shape that now throws. It now carries the accept/refuse pair worked out, names both carve-outs, lists the two refusal classes plusisMalformedFilterErrorin the error-types block, and recordsINVALID_FILTERin the error-codes list, where neither refusal appeared before.Out of scope, deliberately
AggregateParams—packages/types/src/data.tsdeclaresgroupBy: stringwith nowhere/aggregations. Untouched: it is objectui's own contract defect, neither A nor B fixes it, and it is carded separately. Nothing here needed widening it to typecheck.filter/field/function, so an arraygroupByon the LEGACY shape silently drops both the filter and the measure #6864 — the samelooksLikeSpecShapepredicate's second, worse failure (a legacy shape carrying an arraygroupByfalls into the spec-shape branch and loses the filter together with the measure). Awaiting triage; not addressed here and it stays open.Fence honoured: the diff touches
packages/data-objectstack/and.changeset/only. Nothing inpackages/plugin-form/,packages/plugin-timeline/,packages/app-shell/,apps/console/,packages/components/orpackages/fields/. No test skipped, disabled or quarantined; no edit tocontent/docs/releases/; nogit stash.Generated by Claude Code
Generated by Claude Code