Skip to content

Restored multi-region ProportionalDock inner layout renders both regions headerless — real tabs present but header items missing (duplicate-instance last-writer-wins wiring from persistence leak + non-atomic restore of unresolvable descriptors) #1334

Description

@JoshuaRowePhantom

Summary

The owner's real "Phantom.Workspaces" workspace persists a saved dock-layout that, on restore,
renders as follows:

The previously-diagnosed causes (missing inner-scope IRootDock template; DFS-first-only wiring)
were fixed and MERGED (see "Considered / Background" below), and the bug still reproduces.
This body is a corrected, empirically-grounded rewrite of the diagnosis.

Root Cause

Two distinct mechanisms both produce headerless tabs on restore. They are independent, both real,
and coupled through the persistence path.

PRIMARY — duplicate WorkspaceDocument instances + last-writer-wins registration

Root cause: the persistence leak diagnosed in #1335. On every save,
DockSerializer.JsonConverterList bypasses ReferenceHandler.Preserve for
ActiveDockable/DefaultDockable/FocusedDockable, emitting them as full inline clones instead
of $ref markers back to their canonical VisibleDockables sibling. Restore deserializes those
clones as DISTINCT WorkspaceDocument objects; nothing collapses them.

WorkspaceDockFactory.RegisterDocument (WorkspaceDockFactory.cs:62-67) is last-writer-wins on
documentsByTabId[tabId]. With N duplicate WorkspaceDocument instances per tab Id, the entry
that wins is nondeterministic. Consequently the instance whose header services were wired (icon,
favicon, status, running, notification items on WorkspaceDocument.EffectiveTabHeader,
WorkspaceDocument.cs:150) is not necessarily the instance actually attached under a rendered
WorkspaceContentDock. The rendered instance's EffectiveTabHeader still points at the plain
fallback TabHeaderViewModel created in the parameterless constructor with only the
auto-added StatusTabHeaderItemViewModel (WorkspaceDocument.cs:22-27,149-150) — hence real tab
titles are present (the tab VM has a Title) but per-item header content is empty.

This is the mechanism that matches the owner's observed symptom exactly: the LEFT region keeps
its 5 real tabs but they render headerless.
Real tab VMs — wrong document instance wired.

SECONDARY — non-atomic restore + unresolvable descriptor stubs

Independently reproducible in tests. In TryRestoreFromDockLayoutAsync
(MainWindowViewModel.cs:3528-3640):

  1. workspacePane.ContentLayout = layout (:3598) is committed before success is proven.
  2. WireContentDock(primary, ownsTabs: true) calls dock.VisibleDockables?.Clear()
    (WorkspaceDockFactory.cs:84), destroying the primary region's stub documents unconditionally.
  3. For every non-primary region the guard if (stub.TabViewModel is not { } tabVm) continue;
    (WorkspaceDockFactory.cs:99-102) SILENTLY SKIPS any stub whose descriptor failed to resolve
    (ContextLocator returned null / tabResults[i] was null). The stub is left permanently
    uninitialized: its EffectiveTabHeader remains the empty fallback created in
    WorkspaceDocument() (WorkspaceDocument.cs:22-27), it is never Initialized, and it is
    never registered.
  4. If ALL descriptors failed, success stays false and the caller falls back to a single
    default EntityWorkspaceTabViewModel (MainWindowViewModel.cs:3498-3519) — the "one giant
    fallback tab" symptom seen in the secondary repro.

Note the empirical distinction between the primary and secondary mechanisms: in the secondary
repro the LEFT region degrades to a single fallback tab (because success was false and the
caller substituted the default). In the owner's observation, the LEFT region keeps its 5 real
tabs
— so the primary (leak + duplicate-instance) mechanism better matches the observed field
symptom, while the secondary mechanism is a real but distinct failure mode worth fixing in the
same change.

Empirical evidence: a new failing end-to-end test drives the real
OpenWorkspaceAsync → TryRestoreFromDockLayoutAsync path with the real layout shape (double-nested
proportional, mixed typed/base, overlapping tab Ids, unresolvable descriptor):

Phantom.Workspaces.Tests/MainWindowDockTemplateTests.cs
  → Bug1334_RealLayoutShape_DoubleNestedProportional_MixedTypedAndBase_OverlappingIds_BothRegionsRenderHeadedTabStrips

It asserts both regions render headed tab strips and FAILS on HEAD with "TabViewModel is null".
The test is present in the features working tree but intentionally uncommitted.

Why this was invisible to prior tests

Missing-coverage analysis: every prior test in the #1324/#1330/#1334 series used one of

  • BrowserDockTabDescriptor — URL-only, always resolves via ContextLocator.
  • Entities upserted immediately before restore — always resolve.
  • Hand-built single-level dock trees with NO duplicate WorkspaceDocument instances and NO
    ActiveDockable/DefaultDockable/FocusedDockable aliasing.

Both mechanisms above were therefore dead code in the test suite. The new test uses the real
double-nested/mixed/overlapping-Id/unresolvable-descriptor shape and exercises both.

Affected Files

File Lines Role
Phantom.Workspaces/ViewModels/MainWindowViewModel.cs 3528-3640 TryRestoreFromDockLayoutAsync; premature ContentLayout = layout commit at 3598; single-default fallback at 3498-3519
Phantom.Workspaces/ViewModels/WorkspaceDockFactory.cs 62-67 RegisterDocument last-writer-wins on tab Id
Phantom.Workspaces/ViewModels/WorkspaceDockFactory.cs 80-111 WireContentDockVisibleDockables.Clear() before success; silent-skip guard at 99-102
Phantom.Workspaces/ViewModels/WorkspaceDocument.cs 22-27, 140-150 Parameterless constructor; fallback EffectiveTabHeader cached in cachedTabHeader
Phantom.Workspaces/ViewModels/WorkspaceDocumentGenerator.cs 49-56 PrepareDocumentContainer — the correct creation-time wiring seam
Phantom.Workspaces.Tests/MainWindowDockTemplateTests.cs New failing test Bug1334_RealLayoutShape_… (uncommitted, in working tree)
Phantom.Workspaces/Templates/DockDataTemplates.axaml 10-12, 124-136 Existing inner + outer scope IRootDock templates (merged fix; not the cause)

Design / Fix

Per the owner's directive: wire header services at factory document-creation time, not via
FindDocumentDock / visual-tree probing. Three coordinated changes:

(1) Depend on the persistence-leak fix (#1335)

Requires exactly one WorkspaceDocument instance per tab Id at load time (heal-on-load) and at
save time (canonical single-JsonSerializer.Serialize write path). Without this,
RegisterDocument is racing between duplicates and no amount of downstream wiring can guarantee
the correct instance renders. This bug depends on #1335.

(2) Wire per-document services in WorkspaceDocumentGenerator.PrepareDocumentContainer

Move all per-document header/status/notification/running/favicon wiring into
PrepareDocumentContainer (WorkspaceDocumentGenerator.cs:49-56), immediately after
doc.Initialize(tab). Removes:

  • Any FindDocumentDock / visual-tree scanning the owner has objected to.
  • The WireContentDock non-primary post-hoc initialization path (WorkspaceDockFactory.cs:91-110)
    as the primary place per-document services get attached.

Combined with (1), every WorkspaceDocument present in the restored tree passes through
PrepareDocumentContainer (or is initialized in place using the same shared helper) and is fully
wired before it can render.

(3) Make restore atomic and robust to unresolvable descriptors

In TryRestoreFromDockLayoutAsync:

  • Never assign workspacePane.ContentLayout = layout before proving success. Build the whole
    restored tree, initialize every stub, verify at least one initialized tab, THEN swap.
  • Never VisibleDockables.Clear() the primary before success is guaranteed.
  • Give every stub a live placeholder tab VM when its descriptor cannot be resolved (so that
    stub.TabViewModel is never null, WorkspaceDocument.Initialize runs, EffectiveTabHeader is
    populated with the header items, RegisterDocument succeeds, Alt+Digit DockTabOrder sees the
    document, and Ctrl-click new-window anchor resolution finds the owning dock).

Empirical validation: a placeholder-for-null-tab-VM patch was applied locally and made the new
failing test PASS. That change was reverted and not committed; it is the minimal safety patch
that closes the secondary mechanism. The full fix should also address the atomicity of the
ContentLayout swap.

Expected Tests

Considered / Background — attempted and merged but insufficient

Attempt Commit Why it didn't resolve the bug
Add missing inner-scope IRootDock template in the pane DockControl scope (Templates/DockDataTemplates.axaml:124-136) — the previously-diagnosed root cause. e950664c "Fix #1334: render restored multi-region panes with headed regions" Necessary for restored IRootDock graphs to bind a RootDockControl at all, but the observed bug is per-document header emptiness on documents that ARE rendered, not a missing root template.
Uniform wiring for every restored WorkspaceContentDock (drop DFS-first-only wiring; call WireContentDock on every region; primary owns tabs, others re-register their restored documents). e950664c Fixes the asymmetry for non-primary regions in the clean case, but the silent-skip guard at WorkspaceDockFactory.cs:99-102 and the duplicate-instance race defeat it in the real bloated shape.
Base-DocumentDockWorkspaceContentDock migration on restore (MigrateBaseDocumentDocksToWorkspaceContentDock). 4eb823d0 "Fix #1324: guarantee workspace dock type on restore so inner tab headers render" Ensures every region has the correct .NET type + HeaderTemplate, but the owner's LEFT region is ALREADY typed and STILL headerless — so dock type is not the bottleneck for the observed symptom. Migration is also VisibleDockables-only (see #1335).
Visual regression tests for restored multi-region header rendering. 12069e6d "Add visual regression tests for #1334 restored multi-region header rendering" All fixtures used BrowserDockTabDescriptor / just-upserted entities / single-level trees with no duplicate instances, so both current mechanisms were dead code in tests.

Net: the merged commits are correct and worth keeping, but they address the render-template /
per-region-wiring pipeline. The remaining failure is upstream in persistence and restore
instance identity
, which is why the observed symptom persists at features HEAD 12069e6d.

Relationship to other issues

Metadata

Metadata

Labels

bugSomething isn't workingdiagnosedRoot cause identifiednext-upverified-locallyImplementation has been verified locally

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions