Opus re-review of 4fada238e (Slice A review-fix round) left seven residuals.
All seven addressed here, in one commit per the residual-round contract:
NEW-1 (test-coverage): no test pinned the Draggable:true->false change
itself. Added Drag_StartingOnShelfPadding_DoesNotMoveTheShelf, pressing the
shelf's own PADDING (left of the first button's Left=4, below the 12px grip)
and dragging 100px through the real UiRoot press/move/release path. Proven
to fail against the pre-4fada238e shelf: temporarily setting Draggable=true
in the ctor and re-running this test moved the shelf from Left=760 to 764
(clamped to parent.Width-Width) instead of leaving it in place — reverted
after confirming.
NEW-2 (correctness): RetailWindowLayoutPersistence.ClampAllToScreen clamps
generically to screen.Width-handle.Width, 4px (OuterPadding) off what the
shelf's own dock formula (parent.Width-Width-OuterPadding) would produce for
the same new parent size. A screen shrink that triggers that clamp used to
flip _userPositioned permanently even though nothing a user would call
"dragging" happened. PluginSidePanel.OnHandleMoved now re-derives "still
docked" at comparison time as a function of the CURRENT parent size: either
the live dock formula's own result, or what ClampAllToScreen's clamp would
produce from the PREVIOUS docked position — only a position matching
NEITHER flips _userPositioned. New test
ClampAllToScreen_AfterShrinkingTheRoot_DoesNotFlipAnchoring_ButARealDragStillDoes
shrinks the root, runs ClampAllToScreen, confirms the shelf still anchors
top-right through a later reflow and a collapse, then confirms a genuine
grip drag afterward still flips anchoring.
NEW-3 (decision, documented): KeepWindowReachable's per-tick clamp and
ClampAllToScreen's screen-resize sweep both touch plugin windows. Evidence
gathered and recorded as a doc comment on KeepWindowReachable:
RetailWindowManager.MoveTo (line 178) already short-circuits an unchanged
position before ever raising Moved; persistence's ScreenSize and Host.Root's
size derive from the same d.Window.Size and are reconciled every frame
(UiHost.Draw sets Root.Width/Height from screenSize); and because
RetainedGameplayUiFrame.Render ticks BEFORE it draws in the same frame,
ClampAllToScreen always resolves a screen-resize's clamp before
KeepWindowReachable ever observes the new size next tick, making
KeepWindowReachable's own pass a structural no-op for that case (never a
second write). KeepWindowReachable is NOT dropped, though: it is the only
reachability guarantee for (a) a plugin window whose geometry is mutated
directly rather than through MoveTo — pinned by the pre-existing
FullWidthPluginWindowStartsAndStaysReachableAtMinimumCanvas test, which
exercises exactly that with no screen resize at all — and (b) a host wired
with no RetailWindowLayoutPersistence at all. Decision: keep both; they do
not race.
NEW-4 (robustness): RetailWindowLayoutPersistence._attached is mutated
mid-session by WindowRegistered/WindowUnregistered (a callback invoked from
inside one of these loops — e.g. Apply -> Show()/Hide() -> a controller
unregistering another window — could otherwise mutate the list being
enumerated). All five bare `foreach (... in _attached)` loops
(RestoreAllCore, ClampAllToScreen, SaveAll, SaveNamed, RestoreNamed) now
snapshot with .ToArray(), matching Dispose's existing pattern.
NEW-5 (retail-faithfulness): the plugin-shelf hide message hard-coded
"Shift+Ctrl+F1" even though InputAction.TogglePluginManager is rebindable
through Configure Keyboard. RetailUiRuntime.PluginShelfHiddenMessage now
looks up the CURRENT binding via the live InputDispatcher and formats it
through Layout.RetailKeyNames.Describe — the exact formatter Configure
Keyboard's own row captions already use
(Layout/KeyboardConfigController.cs:284) — falling back to an honest
"bind it in Configure Keyboard" message when the action is unbound or no
dispatcher is wired.
NEW-6 (bookkeeping): added divergence-register row IA-27 for the plugin
shelf's repurposing of retail's plugin-manager chord (Shift+Ctrl+F1),
its ASCII </> toggle glyphs, its two acdream-authored system messages, and
its Draggable=false grip-only drag model — citing PluginSidePanel.cs and
the RetailUiRuntime.cs TogglePluginManager case. Header count bumped
23->24 active IA rows.
NEW-7 (correctness): the one-time dock wrote Left directly, so the
first-run docked position went unsaved until some later, unrelated event
happened to trigger a save. PluginSidePanel.OnTick now routes the one-time
dock through the retained-window handle's MoveTo when registered (falling
back to the direct field write when unregistered, as before) —
_dockLeft/_dockTop are set to the TARGET position BEFORE calling MoveTo so
the synchronous OnHandleMoved re-entry it triggers (NEW-2's logic) reads the
new dock position and does not flip _userPositioned. New test
FreshShelf_OneTick_SavesTheDockedPositionImmediately confirms the store has
the docked X/Y after exactly one tick and that a subsequent collapse still
preserves the right (not left) edge.
Verification: dotnet build src/AcDream.App (Release) green; targeted filter
(PluginSidePanel|RetailWindowLayout|Markup|UiRootInput|KeyboardConfig) 145
passed / 2 pre-existing Lane=Manual failures (unrelated, gated on
ACDREAM_PROBE_LIVE_MOUNT=1); full tests/AcDream.App.Tests suite: 7306
passed / 97 skipped / 36 failed — same 36 pre-existing failures by name
(installed-DAT/Cathedral/Linux/Lane=Manual probe tests), +3 passed vs the
7303 baseline (exactly the three new tests this round added).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
718005b210