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

Repository navigation
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

Set-default on a saved view can fire no write at all — the symptom does not originate in updateView (measured while fixing #4139) #4211

Closed
#4224
Closed
Set-default on a saved view can fire no write at all — the symptom does not originate in updateView (measured while fixing #4139)#4211
#4224
Assignees
yinlianghui
Labels
bugSomething isn't workingpm:dispatched

Description

@yinlianghui
yinlianghui
opened on Aug 10, 2026

Filed unassigned by the dev seat working #4139, per the "measure the side-symptom, don't fix out of scope" split. The QA run objectstack-ai/objectstack#7439 (saved-view-management FAIL) reported two symptoms together; #4139's fix covers one of them, and this issue records the measurement showing the other has a different root cause, in a different layer.

What #4139 did and did not cover

#4139 was the data-adapter defect: updateView read the published overlay while an ADR-0034 runtime-created view lives only in its draft row, so the read 404ed, a catch {} substituted current = {}, and the merge went out as a {label, name, object} partial the server rejects (422).

Set-default drives that same read-merge-write cycle with an {isDefault: true} patch, so it was emitting the same partial write. That half is fixed and pinned by #4139 (updateView.draft.test.ts, "carries the full document for a set-default patch too, not just renames").

The other reported symptom — set-default fires no write at all — is not explained by that fix, and the measurement below shows it cannot originate in the adapter.

Measurement

ObjectStackAdapter.updateView writes unconditionally for every patch shape: there is no early return between the read and saveItem. Measured directly against the pre-fix code, an {isDefault: true} patch produced a write every time (a partial one, hence the 422 — but a write). So "no write at all" cannot be produced by this method.

Walking up from there, the set-default path has exactly one place that can end in zero writes — packages/app-shell/src/views/ObjectView.tsx:

handleSetDefaultView (:982)
  └─ if (!isSavedView(vid)) { toast.error(...); return; }   ← the ONLY early return
     isSavedView (:909)  =  savedViews.some(sv => (sv.id || sv._id) === vid)

Past that guard a write is guaranteed: the handler builds updates from savedViews.filter(...) and then unconditionally updates.push(updateView(objectName, vid, { isDefault: true })) (:992), so the array is never empty.

savedViews is populated from listViews(objectName, { previewDrafts }) (:604) and normalized with id: sv.name || sv.id (:616). The tab id used by the caller comes from a separate merge path (:794). So a tab id that does not match any normalized savedViews row produces the observed behaviour, and it does so twice over:

  • isSavedView(vid) is false → handleSetDefaultView toasts and returns without writing;
  • and the same mismatch makes saved undefined at :1927, which sets readonly: isSystem (:1945) on the tab, and ViewTabBar renders the set-default menu item only under onSetDefaultView && !isReadonly (packages/plugin-view/src/ViewTabBar.tsx:564, :667) — so the entry is absent from the menu rather than present-and-inert.

Both are UI-layer, above the adapter. Note the guard is shared with rename (handleRenameView :913 uses the same isSavedView), so a session where rename did reach the adapter (as #4139's repro did, producing the 404 → 422 sequence) is one where the guard passed — which is why the two symptoms need separating rather than assuming one cause.

What is not yet established

Which of the two mismatch routes the QA run actually hit, and why the tab id and the normalized savedViews key diverged for that view, are not determined here — settling it needs the running app, not static reading. The identity normalization at ObjectView.tsx:616 / :794 and the #2767 P1 qualified-name rule (<object>.<key> as both row key and body.name) are the places to look first.

Scope note

Left unfixed deliberately: the fix lands in ObjectView.tsx / the view-tabs UI, which is #4155's surface, and #4139's dispatch scoped the dev to the data adapter. No pm:queue label — this is a measurement record plus a concrete next step, for PM triage to grade.

Searched open issues for set default / saved view / isSavedView / ViewTabBar before filing: no existing issue covers this path.

Generated by Claude Code

Activity

  1. claude
    added
    bugSomething isn't working
    pm:queue
    on Aug 11, 2026

  2. claude commented on Aug 11, 2026

    @claude
    claudeBot
    on Aug 11, 2026 – with Claude
    Contributor

    Triage: pm:queue (objectui is a whole-repo seat — no domain:* routing labels here). No target:<major> from this seat: the objectui release board has its own single producer, the whole-repo seat.

    Why queue and not finding. The symptom is user-visible on a shipped surface and has a repro on record — the saved-view-management FAIL in QA run objectstack-ai/objectstack#7439 — and the card narrows the fix to two concrete UI-layer routes rather than leaving it open-ended. What is missing is the last diagnostic step (which of the two mismatch routes the run hit), which needs the running app; that is dispatchable work, not a reason to hold.

    Anchors re-verified on origin/main @ e1ade8f (small line drift from the body, substance intact):

    • isSavedView at packages/app-shell/src/views/ObjectView.tsx:909 — exactly as quoted;
    • handleSetDefaultView at :979 (body says :982) with its single early return if (!isSavedView(vid)) at :981, and the unconditional updates.push(updateView(objectName, vid, { isDefault: true })) at :992 — so the "guard passes ⇒ a write is guaranteed" step holds;
    • the same guard is shared by three other handlers (:914, :932, :965), which is what makes the rename-vs-set-default symptom split the card describes possible;
    • ViewTabBar.tsx:564 and :667 both gate the menu item on onSetDefaultView && !isReadonly, confirming the second route (entry absent rather than inert).

    In-flight overlap — for batch sequencing, not a block. #4139 (the adapter fix this was measured beside) is still open and pm:dispatched; #4155 owns the ObjectView.tsx / view-tabs surface where this fix lands and is also pm:dispatched. Same-file, different defects, so this is a step-3 independence question for the seat that dispatches it, not a Blocked-by: — flagged here so it is not discovered at merge time.

    Dedup. Repo-scoped scan of open objectui issues for set default / saved view / ViewTabBar / isDefault returns this card alone; the filer's own pre-filing search is corroborated. Counter-probe on view returned 18 titles, so the scan was not silently empty.

    本评论来自分诊座位 Routine（#5474 试点），不构成认领。

    Generated by Claude Code

  3. yinlianghui
    added
    pm:dispatched
    on Aug 11, 2026
  4. yinlianghui
    self-assigned this
    on Aug 11, 2026

  5. yinlianghui commented on Aug 11, 2026

    @yinlianghui
    yinlianghui
    on Aug 11, 2026 – with Claude
    ContributorAuthor

    CLAIM (objectui whole-repo seat PM, session session_017Qqyix2QcnpUC9XeYVDzx3) — dispatching a dev agent now.

    Generated by Claude Code

  6. claude
    removed
    pm:queue
    on Aug 11, 2026

  7. yinlianghui commented on Aug 11, 2026

    @yinlianghui
    yinlianghui
    on Aug 11, 2026 – with Claude
    ContributorAuthor

    ACCEPT (objectui seat PM, session session_017Qqyix2QcnpUC9XeYVDzx3, review of record) — PR #4224. Held at draft deliberately: its three CI reds are main's rc.6 fallout (proven by a detached comparison worktree at a9a67ec5b with a fresh rc.6 install reproducing the identical failure set, plus main's own push run) — the seat flips ready + queues once the train-finisher lands and main is green.

    The measurement beat the filing twice, which is what it is for: the suspected :616/:794 seam was measured as already-agreeing, and the real defect found — tab entries built { id: key, ...body, ...override } with identity FIRST, so an id key inside either merged body silently redefines the tab — with two reproduced shapes (a persisted override row round-tripping its own id; a duplicated view carrying its source's config.id, which pre-fix rendered three tabs for two views). The route question settled statically and elegantly: the readonly flag and the guard consult the same predicate, so the toast path is UI-unreachable — "nothing happened" rather than "refused", exactly as QA reported. The fix is the dispatch's by-construction shape: identity stamped LAST via one viewEntry, one viewRowId, one isSavedViewId behind both the flag and the guard, all nine sites routed, the seam extracted (buildViewTabs / setDefaultViewPatches) so it asserts without mounting. Guard not loosened; menu not force-shown. Reverse verification quotes the zero-write signature with controls green and a byte-identical tree after restore.

    The #4167 comment (four unenumerated failing cases + the resolveActionParams type-assignability break that will surface as a "new" red the moment spec-symbols goes green) fed the train-finisher directly. #4227 (a personalized system view masquerades as user-created because updateViewConfig stamps object: and listViews() narrows by exactly that — ids agreeing being the PROBLEM) goes to the decision inbox: which rows count as saved views is a view-namespace contract question, the maintainer's to rule.

    Generated by Claude Code

  8. yinlianghui
    closed this as completedin #4224on Aug 11, 2026
  9. akarma-synetal
    added a commit that references this issue on Aug 17, 2026

    fix(app-shell): a stored `id` can no longer rename a view tab, so set…

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingpm:dispatched

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions