Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

Personalising a system view makes it look user-created — its override row comes back from listViews() as a saved view #4227

Closed

Description

@yinlianghui

Filed unassigned from #4211's dispatch (PR #4224), per the "measure the side-symptom, don't fix out of scope" split. #4211 made the switcher's two reads agree on a view's identity; this is the adjacent question that fix deliberately does not answer — which rows count as saved views at all — and it fails in the opposite direction.

What happens

Toggle any per-view personalization on a code-defined (system) view — density, sort, hidden columns, column widths, inline edit. From the next load on, that view is presented as user-created: the tab loses its read-only lock badge and gains Rename, Delete, Set as default, Pin and Edit view config in both its dropdown and its context menu.

It is not a display-only slip. handleDeleteView calls dataSource.deleteView(objectName, vid) against a view that lives in code, and handleRenameView writes a label overlay onto it.

Why — one metadata namespace, two meanings

Personalization and saved views are stored in the same type='view' namespace, and nothing distinguishes them on read.

The write side stamps the row so it looks object-owned (packages/data-objectstack/src/index.ts, updateViewConfig):

const merged = { ...(config || {}), object: (config as any)?.object || objectName, name: viewId };
await this.client.meta.saveItem('view', viewId, merged);

The read side narrows by exactly that field (viewItemObjectName, same file):

function viewItemObjectName(item: any): string | undefined {
  const spec = item?.list ?? item;
  return spec?.data?.object ?? spec?.object ?? spec?.objectName;
}

listViews() filters on viewItemObjectName(v) !== objectName and drops only form-family rows, so a personalization row for a system view passes both tests and is returned as an ordinary overlay row. It is keyed by name: viewId — the system view's own id — so it matches that tab exactly.

ObjectView then reads "an overlay row exists for this tab id" as "this view is user-created":

const saved = savedViews.find((sv: any) => viewRowId(sv) === view.id);
const isSystem = !saved;                 // ObjectView.tsx:2022
readonly: isSystem,

and the same predicate is the guard in all five mutating handlers, so the menu entries render and their handlers accept.

listViewOverrides in the same adapter reads these rows for their intended purpose. The two readers disagree about what a row means, and only one of them is right.

Measured

Reproduced at component level while mapping #4211's seam, with rows in the shape updateViewConfig actually writes:

savedViews keys : [ 'crm_lead.default' ]
tab ids         : [ 'crm_lead.default' ]
  tab "crm_lead.default" readonly=false isSavedView=true

crm_lead.default is a metadata-declared view with no user-created counterpart; the only row backing it is the density override. Expected readonly=true.

Not caused by, and not fixed by, #4211

Pre-existing, and untouched by PR #4224. That PR replaced three spellings of a row's identity with one (viewRowId) so the tab id and the overlay key agree; it did not change which rows enter savedViews, and this defect needs no id disagreement — the ids here match perfectly, which is precisely the problem. PR #4224 pins the correct behaviour as a control ("a genuine system view stays readonly — its missing menu entry is CORRECT"), and that control passes today only because its fixture has no override row.

Sketch of the fix, for triage — not a decision

The rows are already distinguishable; nobody looks. A personalization row has no viewKind and no view body of its own, while a real saved view is created through createView with a generated name and a full fullSpec. Options, roughly ordered by how contract-first they are:

  1. Separate the namespaces — write personalization under its own metadata type. Cleanest, and makes "declared = enforced" true on read; costs a migration for existing rows.
  2. Mark the row on write and filter on read — e.g. an explicit discriminant stamped by updateViewConfig and excluded by listViews(). Cheaper, and it is a producer-side fix rather than a consumer-side tolerance.
  3. Infer on read (a row with no view body is an override). Consumer-side leniency of exactly the kind that lets authoring mistakes hide — noted for completeness, not recommended.

Which one is right is a spec/ownership question about the view metadata namespace rather than a rendering detail, so it wants a maintainer read before code.

Activity

  1. added
    bugSomething isn't working
    and removed
    bugSomething isn't working
    on Aug 11, 2026
  2. yinlianghui commented on Aug 11, 2026

    @yinlianghui
    ContributorAuthor

    Inbox disposition (objectui seat PM, session session_017Qqyix2QcnpUC9XeYVDzx3): reviewed for delegated ruling and declined — this is the platform's call, and it is the SAME call as objectstack#7494. Both cards are one contract question about the view namespace in sys_metadata: #7494 asks how personalization rows are SCOPED (org-wide today while the console promises personal), this card asks how they are CLASSIFIED (a personalization row keyed like a saved view masquerades as user-created and gains Rename/Delete/Set-default/Pin). Ruling either alone risks the two answers disagreeing about what a view row IS. A client-side patch here (e.g. filtering listViews() by a provenance marker) would presuppose a marker the platform hasn't defined — exactly the kind of consumer-side invention AGENTS.md #0.1 forbids.

    Folded into the #7494 decision as consumer-side evidence #2 (cross-referenced there). needs-user-decision stays; restart condition = the objectstack ruling on the view-row contract, after which the objectui half (honor the scoping/classification in listViews() consumers and the tab affordance gates) becomes a scoped dispatch.


    Generated by Claude Code

  3. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Maintainer ruling recorded 2026-08-11 (PM session, executing the maintainer's direct instruction in chat — verbatim: 「接受你的全部建议,请更新 issue 的状态和标签」, accepting the four-lens decision-inbox review in full).

    Ruling (paired with objectstack#7494): personalization rows must be distinguishable from saved views — write-side marker and/or read-side exclusion in listViews() — and a system view must never become deletable/renamable because an overlay row exists for it. Per objectstack#7494's ruling the overlay contract is explicitly org-wide shared view settings (a permission gate on the write lands platform-side under that card); console prose/UX stops presenting these as personal preferences. A true per-user scope is parked as a v18 direction on the platform side — do not build client-side workarounds for it.

    State: needs-user-decision → pm:queue (objectui whole-repo seat).


    Generated by Claude Code

  4. self-assigned this
    on Aug 15, 2026
  5. yinlianghui commented on Aug 15, 2026

    @yinlianghui
    ContributorAuthor

    Claiming — PM dispatch. Session: session_01RnQd8iMMUwXQEV1crFmQiQ (objectui whole-repo PM seat). Branch: claude/issue-4227-view-override-masquerade.

    Scope per the maintainer's 2026-08-11 ruling (paired with objectstack#7494): personalization rows become distinguishable from saved views — write-side marker in updateViewConfig and/or read-side exclusion in listViews(), with legacy unmarked rows measured and handled — so a system view never becomes deletable/renamable because an override row exists; PR #4224's system-view control gains the companion fixture WITH an override row. Console/plugin prose stops presenting these as personal preferences (the ruled contract is org-wide shared view settings). ⛔ Not here: the write-side permission gate (platform card under objectstack#7494) and any per-user scope (parked v18, no client-side workarounds). PR will be Fixes #4227.


    Generated by Claude Code

  6. yinlianghui commented on Aug 15, 2026

    @yinlianghui
    ContributorAuthor

    <!-- os-dev-report -->

    {
      "issue": 4227,
      "status": "done",
      "branch": "claude/issue-4227-view-override-masquerade",
      "pr": "https://github.com/objectstack-ai/objectui/pull/4713",
      "pr_number": 4713,
      "pr_url": "https://github.com/objectstack-ai/objectui/pull/4713",
      "head_sha": "3d329df17",
      "changed_files": [
        "packages/app-shell/src/views/ObjectView.tsx",
        "packages/app-shell/src/views/ObjectView.overrideMasquerade.test.ts",
        "packages/data-objectstack/src/index.ts",
        "packages/data-objectstack/src/listViews.test.ts",
        "packages/data-objectstack/src/viewOverlayMarker.test.ts",
        ".changeset/view-override-masquerade.md"
      ],
      "inherited_state": "Handoff worktree /home/user/objectui-issue-4227 (branch claude/issue-4227-view-override-masquerade, tracking origin/main, deps already pnpm-installed) had uncommitted, unpushed work matching the ruling's suggested route: write-side `_isOverride` marker in updateViewConfig, read-side exclusion in listViews() (marker + best-effort legacy-shape predicate isLegacyOverlayRow), 3 prose edits in ObjectView.tsx, 5 new pinning cases in listViews.test.ts, plus 2 new untracked test files (ObjectView.overrideMasquerade.test.ts, viewOverlayMarker.test.ts). No commits existed yet. Reviewed the full diff line by line against the ruling and mechanism assumptions; found it sound and complete, kept it as-is.",
      "premise_still_valid": true,
      "legacy_row_predicate": "isLegacyOverlayRow(item, spec) in packages/data-objectstack/src/index.ts: a row is treated as a pre-marker override iff it has NO nested `config` object AND carries viewKind === 'list' at the top level (item.viewKind ?? spec.viewKind; 'form' already dropped upstream by FORM_FAMILY). Measured against the real write paths, not guessed: a genuine saved view is always created with a nested `config` wrapper (createView's fullSpec / app-shell's ADR-0034 viewEnvelope seam), with viewKind living OUTSIDE config on that shape. A personalization overlay (updateViewConfig) is always flat -- its fields sit at the top level, never wrapped in config. On a flat row, viewKind can therefore only have arrived via the platform's server-side identity inheritance (viewIdentityPatch, @objectstack/metadata-protocol #2555/#7741), which fires only when the write's `name` resolves against a registry-backed (system) view -- a runtime-created saved view has no registry entry to trigger it, so its row never gains a viewKind even if later flattened by a toolbar toggle. So 'flat body + viewKind' reliably signals 'override on a system view'; a flat row with NO viewKind is left alone (this is exactly the existing 'legacy bare spec' pin's territory in listViews.test.ts, and it stays a saved view). Explicitly NOT a complete net: a pre-marker override whose body also got the system view's full type/columns/data copied in by persistViewPatch's spread is structurally indistinguishable from an untouched saved view without the viewKind signal or the new marker -- not caught by design; those rows self-heal on their next write, which now carries the marker.",
      "prose_sweep": {
        "changed": [
          "packages/app-shell/src/views/ObjectView.tsx: buildViewTabs override-wins-per-key comment -> 'org-wide shared view settings (objectstack#7494's ruling...)'",
          "packages/app-shell/src/views/ObjectView.tsx: density/sort/etc hydration comment -> 'ORG-WIDE shared, not a per-user preference; a true per-user scope is a parked v18 direction'",
          "packages/app-shell/src/views/ObjectView.tsx: viewOverrides state comment (~line 995, found during this session's sweep, not the predecessor's) -> replaced 'toggle preferences' with 'toggle state' + explicit org-wide-shared note, for consistency with the other two spots in the same file"
        ],
        "swept_not_changed": [
          "packages/react/src/hooks/useViewSharing.ts and packages/plugin-view/src/ViewTabBar.tsx (ViewVisibility: private/team/organization/public) -- a distinct, legitimate feature (who can SEE a saved view), unrelated to the updateViewConfig personalization-overlay contract this issue is about",
          "packages/plugin-view/src/ManageViewsDialog.tsx 'per-user personalisation (objectui#1520)' -- references a different, unrelated future-work issue about view duplication",
          "packages/i18n/src/locales/*.ts (all 10 packs: de/es/zh/en/ja/ko/ru/fr/ar/pt) -- checked systemViewReadonly and every density/sort/hiddenFields/columnState/inlineEdit-adjacent toolbar string; zero instances of personal/individual-scope framing found"
        ]
      },
      "gates": {
        "head_at_run": "3d329df17 (post-final-commit, matches PR head sha)",
        "results": [
          "pnpm --filter '@object-ui/data-objectstack^...' --filter '@object-ui/app-shell^...' --workspace-concurrency=2 build -- exit 0",
          "pnpm exec vitest run --maxWorkers=2 packages/data-objectstack/src/listViews.test.ts packages/data-objectstack/src/viewOverlayMarker.test.ts packages/app-shell/src/views/ObjectView.overrideMasquerade.test.ts packages/app-shell/src/views/ObjectView.setDefaultViewIdentity.test.tsx packages/app-shell/src/views/ObjectView.defaultViewIdentity.test.tsx packages/app-shell/src/views/ObjectView.viewOverrides.test.tsx -- 6 files / 59 tests passed",
          "pnpm --filter '@object-ui/data-objectstack' --filter '@object-ui/app-shell' type-check -- exit 0",
          "pnpm --filter '@object-ui/data-objectstack' --filter '@object-ui/app-shell' lint -- exit 0, 0 errors (2838 pre-existing any/hooks warnings across both packages, none touching lines this PR changed)",
          "node scripts/check-control-bytes.mjs -- OK, 4238 files scanned",
          "node scripts/check-i18n-call-site-keys.mjs -- OK, every call site resolves",
          "node scripts/check-changeset-presence.mjs -- OK, 1 changeset declared",
          "node scripts/check-changeset-no-major.mjs -- OK"
        ]
      },
      "changesets": [
        ".changeset/view-override-masquerade.md -- patch bump for @object-ui/app-shell and @object-ui/data-objectstack, describing the write-side marker + read-side exclusion behavior change. Not a skip-changeset PR (real user-visible behavior change ships), so no skip-changeset label applies."
      ],
      "notes": "Truth-table readings, all from the real production pipeline (ObjectStackAdapter.listViews() -> buildViewTabs / isSavedViewId), not hand-mocked shortcuts: (1) system view + marker-carrying override row => listViews() returns [], tab stays isSystem/readonly=true, isSavedViewId guard refuses -- pins the fix's primary case. (2) system view + legacy unmarked override row (flat body, viewKind:'list', no config) => same result -- pins the backfill predicate. (3) genuine createView-produced saved view, even reusing a system-view-shaped label ('crm_lead.default'-adjacent naming) => fully manageable (readonly=false, isSavedViewId=true) -- positive control proving the exclusion doesn't over-fire. (4) PR #4224's own no-override-row control (savedViews=[]) still holds unchanged. listViewOverrides() was verified end-to-end to keep returning the override row unchanged (viewOverlayMarker.test.ts) since ObjectView's display-merge path depends on it. isSystem/readonly/all five mutating-handler guards were NOT touched directly -- they derive from savedViews, which is populated straight from listViews(), so excluding the override row there is what makes the pre-existing predicate correct again transitively; confirmed this data flow by reading ObjectView.tsx's savedViews-population effect (dataSource.listViews call) and the isSavedViewId guard call site.",
      "summary": "Fixed objectui#4227: a system (code-defined) view's personalization-overlay row (density/sort/hiddenFields/columnState/inlineEdit, written by updateViewConfig) no longer reads back from listViews() as a user-created saved view, so it can no longer gain Rename/Delete/Set-default/Pin. Two layers per the paired 2026-08-11 ruling: updateViewConfig stamps an explicit _isOverride:true marker on every row (its one production caller is the toolbar toggle, so every row it writes is an overlay); listViews() excludes marked rows plus a measured best-effort legacy shape (flat body + registry-backfilled viewKind) for rows written before this fix. listViewOverrides() is unchanged. Console prose describing the overlay as a 'personal preference' was corrected to 'org-wide shared view settings' in 3 spots in ObjectView.tsx; a repo-wide sweep (including all 10 i18n locale packs) found no other such framing. Write-side permission gating and true per-user scope are explicitly out of scope (platform-side, per the paired objectstack#7494 ruling).",
      "tests": "See gates.results above -- 59/59 new+existing tests passing (17 in data-objectstack: 5 new listViews.test.ts cases + viewOverlayMarker.test.ts's 6; 42 in app-shell across 4 files including the new ObjectView.overrideMasquerade.test.ts's 6 cases and PR #4224's own setDefaultViewIdentity/defaultViewIdentity controls, all re-verified green), type-check and lint clean (0 errors) for both changed packages, all captured post-final-commit at HEAD 3d329df17 which matches the pushed PR's head sha.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  7. yinlianghui commented on Aug 15, 2026

    @yinlianghui
    ContributorAuthor

    os-dev-report

    {
      "issue": 4227,
      "status": "done",
      "branch": "claude/issue-4227-view-override-masquerade",
      "pr": "https://github.com/objectstack-ai/objectui/pull/4713",
      "pr_number": 4713,
      "pr_url": "https://github.com/objectstack-ai/objectui/pull/4713",
      "head_sha": "3d329df17",
      "changed_files": [
        "packages/app-shell/src/views/ObjectView.tsx",
        "packages/app-shell/src/views/ObjectView.overrideMasquerade.test.ts",
        "packages/data-objectstack/src/index.ts",
        "packages/data-objectstack/src/listViews.test.ts",
        "packages/data-objectstack/src/viewOverlayMarker.test.ts",
        ".changeset/view-override-masquerade.md"
      ],
      "inherited_state": "Handoff worktree /home/user/objectui-issue-4227 (branch claude/issue-4227-view-override-masquerade, tracking origin/main, deps already pnpm-installed) had uncommitted, unpushed work matching the ruling's suggested route: write-side _isOverride marker in updateViewConfig, read-side exclusion in listViews() (marker + best-effort legacy-shape predicate isLegacyOverlayRow), 3 prose edits in ObjectView.tsx, 5 new pinning cases in listViews.test.ts, plus 2 new untracked test files (ObjectView.overrideMasquerade.test.ts, viewOverlayMarker.test.ts). No commits existed yet. Reviewed the full diff line by line against the ruling and mechanism assumptions; found it sound and complete, kept it as-is.",
      "premise_still_valid": true,
      "legacy_row_predicate": "isLegacyOverlayRow(item, spec) in packages/data-objectstack/src/index.ts: a row is treated as a pre-marker override iff it has NO nested config object AND carries viewKind === 'list' at the top level (item.viewKind ?? spec.viewKind; 'form' already dropped upstream by FORM_FAMILY). Measured against the real write paths, not guessed: a genuine saved view is always created with a nested config wrapper (createView's fullSpec / app-shell's ADR-0034 viewEnvelope seam), with viewKind living OUTSIDE config on that shape. A personalization overlay (updateViewConfig) is always flat -- its fields sit at the top level, never wrapped in config. On a flat row, viewKind can therefore only have arrived via the platform's server-side identity inheritance (viewIdentityPatch, @objectstack/metadata-protocol #2555/#7741), which fires only when the write's name resolves against a registry-backed (system) view -- a runtime-created saved view has no registry entry to trigger it, so its row never gains a viewKind even if later flattened by a toolbar toggle. So 'flat body + viewKind' reliably signals 'override on a system view'; a flat row with NO viewKind is left alone (this is exactly the existing 'legacy bare spec' pin's territory in listViews.test.ts, and it stays a saved view). Explicitly NOT a complete net: a pre-marker override whose body also got the system view's full type/columns/data copied in by persistViewPatch's spread is structurally indistinguishable from an untouched saved view without the viewKind signal or the new marker -- not caught by design; those rows self-heal on their next write, which now carries the marker.",
      "prose_sweep": {
        "changed": [
          "packages/app-shell/src/views/ObjectView.tsx: buildViewTabs override-wins-per-key comment -> 'org-wide shared view settings (objectstack#7494's ruling...)'",
          "packages/app-shell/src/views/ObjectView.tsx: density/sort/etc hydration comment -> 'ORG-WIDE shared, not a per-user preference; a true per-user scope is a parked v18 direction'",
          "packages/app-shell/src/views/ObjectView.tsx: viewOverrides state comment (~line 995, found during this session's sweep, not the predecessor's) -> replaced 'toggle preferences' with 'toggle state' + explicit org-wide-shared note, for consistency with the other two spots in the same file"
        ],
        "swept_not_changed": [
          "packages/react/src/hooks/useViewSharing.ts and packages/plugin-view/src/ViewTabBar.tsx (ViewVisibility: private/team/organization/public) -- a distinct, legitimate feature (who can SEE a saved view), unrelated to the updateViewConfig personalization-overlay contract this issue is about",
          "packages/plugin-view/src/ManageViewsDialog.tsx 'per-user personalisation (objectui#1520)' -- references a different, unrelated future-work issue about view duplication",
          "packages/i18n/src/locales/*.ts (all 10 packs: de/es/zh/en/ja/ko/ru/fr/ar/pt) -- checked systemViewReadonly and every density/sort/hiddenFields/columnState/inlineEdit-adjacent toolbar string; zero instances of personal/individual-scope framing found"
        ]
      },
      "gates": {
        "head_at_run": "3d329df17 (post-final-commit, matches PR head sha)",
        "results": [
          "pnpm --filter '@object-ui/data-objectstack^...' --filter '@object-ui/app-shell^...' --workspace-concurrency=2 build -- exit 0",
          "pnpm exec vitest run --maxWorkers=2 packages/data-objectstack/src/listViews.test.ts packages/data-objectstack/src/viewOverlayMarker.test.ts packages/app-shell/src/views/ObjectView.overrideMasquerade.test.ts packages/app-shell/src/views/ObjectView.setDefaultViewIdentity.test.tsx packages/app-shell/src/views/ObjectView.defaultViewIdentity.test.tsx packages/app-shell/src/views/ObjectView.viewOverrides.test.tsx -- 6 files / 59 tests passed",
          "pnpm --filter '@object-ui/data-objectstack' --filter '@object-ui/app-shell' type-check -- exit 0",
          "pnpm --filter '@object-ui/data-objectstack' --filter '@object-ui/app-shell' lint -- exit 0, 0 errors (2838 pre-existing any/hooks warnings across both packages, none touching lines this PR changed)",
          "node scripts/check-control-bytes.mjs -- OK, 4238 files scanned",
          "node scripts/check-i18n-call-site-keys.mjs -- OK, every call site resolves",
          "node scripts/check-changeset-presence.mjs -- OK, 1 changeset declared",
          "node scripts/check-changeset-no-major.mjs -- OK"
        ]
      },
      "changesets": [
        ".changeset/view-override-masquerade.md -- patch bump for @object-ui/app-shell and @object-ui/data-objectstack, describing the write-side marker + read-side exclusion behavior change. Not a skip-changeset PR (real user-visible behavior change ships), so no skip-changeset label applies."
      ],
      "notes": "Truth-table readings, all from the real production pipeline (ObjectStackAdapter.listViews() -> buildViewTabs / isSavedViewId), not hand-mocked shortcuts: (1) system view + marker-carrying override row => listViews() returns [], tab stays isSystem/readonly=true, isSavedViewId guard refuses -- pins the fix's primary case. (2) system view + legacy unmarked override row (flat body, viewKind:'list', no config) => same result -- pins the backfill predicate. (3) genuine createView-produced saved view, even reusing a system-view-shaped label ('crm_lead.default'-adjacent naming) => fully manageable (readonly=false, isSavedViewId=true) -- positive control proving the exclusion doesn't over-fire. (4) PR #4224's own no-override-row control (savedViews=[]) still holds unchanged. listViewOverrides() was verified end-to-end to keep returning the override row unchanged (viewOverlayMarker.test.ts) since ObjectView's display-merge path depends on it. isSystem/readonly/all five mutating-handler guards were NOT touched directly -- they derive from savedViews, which is populated straight from listViews(), so excluding the override row there is what makes the pre-existing predicate correct again transitively; confirmed this data flow by reading ObjectView.tsx's savedViews-population effect (dataSource.listViews call) and the isSavedViewId guard call site.",
      "summary": "Fixed objectui#4227: a system (code-defined) view's personalization-overlay row (density/sort/hiddenFields/columnState/inlineEdit, written by updateViewConfig) no longer reads back from listViews() as a user-created saved view, so it can no longer gain Rename/Delete/Set-default/Pin. Two layers per the paired 2026-08-11 ruling: updateViewConfig stamps an explicit _isOverride:true marker on every row (its one production caller is the toolbar toggle, so every row it writes is an overlay); listViews() excludes marked rows plus a measured best-effort legacy shape (flat body + registry-backfilled viewKind) for rows written before this fix. listViewOverrides() is unchanged. Console prose describing the overlay as a 'personal preference' was corrected to 'org-wide shared view settings' in 3 spots in ObjectView.tsx; a repo-wide sweep (including all 10 i18n locale packs) found no other such framing. Write-side permission gating and true per-user scope are explicitly out of scope (platform-side, per the paired objectstack#7494 ruling).",
      "tests": "See gates.results above -- 59/59 new+existing tests passing (17 in data-objectstack: 5 new listViews.test.ts cases + viewOverlayMarker.test.ts's 6; 42 in app-shell across 4 files including the new ObjectView.overrideMasquerade.test.ts's 6 cases and PR #4224's own setDefaultViewIdentity/defaultViewIdentity controls, all re-verified green), type-check and lint clean (0 errors) for both changed packages, all captured post-final-commit at HEAD 3d329df17 which matches the pushed PR's head sha.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    (Supersedes the previous comment on this issue: that attempt mis-escaped the os-dev-report marker into literal HTML entities. This comment uses the plain-text fallback form the dispatch contract names for that case.)


    Generated by Claude Code

  8. yinlianghui commented on Aug 15, 2026

    @yinlianghui
    ContributorAuthor

    os-dev-report

    Follow-up patch round on PR #4713, addressing the PM's review comment (saved-view toggle collision).

    {
      "issue": 4227,
      "status": "done",
      "branch": "claude/issue-4227-view-override-masquerade",
      "pr": "https://github.com/objectstack-ai/objectui/pull/4713",
      "pr_number": 4713,
      "pr_url": "https://github.com/objectstack-ai/objectui/pull/4713",
      "head_sha": "6ac6427ec",
      "premise_still_valid": true,
      "pm_review_finding": {
        "danger_sequence_confirmed": true,
        "evidence": "packages/app-shell/src/views/ObjectView.tsx: persistViewPatch (originally ~line 692) has NO gate on which kind of tab is active -- the onDensityChange/onSortChange/onHiddenFieldsChange/onColumnStateChange/onInlineEditChange handlers (originally ~lines 1731-1745, 1973-1995) call persistViewPatch(viewDef.id, viewDef, patch) for whatever tab is currently active, system or saved, identically. persistViewPatch writes via dataSource.updateViewConfig(objectName, viewIdLocal, {...baseViewDef, ...merged}), which saveItem('view', viewId, merged)s the SAME (type='view', name=viewId) key a saved view's own row lives at (confirmed: the real production 'create saved view' path is packages/app-shell/src/views/runtime-metadata-persistence.ts's viewEnvelope + persistRuntimeMetadata/createRuntimeMetadata, calling metadataClient.save('view', qualifiedName, envelope, {mode:'draft'}) on the SAME qualified name persistViewPatch later writes to -- ObjectStackAdapter.createView's own flat fullSpec write is a SEPARATE, unused-by-the-console path, not the real collision surface). So the first commit's unconditional _isOverride:true stamp flagged the saved view's own row as an overlay, and listViews() excluded it on the next read -- the user's own saved view vanished from the switcher the moment they toggled its density.",
        "metadata_keyed_by_type_and_name_confirmed": true
      },
      "fix": "updateViewConfig(objectName, viewId, config, opts?: {isSavedView?: boolean}) -- the marker is withheld when opts.isSavedView is true. ObjectView's persistViewPatch computes isSavedViewId(savedViewsRef.current, viewIdLocal) (the SAME predicate already gating the readonly flag and all five mutating handlers -- rename/delete/pin/set-default/config-save, which were already unaffected since they route through dataSource.updateView, never updateViewConfig) and passes it through. No new shape-inference was added; the classification the caller already computes is reused. Implementation note: persistViewPatch's useCallback deps are evaluated before the savedViews state exists further down the component (temporal dead zone), so the signal is mirrored into a savedViewsRef (same pattern as the file's existing identityPolicyRef) instead of being a direct dependency. Also updated the DataSource interface (@object-ui/types) and the VIEW_OVERLAY_MARKER / updateViewConfig doc comments, which had claimed the write was 'unconditionally' an overlay -- no longer accurate.",
      "new_truth_table_row_pinned": "packages/data-objectstack/src/viewOverlayMarker.test.ts, describe('a toggle on a SAVED view must not make it vanish'): (1) 'createView -> toggle density on THAT SAME view -> listViews() still returns it (the fix)' -- writes a viewEnvelope-shaped (nested config, viewKind outside it) row directly to the fake metadata store (the real production shape, not the adapter's own unused flat createView), confirms listViews() sees it, calls updateViewConfig(..., {isSavedView:true}) simulating the toggle, confirms listViews() STILL returns it afterward with the toggled field present. (2) 'the same sequence WITHOUT the fix (isSavedView omitted)' -- same setup, omits the flag, asserts listViews() now returns [] -- pins the regression this closes so the first test's assertion is not vacuous.",
      "changed_files": [
        "packages/app-shell/src/views/ObjectView.tsx",
        "packages/data-objectstack/src/index.ts",
        "packages/data-objectstack/src/viewOverlayMarker.test.ts",
        "packages/types/src/data.ts",
        ".changeset/view-override-masquerade.md"
      ],
      "gates": {
        "head_at_run": "6ac6427ec (post-follow-up-commit, matches PR head sha)",
        "results": [
          "pnpm --filter '@object-ui/data-objectstack^...' --filter '@object-ui/app-shell^...' --workspace-concurrency=2 build -- exit 0 (rebuilt after the @object-ui/types interface change)",
          "pnpm exec vitest run (9 files: listViews/viewOverlayMarker/onSaveAdvisory/viewCacheInvalidation.pin/listViewOverrides in data-objectstack + overrideMasquerade/setDefaultViewIdentity/defaultViewIdentity/viewOverrides in app-shell) -- 93/93 tests passed",
          "pnpm --filter '@object-ui/types' --filter '@object-ui/data-objectstack' --filter '@object-ui/app-shell' type-check -- exit 0 (first run hit stale packages/types/dist/*.d.ts TS6305 errors, resolved by rebuilding the dependency closure before re-checking)",
          "pnpm --filter '@object-ui/types' --filter '@object-ui/data-objectstack' --filter '@object-ui/app-shell' lint -- exit 0, 0 errors",
          "node scripts/check-control-bytes.mjs -- OK",
          "node scripts/check-i18n-call-site-keys.mjs -- OK",
          "node scripts/check-changeset-presence.mjs -- OK, 1 changeset declared (updated to add @object-ui/types: patch)",
          "node scripts/check-changeset-no-major.mjs -- OK"
        ]
      },
      "changesets": [
        ".changeset/view-override-masquerade.md -- updated to add '@object-ui/types': patch alongside the existing app-shell/data-objectstack patches, with a new paragraph describing the follow-up (opts.isSavedView) behavior."
      ],
      "summary": "PM review on PR #4713 correctly identified a real regression the original round-trip tests missed: persistViewPatch fires identically for a toggle on a system view OR an already-saved view, and the first commit's unconditional _isOverride marker stamp made a saved view's OWN row look like an overlay the instant its toolbar was touched, vanishing it from listViews(). Fixed by threading the caller's existing isSavedViewId classification through updateViewConfig(..., {isSavedView}) so the marker is withheld for that case -- no new shape-inference, reusing the same signal that already gates the five mutating handlers (which were unaffected, since they route through the separate dataSource.updateView). New regression tests pin both the fixed sequence and, side by side, the same sequence without the fix (to prove the assertion isn't vacuous). PR body and .changeset updated; this is a patch round on the same PR/branch, not a rework.",
      "tests": "93/93 tests passing at HEAD 6ac6427ec (post-follow-up-commit, matches PR head): the original 59 plus 34 more pulled into this run's file set (onSaveAdvisory, viewCacheInvalidation.pin, listViewOverrides -- exercised because updateViewConfig's signature changed) plus 4 new cases in viewOverlayMarker.test.ts (opts.isSavedView withholds marker; the fixed create->toggle->listViews sequence; the unfixed version demonstrating the regression). Type-check and lint clean (0 errors) across all three touched packages, all captured post-commit at the pushed head.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions