Repository navigation
fix(core): refuse a retired string sort ENTRY out loud instead of dropping it in silence - #9984
Merged
Merged
Conversation
…ropping it in silence `normalizeSortEntries` refused the retired scalar clause (`sort: 'name asc'`) with a full prescription, while the same clause one container deeper — `sort: ['name asc']` — fell through the array arm's skip, returned `undefined`, and printed nothing. An authored row order disappeared with no trace in the console, which is the exact failure the module's own docblock argues against. A string entry now gets its own `console.error`, deduped per spelling like its scalar sibling. The dedupe keys are arm-tagged so the two arms cannot silence each other; the bound each keeps is one line per distinct spelling per arm, however many blocks inherit the same bad view. No input starts being accepted. `['name asc']` still contributes no ordering, per the director ruling that retired the spelling: one `sort` spelling, the array, everywhere. Mixed arrays keep lowering their good entries, and the message says so rather than claiming the query lost its ordering. The `field`-less object entry keeps the silence its docblock declares, and `''` keeps the silence the scalar `!sort` guard already gives it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
Contributor
|
changeset-claim-re-read
|
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
|
…pped in silence The `object-grid` probe table pinned `expect(errorSpy).not.toHaveBeenCalled()` for `sort: ['name desc']`, and its comment routed the widening of that refusal to "that card's business, not this one's". That card landed, so the assertion and the sentence above it were both false. The pin now asserts the sink fired exactly once and that the message is the ARRAY-ENTRY refusal specifically, not merely that `console.error` was called: a bare call count, or a match on the ruling number, would be satisfied just as well by the scalar refusal one describe down, and the input here is an array. `expectNoOrderBy(params)` stays first and stays unchanged. The RETURN value is the half a later reader is most likely to get wrong, and nothing moved it — a string entry still names no field and still survives nothing. Loud is not accepted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
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 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 #9955
What was wrong
packages/core/src/utils/sort-query.tsrefused the retired scalarsortclause outloud, and dropped the same clause one container deeper in complete silence. The
author's row order disappeared and the console said nothing.
Re-measured at source on this branch's base, objectui
origin/main8a7e09fa1— thearray arm had exactly two statements per entry,
continueandout.push, and no sinkcall anywhere between the
Array.isArrayguard and the return.Three arms, before and after
Each row is a real run of
convertSortToQueryParamsfrompackages/core/src/utils/__tests__/sort-query.test.tson this branch.$orderbyBEFORE$orderbyAFTER'name asc'undefinedundefined['name asc']undefinedundefined[{ field: 'name', order: 'desc' }]{ name: 'desc' }{ name: 'desc' }[{ order: 'asc' }]undefinedundefined[{ field: 'a', order: 'asc' }, 'b desc']{ a: 'asc' }{ a: 'asc' }'b desc')Only the SUBJECT row moves, and it moves only in the diagnostics column.
⭐ CONTROL C is the load-bearing one.
[{ order: 'asc' }]is also an entry thatcontributes nothing, and its silence is declared in
normalizeSortEntries' docblock:"an entry with no usable
fieldis SKIPPED (it names nothing to order by)". It stayssilent. That is what shows the seam was cut at
typeof entry === 'string'and not at"this entry has no field", which would have converted a designed silence into noise on
every partially-authored view.
⛔ The accepted input set is UNCHANGED — by one key and by one shape
sort: ['name asc']still contributes no ordering. Nothing that was refused is nowaccepted, and nothing that was accepted is now refused. Making the array-of-strings
work would re-admit the retired spelling one container deeper and reverse the director
ruling quoted in this file's own docblock — decision batch #77, 2026-09-07, objectui#8221,
maintainer approved, option B: "The platform has one
sortspelling, the array,everywhere."
Loud is not accepted. Pinned, not merely asserted, by
ACCEPTED INPUT SET UNCHANGED — the string entry contributes exactly nothing, whichcompares five arrays against their own string-free twins on both
convertSortToQueryParamsandnormalizeSortEntries— and which is the one new casethat stays green under the ablation below, because it pins the non-change rather than
the change.
The two docblock sentences that look contradictory are both still true, and neither was
edited:
yields
undefinedrather than a guess" — governs the RETURN VALUE. Unchanged.DIAGNOSTIC. This is the one that was being violated, and the only one this branch moves.
The dedupe key — the decision, and why the line count stays bounded
A sibling reporter, and ARM-TAGGED keys in the one existing Set.
retiredKey(ARM, SPELLING)returns the arm token, a newline, then the spelling, so thetwo key spaces are disjoint by construction: the arm token is one of two fixed words
and neither contains a newline, so the text before the first newline names the arm no
matter what an author wrote. A bare string prefix would only have been disjoint until
somebody authored the prefix — the spellings come out of authored JSON.
Why not reuse
reportRetiredSortSpellingwith the entry as its key:carries no
$orderby" because refusing the whole value refuses the whole ordering. Onebad entry among good ones does not:
[{ field: 'a', order: 'asc' }, 'b desc']stillorders by
a. Reusing the scalar text would tell an author their rows are unorderedwhile they are ordered by everything except the key they are hunting for — worse than
silence.
both spellings would print a sentence about the wrong container. Pinned by
CONTROL A — the scalar arm is untouched, and the two messages differ.The bound: ONE line per distinct spelling per arm, however many blocks inherit the
same bad view. That is the property the existing dedupe was built for — this sink runs
inside the query memo of every object-bound block, so a related-list derivation over an
object with N lists re-enters it N times per pass. Pinned by
BOUNDED — N blocks inheriting one bad view print ONE line, per spelling, which drives 25 renders of one badview and asserts one line, then a second spelling and asserts two, then clears the seam
and asserts three.
The message text
console.error, unconditional, not dev-gated — the same channel and the same severityclass as the scalar sibling, for the reason its docblock already argues: this REFUSES an
authored row order, so the page renders in an order the author did not ask for. Not the
warnOnUnknownActionKeysclass, which reports keys nothing was ever going to read.Asserted against
console.warnin the same body.Both halves objectui#9031 ruled load-bearing survive in this arm too, and they are judged
rather than eyeballed: the new case
objectui#9031 — the ENTRY message prescribes metadata the installed spec ACCEPTSparsesthe entries back out of the real emitted string and feeds them to
SortItemSchemafrom the INSTALLED
@objectstack/spec, exactly as the scalar pin does. The message namesARRAY_FORM_EXAMPLE, statesorderis required at publish, and states that the ascendingdefault is a runtime tolerance and not permission to omit the key.
One decision worth a reviewer's eye: the empty-string entry
['']stays silent, and that is symmetry with the scalar arm rather than an exceptionto the seam.
sort: ''never reaches the loud scalar arm either — the!sortguardanswers first, and the pre-existing case
stays silent for values that were never the retired spellingsays why in words: an emptyspelling is "the author asked for nothing", and reporting it would train readers to ignore
the message. Inverting that one container deeper would recreate exactly the inconsistency
this card is about, in the other direction.
Whitespace is NOT in the carve-out:
' 'is truthy, is loud on the scalar arm today,and is loud as an entry too. Both legs are the control in
the '' entry keeps the scalar arm's silence — symmetry, not an exception.Reverse verification (ablation) — measured, on disk, from the committed state
Declared direction before running: turns RED. Run through
scripts/ablation-replace.mjs, which proves the mutation reached the disk and proves therestore:
The 7 that fall are exactly the new objectui#9955 cases that assert the diagnostic. The 32
that hold include every pre-existing scalar refusal — so the sink is demonstrably not dead,
and the new assertions are not vacuous.
The plugin-grid pin this change invalidates — updated in the same branch
This section was rewritten by the⚠️ ONE RED」 that described a state which lasted one round; the evidence below is the seat's own and the implementer's, each attributed in place.
domain:ui#2seat at 2026-09-19T02:04Z, under the standing clause that a dev writes a PR body once and the seat writes later corrections. It replaces a section headed 「packages/plugin-grid/src/__tests__/gridArrayArmOrderby-8973.test.tsxhad one case pinningthe very silence this card removes, and that pin anticipated this card by name. Its own
comment read:
objectui#9955 is that card, and it landed the widening. The implementer stopped at the face
boundary and asked rather than riding; the seat granted route A (verdict
5738396331) and expanded the declared face by that one file, on three readings taken inthat act:
afterEach, and an appended section — and ⛔ does not touch this case's hunk;errorSpyassertion on that branch is one of the three pre-existing ones, and itsappended section carries no
errorSpyassertion and no array-of-strings case at all;the serialization the in-flight rule buys is buying nothing here — while the wait it costs
is unbounded.
What the update asserts
expectNoOrderBy(params)stays first and unchanged — the RETURN value moved nothing,and that is the half a later reader is likeliest to get wrong. The diagnostic half becomes
toHaveBeenCalledTimes(1)plus three assertions that discriminate the entry refusalfrom the scalar one: the message must carry the entry sentence's own opening, must not
carry the scalar sentence's, and must quote the value that arrived. A bare
toHaveBeenCalled(), or a match onobjectui#8221, would be satisfied just as well by thescalar message — and the input in this case is an array.
The fence held
git diff -U0on that file yields exactly one hunk header,@@ -188,5 +188,22 @@. Thescalar-refusal case, the imports, the
afterEachand the module docblock — PR objectui#9924'shunks — are all outside it.
packages/core/src/utils/sort-query.ts, its test and thechangeset are byte-identical to the first commit.
The reading
Same command, same denominators, both sides: the composite affected set went from
Test Files 1 failed | 165 passed (166)/Tests 1 failed | 3400 passed (3401)toTest Files 166 passed (166)/Tests 3401 passed (3401). ⭐ The equality of the totals isthe reading — it says the one failure is gone and nothing else moved. The whole
plugin-gridsuite is green on its own: 143 files, 1348 tests.An advisory is posted on PR objectui#9924 (
5738398717) so its author is not surprised; itasks nothing of that seat.
Verification
Run at commit
09d61f337, from the repo root, per the repo's canonical invocation.pnpm exec vitest run packages/core/— 158 files, 3319 tests, all passed.pnpm --filter @object-ui/core run type-check— exit 0 (both legs:tsc --noEmitand
tsc -p tsconfig.test.json), afterpnpm --filter '@object-ui/core^...' build,without which every reading is a stale-
distartefact (TS6305).exported symbols did not —
deriveRelatedLists.inheritSort,RecordDetailView.relatedListInheritedSort-5795,serverSorting,ObjectView.sortSink,ObjectView.namedViewSortArity,gridRetiredStringSort-8767,ObjectCalendar.sortMembersReachTheWire-8171,ObjectKanban.structuredMembersReachTheirSinks-8313,kanban-calendar-filter-sort-8174,ListView— all passed.pnpm exec eslint . --no-inline-config --format json— the whole repo, not anarrowing: 5151 files linted, 95 errors and 13387 warnings, every one of them
pre-existing; both files this branch touches report 0 errors and 0 warnings. The
population and the file count are read from that JSON, not estimated.
pnpm check:control-bytes— OK, 8050 tracked text files scanned.pnpm check:new-line-citations— 0 new citations, exit 0.node scripts/check-changeset-presence.mjs— 2 published source files changed,1 changeset declared, exit 0.
The one red is the out-of-face pin described above, and only that.
Scope
⛔ Three things this branch deliberately does not touch, each declared out of scope by the
dispatch order:
field-less OBJECT entry — its silence is declared in the docblock and is pinnedas CONTROL C to keep it that way.
object-gridwire shape — route B on objectui#8767, declined by the maintainer byname on 2026-09-10. Nowhere near this branch.
Acceptance notes
noted, not filed— the dedupe Set is now keyed byARM + newline + SPELLINGwhile thesibling sink
reportRetiredFieldType(same module family, same "retired spelling" idea)still keys on a raw value. Not a defect and not filed: that sink has one arm, so it has
nothing to collide with. Successor that would meet it: whoever gives that sink a second
arm. Named here so the next reader of the two does not read the asymmetry as an
oversight.
This PR is a DRAFT and stays one: landing is the claiming seat's act after review.
🤖 Generated with Claude Code
https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
Generated by Claude Code
Generated by Claude Code