fix(plugin-kanban): gate ObjectKanban's record query on the object definition - #6420
Merged
Merged
Conversation
…finition The board ran its fetch effect twice on every standalone mount: `objectDef` sat in that effect's dependency list while a separate effect resolved it, so the first query went out before the definition landed and carried no `$expand` at all, and a second, expanded one followed. The definition now gates the query rather than refining it afterwards — one query per mount, expanded the first time. The gate is on the read having SETTLED, not on the definition being truthy: an adapter with no `getObjectSchema`, and a read that throws, settle with nothing to report and the board falls through to an unexpanded query instead of waiting forever. Chosen on measurement (recorded in the PR body and the changeset): the first response never reached the screen in the regimes that matter, and a schema read is one small cached GET behind the discovery call `find` already awaits. Part of #6271 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
…ble fetch `awaitSchemaFetch` waited for a SECOND `find` call, because only the second carried the expand set — a synchronisation built on the defect #6271 removes. Same condition, spelled against the ordering that now holds: one call, issued after the definition settles. The comment records why one call is sufficient and what re-opens the vacuum if the gate is ever removed. Also drops a `.apply()` in the new pin's adapter stub (`prefer-spread`). Part of #6271 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 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 #6271
ObjectKanbanran its fetch effect twice on every standalone mount:objectDefsat in that effect's dependency list while a separate effect resolved it, so the first query went out before the definition landed —buildExpandFieldssaw no fields, and the query carried no$expandat all — and a second, expanded one followed.The definition now gates the query instead of refining it afterwards. One query per mount, expanded the first time.
The measurement the card was gated on
Triage named one deciding reading — how long schema resolution takes on a cold boot — and forbade picking an arm before taking it. Three measurements, all against real code.
M1 — real
ObjectStackAdapter, realMetadataCache, real loopback HTTP.connect()+ the metadata GET, at two transport latencies:getObjectSchema(connect + meta GET)MetadataCachehit, n=20 avg)So a schema read is one small GET behind the same discovery call
findalready awaits, and every reader after the first — inside a 5-minute TTL — pays 0.01ms and no request. Concurrent readers coalesce onto one in-flight request rather than adding a second.M2 — the mount timeline of the real component, instrumented adapter, DOM polled every 2ms, per-query payloads made distinguishable so the DOM says which round trip is on screen. Before the change:
⭐ The finding that decided it: wherever the schema resolves no slower than the row query, the first response never reaches the screen. The definition lands, the effect re-runs, its cleanup flips⚠️ framed this as "a redundant request traded against a slower first paint"; in the measured regimes there was no trade to make.
isMountedfalse, and the unexpanded rows are discarded on arrival. That round trip bought no earlier paint — it bought a query whose answer was thrown away. The card'sThe two regimes where it did paint early need the metadata GET to be 5–10x slower than a 100-row expanded query, and what they paint is not a graceful partial board: with no
columnsauthored, lanes are derived from the DATA's distinct values, so the board shows one raw-valued lane (qualified) and re-lanes wholesale ~100–190ms later when the declared options arrive.M2 again, after the change — same harness, same profiles:
Faster in four of five profiles (one less query competing, and nothing left to overwrite); +5.9ms in the pathological one, where the early paint it gives up was a raw-lane board.
M3 — the hosted paths. Rendering
object-viewandlist-viewwith a kanban view: both hosts read the definition on the same adapter (2getObjectSchemacalls, 1.1ms apart underobject-view), which on the real adapter is one request with the second reader coalesced or warm. Both also hand the board its rows viadata, so the board's internal fetch — this effect — never runs there at all. The double fetch is specific to the standalone board, which is exactly the mount the card measured.Verdict: schema resolution is fast and shared ⇒ gate the first fetch (triage's arm A).
What changed
objectDefbeing truthy. Those differ for exactly the boards least able to report it: an adapter that exposes nogetObjectSchema, and a read that throws. Both now settle with nothing to report, and the board falls through to an unexpanded query — under a truthy-value gate both would wait forever and render empty, with no error and no request.data,bind, inlineschema.data) are untouched: they never ran this effect, and they still read the definition for lane titles and labels.Ghost-assertion guard — both directions, ablated
Ablation A — the pin against unmodified
origin/main. Nothing mutated butObjectKanban.tsx, restored to the base blob2c99db8(verified: disk hash equals the base blob; marker countobjectDefReady5 → 0,setObjectDef0 → 2). Predicted RED on the two discriminators. Observed 3 failed | 2 passed:One failure past the prediction, reported as observed: the rejection-path test also carries an ordering assertion, which is a discriminator rather than a control — its control half (the board still queries, once, and paints) passed on
main. Restored by hash afterwards:git diff HEADempty, disk hash equals the HEAD blob.Ablation B — proving the controls are not decoration. The guard's rule is that a query-count assertion which would pass with ZERO fetches is not a pin. So the mis-fix was implemented:
if (!objectDefReady)→if (!objectDef)(mutation confirmed on disk — marker counts 1 → 0 and 0 → 1, file hash differs from the HEAD blob, and the run aborts if it does not). Predicted: the two no-definition controls go red, the rest stay green. Observed exactly that — 2 failed | 3 passed, both failures beingexpected 'Open0…' to contain 'Q3 renewal'after a 1020mswaitFortimeout: the board never fetched at all. Restored inside anEXIT/INT/TERMtrap with absolute paths; verified by hash,git diff HEADempty.The coupling, recorded in the code
Triage asked that whichever arm lands notes the coupling beside the
isOpaqueIdsuppression, not only here. It does — a block aboveOPAQUE_ID_REsays what changed and what did not. Half of what that predicate hid was this fetch ordering, and that half is gone. It is not thereby redundant: unexpanded rows still reach the card path from sources the gate does not sit in front of, and the first of those is measured rather than assumed —ObjectViewhosts this board and its own query goes out as{ $top: 100 }, so every card in that path is built from raw ids (see the finding below). Author-suppliedbind/inline rows and backends that decline an expansion are the others.Verification
All on
88e10b836(the final commit), each command's own verdict line quoted:pnpm exec vitest run packages/plugin-kanban/→Test Files 18 passed (18)/Tests 103 passed (103)pnpm --filter '@object-ui/plugin-kanban' run type-check(tsc --noEmit && tsc -p tsconfig.test.json) → clean, after building the dependency closure (pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-kanban^...' build; without it the run reports 40 phantomTS2307s from missingdist/*.d.ts). Both edited test files confirmed present in the program via--listFiles, so that green covers them.ObjectView.kanbanConditionalFormatting,ObjectView.tableColumnsForwarding,ListView.test,ElementDataSourceGate,react-page-state,react-page-scope→Test Files 6 passed (6)/Tests 206 passed (206)eslint .inpackages/plugin-kanban(the exact unitturbo run lintruns for it) → 34 files collected, 0 errors.ObjectKanban.tsx's warning profile is byte-identical to base: 33 warnings, same five rule ids, measured by linting the base blob through--stdin-filename.node scripts/check-control-bytes.mjs→OK (scanned 5306 tracked text file(s));check-vi-mock-specifiers,check-package-self-import,check-phantom-dependencies,check-changeset-no-major,check-changeset-presence,check-changeset-fixed→ all✅.Lint narrowing, declared: the repo-wide scan is CI's. What ran here is this package's complete lint unit; the population and the 34-file count come from eslint's own config resolution (
--format json), andeslint.config.jsdeclares noparserOptions.project/projectService— zero hits — so no rule reads cross-file type information and a diff confined to this package cannot move any untouched file's verdict.One fixture moved with the fix
expandableFamily.identity-5874.test.tsxsynchronised on the defect: itsawaitSchemaFetchwaited for a secondfindcall, because only the second carried the expand set. Same condition, re-spelled against the ordering that now holds (one call, issued after the definition settles), with a comment recording why one call is sufficient and what re-opens the vacuum if the gate is ever removed. Its assertions are unchanged.Out of scope — filed, not fixed
$expand— the ref it reads the schema from is empty on the one run it makes #6419 —ObjectView's non-grid fetch never injects$expandat all: it reads the schema through a ref that is still empty on the one run its effect makes. Measured in the same run (find[0] params={"$top":100}). Different package, and it changes what six child views receive, so it is not a rider on this PR.Generated by Claude Code