refactor(core): rowHeightToDensityMode abstains for an off-spec rowHeight, matching the spec bridge (#4440) - #4447
Merged
Conversation
…ight (#4440) `@object-ui/core` coerced any unknown `rowHeight` to `comfortable`, while `@object-ui/react`'s spec bridge declined to answer for the same string after #4352 (PR #4439) — one metadata-driven system holding two answers for one input. AGENTS.md #0.1 decides which survives: the strict one. An off-spec `rowHeight` now yields no density, and the caller's own "nothing was said" default applies. `ListView`, the single in-repo caller, lands that abstain on `'compact'` — the default it already used for an ABSENT `rowHeight`, and the one `ObjectGrid` picks independently. It deliberately does NOT let `undefined` reach `useDensityMode`, whose parameter default is `'comfortable'`: that compiles and would rebuild the retired coercion one frame lower. Also closed: the lookup guarded membership with `in`, which walks the prototype chain, so `rowHeight: 'toString'` returned `Object.prototype.toString` — a function — from something typed `DensityMode`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
ACCEPT — PM 复核 (session
Flipping ready + arming auto-merge. Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 12, 2026 09:03
This was referenced Aug 12, 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 #4440
Implements the delegated ruling on #4440:
@object-ui/core'srowHeightToDensityModestops coercing an unknownrowHeighttocomfortableand gives the same no-mapping answer@object-ui/react's spec bridge has given since #4352 (PR #4439). One system, one answer. AGENTS.md #0.1 — one strict contract beats N dialects, and a badrowHeightgets fixed at the producer.1. Empirical check — run FIRST, as the ruling required
The card's own caution was that this fallback sits on a live render path, unlike #4352's unreachable keys. So the callers were enumerated and the rendered outcome measured on both sides before anything was edited.
Callers of
rowHeightToDensityMode— the complete setpackages/plugin-list/src/ListView.tsx:1038useDensityMode, whosedensity.modedrives the toolbar button, the row padding/font, and therowHeightforwarded to every child viewExactly one in-repo caller (the card cited
ListView.tsx:877; the line has moved, the caller has not). Plus out-of-tree hosts:packages/core/src/index.ts:72re-exports the whole module, so this is a published surface.Checked and not callers:
app-shell/ObjectView.tsximportsDENSITY_MODE_TO_ROW_HEIGHT(the write direction only);plugin-grid/ObjectGrid.tsxreadsschema.rowHeightdirectly and never routes through this function.What can put an off-spec value into that caller
app-shell/ObjectView.tsx:1545(viewDef.rowHeight ?? listSchema.rowHeight),plugin-view/ObjectView.tsx:1001ObjectView.tsx:1572Record< DensityMode, RowHeight >codomaindensityModefold,normalize-list-view.ts:221ListView.tsx:1698, density forwarded to childrencompact/medium/tallIs any off-spec value actually authored? Sweep re-run at today's HEADs
objectui@d7f3e308b— everyts/tsx/json/yml/yamlrowHeighthits, all classified: i18n labels for the key name (9 locales), the booleanuserActions.rowHeightflag, pixel-number props on unrelated widgets (gantt / VirtualGrid / dashboard), the typed tables, pass-throughsobjectstack@5d24f4b94— showcase, CRM,packages/spec,platform-objectscompact/medium/short/tallhotcrmlead.view.ts:182authorsrowHeight: 'medium')Independently confirms PR #4439's three-repo sweep, at newer HEADs.
TODAY vs AFTER, measured on the live render path
Not argued from the mapping — rendered.
packages/plugin-list/src/__tests__/ListView.density.offspec.test.tsxrenders the realListViewand reads the density button'saria-label, which isdensity.modeverbatim.schema.rowHeightcompact/shortmediumtall/extra_tallcomfortable,spacious,small,large,gargantuan42(non-string, from a stored view)toString/constructor[Function toString]What the changed rows mean on screen: density
comfortable→compactis row height 40px → 32px, paddingpy-2 px-3→py-1 px-2, fonttext-sm→text-xs(react/src/hooks/useDensityMode.ts:46-62), and therowHeightforwarded to child views goes'medium'→'compact'.Reading: reachable code path, unreachable input class. The delta is real and is reported as the ruling asked, but no metadata in existence reaches it. It moves the caller onto the bridge's answer, which is the intended direction.
2. Where the abstain lands — the measurement that changed the implementation
undefinedhas to land somewhere, and the obvious landing is a trap.rowHeightToDensityMode(schema.rowHeight) ?? 'compact'— the consumer's own "nothing was said" default. It was already there in the branch this replaces (if (schema.rowHeight) … return 'compact'), andObjectGridindependently picks the same one (schema.rowHeight ?? 'compact').undefinedflow intouseDensityMode. Its signature isinitialMode: DensityModeValue = 'comfortable', soundefinedcompiles and lands oncomfortable— rebuilding the exact coercion this PR retires, one frame lower, while every unit test of the mapping goes green. Reverse verification B below measures this: it is invisible to core's own pins and to the agreement pin, and only the render pin catches it.3. Ruling condition 2 — the
Record< RowHeight, … >typingAlready present, nothing added.
normalize-list-view.ts:47has declaredROW_HEIGHT_TO_DENSITY_MODE: Record< RowHeight, DensityMode >withRowHeightre-exported from@objectstack/spec/uisince objectstack#4115. The upstream-enum-growth build-failure property #4439 bought for the react twin was already held here. Measured, not assumed — and the honest answer to a condition can be "it is already true".4. Ruling condition 3 — the agreement pin, and why it lives where it does
packages/react/src/spec-bridge/__tests__/RowHeightDensityAgreement.test.ts(new file).Placement is decided by the dependency direction, and core cannot host it.
@object-ui/reactdepends on@object-ui/core, so this package can see both surfaces; core importing react would invert the graph. The pin therefore imports core's function by its published specifier (@object-ui/core, a declared dependency —check:phantom-depsgreen) and reaches the bridge throughSpecBridge.transformListView, whose parameter isany— the untyped boundary a host's stored JSON crosses, and since #4352 the only way an off-specrowHeightcan enter the bridge at all. A new file rather than an edit toSpecBridge.test.ts, so it cannot collide with in-flight work on that surface.It asserts both surfaces agree for the five spec values (controls) and for the off-spec class, plus the invariant itself (
core(x) === bridge(x)for every off-specx) so a future fallback re-added to either side breaks it even if re-added to both differently.One honest exclusion, stated in the file:
Object.prototypemember names are deliberately not in the pin. Core abstains for them now; the bridge still indexes its table with an unguarded key and hands backObject.prototype.toStringas a density. That is a different defect in source outside this card's surface — filed as #4442, with the comment pointing there.5. A latent defect the empirical check surfaced
The guard was
rowHeight in ROW_HEIGHT_TO_DENSITY_MODE, andinwalks the prototype chain. SorowHeightToDensityMode('toString')returnedObject.prototype.toString— a function, from a signature that saysDensityMode. Caught by a red-first case, not by reasoning:Now an own-property check, matching the repo convention (
Object.prototype.hasOwnProperty.call, as incore/src/utils/freeze-schema.ts:135).6. Red-first, captured against the unmodified source
All three pins were written and run before the source was touched:
The 50 green are the controls: the five spec mappings on each of the three surfaces, plus absent-rowHeight. After the change: 70 passed (70).
7. Verification (local, all green)
pnpm --filter '@object-ui/plugin-list^...' build— exit 0, before any judging.packages/core/ packages/react/ packages/plugin-list/— 154 files, 2772 tests passed.tsc --noEmitandtsc -p tsconfig.test.json, six runs, all exit 0..d.ts, not a stale one:turbo run type-check --concurrency=2repo-wide — 78 successful, 78 total. Repo-wide is a superset of the...@object-ui/coreprefix filter (downstream consumers), which is the direction a return-type widening breaks.eslinton all five touched files — 0 errors. 168 warnings, every one pre-existingno-explicit-any/exhaustive-depsinListView.tsx; none in the edited region (1033-1048), none in the three test files.check:control-bytes— OK, 4148 tracked text files; plus a self-scan of the touched files across the wider control-byte range: no hits.check:phantom-deps— OK.check:spec-symbols— OK.check-changeset-presence— 1 changeset declared.check-changeset-no-major— OK.8. Reverse verification (both directions predicted before running)
Taken out with
git checkout origin/main -- FILE, restored withgit checkout HEAD -- FILE— nevergit stash.A. Restore the coercion in core → the pins red. Predicted the normal direction (the assertions read a value the restored branch produces, not a count a gate reports). Measured 21 failed | 49 passed, on all three surfaces:
21, not the 20 of the red-first run, and the extra one is worth naming rather than rounding off:
renders an absent rowHeight as Compactalso fails here. This state is a mixture — old core, new caller — and the old function coercedundefinedtocomfortabletoo, which is precisely why the caller needed itsif (schema.rowHeight)guard. That the guard becomes unnecessary once the function can abstain is evidence for the change, not a defect in the pin.B. Restore only the CALLER, keeping core fixed → only the render pins red. Predicted: core's unit pins and the agreement pin stay green (the mapping is correct), the six off-spec render cases go red because
undefinedreachesuseDensityMode's= 'comfortable'default, and the absent-rowHeight control stays green (the old caller's explicitreturn 'compact'). Measured exactly that — 6 failed | 64 passed, one file red:This is the measurement that makes the render pin load-bearing rather than a duplicate of the unit pin: the §2 trap is green on every mapping-level assertion and red only here.
Controls green in both directions, and after restoring:
git statusclean,git diff HEADempty,packages/core/ packages/react/ packages/plugin-list/back to 154 files / 2772 tests passed.9. Changeset grading — measured
.changeset/rowheight-density-one-answer-4440.md:'@object-ui/core': minor,'@object-ui/plugin-list': minor. Nevermajor, per the version-alignment rule.The measurement, run as part of reverse verification: unlike #4439 — where the emitted
.d.tswas byte-identical andminorrested on runtime behaviour alone — here the published type itself moves:The function is re-exported by
packages/core/src/index.ts:72, so it is published surface, and a host assigning the result straight into aDensityModemust now say what an off-spec row height means to it.plugin-listis graded alongside it because its rendered output changes for the same input class (§1).patchwould only have been defensible if nothing observable changed; two things do.10. Surface discipline and out-of-scope findings
Touched:
packages/core/src/utils/normalize-list-view.ts+ its test,packages/plugin-list/src/ListView.tsx+ one new test, one new test inpackages/react/src/spec-bridge/__tests__/, and the changeset. Nothing inapp-shell/console/ai/**,eslint-rules/**, root config, or the #4425 sweep suite. No bridge source was edited.Filed unassigned, not fixed here:
mapDensityindexesROW_HEIGHT_TO_DENSITYwith an unguarded key, sorowHeight: 'toString'returns a function as the density. The same hole core had; its fix lives in the surface this card was told to leave alone, and extending this PR's agreement pin to prototype keys is the natural test half of it.medium, the third answer to the question #4440 unified #4443 (finding) — a standaloneObjectGridstill renders an off-specrowHeightasmediumvia its styling ternary's terminalelse, while resolving an ABSENT one tocompact: the third answer to the question this PR unified, on a surface finding(core): rowHeightToDensityMode coerces an off-spec rowHeight tocomfortable, the opposite of what the spec bridge now does with the same input #4440 does not name.Generated by Claude Code