Repository navigation
feat(spec,lint): a list-view grouping level can order its groups by another field (grouping.fields[].orderBy) - #25828
Conversation
…r field (orderBy) WIP: GroupingFieldSchema.orderBy, the header query's min/max order node, listViewGroupOrderColumn, and their pins. Lint, door pin, artifacts and changeset follow. Claude-Session: https://claude.ai/code/session_01E1GNboWQRhh5vidwh2hxTY Co-authored-by: Claude <noreply@anthropic.com>
…r does WIP: existence, a dotted path at the aggregate axis, and the new list-view-group-order-unorderable for a type min/max cannot order; explanations, barrel export, changeset, authorable surface. Claude-Session: https://claude.ai/code/session_01E1GNboWQRhh5vidwh2hxTY Co-authored-by: Claude <noreply@anthropic.com>
…r, both tiers Claude-Session: https://claude.ai/code/session_01E1GNboWQRhh5vidwh2hxTY Co-authored-by: Claude <noreply@anthropic.com>
…ping orderBy (#25786) Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1GNboWQRhh5vidwh2hxTY
… doors; state the whole-set sort An orderBy name is compiled verbatim into the header query's min / max node, so a padded spelling is refused by name, as a padded grouping field is. The unknown / dotted refusals are the data door's ingress gate, the type refusal is engine.aggregate's type door; the prose now says which. The header set is sorted whole: a limit on the header query slices before any sort. Equal order values fall back to the group key in the same direction. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1GNboWQRhh5vidwh2hxTY
… Schema publishes it A superRefine is dropped by z.toJSONSchema and would have added eighteen x-dropped-refinements sites; a native .regex() publishes the same rule as the schema's pattern, which a validator reading packages/spec/json-schema then refuses too. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1GNboWQRhh5vidwh2hxTY
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1GNboWQRhh5vidwh2hxTY
…y uses a regex Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E1GNboWQRhh5vidwh2hxTY
|
Independent full review: VERDICT ACCEPT (review 1 CHANGES-REQUIRED → review 2 ACCEPT-WITH-NITS → nit closed) The review was run by an isolated reviewer subagent at CONTRACT_REVIEW_TIER. It was given the card, the dispatch order, AGENTS.md and the code, not the author's conclusions, and it read the whole diff Review 1 on the tree at
Fixes, in
Review 2 on the delta Not reviewed: HotCRM's Generated by Claude Code |
📓 Docs Drift CheckThis PR changes 2 package(s): 14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 144 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a0bdbb71d26ce7b322a4b2181545e3c67241d8f7 && git checkout a0bdbb71d26ce7b322a4b2181545e3c67241d8f7
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin abebc4e14d43d8518e2dade88eeaa07e536c3b17 21dc9ad78953f57f991b69c25800a2db82db8d9d && git checkout -B drift-repro abebc4e14d43d8518e2dade88eeaa07e536c3b17 && git merge --no-ff 21dc9ad78953f57f991b69c25800a2db82db8d9d
node scripts/docs-audit/affected-docs.mjs --json abebc4e14d43d8518e2dade88eeaa07e536c3b17
|
Contract reviewServed-tier: Read against card #25786 (triage 6397346626, claim 6398321686, os-dev-report 6400232700), the PR body, the independent verdict 6400202111, the net diff ① Derived judgmentsThe key — right. The compiler — right, and conservative in the right places. The measurement claim — verified in the cited code. "The server needs no new code" holds. Observations, none blocking.
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS |
维护者速读改了什么 为什么改 风险与代价(含回滚) 席位意见 你要做的 席位核对Seat
|
Fixes #25786
Clause-②: yes
Metadata-protocol: added
A list-view grouping level can now order its groups by another field of the same object:
{ field: 'period_label', orderBy: 'period_start', order: 'desc' }. Before this, a period label grouped as text (Nov 2026,Oct 2026,Q1 2027,Q4 2025), so HotCRM's forecast list carried a second, outer grouping level onperiod_startand showed two header rows per forecast.What changes
packages/spec/src/ui/view.zod.ts).GroupingFieldSchemagainsorderBy?: string. It is optional with no default; absent, or naming the grouping field itself, means the level orders by its own key exactly as before. Its name is refused padded, asfield's is (spec:GroupingFieldSchema.fieldaccepts a padded field name that three objectui readers bucket wrong — refuse non-trimmed names at the producer (ruling C on objectui#7347) #17360's ruling). The rule is a native.regex(), so the published JSON Schema states it aspatterninstead of adding anx-dropped-refinementssite.packages/spec/src/ui/view-grouping-query.ts).compileListViewGroupQueryadds one node per ordered level,minoverorderByfororder: 'asc'andmaxfor'desc', aliased bycolumnSummaryAlias(min_period_start). Each group sits where its first row would sit if the rows were sorted byorderBy.groupBycolumn, so there is still one header per label.minis the same node.alias_collision.orderByisgrouping_order_by_blank(INVALID_QUERY/ 400).depthlimits the nodes to the grouped levels.listViewGroupOrderColumn(grouping, level), names the header-row column a level sorts by.packages/lint/src/validate-list-view-field-refs.ts).grouping.fields[].orderByis judged the way the server judges it, onos validate,os buildand the runtime view save door:list-view-field-unknown(error);list-view-field-dotted(error), on a newaggregateaxis;min/maxcannot order on every backend is the newlist-view-group-order-unorderable(error). The pair is asked ofisAggregateCompatibleWithFieldType, as the engine's type door asks it, so the string classes (collation-dependent) and the virtualformulaare refused, along with every other type outside the numeric, temporal and boolean classes.os explainentry.What the server needs (measured, as the order asked)
No new server code. The group sort stays the consumer's (
EngineAggregateOptionscarries noorderBy, andorderwas already applied by the consumer over the header rows). What the header rows lacked was the value to sort by, and the existing aggregate door already serves it:POST /api/v1/data/:object/query→protocol.findData→engine.aggregateanswersmin/maxoverdateanddatetimeoperands on both tiers. That is driver-sql pushdown, and the in-memoryapplyInMemoryAggregationbehind a driver that does not push down.typeofper column on both tiers, asc and desc. This is pinned inpackages/rest/src/list-view-grouping-query-door.test.ts§11, with the tier asserted by the fork counters, not assumed.400 INVALID_FIELD:findData's ingress (assertAggregationFieldsExist);engine.aggregate's type door.textoperand and a dottedperiod_start.year.limiton the header query is sliced byfindDatain emission order before any sort, so a paged header set honours neitherordernororderBy. That is the limit already recorded onGroupingConfigSchema("Header cardinality is unbounded"), unchanged here; bounding it isorderBy+limiton the aggregate verb, an engine-contract card of its own.Evidence
Final head
21dc9ad7.mainwas merged twice: once before the review (6679644a), and again after the sibling spec card #25755 landed (abebc4e1). There were no conflicts, and the whole workspace was rebuilt each time (pnpm build, 76/76).@objectstack/spec: full suite, 800 files, 23119 passed / 1 todo.@objectstack/lint: full suite, 163 files, 7331/7331.@objectstack/rest:list-view-grouping-query-door.test.ts, 53/53 (9 new in §11).scripts/ablation-replace.mjs, with the file restored to its HEAD blob andgit diff HEADempty afterwards:|| trueon the compatibility check): 8 red;...orders.slice(0, 0)): 5 red;/[\s\S]*/: 6 red.node scripts/pm/dispatch-gates.mjs --commandson the final diff derived 114 commands. All 114 ran on the final head, each exit code recorded before any pipe: 114 × exit 0.--ranreconciles to "114 derived, 114 run, 0 NOT-MEASURED, 0 UNRUN".pnpm build, and the self-test fixture commit fetched into the shallow clone.check-changeset-fixed,check:meta-url-spelling,check:authz-resolver,check:error-code-casing,check:filter-alias-parity),check:error-code-provenance, andtypecheckfor spec, lint and rest.!in the title, andClause-②carries no(narrowing)arm.Review
An independent full review of the whole diff (a fresh reviewer at the contract-review tier) returned CHANGES-REQUIRED with five findings. All five are fixed in
5d64c378,6a3bf131andd37faf8f, and the delta re-review's one nit in5df57e7f:orderBywas accepted by the schema;The delta re-review verdict is posted as a comment on this PR.
Acceptance notes
orderByis declared and compiled, but the console grid ignores it until the objectui child lands: render the label fromfield, order the headers bylistViewGroupOrderColumn, nulls last, ties by key in the same direction. That child card is filed in objectui withBlocked-by:this PR. HotCRM's one-line view edit (dropping its outerperiod_startlevel) comes after it, and is not this PR.grouping.fields[].fieldis not judged by lint (pinned silent invalidate-list-view-field-refs.test.ts), while the data door refuses it:400 INVALID_FIELD, "Unknown field 'period_start.year' … a related record's column cannot be a grouping target", measured through the §11 fixture. This predates the PR and is out of scope here; it is listed in the report.fieldkeeps itssuperRefineand itsdropped-refinements.baseline.jsonrow, whileorderBypublishes the same non-padded rule as a JSON Schemapattern. Movingfieldto the same.regex()would shrink the ledger, but that is a separate change.Generated by Claude Code