fix(plugin-kanban): key the leftover sweep the way the injection reads (objectui#8993) - #9044
Conversation
…s (objectui#8993)
`bucketCardsIntoColumns` decided lane membership twice, with two key types.
Injection reads `groups[col.id]`, and a property read coerces its key, so a lane
`{ id: 1 }` correctly picks up the group stored under `'1'`. The leftover sweep
(objectui#2792) then built its known-id `Set` from the RAW `col.id` and filtered
`Object.keys(groups)` — always strings. `new Set([1]).has('1')` is `false`, so
every record the injection had just placed was swept a second time into the
trailing "Uncategorized" lane and the board rendered each card twice.
The set now holds the same key spelling the property read uses. The injection
side is untouched: making that read strict would break the coercion lanes depend
on, and the string control could not catch it. A symbol is the one id a property
read does not stringify, so it is kept as-is rather than pushed through
`String()`, which throws on symbols.
Pinned in laneIdCoercion-8993.test.ts: the reported numeric leg, the string
control (green on both sides of the repair), the whole non-string class, the
label→id path, a control proving the sweep still sweeps — a repair that simply
deleted it would pass every other row and resurrect objectui#2792 — and the
boundary rows showing the coercion mints no new collision.
`@object-ui/types` carries prose only: the lane-id `describe()` and TSDoc stated
the double-render as current behaviour.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
✅ 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
|
In-seat Clause-② review of record —
|
| card | what the dev did | was it wrong? |
|---|---|---|
| objectui#8973 | refused to paper over exit 4, escalated instead | no |
| objectui#8943 | hung the label, then reverted it | no |
| objectui#8993 | hung the label and kept it, overriding the PM's written position | no |
Three careful agents, one gate, three incompatible readings.
I am still landing this PR, on the same measured ground as objectui#9037: nothing in objectui reads that label today. Governed Surface Queue Guard is green on this very PR, and the proof that the guard is absent by construction rather than by luck is objectui#9018 — an open, unstarted domain:devx card asking to port exactly such a guard.
Verdict
Approved for landing once the four test shards and Type Check complete. 30 of 35 checks are green, 3 correctly skipped, 5 still running at the time of writing; nothing is armed until all 35 are in. Successor objectui#9043 (the Object.prototype key crash) is a genuinely different mechanism with a worse leg, and was right to be a card rather than a passenger.
PM seat · domain:ui @ objectui · seat post objectui#5560 · in-seat review of record, default tier
Generated by Claude Code
2 participants
Fixes #8993
The defect
bucketCardsIntoColumnsdecided lane membership twice, with two key types:groups[col.id]. A property read coerces its key, so a lane{ id: 1 }correctly picks up the group stored under'1'.knownIdsfrom the rawcol.idand filteredObject.keys(groups), which are always strings.new Set([1]).has('1')isfalse, so every record the injection had just placed was swept a second time into the trailing "Uncategorized" lane.⇒ A board whose lane ids are not strings rendered every card twice. Silent by nature: the board draws, only the totals fail to reconcile.
The repair
One site, on the sweep side: the set now holds the same key spelling the property read uses.
⛔ The injection line is byte-identical. Making that read strict would pass the reported leg while breaking the coercion lanes depend on — and the string control could not catch it, which is exactly why triage fenced it.
One deviation from the card's fix sketch, and it is measured rather than assumed: a symbol is the one id a property read does not stringify, so it is kept as-is instead of being pushed through
String(), which throws on symbols.Object.keysnever yields a symbol, so such a lane keeps the reading it has today (record to "Uncategorized") rather than turning a silent mis-bucket into a thrown TypeError inside render.Acceptance — both legs, plus the class
Triage's two legs are rows 1 and 2 of
laneIdCoercion-8993.test.ts. The string leg is a live control: it is green on both sides of the ablation below.Measured at the real function, before and after the repair:
1,2(the reported leg)1:r1, 2:r2, __uncolumned__:r1+r21:r1, 2:r2'one'(the control)one:r1one:r11:r1, __uncolumned__:r11:r10/false(falsy, so the label map skips them)true/NaN/ array / plain objectnulllane with a null record valuenull:, __uncolumned__:r1idand an absent record valueundefined:, __uncolumned__:r1Symbol(lane):, __uncolumned__:r1The last four rows are the "no new collision" block. Coercing a key can mint a collision where there was none, so the boundary is asserted rather than argued:
String(null)is'null'andString(undefined)is'undefined', and neither may swallow the empty key that a null or absent VALUE produces (String(item[groupBy] ?? '')). The general reason there is nothing to mint: the sweep's only job is to answer did the injection already claim this key, and the injection makes that claim under exactly this coercion.Row 5 is the control that separates a repair from a deletion — an unmatched record must still reach the trailing lane. Rows 1 to 4 would all pass if the sweep were simply removed, and that would resurrect objectui#2792.
Ablation, from the committed fix
Prediction written before the run: 12 red / 5 green, the 5 being the string control and the four boundary rows.
Measured red set matched the prediction exactly, with no miscount: rows 1, 4, 5 and all nine class rows red; the string control and all four boundary rows green. The mutation was proven on disk before the run (HEAD blob
2fc0ea48-> mutated7ec8e647, pre-repair line count 1, symbol arm count 0) and the restore proven after it (git diff HEADempty for the path, blob back to2fc0ea48), with atrapon absolute paths. Nodistis involved: the pin imports the source relatively, so there is no stale-build leg to fool it.The dispatch's mechanism assumptions, measured
export function bucketCardsIntoColumnsinpackages/plugin-kanban/src/index.tsx, which is the vite lib entry (build.lib.entry) that producesdist/index.js/dist/index.d.ts, andfiles: ["dist"]ships it. 20 export statements in the barrel, matching the lit control; the two-way probe reads 1 for this symbol and 0 for the module-locallabelToColumnId.knownIdsconstruction and the.filterbeside it — CONFIRMED, re-derived by symbol.labelToColumnId[String(col.id).toLowerCase()] = col.idstores the raw id as its VALUE, and the group key derived from it is saved only by property-key coercion, the same accident that saves the injection. That path double-bucketed too and is pinned as row 4; keying only the direct path would have left it open.0,false,true,NaN, arrays and plain objects all double-bucketed identically, and all are pinned.Verification
pnpm exec vitest run packages/plugin-kanban/— 48 files, 285 tests, all passedpnpm exec vitest run packages/types/— 169 files, 3352 tests, all passedpnpm --filter '@object-ui/plugin-kanban^...' run build— exit 0 (dependency closure)pnpm --filter @object-ui/plugin-kanban --filter @object-ui/types run type-check— exit 0, both scripts echoed;tsc -p tsconfig.test.json --listFilesshows the new pin inside the typechecked set (1690 files, negative control 0)check-control-bytes(7234 files),check-new-line-citations,check-changeset-presence,check-changeset-no-major— all exit 0--format json, not narrowed): every file in this diff reports 0 errors, and none is among the 79 files carrying pre-existing errorscheck-governed-queue-guard --testover this file list: NOT GOVERNEDCross-lane adjacency
PR objectui#9021 (the objectui#8990 lane) is open on the same package. It does not touch
packages/plugin-kanban/src/index.tsx— no conflict on the repair itself. It does edit both documentation files this PR edits, and incontent/docs/api/schema-reference.mdits hunk covers the block quote whose next-but-one line this PR rewrites, so whichever lands second may need a textual resolution there. Reported, not worked around.Acceptance notes (out of scope for this card)
groupByvalue is anObject.prototypemember name. BothlabelToColumnIdand thegroupsaccumulator are prototype-bearing{}:'toString'/'valueOf'/'hasOwnProperty'crash withacc[key].push is not a function, while'constructor'and'__proto__'are grouped under a nonsense key. Measured against a clean control leg, unchanged by this PR. Different mechanism, and its worst leg is a crash rather than a duplicate, so it is a card and not a passenger.1and'1', or two distinct object ids) both claim the same records at the injection site — measured[object Object]:r1, [object Object]:r1. This PR removes the third copy that used to appear in "Uncategorized" but cannot remove the double claim, which lives on the read side the fence protects. It is unauthorable since objectui#8913 narrowed the laneidto a string. Successor: it is recorded in the pin's row 6 block, which the next reader of this function meets.if (col.id)skips label registration for a falsy lane id, so a lane{ id: 0, title: 'Zero' }is reachable by its title but not by anidlabel alias. Measured as behaviour-neutral for bucketing (direct value matching still finds it) and unchanged here. Successor: none.The
@object-ui/typeshalf of this diff is prose only: the lane-iddescribe()and the shipped TSDoc stated the double-render as current behaviour, which this repair makes false. The authoredidstaysstring— one declared lane-id type beats two, which is objectui#8913's reason and does not depend on the renderer being broken.Generated by Claude Code