Repository navigation
fix(data-objectstack): classify a view overlay by its _isOverride marker only (objectui#10210, ruling B) - #10427
Merged
Conversation
…arker only (objectui#10210, ruling B) Retire `isLegacyOverlayRow`, the shape guess that also treated a flat view row carrying `viewKind: 'list'` as a personalization overlay. An "Edit view config -> Save" on a code-defined view stored that same shape before PR #10332, so `listViews()` dropped the user's own view, the tab was stamped read-only, and publishing made it permanent. Under the maintainer's ruling B (objectui#10210, comment 5824008636) the marker is the only discriminant: such rows heal on read with their edits, and the one exposed class (overlay rows written before the marker and never touched since) is named in the changeset and pinned. The pins that asserted the guess are rewritten with the reason, not deleted: `listViews`, `viewOverlayPatchOnly` (and the header of `viewOverlayMarker`), plus the two app-shell pins that ran the real adapter through the same guess. The two pending changesets this makes false are corrected in place. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C
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
|
Contributor
Author
Contract reviewServed-tier: Rendered by an isolated review subagent spawned by the ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS 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.
Fixes #10210
Clause-②: no
This carries out the maintainer's ruling B on objectui#10210 (ruling comment 5824008636, 「10210 同意」).
data-objectstackretiresisLegacyOverlayRow, so a view row counts as a personalization overlay only when it carries the_isOverridemarker. The ruling puts the B work on this card and asks for no separate execution card. The write half landed earlier as objectui#10332 (Part of). Implemented under thedomain:uiseat 4 claim 5824191795 on branchclaude/issue-10210-retire-overlay-shape-guess, sessionhttps://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C.What changed
packages/data-objectstack/src/index.ts:isLegacyOverlayRowis deleted. That was the shape guess: a flat row (no nestedconfig) withviewKind: 'list'.isPersonalizationOverlayRownow reads only the marker, on the item or on its{list: …}body. Four comments the change made false are corrected: the marker docblock,VIEW_OVERLAY_OWNED_KEYS,narrowPersonalizationOverlay, and the comment in thelistViewsfilter. No export, key, schema or signature moves.packages/data-objectstack/src/viewOverlayMarkerOnly-10210.test.ts, covers the heal, the accepted exposure and a same-shape marker control..changeset/10210-overlay-marker-only.mddeclares apatchfor@object-ui/data-objectstack: a fix to how the view list reads rows, with no API change. It says broken views recover on read, and it names the one exposed class exactly as the ruling words it.Mechanism assumptions, measured
origin/main(378a4f6),isPersonalizationOverlayRowreturnedisLegacyOverlayRow(item, spec)when the marker was absent.git grepfinds two callers of the classifier, both inindex.ts. Here is what each did with a row the guess classified:listViews()filter: dropped it from the saved-view list, so the app-shell tab was stampedreadonly(isSystem = !saved). Now the row is returned as a saved view: a flat row as stored, with_draftkept in draft preview.narrowPersonalizationOverlay(), which app-shell'ssanitizeViewOverridecalls for bothloadViewOverridesread branches (the display merge): narrowed it to identity plus the five owned keys. Now it returns the row by reference, whole.updateViewConfigstill stamps_isOverridelast on a system-view target (pinned unchanged inviewOverlayMarker.test.ts), and the config save now writes a nested-configenvelope. So no current writer produces an unmarked overlay.viewKind/object/labelfrom the registry entry it shadows, as the platform'sviewIdentityPatchdoes. Its keys are{label,type,columns,name,isDefault,id,viewKind,object}, plusfilterbecause the draft edited one.listViews()returns it with the edited label, columns and filter, in both the published read and the?preview=draftread, andnarrowPersonalizationOverlayreturns it whole. Through the real app-shell pipeline (listViewsintobuildViewTabsintoisSavedViewId), the rewrittenObjectView.overrideMasquerade.test.tscase asserts that the tab is not read-only, that the mutating-handler guard admits it, and that the tab shows the edited label and columns.listViews()with its frozen label, columns and filter. Merged over a newer code definition, that frozen copy wins. This is asserted in the new pin, inviewOverlayPatchOnly.test.ts, and end to end in app-shell'sObjectView.overlayPatchOnly.test.ts, where the admin's filter, columns and label edit is covered by the frozen copy.listViews.test.ts, one inviewOverlayPatchOnly.test.ts, one in app-shellObjectView.overlayPatchOnly.test.tsand one in app-shellObjectView.overrideMasquerade.test.ts.viewOverlayMarker.test.tsstayed 8/8 green. Only its header prose described the guess, so the header is the part rewritten there. Each rewritten file carries a comment naming this ruling.HEADand verified the restore by blob hash and an emptygit diff HEAD. Result: 9 red, 46 green over the six files. All 4 rewritten assertions and 5 of the 6 new cases went red. The sixth new case is the same-shape marker control, which is green on both trees by design. The 8 cases inviewOverlayMarker.test.tsstayed green because nothing there was rewritten. Nodistis involved: the data-objectstack tests import./index, and app-shell resolves@object-ui/data-objectstacktosrcthrough the root vitest alias.it(counts before and after (text count, then executed-test count from the vitest JSON report):data-objectstack/src/listViews.test.tsdata-objectstack/src/viewOverlayMarker.test.tsdata-objectstack/src/viewOverlayPatchOnly.test.tsapp-shell/src/views/ObjectView.overlayPatchOnly.test.tsapp-shell/src/views/ObjectView.overrideMasquerade.test.tsit(+ 1it.each(with 2 rows / 4it(+ 1it.each(with 1 row / 4data-objectstack/src/viewOverlayMarkerOnly-10210.test.ts(new)In
ObjectView.overrideMasquerade.test.ts, the unmarked row moved out of theit.eachtable into its ownit(, because the table's shared body asserts exclusion. The executed count is unchanged.Surface breach: a bounded in-place fix, declared
The claim's surface is
data-objectstack/src/index.ts, the three named pins, a new pin and one changeset. The measurement above found two more pins that asserted the guess through the real adapter, both in app-shell:ObjectView.overrideMasquerade.test.ts(itslegacy unmarked rowcase) andObjectView.overlayPatchOnly.test.ts(a PRE-MARKER legacy row is narrowed …). Leaving them would leave this PR red in CI. I rewrote them in place, the same way as the named pins. The dispatch said "stop on breach"; the role file allows a bounded in-place fix when all four conditions hold, and it wins when the two conflict. That conflict is raised in the report, not settled silently here. The four conditions:packages/data-objectstack/. The release PR's 1732 files were read in full, matching itschanged_files.The seat needs to add these two paths to the claim surface. The same edit also repairs a stale cross-file line citation in
ObjectView.overrideMasquerade.test.ts(it cited thesavedViewsnormalization by an old line range; it now cites it by content), as AGENTS.md #11 asks for any file a change touches anyway.Pending changesets corrected in place (
check-changeset-overwrite, case 2: prose correction; front matter unchanged).changeset/10210-view-config-save-envelope.md(objectui#10332, still pending). Itsdata-objectstackchange. One present-tense clause ("is the shape the list reader treats as …") is put in the past tense..changeset/view-overlay-write-patch-only-5233.mdsaid "rows written before this land are still tolerated on read". That is now true only for rows carrying the marker, so the sentence is scoped to them and points to this change for rows older than the marker..changeset/hollow-view-overlay-hydration-pin-5773.mdnamespackages/data-objectstack/src/index.ts. I re-read it, and no sentence becomes false: its fixtures are realupdateViewConfigwrites, which carry the marker, andInterfaceListPage's hydration does not narrow.Verification, all at HEAD
9736e8d20pnpm exec vitest run --maxWorkers=2ran the wholepackages/data-objectstack/package plus the 12 app-shell and console suites that namelistViews(,_isOverride,narrowPersonalizationOverlay,sanitizeViewOverride,loadViewOverridesorlistViewOverrides: 1072 passed, 0 failed (the base had 1066 of 1066; the difference is the 6 new cases). A second run covered 5 more suites that import the adapter and namelistViews(metadataReadWarningToast, coreelement-data-source, and the kanban, list and timelineelementDataSourcesuites): 66 passed. Every suite that uses the real adapter and names an overlay reader was run: 19 of 19.pnpm --workspace-concurrency=2 --filter '@object-ui/data-objectstack^...' run build(exit 0), thenpnpm --filter @object-ui/data-objectstack type-check(exit 0). Its program lists all 4 touched data-objectstack test files (--listFilesOnly). Next,turbo run build --filter='@object-ui/app-shell^...' --concurrency=2: 28 of 28 tasks succeeded. Thenpnpm --filter @object-ui/app-shell type-check(tsc --noEmit && tsc -p tsconfig.test.json) exited 0, andtsconfig.test.jsonlists both edited app-shell test files.check-changeset-presence("7 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)"),check-changeset-no-major,check-changeset-fixed,check-changeset-overwrite(report-only; it names the two corrections above),check-changeset-claims("No pending changeset names a file this change touches"; the self-contradiction reading is clean over 3 bodies),check-pending-changeset-literals,check-new-cross-file-line-citations("VERDICT new-cross-file-line-citations: 0 new citation(s)"),check-control-bytes,check-shell-escape-residue,check-test-path-roots,check-vi-mock-specifiers,check-vi-mock-inherit,check-vi-mock-override-shape,check-unreferenced-sourcesandcheck-object-metadata-write-doors.check-governed-queue-guard --testover the 10 paths says NOT GOVERNED.eslint.config.js**/*.{ts,tsx}and**/*.test.{ts,tsx}blocks, the same config each package'seslint .resolves to.eslint --format jsonover the 7 touched TypeScript files gives 7 files, 0 errors, 183 warnings, allno-explicit-any.--stdin:index.ts124 → 122,ObjectView.overrideMasquerade.test.ts7 → 8, the other four unchanged, and 10 in the new pin, in line with its sibling pins.parserOptions.projectorprojectService), so this diff cannot move the verdict for any untouched file.pnpm testruns it.Acceptance notes (not filed)
buildViewConfigSaveBodydocblock inpackages/app-shell/src/views/ObjectView.tsx: "a flat row carryingviewKindis exactly the shape the adapter's legacy-overlay net reads as a personalization overlay".ObjectView.viewConfigSaveEnvelope-10210.test.ts.buildPersistedViewBodydocblock: "already harmless on read since PR fix(data-objectstack,app-shell): a view overlay contributes only the keys it owns #5272 narrowed the merge" now holds only for marked rows.Each describes history accurately and is stale only in tense or scope. This is drift, not a defect.
persistViewPatch's saved-view branch (isSavedViewIdis true), writing the row whole and without the marker. Unlike before the ruling, the row does not re-mark itself on the next touch; it stays a plain row. This follows the ruling's "reads as a plain row", and I record it only so it is not rediscovered later.Generated by Claude Code