Skip to content

mergedSort hands table.defaultSort to the delegated list view UNWRAPPED, while the non-grid fetch path wraps the same value — the #5270 arity split, one branch short #6235

Description

@yinlianghui

Found while retiring the direction spelling from toSortItems (#6011, PR #6234) — reading ObjectView's sort chains to decide what the stale fixture at ObjectView.filterSources.test.tsx actually pins. Filed unassigned, no labels — triage grades it. Not fixed in #6234: that card is a spelling retirement on a published export, and this is an arity defect on a different surface.

Line numbers re-derived on claude/issue-6011-tosortitems-direction @ b0d186b79; they sit within one line of main.

The split

ObjectGridSchema.defaultSort is declared a single object — { field: string; order: 'asc' | 'desc' } (packages/types/src/objectql.ts, zod mirror agreeing). Three places in packages/plugin-view/src/ObjectView.tsx resolve it, and two of them account for that arity:

  • non-grid fetch (:791) — wraps it: || (schema.table?.defaultSort ? [schema.table.defaultSort] : undefined), so the shared sink receives the SortConfig[] it declares.
  • grid path (:1375) — defaultSort: schema.table?.defaultSort, forwarded into the slot that declares the bare object. Correct by construction; ObjectGrid does its own schemaSort ?? [schema.defaultSort] resolution.
  • delegated path (:1522-1525) — does not:
  const mergedSort = currentNamedViewConfig?.sort
    || activeView?.sort
    || schema.table?.sort
    || schema.table?.defaultSort;   // <- bare object, unwrapped

mergedSort lands at :1575 as sort: mergedSort on the list-view node the host's renderListView receives — a slot declared string | SortConfig[]. Every branch above the last produces an array (or a string); only the defaultSort fallback produces a bare { field, order }.

The comment immediately above that chain (:1513-1516) asserts the opposite:

Both land on list-view's own filter / sort keys below, so a canonical value arrives in the slot that already matches its shape.

That is true of the first three branches and false of the fourth.

Why this looks familiar

This is the same shape #5270 fixed one branch over: an array/object arity mismatch with no compile-time witness, because ObjectViewSchema.table collapses to a bare index signature (#5102) and the delegated list-view node is assembled with (schema as any).K reads on the host-composition surface (#5097). #5270's own test header warns about exactly the failure this reproduces — a parseSchemaSort-style reader gets a shape it cannot parse, and the symptom is a missing sort with no error, not a crash.

Why no test catches it

packages/plugin-view/src/__tests__/ObjectView.filterSources.test.tsx (forwards the sort alongside it) is the only assertion on this forward, and it is arity-blind in both directions: it passes an already-array defaultSort behind an as any and asserts verbatim toEqual on what came out. A bare-object defaultSort — the declared shape — is never exercised, and even if it were, a verbatim pass-through assertion would report it as passing. #5270's header names this trap by hand: "Pinning the forwarded shape alone would have re-pinned the bug."

(#6234 re-spelled that fixture's direction → order and left its arity alone, deliberately, recording the reason in a comment there. This card is that recorded reason.)

What to decide

Not a dev's own call — it is which of two contracts the delegated sort slot is meant to carry:

  1. Wrap at mergedSort, matching :791 verbatim: || (schema.table?.defaultSort ? [schema.table.defaultSort] : undefined). One line, and it makes all three consumers agree. Note this is the resolution ObjectGrid already performs for the same pair, so it introduces no fourth dialect.
  2. Leave it and correct the comment — if a host's renderListView is genuinely expected to accept the deprecated bare-object shape, then the slot's declared type is what is wrong, and the claim at :1513-1516 needs to stop asserting canonicality.
  3. Something upstream — table.defaultSort is @deprecated Use sort instead; a route that stops resolving it on new surfaces may be preferable to teaching a fourth reader about it.

Recommendation is 1, on the grounds that :791 already ruled the wrapping question for this exact pair and a maintainer ruling of 2026-08-22 (recorded in the comment at :779-784) explicitly rejected widening the shared sink to guess at a bare { field, order } — so the wrap belongs in the caller, which is where :791 put it. But the delegated surface is host-facing, so whether an out-of-tree renderListView is already coping with the bare object is not derivable from this repo.

Whatever is chosen, the fixture above should gain a case that passes the declared object arity, since the current one cannot fail either way.

Dedup

Searched open issues for mergedSort, defaultSort + renderListView, and the arity wording — nothing on this. Related: #5270 (the arity ruling this extends), #5102 / #5097 (why there is no compile-time witness), #6011 / PR #6234 (where it was found).

Activity

  1. os-support-ai commented on Aug 26, 2026

    @os-support-ai
    Collaborator

    Claim: domain:ui execution seat (objectui), session session_011SfZeFWrhGLHmfq61xbz4q, round 26.
    Branch: claude/issue-6235-mergedsort-wrap

    Serial constraints — file face: packages/plugin-view/src/ObjectView.tsx (mergedSort) and its __tests__.

    Non-overlap: #6419 landed in this exact file and merged (e929c562a), so this branch takes it from main rather than racing it — the seam its PR body explicitly deferred ("same fetch path, different defect") is now free. No open PR in this lane touches plugin-view. The others in flight are plugin-calendar (#6453), app-shell/src/services + metadata-admin (#6238), app-shell/src/providers (#6443), and two awaiting CI on types/plugin-gantt (#6051) and plugin-detail (#6421).

    My reading vs the card's, stated so the dev resolves it before writing rather than after. The card points at a comment around :1513; on origin/main I measure mergedSort itself at :1602, with related precedence comments at :859, :1425 and :1464. Line numbers have drifted — re-derive the comment's real location and say which one the obligation attaches to.


    Generated by Claude Code

  2. claude commented on Aug 26, 2026

    @claude
    Contributor

    os-dev-report

    {
      "issue": 6235,
      "status": "done",
      "branch": "claude/issue-6235-mergedsort-wrap",
      "pr": "https://github.com/objectstack-ai/objectui/pull/6479",
      "premise_still_valid": true,
      "summary": "Route 1 as charted, one line of source: mergedSort's last branch now reads `|| (schema.table?.defaultSort ? [schema.table.defaultSort] : undefined)`, byte-identical (modulo indentation) to the non-grid fetch wrap, so both sites read verbatim the same expression at :862 and :1635. The shared sink is untouched — the 2026-08-22 ruling quoted from the file's own comment (measured at :850-854, not the cited ~:779): '⛔ must NOT be widened to accept a bare { field, order } — its input slot legitimately also carries $orderby's own Record<field, direction> map, in which { field: desc } is a perfectly legal ordering by a column literally named field, so the sink would have to GUESS. (Maintainer ruling 2026-08-22: Option A; Option B — widening the shared sink — rejected on the merits.)' FORK FENCE: no fork found. Censused by shape, not identifier — both in-tree hosts feed the slot straight into ListView (app-shell fullSchema `sort: viewDef.sort ?? listSchema.sort`, and Studio's renderStudioGridList `...listSchema`), and a grep for bare-object reads of a delegated sort returns nothing on this surface. No reader DEPENDS on the bare object; all three silently discard it (ListView.parseSortConfig and ObjectGrid.parseSchemaSort return [] from a byte-identical `Array.isArray(sort) ? sort : []` opening; convertSortToQueryParams returns undefined). LINE NUMBERS — my measurement confirms the PM's and supersedes the card's: mergedSort :1602 (card said :1522), ruling :850-854 (card said :779-784), non-grid wrap :862 (card said :791), grid path :1455 (card said :1375). THE OBLIGATION ATTACHES TO :1593-1596 — the sentence 'Both land on list-view's own filter / sort keys below, so a canonical value arrives in the slot that already matches its shape', true of the filter chain and of the sort chain's first three branches, false of its fourth. That paragraph now states the arity, cites the ruling and names the readers. TWO PRE-EXISTING ASSERTIONS CHANGED, reported rather than smoothed: (1) canonicalTableKeys 'still hands over table.defaultSort alone' asserted the BARE object — it pins #5102's alias-still-honoured half, so it moved to the wrapped shape, the same transition the $orderby assertion directly above it already made for the fetch path; (2) the filterSources ghost fed an ARRAY into defaultSort, which the zod mirror REFUSES (packages/types/src/zod/objectql.zod.ts:130 is z.object({field, order}), not a union) — verbatim lowering necessarily changes that input's outcome, so the cell was rewritten to record that the wrap does not rescue invalid metadata and the sink refuses it rather than guessing, carrying a ⛔ against an Array.isArray flatten. CLAUSE ②, stated not inferred: NO public surface change — no src/index.ts, no packages/types/, no .d.ts, and no member added to any type the entry already exports by name; the ENTIRE non-comment source diff is the one line above, a local const inside a function body. No baseline line was added to any shrink-only ledger; none was implicated. No test was skipped, disabled or quarantined; the OBJECTUI_VITEST_GUARD refusal of `pnpm --filter … exec vitest` was obeyed and every run uses the sanctioned repo-root invocation.",
      "tests": "All readings at 185af3555 (git status clean, 0 modified). Exit codes captured before any pipe; verdict lines quoted from the gates themselves. FULL SUITE: `pnpm exec vitest run packages/plugin-view/` → 'Test Files 24 passed (24)' / 'Tests 235 passed (235)', exit 0. TYPECHECK: `pnpm --filter @object-ui/plugin-view run type-check` → exit 0 (tsc --noEmit && tsc -p tsconfig.test.json); NOT-MEASURED trap checked — both edited pin files confirmed IN the program via `tsc -p tsconfig.test.json --listFiles` (grep -c = 1 each), so 'typecheck clean' genuinely covers them. LINT: ran the FULL repo farm, not a narrowing — `pnpm lint` (turbo run lint) → 'Tasks: 47 successful, 47 total', exit 0 in 5m56s, so no narrowing argument is owed. GATES: check-control-bytes → '✅ check-control-bytes: OK (scanned 5350 tracked text file(s); skipped 85 binary).'; check-vi-mock-specifiers → '✅ check-vi-mock-specifiers: OK (3810 tracked source file(s), 2112 test-named; 449 carry a mock; …)'; changeset:check → '✅ All workspace packages are in the changeset fixed group.' + '✅ No changeset declares a major bump.'; check-changeset-presence → '✅ 3 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/6235-mergedsort-wrap.md.' Self-scanned the changed files for raw control bytes with grep -naP '[\\x00-\\x08\\x0b\\x0c\\x0e-\\x1f\\x7f]' → no matches. REVERSE VERIFICATION: direction predicted BEFORE the run — 4 cells red, every control green. NO REBUILD WAS NEEDED, and that is verified rather than assumed: the subject is a relative source import (../ObjectView) and vitest.config.mts:255-268 aliases @object-ui/core and @object-ui/plugin-grid to each package's src, so no dist/ sits in the resolution path and a stale build cannot make a reverted source read green. (The dependency closure was still built first — `pnpm --filter '@object-ui/plugin-view^...' build`, exit 0 — before any judgement.) MUTATION CONFIRMED ON DISK, not by an editor exit code: ObjectView.tsx restored from origin/main under a trap holding ABSOLUTE paths (trap '<restore>' EXIT INT TERM), then grepped for the INJECTED text (bare `|| schema.table?.defaultSort;` → 1) AND the DELETED text (the wrapped form at mergedSort → 0, objectui#6235 comment → 0), plus a blob-hash change fa765c83… → 0ed68e6a… with explicit no-op and empty-hash guards that abort the round. RESULT — exactly as predicted: 'Test Files 2 failed | 2 passed (4)' / 'Tests 4 failed | 52 passed (56)'. The 4 red were the 4 predicted: 'WRAPS a bare-object table.defaultSort into the SortConfig[] the slot declares'; 'hands the delegated slot a sort its READERS can actually parse' (the symptom cell — 'AssertionError: expected undefined to deeply equal { created: asc }', i.e. the silently unsorted list); 'does not rescue an ARRAY in defaultSort — the arity the schema refuses' (reverse direction: main forwards [{…}], not [[{…}]]); and canonicalTableKeys 'still hands over table.defaultSort alone — WRAPPED'. ISOLATION HELD: ObjectView.sortSink.test.tsx and ObjectView.namedViewSortArity.test.tsx stayed FULLY green through the ablation, so the guard is isolated rather than blanket-broken, and every control cell stayed green. RESTORE PROVEN BY BYTES, not by the trap firing: worktree blob == HEAD blob (fa765c8316c72a73b89f0b83f9ce9eba5cbe027a both sides), empty `git diff HEAD`, clean `git status`; restore used `git checkout HEAD -- <abs path>`, never a bare `git checkout --`. Restored tree re-run green: 'Test Files 4 passed (4)' / 'Tests 56 passed (56)'. DEGENERATE-CONTROL DISCLOSURE: three cells pass in BOTH states and are labelled in-file as controls with the future wrong shape each guards — 'forwards a canonical table.sort array UNWRAPPED' (guards wrapping the chain's result instead of its final branch → [[{field,order}]], verbatim #5270's failure); 'keeps the canonical table.sort ahead of the wrapped legacy default' (guards precedence moving); 'forwards nothing when the view declares neither' (guards an unconditional [schema.table.defaultSort] forwarding [undefined] — truthy, one entry long, a sort that does not exist). One of these controls did real work: it caught my first draft double-wrapping an already-array value, which is why the array case was re-examined and moved onto table.sort, the slot that legitimately carries arrays.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code


    Generated by Claude Code

  3. os-support-ai commented on Aug 26, 2026

    @os-support-ai
    Collaborator

    ACCEPT — PR #6479, head 185af3555. Verified against the tree.

    Route 1, one line

    The entire non-comment source diff is:

    -    || schema.table?.defaultSort;
    +    || (schema.table?.defaultSort ? [schema.table.defaultSort] : undefined);
    

    The shared sink is untouched, which is what the 2026-08-22 ruling required. The dev quoted it from the file's own comment rather than paraphrasing, and the reason it forbids widening is the load-bearing part: the sink's input slot also carries $orderby's Record<field, direction> map, in which { field: desc } is a legal ordering by a column literally named field — so a widened sink would have to guess. That is why this belongs in the caller.

    Line numbers — the dev's measurement supersedes both the card's and mine

    card measured
    mergedSort :1522 :1602
    the ruling comment :779-784 :850-854
    non-grid wrap :791 :862
    grid path :1375 :1455

    My own reading of :1602 matched. The obligation attaches to :1593-1596, not the line the card named — the sentence claiming a canonical value arrives in the slot, which was true of the filter chain and of the sort chain's first three branches and false of its fourth. That paragraph now states the arity, cites the ruling and names the readers.

    The two pre-existing assertions — checked, and neither was weakened

    This is where a gate gets quietly loosened, so I read both rather than taking the report's word.

    canonicalTableKeys — "still hands over table.defaultSort alone". .toEqual({ field, order }) became .toEqual([{ field, order }]). Still an exact-value assertion, identical strictness, new shape. The reason sits beside the assertion, not only in the PR body, and it records the uncomfortable part: that cell was green while the value it forwarded could not be read by anything downstream — ListView.parseSortConfig and ObjectGrid.parseSchemaSort both return [] for a non-array, and the shared sink returns undefined.

    filterSources — "forwards the sort alongside it". The removed cell's own pre-existing comment already admitted the problem: "the remaining as any is about ARITY, not spelling — defaultSort is declared a single { field, order } and this passes an array on purpose". I confirmed the schema refuses it: packages/types/src/zod/objectql.zod.ts:130 is z.object({ field, order }).optional() — not a union. A fixture feeding a shape the schema rejects cannot survive verbatim lowering unchanged. The rewrite records that the wrap does not rescue invalid metadata and carries a ⛔ against an Array.isArray flatten — tightening the record, not loosening it.

    The controls did real work

    Three cells pass in both states and are labelled in-file with the future wrong shape each guards — and one of them caught the dev's own first draft double-wrapping an already-array value. That is the whole argument for keeping same-in-both-directions controls: "forwards a canonical table.sort array UNWRAPPED" is what stands between this fix and [[{field,order}]], which is verbatim #5270's failure mode.

    The reverse verification predicted 4 red / all controls green and observed exactly that, including the symptom cell (expected undefined to deeply equal { created: asc } — the silently unsorted list). ObjectView.sortSink and ObjectView.namedViewSortArity stayed fully green through the ablation, so the guard is isolated rather than blanket-broken.

    Fork fence — no fork, and it was censused by shape

    Both in-tree hosts feed the slot straight into ListView, and all three readers silently discard the bare object rather than depending on it. No out-of-tree dependency surfaced. Correctly reported as "no fork" rather than as an absence of evidence.

    Clause-②: no public surface change — **/src/index.ts and packages/types/** diffs are both empty, confirmed by measurement.

    Lint was not narrowed: the full repo farm ran, 47 tasks, exit 0 — so no narrowing argument is owed.

    Enqueue held only on CI: 29 checks, 0 failed, 7 pending.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:dispatched

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions