Repository navigation
fix(app-shell): stop declaring and writing a field-level sortOrder - #6463
Merged
Merged
Conversation
`FieldSchema` refuses `sortOrder` by name and the spec has no field-level
ordering key at all — it models field order by declaration order in the
object's `fields` record. `toFieldPayload` copied the key onto the payload
`saveFields` PUTs, so it was one reorder feature away from the hard 422
`INVALID_METADATA` that blocks every subsequent save of an object; it stayed
latent only because nothing populated it and `JSON.stringify` drops the
`undefined`.
Removed in one go from the wire shape (`FieldMetadataPayload`), its writer
(`toFieldPayload`) and the UI model (`DesignerFieldDefinition`), per
objectui#4687's resolution rather than objectui#6041's rename — the
near-spelling `sortable` is a boolean ("whether field is sortable in list
views"), a different concept. The `KNOWN_UNPARSEABLE_KEYS` entry goes with it;
that ledger ratchets in both directions.
The object-level `sortOrder` (objectui#6223, kept on `ObjectDefinition`) and
the saved-view `sortOrder` in `ObjectView` share the spelling and nothing else,
and are untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
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 26, 2026
os-support-ai
marked this pull request as ready for review
August 26, 2026 01:56
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 #6045
toFieldPayloadcopiedsortOrderonto the payloadsaveFieldsPUTs, andFieldSchemarefuses that key by name. Field-level sibling of #6223, same#5761 family. Removed from the wire shape, its writer, and the UI model.
The premise, re-derived on today's tree — and it holds
The card says the key is latent only because nothing populates it. Confirmed
on
origin/main854222c5e, with a positive control so the search is not takenon faith. Only two sites construct a
DesignerFieldDefinition—FieldDesigner's create/update handlers andMetadataFieldsPage.toDesignerField— and the same search that finds a key those sites do populate finds nothing
for this one:
So
toFieldPayloademittedsortOrder: undefined,JSON.stringifydropped it,and the key never reached the wire. Nothing populates it today — the card's shape
is unchanged and this is a removal, not a live key.
plugin-designer's own save path is separate and was already clean:fromDesignerFieldnever names the key, and itscarryOverspread cannot carryone back out because no writer ever put one into the stored document — so no
RETIRED_FIELD_KEYSentry is needed there.Resolution: deletion, not a rename — #4687's shape, not #6041's
The spec has no field-level ordering key at all. It models field order by
declaration order in the object's
fieldsrecord, so a designer that wantsexplicit ordering reorders that record rather than carrying an index. There was
nothing to map onto, and nothing was invented to map onto.
The near-spelling is not a rename target, and that is now asserted rather than
left as prose:
sortableis a boolean ("whether field is sortable in list views") — adifferent concept. The control on the first line is what makes this a key-by-key
result rather than a schema refusing everything.
Removed in one go so nothing is left declared-but-unwritten:
FieldMetadataPayload(wire),toFieldPayload(writer),DesignerFieldDefinition(UI model), plus the
KNOWN_UNPARSEABLE_KEYSentry — that ledger ratchets in bothdirections, so an entry left behind for a resolved key is as red as a missing one.
Two keys share this spelling and are NOT touched
The census was on the shape (a field-metadata payload key
FieldSchemarefuses), not on the identifier — a grep on the bare name hands you both of these:
sortOrder—ObjectSchema's. Object-level metadata payloads emit three keysObjectSchemarefuses by name (group,sortOrder,relationships) — the #5761 parity gate reaches field shapes only #6223 removed it from theobject wire shape and deliberately kept it on the
ObjectDefinitionUImodel as the Object Manager's display order. Still there, still reported by the
gate as
uiOnly(see the before/after below).sortOrder—ObjectView.tsx:554, documented at:600as"
sortOrderper saved view, in the new order". Per-view display order on adifferent document entirely. Untouched.
Evidence: before/after on the gate, with the controls in the same output
A removal has no new behaviour to pin, so the structural claim is a measured
before/after rather than an invented assertion. Both runs end in the gate's own
verdict line,
designer-field-key-parity: OK:Three positive controls sit in that same unchanged output, which is what makes the
two disappearances a removal rather than a blinded scanner:
enabledis stillledgered (the ledger was not emptied),
sortOrder (ObjectDefinition, vs ObjectSchema)is still reporteduiOnly(the object-level half is untouched),and
referenceTo (DesignerFieldDefinition, vs FieldSchema)is still reporteduiOnly(the UI-model scan still finds refused keys there).Evidence: reverse verification, on the committed fix
Predicted direction stated before the run: red. The mutation was proven on
disk before anything was measured (injected-text
grep -c= 1, andgit hash-objectdiffering from theHEADblob), and restored by hash:The second predicted direction is the interesting one, and it also held: the
parity gate stayed GREEN on the mutated tree (
MUTATED_GATE_EXIT=0,designer-field-key-parity: OK). The mutation restored only the copier, not thedeclaration, and the gate reads declarations. So the runtime pin is not
redundant with the gate — it covers the half the gate's own coverage notes say it
cannot see. That is why the new test is a runtime assertion on the PUT bytes
rather than a second reading of the gate.
Which assertion carries that weight is stated in the test file's header rather
than left implicit: the smuggled case is the one that reds on a revert. The
plain "a normal field PUTs no
sortOrder" case would still pass on a revert —deliberately, exactly as #6223's half-filled case does — because the key was
latent precisely because
JSON.stringifydrops theundefined.One fixture was replaced, not respelled
MetadataService.specKeyObjectPayload.test.tscarriedit('leaves the FIELD-level sortOrder alone - that key is objectui#6045...'),which asserted
saveFieldsstill put the key on the wire. That was correct when#6223 landed: the object half had to be provable without quietly resolving the
field half. It pins exactly the branch this card deletes, so it is replaced with
the claim it was really making — the object half is judged on the object document,
and reverting this card cannot make that case green or red.
Verification
All exit codes captured before any pipe; every gate result quoted from the verdict
line the gate itself printed. Union run on the final commit
c324804c9.node scripts/check-designer-field-key-parity.mjsdesigner-field-key-parity: OKpnpm exec vitest run packages/app-shell/src/services/ scripts/__tests__/check-designer-field-key-parity.test.ts packages/plugin-designer/Test Files 20 passed (20)/Tests 164 passed (164)pnpm --filter @object-ui/types type-checkpnpm --filter @object-ui/app-shell type-checkpnpm --filter @object-ui/plugin-designer type-checkpnpm --filter @object-ui/app-shell lint(eslint .)pnpm --filter @object-ui/types lint(eslint .)node scripts/check-changeset-presence.mjsnode scripts/check-changeset-no-major.mjsnode scripts/check-control-bytes.mjsnode scripts/check-phantom-dependencies.mjsnode scripts/check-spec-symbol-derivation.mjsnode scripts/check-vi-mock-specifiers.mjsnode scripts/check-package-self-import.mjsTwo notes, so the scope of the table is not over-read:
Run cold they were
exit 2with a wall ofTS2307: Cannot find module '@object-ui/...'in files this PR never touches. Afterturbo run build --filter='@object-ui/app-shell^...' --filter='@object-ui/plugin-designer^...'(28 tasks, exit 0) all three are exit 0.
app-shell'stsconfig.test.jsonsetspaths: {}, so it resolves@object-ui/typesthrough the built.d.ts—--listFilesconfirms both edited test files are in that program (2 hits of4441), and
packages/types/dist/designer.d.tsnow carriessortOrderonlyinside
ObjectDefinition, withDesignerFieldDefinitiongoinggroup?->description?with nothing between.node scripts/check-readme-exports.mjsexits 1 locally, and it is aprerequisite, not a finding. All 69 items read
its type entry ./dist/index.d.ts is not on disk -- run pnpm build first, for 8 packagesoutside the closure built above. Zero of them name
packages/types,DesignerFieldDefinitionorsortOrder. CI builds the whole workspace, so itmeasures what this run could not.
Declared narrowing: the other packages'
lintand the remainingcheck:*gates were left to CI, which runs the farm exactly once regardless. The gates
above are this card's own family plus those derived by hand from the changed
paths (
objectuihas nodispatch-gatesderiver; the one inobjectstack/scripts/pm/answers only about its own tree).Draft on purpose — the PM lands this. Not marked ready, no auto-merge.
Generated by Claude Code
Generated by Claude Code