Repository navigation
fix(plugin-grid): refuse the retired string sort clause at object-grid's own read site (objectui#8767) - #8960
Conversation
…rid's own read site (objectui#8767) `ObjectGrid` reads `schema.sort` and lowers it with PRIVATE code, so PR #8758's narrowing of the shared sink `convertSortToQueryParams` never reached it: after that PR a bare `object-grid` still forwarded a runtime string verbatim to `$orderby`, while the SAME key routed through `object-view` was refused with a diagnostic. One key, two meanings, chosen by which block you are on — the per-block divergence the objectui#8221 ruling declined by name. Route C, as ruled by the maintainer 2026-09-10: keep the wire shape, refuse the string. The string arm now calls the shared sink for its refusal alone — that reporter names the offending spelling once per spelling and answers `undefined`, so the query carries no `$orderby` — and the return value is deliberately unused, because the array arm keeps lowering to this block's own `"field order"` join string. Route B (routing the whole key through the sink, which would send its `{field: direction}` map instead) was explicitly not taken; the export path and the header-arrow reader `parseSchemaSort` are untouched. - `gridRetiredStringSort-8767.test.tsx` pins BOTH halves, so the pin cannot pass by refusing everything: a string reaches no `$orderby` and reports once per spelling; the array arm still emits `'name desc'` and `'status asc, name desc'` with no diagnostic. - `serverSorting.test.tsx`'s "replaces the declared sort" case authored the retired string and asserted it went out verbatim — that assertion is this card's own subject, so its fixture is re-authored in the declared array spelling and its #3106 subject is preserved. - Two comments this change falsifies are corrected, comment-only: the `parseSchemaSort` docblock no longer claims the fetch path reads all three spellings, and both it and the header-arrow read site now record that this reader is WIDER than the fetch path and that closing the gap is route B. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jmxdo7bmeqCQHLSfmLVX9w
✅ 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
|
Contract review — PR #8960 / card #8767 — VERDICT: REWORK (two bounded items; the refusal itself is judged correct)Contract-review seat, subagent 0. Tier fuse (read before any judgement)
1. Independence pair — and this is a SELF-REVIEW
2. Carrier state — the card is 漏挂 (never hung), not 已清标
3. ① Derived judgements — accept set and public face, measured(a) What changed, per input class. Probe rendered the real
Verdict on (a): exactly one arm moved — every truthy string — and it moved the same way on the bare grid and through (b) Consistent with #8758's sink, or a second semantics? For the string arm there is literally one semantics: the grid calls (c) (d) ⛔ But the same file at :170 ( (e) New pin (f) #8961 (arrow says 4. ② SemverChangeset: 5. ③ Boundary flags — each answered
6. VERDICT: REWORKThe refusal, its control legs, the semver level and the successor card are judged correct. Two bounded items, neither reopening the ruling: R1 — changeset body, two factual corrections (
R2 —
Dispatch-seat actions surfaced, not REWORK items: hang 7. NOT measured
Generated by Claude Code |
⛔ Landing withheld — REWORK adopted, plus the carrier the review caught me missingPM seat ( All 33 check runs are green on The carrier defect is mineThe review measured that That is the dispatching seat's to fix, and I have hung it on the card. ⛔ It stays on both carriers until a PASS against the repaired head. Route C itself is not in question — and I re-measured whyThe review's R1(a) is right that the changeset's sentence is false, and the reason matters enough to state precisely, because the maintainer principle now in force («我们的项目以 objectstack 协议为准», relayed on objectui#8934 Measured in this seat,
⇒ But that is exactly why the changeset sentence has to be repaired rather than defended: "the only one R1(b) turned up a second defect, in code this PR does not touch
R2 — the frozen-defect legThe review is right, and it is right by the implementer's own stated principle. What I am NOT asking for⛔ Do not touch the refusal mechanism, the control pins, the SELF-REVIEWThe review declares SELF-REVIEW: reviewer and implementer are both subagents of this one dispatching session, so context isolation is not independence. Recorded here rather than treated as an independent clearance — and acted on in the stricter direction (withholding a landing), which is the direction SELF-REVIEW is safe in. PR stays open and Generated by Claude Code |
…and pin the arrow-vs-wire divergence instead of freezing it green (objectui#8767)
Contract review VERDICT: REWORK, two bounded items. The refusal mechanism, the
control pins, the `minor` grading and objectui#8961's parking are judged correct
and are untouched — `ObjectGrid.tsx` is byte-identical to the reviewed head.
R1 — the changeset body carried two sentences that are false about the contract:
- "the only one `@objectstack/spec` accepts". Measured against the installed
`@objectstack/spec@17.4.0`: `ui.ObjectGridPropsSchema.sort` is
`z.unknown().optional()`, so `safeParse` PARSES `'name desc'`, `'name'`, `42`,
`['name desc']` and `{ name: 'desc' }` alike, while an undeclared `bogusProp`
is REFUSED with `unrecognized_keys` — the control that shows those readings
are real. The protocol's validator does not refuse the string. What the
protocol DECLARES is the array, in three places: the key's own `describe`
("Initial sort (array of { field, order })"), the sibling
`ElementRecordPickerPropsSchema.sort` typed as `z.array(SortItemSchema)`
(which refuses a string outright), and the `defaultSort` retirement text
("wrap the value in an array"). The sentence is now a claim about what the
protocol declares, which is true, rather than about what it accepts.
- "(`order` is optional and means `'asc'`)". `SortConfig.order` is REQUIRED in
`@object-ui/types` and in its zod mirror, and the protocol's `SortItemSchema`
requires it too (measured: it refuses `[{ field: 'name' }]` with
`invalid_value` at `0.order`). Worse, this block's untouched array arm
interpolates whatever is there, so an omitted `order` lowers to
`$orderby: 'name undefined'`. The changeset now says both keys are required
and names that behaviour; the defect itself is pre-existing, is NOT widened
into this diff, and is filed as a successor card.
R2 — `serverSorting.test.tsx` had a defect frozen green at the arrow case:
- its fixture is re-authored in the declared array spelling, the same principle
this PR already applied to the sibling case;
- the string's behaviour is now an EXPLICIT pin naming objectui#8961, asserting
both halves — the arrow is drawn AND the query carries no `$orderby` AND the
disagreement is announced once. A test that asserted only the arrow was a
green certificate that the state is intended, not a record of a divergence;
- the `parseSchemaSort` describe title ("the header reads what the fetch path
reads") was falsified by this PR and is renamed to say what that block
actually pins: the reader's own contract, wider than the fetch path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jmxdo7bmeqCQHLSfmLVX9w
✅ 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
|
Contract reviewRound 2 (delta) — PR #8960 / card #8767 — judged head 0. Tier fuse, re-read this round
1. Independence pair — SELF-REVIEWSELF-REVIEW. Implementer and reviewer are subagents of the same dispatching session. Per 2. Scope of this round, and what did NOT move
3. ① Derived judgements — the delta, item by itemR1(a) — the sentence "the only one
The text also says plainly that "this change is not a narrowing the validator already performed" — correct, and it keeps route C's justification where it belongs: the protocol's declared shape, which is what the maintainer's ruling rests on («Keep the wire shape; make R1(b) — " R2 — the three steps, each verified in the diff and by ablation.
Regression surface. Seven related files at 4. ② Semver — deltaFrontmatter unchanged, 5. ③ Boundary flags — delta
6. Carrier state on this headTimelines (REST): card #8767 — 7. NOT measured this round
VERDICT: PASS Generated by Claude Code |
2 participants
Fixes #8767
Authored by the
os-devseat of sessionhttps://claude.ai/code/session_01Jmxdo7bmeqCQHLSfmLVX9w. That reference is written here as prose in a code span on purpose: an attribution footer on a pull request body is normalised to the session-URL form on creation and degraded back to the bare form by every later edit, so a PR that is edited even once loses the reference from its footer. The code span survives the edit path.Route C, as ruled by the maintainer on 2026-09-10: keep the wire shape; make
ObjectGridREFUSE a stringschema.sortwith PR #8758's own diagnostic, before its own lowering. Route B (routing the whole key through the shared sink) and route A (leave it) were both declined on the card and are not re-opened here.Rework: the two repairs
R1 — two sentences in the changeset were false about the contract
Both were claims about the spec, and both were wrong. Re-derived on this branch rather than taken from the review.
(a) "the only one
@objectstack/specaccepts". False. Measured against the installed@objectstack/spec@17.4.0by callingui.ObjectGridPropsSchema.safeParsedirectly:sort'name desc''name'42['name desc']{ name: 'desc' }[{ field: 'name', order: 'desc' }]bogusProp— CONTROLunrecognized_keysThe control is what makes the six PARSES a reading rather than a schema that accepts anything. The cause is in the protocol source:
ObjectGridPropsSchema.sortisz.unknown().optional(), so the protocol simply does not police this field.z.unknown()is the protocol declining to police, not the protocol blessing the string — and what the protocol declares and documents is the array, in three independent places, all re-read onobjectstackfor this round:describe:Initial sort (array of { field, order });ElementRecordPickerPropsSchema.sort, which spells the identical intent as a typedz.array(SortItemSchema)— measured: it PARSES[{ field: 'name', order: 'desc' }], REFUSES'name desc'(invalid_typeat the root) and REFUSES['name desc'](invalid_typeat0);defaultSortretirement text, which instructs authors to rename the key tosortand wrap the value in an array.⇒ Route C moves this block toward the protocol's declared shape. But the sentence still had to be repaired rather than defended, because "the only one the spec accepts" is a claim about the validator, and the validator accepts the string. The changeset now makes the true claim — about what the protocol declares — and states plainly that the validator does not refuse the string.
(b) "(
orderis optional and means'asc')". False twice over.SortConfig.orderis required on both declared faces:packages/types/src/objectql.ts(order: 'asc' | 'desc', no?) and its zod mirrorpackages/types/src/zod/objectql.zod.ts(z.enum(['asc','desc']), no.optional()). The protocol agrees: its reusableSortItemSchemarequiresorder— measured,z.array(SortItemSchema).safeParse([{ field: 'name' }])refuses withinvalid_valueat0.order.orderlowers to$orderby: 'name undefined'. That sentence came from the shared sink's own diagnostic, which this PR now makesobject-gridprint — so the grid was prescribing a migration that produces garbage on the grid.The changeset now says both keys are required and names that behaviour. ⛔ The defect itself is pre-existing on the arm this PR does not touch, and widening the diff to reach it is the direction the ruling refused. It is therefore out of scope here, and carried instead by #8973, which holds the full probe table. (Deliberate wording: a closing keyword must never sit near another card's number — the merge parser matches keyword-plus-reference and does not read negation, so a sentence saying a card is not addressed would close it anyway.)
R2 — a defect was frozen green in
serverSorting.test.tsxThe review is right, and right by this PR's own principle. The sibling case at
:141was re-authored in the array spelling precisely so a pin would stop asserting something the wire no longer does; the arrow case was then left authoring the retired string, green, while the wire carries no ordering — the same shape, kept. Leaving the divergence visible was the correct instinct; leaving it visible as a green test that asserts only the arrow is not, because a green test does not expose a state, it certifies it.Three edits, one stroke:
:141was;sortspelling the fetch path now refuses —parseSchemaSortis wider than the wire #8961, asserting both halves — the arrow is drawn,hasOwnProperty(params, '$orderby')isfalse, and exactly one retired-spelling diagnostic is emitted. That case is now the one that has to change when finding(plugin-grid): after #8767 the header arrow still reads the retired stringsortspelling the fetch path now refuses —parseSchemaSortis wider than the wire #8961 closes the gap;parseSchemaSortdescribetitle — "the header reads what the fetch path reads", falsified by this PR and left standing while twoObjectGrid.tsxcomments were corrected — is renamed to say what that block actually pins: the reader's own contract, wider than the fetch path.The measurement, re-derived on this branch's own base
72bcd7783Precondition, by commit and ancestry rather than by an API field:
The control leg matters because a negative
--is-ancestoron a shallow checkout is a false negative; this clone is not shallow and the control answers 0, so the positive reading stands on its own.The divergence itself, on the same base:
convertSortToQueryParams(packages/core/src/utils/sort-query.ts:132) refuses a string at:141-144viareportRetiredSortSpelling(:110-122, an unconditionalconsole.error, deduped per spelling) and returnsundefined.packages/plugin-grid/src/ObjectGrid.tsx:1465takesconst schemaSort = schema.sort;and:1852-1859lowered it privately: a string went verbatim toparams.$orderby; an array became a"field order, field order"join string.convertSortToQueryParamslights up seven sibling packages non-test undersrc/—plugin-calendar,plugin-gantt,plugin-map,plugin-timeline,plugin-view,plugin-form,app-shell— andplugin-grid= 0, absent fromObjectGrid.tsx:39's@object-ui/coreimport list. So "ObjectGrid is not among them" is a reading, not an assumption.What changed, and why it is the minimal route-C change
One arm moves. In
ObjectGrid's own query memo the string arm no longer assigns$orderby; it calls the shared sink for its refusal alone:Three consequences worth stating explicitly:
reportRetiredSortSpellingis module-private tosort-query.ts, and the dispatch forbids changing that shared file. The only exported route to that one reporter is the sink itself, so the string arm calls it. The diagnostic, its wording and its once-per-spelling dedupe are all feat(core)!: retire the legacy stringsortclause — one spelling, the array (objectui#8221) #8758's, unmodified."field order"join string, so the export path (:3046) and the header-arrow readerparseSchemaSort(:4029) read exactly what they read before. Routing the whole key through the sink would send itsfield-to-directionmap where every grid today sends a string; that is the other route, and it is not taken here.else if (schemaSort)branch, so it does not silently hand the query to the legacydefaultSortarm. Pinned.The join-string pins — including a third one the dispatch did not name
The dispatch flagged two live
expect(params.$orderby).toBe('name desc')pins and asked which, if any, author a string. Read end to end:sortObjectGrid.elementDataSource.test.tsx:75HOT_VIEW.sort = [{ field: 'name', order: 'desc' }]— arraygridDefaultFiltersLowering.test.tsx:275defaultSort: { field: 'name', order: 'desc' }— the legacydefaultSortleg, notsortgridDefaultFiltersLowering.test.tsx:286sort: [{ field: 'name', order: 'desc' }]— arrayNeither of the two named pins authors a string. A third one does, and it is not in the dispatch's list:
packages/plugin-grid/src/__tests__/serverSorting.test.tsx:141—renderGrid(ds, { sort: 'name desc' }), assertingexpect(lastFindParams(ds).$orderby).toBe('name desc')under the comment "The declared sort goes out as the string form it was authored in." That is a direct pin of the retired lowering at this exact read site — this card's own subject — and it would have gone red.Its test name is "replaces the view's declared sort rather than stacking on it": its subject is objectui#3106's header click, and the string spelling was incidental to it. So its fixture is re-authored in the declared array spelling (the assertion value
'name desc'is unchanged, because the array arm is unchanged), which keeps its #3106 subject intact; the refusal it used to contradict is pinned separately, with both halves, in a dedicated file. Saying which, rather than editing quietly.A fourth site authors a string — the header-arrow case in the same file (
sort: 'status desc'). The first round left it string-authored with a comment, on the reasoning that this kept theparseSchemaSort-vs-fetch-path divergence visible. ⛔ That was wrong and the rework fixed it (R2 above): its fixture is now the declared array spelling, and the divergence is asserted by a dedicated pin naming #8961 — arrow drawn and$orderbykey absent and one diagnostic — instead of being left as a passing test that reads like the state is intended.Two comments this change falsifies, corrected — comment only
Both are claims that were true before this commit and are not after it. Neither touches behaviour:
ObjectGrid.tsxparseSchemaSortdocblock: it asserted "this grid's own fetch path already reads all three" spellings. It no longer does. The docblock now names the one declared spelling and records that this reader is wider than the fetch path, and that closing the gap is the other route.ObjectGrid.tsx:4019(the header-arrow read site): it asserted "the arrow on screen and the$orderbyon the wire are the same sort". One spelling now escapes that, and the comment says so.Ablation — every mutation proven on disk before any result was read
Resolution path first:
plugin-gridtests import../ObjectGridby relative path, and the root vitest config aliases every@object-ui/*specifier to that package'ssrc. Nothing here resolves throughdist, so no build gates these legs — but each mutation is still proven on disk by anchor counts on injected and removed text plus agit hash-objectdiffering from the HEAD blob, and each restoration is proven by blob hash, never by an exit code. Both legs restore fromtrap ... EXIT INT TERMusing absolute paths andgit checkout HEAD -- PATH.HEAD blob of
packages/plugin-grid/src/ObjectGrid.tsxat the commit under test:064b293dcd210d1d62622bc8aff059b59489d80e.Predictions were recorded before each run.
Leg 1 — delete the refusal (restore
params.$orderby = schemaSort;).Predicted: the four refusal cases red, the two array CONTROL cases green, the other three files green. Observed, exactly:
Leg 2 — mutate the ARRAY arm (
Predicted: the two array CONTROL cases red, both surviving join-string pins red, the re-authored
serverSortingcase red, the four refusal cases green, and — the asymmetry that makes it a control —gridDefaultFiltersLowering:275(the legacydefaultSortleg, a different code path) green. Observed, exactly:Leg 1 says the refusal is what makes the new pin pass. Leg 2 says the array assertions really measure the join output — so "the array arm still produces the same
$orderbyit does today" is a measurement, not a vacuous claim — and that the pin cannot pass by refusing everything.Verification
Exit codes captured into files before any pipe; heavy runs serialized through the container's shared verify lock and read from its
VERDICTline.pnpm exec vitest run packages/plugin-grid/Test Files 121 passed (121),Tests 1070 passed (1070)pnpm exec vitest runon the four sort files, final treeTest Files 4 passed (4),Tests 36 passed (36)ObjectGrid.tsxplussort-query.test.tsandObjectView.sortSink.test.tsxTest Files 6 passed (6),Tests 50 passed (50)turbo run type-check --filter=@object-ui/plugin-grid --concurrency=2> @object-ui/plugin-grid@17.6.0 type-checkand> tsc --noEmit && tsc -p tsconfig.test.jsonpnpm --filter @object-ui/plugin-grid lint> @object-ui/plugin-grid@17.6.0 lint/> eslint .;✖ 794 problems (0 errors, 794 warnings); zero lines matching the eslint severity-error shape and zero occurrences of the literal lowercase worderroranywhere in the lognode scripts/check-changeset-presence.mjspnpm changeset:checkmajordeclaredpnpm check:control-bytesnode scripts/check-governed-queue-guard.mjs --teston all four changed pathsRe-run on the rework head
52d28eb4b:plugin-gridtest filesTest Files 5 passed (5),Tests 45 passed (45)turbo run type-check --filter=@object-ui/plugin-grid --concurrency=2pnpm --filter @object-ui/plugin-grid lint✖ 794 problems (0 errors, 794 warnings); zero severity-error lines and zero occurrences of the literal lowercase worderrornode scripts/check-changeset-presence.mjspnpm changeset:checkpnpm check:control-bytesplugin-gridsuite was not re-run locally on52d28eb4b. The container's shared verify lock returnedexit 99(never acquired) twice, after 9 minutes of budget each, against a single holder that held it for 23 minutes; the third attempt ran a narrowed set. The narrowing is sound rather than merely convenient: the only source file this round changed isserverSorting.test.tsx, which nothing imports, plus a.changeset/*.md;ObjectGrid.tsxis byte-identical to76a9ead40, where the full suite was green atTest Files 121 passed (121)/Tests 1070 passed (1070). CI runs the whole farm on the pushed head.The one-off probe behind #8973's table ran inside that same lock hold as a temporary test file, and was removed in the same script under a
trap ... EXIT INT TERM; its removal is confirmed by the tree being clean of it (git statusnames only the two intended files).A first
pnpm --filter @object-ui/plugin-grid type-checkexited 2 withCannot find module '@object-ui/core'— that is the dependencydistnot yet built, i.e. PREREQUISITE NOT MET, not a red gate. It is recorded here rather than reported as a failed measurement; the turbo run above is the real reading.Scope
packages/plugin-grid/**plus one.changeset/*.md.packages/core/src/utils/sort-query.tsis read and reused, not changed. The wire shape, the export path,parseSchemaSort's behaviour, its exported signature andobject-calendarare all untouched.Two successor cards carry what this ruling parked, both filed unassigned and ungraded: #8961 (the header reader is wider than the fetch path) and #8973 (the array arm interpolates missing keys into
$orderby). They are two halves of one question —object-gridstill owns two private lowerings of a key every sibling block routes through the shared sink — and #8973 says so, so triage may fold it into #8961 if it prefers one card.Generated by Claude Code