test(plugin-kanban): split the contractEnvelope-6839 waits by expected outcome - #8533
Merged
os-justin merged 1 commit intoSep 8, 2026
Merged
Conversation
…d outcome The pin waited on the `'Negotiation'` column header and then read the cards synchronously. Measured here, that header is not a first-paint signal: the board reaches `KanbanImpl` through `React.lazy` behind a `Suspense`, so the header appears only when that chunk resolves — after `render`, after `find` is called, and after `find` settles. `KanbanImpl` mirrors its `columns` prop into `boardColumns` state, and both the header text and the cards draw from that mirror, so the header lands in whatever state the mirror was seeded with at its own mount. Chunk load and data commit are two independent races and nothing ordered them; when the chunk won, the reveal drew an empty list and the read saw 0. That is the `expected +0 to be 2` failure on shard 2/4. The four cases need opposite waits, so they no longer share one. The three positive arms wait FOR the rows, which is immune to which race won. The `records` refusal arm has no arrival to wait for, so it takes a settled read: `find` has answered, the board has drawn the card list itself (not merely the header), and React's queued work is flushed. Waiting on the list is what separates "the list is empty" from "the list is absent" — `cards()` answered `[]` for both. No timeout was raised, no case skipped, and `cardsThrough` is still called once per case and never inside a `waitFor` predicate (objectui#7802). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
os-justin
marked this pull request as ready for review
September 8, 2026 07:29
os-justin
enabled auto-merge
September 8, 2026 07:29
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. Which half objected:
📦 Bundle Size Report
Size Limits
|
os-justin
deleted the
claude/issue-8532-kanban-contract-envelope-rows-wait
branch
September 8, 2026 07:58
This was referenced Sep 8, 2026
Merged
This was referenced Sep 8, 2026
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 #8532
Test (shard 2/4)was red onmain, failing identically on PRs whose diffs cannot reachplugin-kanban(#8494, #8521) and stalling five queued PRs. Only the firstitinObjectKanban.contractEnvelope-6839.test.tsxfailed, always withexpected +0 to be 2.What the mechanism actually is
The brief's diagnosis — a mount signal read as a rows signal — is right in direction, but it names the wrong gate. Measured here,
'Negotiation'is not a first-paint signal either.ObjectKanbanreachesKanbanImplthroughReact.lazy(() => import('./KanbanImpl'))behind aSuspense(packages/plugin-kanban/src/index.tsx). Instrumented on this tree, the header is absent atrender, absent whenfindhas been called, and still absent whenfindhas settled; it appears only when that chunk resolves:KanbanImplthen mirrors itscolumnsprop intoboardColumnsstate and re-syncs through auseEffect(KanbanImpl.tsx), and both the header text and the cards are drawn from that mirror. So the header lands in whatever state the mirror was seeded with at its own mount.Chunk load and data commit are therefore two independent races, and nothing in the helper ordered them. When the data commit wins, the reveal carries the rows and the read sees 2 — every local run, where the chunk is cold. When the chunk wins, the reveal draws the column with an empty list and the synchronous read sees 0.
cards()answered[]both when the list was absent and when it was empty, so nothing distinguished the two.The repair
The four cases need opposite waits, so they no longer share one:
data, bare array,valuewaitFor(() => expect(cards()).toHaveLength(2))records(refusal)findhas answered, the board has drawn the card list itself, and React's queued work is flushedWaiting on the list rather than the header is the part that matters for the refusal arm: it is the very node
cards()reads, and the board draws it on every arm (a zero-card column fills it with a dashed placeholder that is not alistitem), so an empty list is no longer indistinguishable from a missing one.No timeout was raised, no case skipped or quarantined, and
cardsThroughis still called once per case and never inside awaitForpredicate (objectui#7802). The predicates that are now insidewaitForare purescreenreads — they mount nothing, which is exactly the propertycardsThroughdoes not have.Ablation — reversed, because the subject is a test
A green run proves nothing here, so the repair was checked against the production behaviour it pins.
@object-ui/coreis aliased topackages/core/srcinvitest.config.mts, so the mutation reaches the runner from source with nodistin the resolution path. Each leg proved the mutation on disk in both directions and restored by state (git diff HEADempty andgit hash-objectequal togit rev-parse HEAD:PATH), under atrap ... EXIT INT TERMwith absolute paths.extractRecords'dataarmdatawent red:the declared rows member must still draw both cards: expected [] to have a length of 2 but got +0valuearmvaluewent redrecordsrefusal still holds at 0The refusal case stayed green through both ablations, and each positive ablation reddened exactly one case. The failure message is the case's own
becausestring, carried into thewaitFortimeout.Reproduction — declared honestly
I could not reproduce the original failure locally. The file is green cold, green with the lazy chunk pre-warmed, green with a deliberately expensive first render (60 columns / 900 rows), and green 8/8 under real contention (six
niced CPU hogs, load average 6.07 on 4 cores). Locally the cold dynamic import always loses the race to the data path, which is precisely why the reveal commit carries the rows here and need not on a runner whose transform cache is warm and whose scheduler is contended.The repair does not rest on that reproduction: it removes the dependence on which race won, rather than widening the window on the same race.
Verification
pnpm exec vitest run packages/plugin-kanban/— 33 files, 214 tests, all passingpnpm --filter '@object-ui/plugin-kanban^...' buildthenpnpm --filter '@object-ui/plugin-kanban' type-check— exit 0;tsc -p tsconfig.test.json --listFilesconfirms the changed test file is in the program (1669 files), so this is measured coverage rather than assumedcheck:control-bytes,check:vi-mock-specifiers,check:vi-mock-inherit,check-changeset-presence,check-changeset-no-major,check-changeset-fixed,check-changeset-overwrite,check-unreferenced-sources— all exit 0check-governed-queue-guard --test— NOT GOVERNED, ordinary routeno-irregular-whitespaceskipStrings: truegap) — 0 hits, with a lit control that firedDeclared narrowing: lint was run on the changed file only, not repo-wide.
eslint --print-configreports noparserOptions.projectand noprojectService, so type-aware linting is not enabled and a one-file diff cannot move the verdict on any untouched file;--format jsonreports 1 file linted, 0 errors, 0 warnings. Repo-wide lint belongs to CI.Changeset: empty frontmatter — test-only, no published source changed.
Left in draft for contract review; auto-merge not armed.
🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code