Uh oh!
There was an error while loading. Please reload this page.
fix(non-grid,i18n,react): correct the row ceiling changeset, its pins and their rationale comments - #7517
Merged
Conversation
…e comments Post-merge correctives to #7210's platform row ceiling (#7507), from the Clause-② review that ran as an audit after the PR landed ungated. - Changeset: the four view packages carry the behaviour break, so they move from `patch` to `minor`; the example footnote is replaced with the copy that actually renders (measured: no thousands separators on either the provider or the provider-less path); `NonGridCeilingResult` joins the export list. - Map and calendar pins now grade the cap they are named for. Both were green under a mutation that drew 2,001 rows, because they asserted `$top` and the footnote and never the count that reached the view. - `ObjectCalendar`'s external-`data` sync clears `rowCeiling`, the one `setData` path that did not. Latent today, pinned now. - Comment-only: the false "the only package all four already depend on" rationale on `@object-ui/react`'s entry (all four also depend on `core`, `components` and `types` — the real reason was the round's barrel fence, and the home is an open decision on #7508), and the stale "~1 KB of headroom" note in the ten locale packs, which named a chunk that no longer holds them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMrWaQw3XS5DxTHxp4yRyC
…ocblocks All four predicted a mechanism that does not happen. Measured on this branch, each ablation proven on disk and restored byte-identically: - gantt, tree, calendar, map: deleting `$top: NON_GRID_ROW_CEILING_TOP` goes red at the `$top` ASSERTION and nowhere else. Not at the footnote (gantt, calendar, map) and not at the row count (tree): an adapter with no `$top` answers with the whole filtered set, `applyNonGridRowCeiling` slices it to the ceiling from the rows in hand, and both the drawn count and the note stay correct. The probe row bounds the response; it is not the detector's only input once an unbounded response has arrived. - calendar and map additionally record the second ablation their new count assertions exist for: handing the view the raw response instead of the capped rows is red at the count, 2001 against 2000 — the mutation that left both files green at 4/4 before this branch. A pin whose docblock names the wrong discriminator is worse than one that names none: the next reader trusts it while deciding what an edit may safely break. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMrWaQw3XS5DxTHxp4yRyC
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
|
os-project-manager
marked this pull request as ready for review
September 3, 2026 16:29
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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#7507
Six of the seven post-merge correctives the Clause-② audit of #7391 found (audit comment 5527977938). The seventh — the five symbols published on
@object-ui/react's sole entry — is a maintainer decision on #7508 and is not touched here: this branch adds, moves, renames and retires zero exported symbols. Clause-② remains no.Verified at
a221c9288: 170 test files / 1687 tests green (plugin-calendar,plugin-map,plugin-gantt,plugin-tree,i18n, and the react ceiling unit), plustype-checkgreen for all six touched packages via turbo (21 tasks, deps built first).What was re-measured, and what it said
Nothing below was taken on the audit's say-so. Every claim was re-derived on this branch first; where an ablation is quoted, the mutation was proven on disk by grepping for the injected marker and the removed text, and the restore was proven byte-identical against the
HEADblob hash. Vitest resolves every@object-ui/*specifier to that package'ssrcthrough the root config's alias table, so these ablations need no rebuild — there is nodistin the resolution path to go stale.Baseline, before any change: 5 files / 16 tests green.
1. Changeset level and copy —
.changeset/7210-non-grid-row-ceiling.mdThe four view packages move
patch→minor. They carry the behaviour break (a result set above 2000 rows is no longer fully drawn); the fixed group makes the released version identical either way, but the per-package CHANGELOG would have filed a break under "Patch Changes".The example footnote is replaced with the copy that actually renders. Measured, not copied: a throwaway probe rendered
NonGridRowCeilingNotethree ways and asserted a deliberately wrong string so the run could not pass silently — 3 failed, 3 printed values:(delimiters shown as brackets; the probe printed them as angle runs.)
No separators on either path. The i18next instance declares no
format(packages/i18n/src/i18n.ts) and the provider-less fallback interpolates throughString(v)(fallbackInterpolation.ts). The probe file was deleted.NonGridCeilingResultjoins the export list. It is already exported (packages/react/src/index.ts,export type { NonGridCeilingResult }) — listing it is documentation of a published symbol, not a new export.2. The map and calendar pins now grade the cap they are named for
Audit finding reproduced first. With
setData(capped.rows)replaced bysetData((result as any).data ?? capped.rows)in both components,ObjectMap.rowCeiling-7210andObjectCalendar.rowCeiling-7210stayed green, 4/4, drawing 2001 rows. They pinned$topand the footnote and never "draws at most N".CalendarViewis stubbed the way the gantt pin stubsGanttView(and the wayObjectCalendar.unscheduled-7071already stubs it, viaimportOriginal), exposingdata-event-count. The above-ceiling case asserts it equalsNON_GRID_ROW_CEILING; the below-ceiling case asserts12, so the pair cannot be satisfied by a view that caps everything unconditionally.enableClustering={false}makes the count observable without changing what reached the view (clustering is a pure function of the marker array), and the DOM marker count is then literally the ruling's "the DOM row count equals the ceiling". The below-ceiling case asserts 20 markers with no prop at all — 20 is under the map's own clustering threshold.Ablation, prediction stated before the run: under the same uncapped mutation both pins go red at the new count assertion, and the new
externalDataCeiling-7507pin goes red at its countwaitFor. Measured: exactly that — 3 failed / 4 passed, red atObjectCalendar.rowCeiling-7210.test.tsx:124,ObjectMap.rowCeiling-7210.test.tsx:125,ObjectCalendar.externalDataCeiling-7507.test.tsx:120, each2001against2000. Restored byte-identically; the pins are green again.3. Four docblocks stated a reverse-verification mechanism that does not happen
Measured by deleting
$top: NON_GRID_ROW_CEILING_TOPfrom each component in turn:$topassertion, 1 failed / 2 passed$topassertion only; the row count stays 2000$topassertion only$topassertion onlyOne mechanism explains all four: an adapter with no
$topanswers with the whole filtered set, soapplyNonGridRowCeilingstill sees more rows than the ceiling, still slices to it, and still reportstruncatedfrom the rows in hand. The drawn count stays 2000 and the note still names both numbers. Losing the$topis a bandwidth regression, not a correctness one — which is exactly why the$topassertion is not redundant with the footnote.The calendar and map docblocks were re-measured after their new count assertions landed, as asked: the
$topablation is unchanged (still$top-only, at:115and:94), so each of the four now records the measured mechanism, and calendar and map additionally record the second ablation their count assertions exist for.4. Calendar external-
datasync leftrowCeilingstaleObjectCalendar.tsxhad fivesetDatapaths; four also resetrowCeiling, the external-datasync did not. A component that drew its own truncated fetch and was then handed a shortdataarray kept a footnote describing rows no longer on screen. Latent today —ObjectViewpassesdatafrom mount — and fixed with the one line the other four paths already carry.Pinned in
ObjectCalendar.externalDataCeiling-7507.test.tsx, with a live control: the first assertion proves the note really was raised by this component's own fetch, so its later absence is a state change and not a query that matched nothing.Ablation, prediction stated before the run: remove the reset and the new pin goes red at
expect(screen.queryByRole('note')).toBeNull(), whilerowCeiling-7210stays green. Measured: exactly that — 1 failed / 3 passed, red atObjectCalendar.externalDataCeiling-7507.test.tsx:131.5. The false dependency comment —
packages/react/src/index.ts"the only package all four already depend on" is measured false:
packages/plugin-{gantt,calendar,map,tree}/package.jsoneach also list@object-ui/core,@object-ui/componentsand@object-ui/types. (@object-ui/i18nis not common to all four —plugin-mapdoes not depend on it.) The comment now states the real reason — the round's barrel fence over exactly those three, plus@object-ui/reacthaving no subpath export — and cross-links #7508 so the next reader finds the open decision instead of a settled-sounding rationale. Comment text only; the export statements are untouched.6. Stale headroom comment —
packages/i18n/src/locales/*.ts(ten packs)"~1 KB of headroom" named a budget line that no longer holds these bytes. Since #7399 the catalogues have their own chunk:
i18n-locales, ceiling455_000against a recorded baseline of446_076— 8,924 B, about sixty short keys' worth across ten locales at the ~147 gzipped bytes/key the gate's own docblock measures. (framework, which the old comment was implicitly about, is now71_000over61_465.) Read live fromPER_CHUNK_GZIP_CEILINGS/PER_CHUNK_BASELINE.pnpm check:eager-closurefor the number in force. A fresh figure needs a fullapps/consolebuild, which isBundle Analysis's job on this PR.Verification
vitest run—plugin-calendar,plugin-map,plugin-gantt,plugin-tree,i18n, react ceiling unit, ata221c9288turbo run type-check— the six touched packages,--concurrency=2(deps built first)type-checktasks executedcheck:control-bytes,check:vi-mock-specifiers,check:vi-mock-inherit,check:i18n-keys,check:i18n-drift,check:i18n-dead-keysa221c9288node scripts/check-changeset-presence.mjseslint --no-inline-config --format jsonover the changed set@typescript-eslint/no-explicit-anyin the fixtures'as anyidiom (lint.ymlsets no--max-warnings)The lint narrowing is a measurement, not a skip. The population came from eslint's own config, not from a guess: its one
filesentry is**/*.{ts,tsx}, so 17 of the 19 changed files are in scope (the two.changeset/*.mdare not), and--format jsonreported 17 results — every in-scope changed file judged. The config declares noparserOptions.project/projectService, so type-aware linting is not enabled: a file's verdict is a function of that file's own source and the shared config, neither of which this diff moves for any untouched file.pnpm lintruns the whole farm on this PR regardless.One check is
NOT MEASUREDrather than green:check:readme-exportsexits 1 on this tree withits type entry ./dist/index.d.ts is not on disk — run pnpm build firstand apopulation COLLAPSEDverdict. That is a prerequisite failure, not a finding: it needs every package built. It cannot go red on this diff either way — the gate judges README docs against the export surface, and this branch changes no export.Fences observed
@object-ui/react唯一入口上发布了 5 个符号，而裁定 a′ 写的是「the constant is internal」——就地追认、迁 core、还是收回？ #7508.ObjectCalendar.expandFls-7230.test.tsx:145) is untouched — it is a resolution, verified in both directions, not a weakening.MAX_EAGER_CLOSURE_GZIP_BYTESandPER_CHUNK_GZIP_CEILINGSare read, never written.content/docs/releases/untouched.Changeset
.changeset/7507-non-grid-ceiling-correctives.mduses the empty-frontmatter exemption, deliberately: every corrective here lands inside a change that has not been released —.changeset/7210-non-grid-row-ceiling.mdis still pending onmain— so the calendar fix repairs a defect no user has seen, and the release note it belongs to is the #7210 changeset, corrected in place.🤖 Generated with Claude Code
https://claude.ai/code/session_01EMrWaQw3XS5DxTHxp4yRyC
Generated by Claude Code