fix(plugin-calendar,plugin-map): honour filter, sort and the row ceiling on inline value data - #9138
Draft
os-tesla wants to merge 2 commits into
Draft
fix(plugin-calendar,plugin-map): honour filter, sort and the row ceiling on inline value data#9138os-tesla wants to merge 2 commits into
value data#9138os-tesla wants to merge 2 commits into
Conversation
…ing on inline `value` data Ports objectui#8769's repair off `ObjectGantt` to the two siblings that carry a hand copy of the same short-circuit. Their fetch effect exited on `provider: 'value'` with `setData(dataItems)` BEFORE the adapter query, which is the one site in each file that lowers `schema.filter` onto `$filter`, `schema.sort` onto `$orderby` and the objectui#7210 ceiling onto `$top`. An authored `filter` therefore reached nothing and every authored row was drawn — the fail-OPEN direction, because the key that was dropped is the key that NARROWS. Not a literal transplant: `ObjectGantt` resolves ONE adapter for every provider and could delete the branch and fall through, while these two call `find` inside their `dataProvider === 'object'` arm, behind an `$expand` projection an inline set has no metadata to build. So each resolves `ValueDataSource` for the inline provider only, leaving the `api` arm exactly as it was, and no dependency array moves. Part of objectui#9061.⚠️ BLOCKED on a decision — see the PR body: routing the inline rows through `ValueDataSource` makes them pass through that adapter's `JSON.parse(JSON.stringify(...))` constructor clone, which retires objectui#6018's pinned guarantee that a map's inline rows never have to be serializable. Three standing assertions are left RED and untouched on purpose. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
…ange which query keys apply The changeset and both docs sets for objectui#9061: an authored `filter` / `sort` narrows and orders inline rows on every provider, the objectui#7210 ceiling applies to inline rows and is applied to the FILTERED set, and the two consequences of routing inline rows through the in-memory adapter — deep-copied row identity, and the JSON round-trip that makes inline rows have to be serializable. Part of objectui#9061. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
github-actions
Bot
added
documentation
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
|
Collaborator
Author
|
Director seat pointer (director seat, summon #22, Generated by Claude Code |
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.
⛔ DO NOT MERGE AS-IS — this pull request is a DECISION REQUEST. It is complete, measured and green on everything it set out to do, and it leaves three standing assertions RED on purpose. The red is the decision, not an oversight. See "The decision" below.
Part of objectui#9061. Deliberately not a closing keyword: the card must stay open until the decision below is taken.
What this does
ObjectCalendarandObjectMapeach held a hand copy of theprovider: 'value'short-circuit objectui#8769 removed fromObjectGantt. Inside the record-fetch effect, the inline branch calledsetData(dataItems)plussetRowCeilingwith truncated false and RETURNED — before thefinda few dozen lines below, which is the ONE site in each file that lowersschema.filteronto$filter,schema.sortonto$orderbyand the objectui#7210 ceiling onto$top.Consequence, identical in both: an authored
filterreached nothing and every authored row was drawn. The fail-OPEN direction — the key that was dropped is the key that NARROWS, so the view answered a wider question than the author asked. Nothing was exposed that was not already in the authored schema.Not a literal transplant, and the difference is structural.
ObjectGanttresolves ONEeffectiveDataSourcefor every provider, so its repair was to delete the branch and let the inline case fall through. These two callfindINSIDE theirdataProvider === 'object'arm, behind an$expandprojection an inline set has no metadata to build (and, on the calendar, behind anobjectSchemaReadygate scoped to that same arm). Falling through would throwDataSource required for object/api providerson a view that needs no DataSource at all. So each resolves aValueDataSourcefor the inline provider only; theapiarm keeps exactly the behaviour it had, and no dependency array moves — nothing about WHEN either effect re-runs changes.The decision this is blocked on
Routing inline rows through
ValueDataSourcepasses them through that adapter's constructor, which deep-clones withJSON.parse(JSON.stringify(...)). That imposes a requirement these two renderers do not have today: inline rows must be JSON-serializable.Measured, three surfaces, same input (a record carrying a back-reference to its own graph — what an expanded lookup looks like once a host hands the resolved rows to the block as inline data):
ObjectGantt(untouched here)TypeError: Converting circular structure to JSON, from the render body, via its ownJSON.stringifymemo keyObjectMapObjectCalendarSo the platform is already inconsistent about this, and this PR makes the two siblings match the gantt — by retiring a pinned guarantee on the map. That is a ruling, not an implementation detail, and it is not this PR's subject, so it is left for a maintainer:
ValueDataSourceclones "to prevent external mutation"; on a read-only query the clone is pure cost. Replacing the JSON round-trip in@object-ui/core(for example withstructuredClone, which handles cycles andBigInt) makes every assertion below green with no change to this diff.ValueDataSourceconsumer (aDatevalue would survive as aDateinstead of becoming a string), so it needs its own card and its own measurement. Recommended.ViewData.itemsis declared serializable metadata in@objectstack/spec, a circular record graph is off-contract, and the failure is LOUD (an error panel naming the cause) rather than silent. Then objectui#6018's third case is re-based, the twoObjectMap.filterConfigfixtures below are re-based, and this lands as written.⛔ An agent seat should not pick between A and B on its own: option B deletes a guarantee a previous card chose deliberately and recorded, and it does so as a side effect of a PR about something else.
The three RED assertions, named
All three are untouched in this diff — no fixture was edited to make anything green.
packages/plugin-map/src/ObjectMap.dataConfigMemo.test.tsx— "renders inline data the serializer cannot handle — identity needs no round-trip" (objectui#6018). This one IS the decision.packages/plugin-map/src/ObjectMap.filterConfig.test.tsx— "really is the DEFAULT config that applies, not 'no config at all'". Collateral: it authors a legacyfilter.mapstash alongside inline rows and relied on the filter being DROPPED; honoured as an object-shaped$filter, it now narrows to zero markers.packages/plugin-map/src/ObjectMap.filterConfig.test.tsx— "appliesschema.mapwhen an array filter is authored alongside it". Same collateral: it authorsfilter: [['owner', '=', 'me']]against a row that has noowner.2 and 3 are ordinary fixture triage under option B (give the row the field the filter asks for, or move the case onto an adapter that ignores
$filter) — they are NOT a second decision, and they were left red only so that the diff shows exactly which standing assertions the port disturbs.⭐ The row-ceiling measurement the brief asked for
An inline list that exceeds the ceiling now renders FEWER rows than the author supplied. Measured: 2,501 inline rows in, 2,000 drawn, on both renderers. Reported rather than buried, because "inline data is authored, not fetched" is a real objection.
It is shipped anyway, and the reason is that the cut is loud: both renderers already mount
NonGridRowCeilingNoteunconditionally at their top level, so the footnote naming BOTH numbers (2,000 of 2,501) renders on the inline path with no extra wiring.ceilingNotepins that in each file. objectui#7210 ruling a-prime's budget is measured in DOM elements per record, its own measurement table was taken over the inlinevalueprovider, and its text carves out no provider — an inline row costs the browser what a fetched row costs. The exemption that REMAINS is the hostdataReact prop, and only it: those rows are not ours to cap and we issued no query whose total a footnote could name.ceilingOrderpins the other half from the observable side: the ceiling is applied to the filtered set (2,400 rows, 800 of them matching, 800 drawn, no footnote), matching the fetching path where$filterand$toptravel in one query.Acceptance, and how each row is met
New:
packages/plugin-calendar/src/ObjectCalendar.inlineQueryKeys-9061.test.tsx(8 cases) andpackages/plugin-map/src/ObjectMap.inlineQueryKeys-9061.test.tsx(9 cases). 17 passed / 17 on the head of this branch.twoSidedFilterrenders the SAME rows and the SAME filter twice — once inline, once through a context adapter that is itself aValueDataSourceover those rows — and reads the disagreement, so "the filter was ignored" cannot be confused with "the filter matched everything".inlineSortreads the drawn id order off the view (the calendar stub carriesdata-event-ids; the map stub carries each marker's longitude, which the marker array takes straight off the record).staticDataSpellingon both;arrayShorthandSpellingon the map only — see the divergences below.ceilingCap,ceilingNote,ceilingOrder, per renderer.providerBackedControlrenders a NON-inline view with the same filter, the same sort and the same rows through the context adapter, and asserts the rows and their order are unchanged. It is green before this repair, after it, and on both ablation legs — that is what bounds the change to the inline path.controlrenders an inline view authoring NONE of the three and asserts the authored rows, in the authored order, with no footnote.Ablation — each renderer's fix removed SEPARATELY
Each leg restored the ONE file from the branch point, proved the mutation reached the disk before reading anything (repair marker
1 -> 0, pre-repair line1 -> 2, blob hash changed), then restored withgit checkout HEAD -- pathand proved the tree clean by state, never by an exit code.ObjectCalendar.tsxrevertedtwoSidedFilter,inlineSort,staticDataSpelling,ceilingCap,ceilingNote,ceilingOrder;controlandproviderBackedControlgreenObjectMap.tsxrevertedarrayShorthandSpelling;controlandproviderBackedControlgreenNeither renderer's repair covers the other. Both needed porting.
Restore proof, both legs:
blob 460e680d… == HEAD blob 460e680d…; git diff HEAD emptyandblob 42d8b312… == HEAD blob 42d8b312…; git diff HEAD empty.Where the two files DIVERGE — verified per file, not assumed
The brief asked for this explicitly. The short-circuit itself is byte-identical in shape; four things around it are not.
ObjectMap.getDataConfignormalizes a bare array underdatainto{ provider: 'value', items }before delegating to the shared ladder (objectui#5305).ObjectCalendarhas no such head — three spellings reach the repair on the map, two on the calendar. Hence the extra map-only case.ObjectMap'srowCeilingstate said "A hostdataprop and an inlinevalueset are never truncated by us". Corrected here.ObjectCalendar's equivalent docblock never made that claim and is untouched.ObjectCalendar's effect guards every state write withisMounted;ObjectMap's does not. Each new branch follows its own file.ObjectGantt's "an inline set is never capped and never footnoted" case. Neither sibling has one —ObjectMap.rowCeiling-7210andObjectCalendar.rowCeiling-7210both grade theobjectprovider only — so the ceiling cases here are NEW coverage, not an inversion. Read both files' case lists before writing.They do not need different repairs: the same shape lands on both, with the same three keys on the same query, and both pay the same serializability cost. The divergences above are about their surroundings.
The fourth surface — reported, not fixed
Triage asked whether any further surface carries this, because a short-circuit found on three components is one that was copied. It does:⚠️ With one measured difference —
packages/plugin-tree/src/ObjectTree.tsxholds the same inline exit before the same$filter/$topquery.ObjectTreenever readsschema.sortat all (zero hits forconvertSortToQueryParamsand forschema.sortin the whole file), so two keys are dropped there, not three. Filed as objectui#9136 and left alone, per this card's fence. It inherits the decision above unchanged.Two further inline branches were looked at and are deliberately NOT graded:
ObjectGrid(a grid, which pages its own rows and sits outside objectui#7210's non-grid ceiling) andListView(an inline branch that applies its own search term and delegates to a child view). Each needs its own reading.Clause-2 declaration holds
Declared no, and the diff keeps it: no declared type, registration input, exported symbol, authorable key or accept-set moves.
OBJECT_CALENDAR_DATA_SOURCEandOBJECT_MAP_DATA_SOURCEare byte-identical — neitherindex.tsxis in this diff at all.filterandsortwere already declared and already read on theobjectpath; this is only about whether they are read on one more path.Verification
pnpm exec vitest run packages/plugin-calendar/ packages/plugin-map/— 3 failed / 361 passed, and the 3 are exactly the named reds above. Every other case in both packages, including all 17 new ones, is green.pnpm exec vitest run packages/plugin-view/ packages/sdui-parser/ examples/schema-catalog/ apps/console/src/__tests__/public-block-binding-reach.test.tsx packages/react/src/__tests__/SchemaRendererProvider.smoke.test.tsx— 85 files / 2704 tests, all green.pnpm turbo run type-check --filter=@object-ui/plugin-calendar --filter=@object-ui/plugin-map— 17 tasks successful.eslint . --no-inline-config --format jsonover the whole repo at7bd112303: 4807 files judged, and all four files this PR touches underpackages/carry 0 errors (only pre-existingno-explicit-anywarnings shared with their neighbours). The 79 error-carrying files are all untouched by this PR.check:control-bytes,check:vi-mock-specifiers,check:vi-mock-inherit,check:vi-mock-override-shape,check:changeset-claims,check:new-line-citations,check:doc-fences,check:element-data-source-declaration,check:unreferenced-sources,check:shell-escape-residue,check-changeset-presence,check-changeset-no-major.check:readme-exportsneeds builtdist/for 36 packages and refused with "runpnpm buildfirst" over packages this PR does not touch — a prerequisite, not a red. Declared to CI.Acceptance notes
Out-of-scope observations, noted and not filed:
packages/plugin-map/src/index.tsxsays the map's fetch "issues no$top", andpackages/plugin-calendar/src/index.tsxsays the calendar "fetches the whole window rather than a capped page". Both stopped being true at objectui#7210, before this card. Not corrected here: both sentences sit directly above anElementDataSourceMappingdeclaration this PR's clause-2 declaration says it will not touch, and neither is made wrong by this change. The carrier would be whoever next edits those two registrations.ObjectMap.filterConfig.test.tsx's "ignores a MapConfig stashed underfilter.mapand falls to the default config" still passes, but after this change it would pass for two reasons at once (the default config finds no coordinates, AND the legacy stash now narrows to zero rows as an object-shaped$filter). Worth re-basing alongside the two red cases if option B is taken. Carrier: whoever re-bases those fixtures.Prepared by an automated developer seat, session
session_01UzHd6hDYatoDn17BuwKxnZ.🤖 Generated with Claude Code
https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
Generated by Claude Code