Files
ARCADIA-Studio/harness/multiselect-architecture-review-20260923.md

230 lines
16 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Architecture multi-selection critique — 2026-09-23
Independent read-only review of `b8685416f56e53e30bd11ecf93a54cea10ac8458`
for issue #29 by `transition_picker_review`. This is a proposal, not passing
implementation or native evidence. Only this note was written; no Cargo, GUI,
production changes, or coordination posts were made. The parent owns synthesis.
Architecture geometry can be a foundation, but does not complete the requested
coherent outcome across Allocation and Behavior. See the parallel
[Behavior review](multiselect-behavior-design-20260923.md).
## Verified constraints
| Boundary | Evidence at reviewed commit | Consequence |
| --- | --- | --- |
| Selection | `app.rs:790–835,1107,2477` uses semantic `selected` and one `selected_node`; `refresh_draft` derives placement again from semantic identity. | A new group cannot be inferred from the inspector selection or rebuilt as a singleton after each refresh. |
| Identity | `validation.rs:794` permits one placement per element per diagram; `projection.rs:169` resolves port endpoints to their owner placements. | Placement identity and active diagram still matter. A port does not become a separately movable card. |
| Ordinary movement | `diagram.rs:463,985,1166` previews one root plus descendants; `app.rs:3296` submits `MoveNode` against the then-active diagram after `apply()`. | New group intents need captured context and exact preview/commit parity, not repeated old messages. |
| Routes | `projection.rs:191` translates pins only when both actual endpoint owners share the same nonzero delta; State adds boundary checks. | A single simultaneous displacement map is necessary for unrelated selected endpoints. Labels/attachments retain relative data. |
| Allocation | `diagram/work_drag.rs:65` captures a work scope; `app/allocation_drag.rs:429,568` selects one work root on press and after commit. | Existing handlers would collapse a group. Work-body drag changes leaf allocations, not merely coordinates. |
| Admission | `app/allocation_drag.rs:353–411` checks owner, drafts, snapshot, context, camera and lifetime; `diagram/work_drag.rs:118` cancels on view/input changes. | Reuse this admission discipline, including rejection before shared state mutation. |
| Focus | `diagram/focus.rs:195` drops all keyboard events when canvas lacks focus. | Canvas-local modifier tracking alone misses Shift pressed while the inspector owns focus. |
| Removal | `app/diagram_removal.rs:114–260` distinguishes placement Hide from confirmed semantic Delete and captures one target/token. | Group removal needs an explicit set and a single combined confirmation. |
## Selection contract
Use transient selection scoped to document-instance and surface identity, with
ordered unique typed targets and one primary member for the existing inspector.
For ordinary diagrams, movable members are exact node placements. The owner
must distinguish a reopened document even if its project UUID is unchanged.
Selection ownership should not itself be tied to model revision: surviving
members should remain selected after a successful group move or primary-property
edit. Gesture authority must additionally capture revision/snapshot, epoch,
selection generation, camera and actual canvas bounds.
Recommended interactions, consistent across applicable families:
- Plain click an unselected card replaces selection. Shift+click toggles it;
adding makes it primary, and removing the primary promotes the most recent
remaining member. Shift selection must precede port/route/reorder handle logic.
- Pressing an already selected card preserves the set so dragging works. The
synthesis chooses collapse to that member on a completed stationary release;
this is acceptable consistently across families. A cancelled, refused, or
threshold-crossing drag must retain the group even if it ends at its origin.
- Right-click a member retains the group. Right-click elsewhere selects that
target before opening its menu. A stationary blank click clears; a pan gesture
retains selection. Do not replace existing blank-drag pan with marquee silently.
- Paint every member, distinguish primary without relying only on color, and show
the count. Inspector edits remain explicitly primary-only. Do not expose
singleton commands under a group label or imply bulk property editing.
- Track modifiers passively through the relevant input owner even before it has
keyboard focus; this does not grant permission for canvas shortcuts in text
fields. Freeze press-time modifiers and clear them on window focus loss.
Typed edge/port selection can be shared with other families, but command
capabilities must remain explicit. Edges do not have node coordinates; port
anchors are owner-local. A mixed set must never silently move only its primary
or eligible subset. If placement-only grouping is delivered first, report that
scope and give an explicit unavailable-action reason for other target types.
## Simultaneous movement and alternatives
Prefer a pure planner which accepts the original diagram, selected placement
IDs and one delta, unions roots with visual descendants, translates each placement
once, calls `translated_routes` once on the original geometry, and emits one
`Batch(EditNode..., SetEdgeRoute...)`. Use the same plan for preview and commit.
Snap one shared delta, based on the grabbed anchor, rather than snapping each
node independently and changing their spacing. Nonselected records and other
diagrams must remain exact.
Sequential `Batch(MoveNode(A), MoveNode(B))` is unsuitable: parent+child can move
twice, and a route between unrelated selected endpoints never sees both owners
in the same displacement map. A dedicated serialized `MoveNodes` command could
eventually express public API intent, but adds schema/documentation/compatibility
cost without being needed for the GUI's atomic transaction. Selection itself
does not belong in the model/source schema. An intent command also would not
remove the need for preview planning or stale-context checks.
Use visual ancestry for placement closure, semantic work ancestry for allocation
leaf normalization, and semantic deletion dependencies for model removal. These
are different graphs. Folded descendants travel with a moved visual parent,
but selecting parent+child must not add a second delta. Selected port owners
already carry their ports and relative anchor/label data; do not translate that
data again. Internal routes translate once; one-ended routes keep world pins;
automatic routes reroute. Reuse State boundary rules without broadening their
meaning. Cache keys must include the captured selection/plan as well as delta:
the existing singleton preview-position cache key is insufficient.
## Allocation decision
Prefer the parent's proposed shared-destination, all-or-nothing group allocation
over always-arrange multi-selection. Shift changes cardinality, not drag meaning;
silently disabling reassignment only for groups would be unpredictable.
Normal Allocation drag of a work-only selection should normalize semantic roots
and all leaves once, evaluate one frozen performer/outside destination for the
entire set, and preserve semantic ownership. Every constituent subtree must
satisfy existing role, mixed-allocation and geometric-coherence rules; one
refusal rejects the whole operation. Separate valid roots starting under different
performers are not inherently ineligible for an explicit common target. Never
weaken role-exit or collateral-change protections to make a group succeed.
Exclude all moving placements from frozen drop occlusion. Allocate new relation
UUIDs once per leaf, reuse them on target revisits, and commit precisely the
previewed candidate through one Document transaction and one Undo step. Layout
must preserve group spacing while respecting the destination's constraints; an
impossible placement refuses the group rather than packing a subset.
Arrange only uses the pure simultaneous geometry planner and preserves every
allocation record. A mixed performer/work/other set cannot promise semantic
reassignment: refuse a normal work-primary drag with actionable Arrange only
guidance, or offer an explicit group Move action. This remains a dependency of
full #29 acceptance even if geometry support lands first.
## Other coherent actions and lifecycle
Group Hide removes exactly the selected placements from the active diagram,
preserving semantic elements and other diagrams. Do not reuse movement's
descendant closure: hiding a parent must not silently hide unselected children.
Ports/edges without placements must not fall through from Hide to model Delete.
Group Delete from model needs one combined impact preview over unique semantic
targets, with overlapping descendants/incidents counted once. Avoid sequential
deletes of a parent followed by an already-removed child. One invalid/read-only
member rejects the whole command before any mutation.
Fold/Unfold should apply one explicit state to all eligible selected containers,
not toggle each member into the opposite mixed state. Select All visible cards,
clear selection, nudge, and align/distribute are useful discoverable related
actions; alignment is geometry-only and not automatically suitable for derived
Behavior layouts. Duplication and bulk semantic-property edits need separate
contracts. Existing menus/help should advertise Shift+click, group count,
Arrange only, Hide versus Delete, and unavailable-action reasons.
Selection-only changes create no model/source/history entries. Changing the
primary must not discard an unfinished inspector draft. A group gesture requires
clean admitted input; do not auto-apply an unrelated draft merely to start or
finish it. Dirty/read-only/pending/modal contexts keep their current input owner.
Opening a modal cancels drag authority but can retain visible selection; closing
it must not revive that drag. Source apply, Undo/Redo and property changes prune
vanished/invisible members while retaining survivors. Explicit UUID edits need
known identity remapping or safe pruning, never alias/name matching. Switching
document/diagram/layer/owner clears the active set. Native pointer ordering and
press/release batching must remain respected. Successful moves retain the group;
Hide/Delete prune it. Automatic reselection after Undo of deletion is a separate
UX decision, not an implied model-history guarantee.
## Falsifiers before acceptance
1. Shift held in a clean inspector before clicking the second function adds it;
no property text, model, source or history changes. Shift-click a selected
route/port handle does not start another editing gesture.
2. A parent and its child, plus an unrelated sibling, move once by one delta;
folded descendants and both-ended custom pins follow once. One-ended pins,
attachments, label offsets, allocations and other diagrams remain exact.
3. Two unrelated selected function endpoints move with their internal route;
preview projection/routing equals committed projection/routing. Undo/Redo is
one exact operation; zero displacement adds no history.
4. Allocation of two valid roots to one performer succeeds atomically. Add one
role-exit/collateral/ineligible member: candidate and release both refuse
without partial allocation. Arrange only moves the same set with allocation
records byte-for-byte unchanged.
5. During a drag, change selection, dirty source, open a modal, lose focus,
resize/pan/zoom, replace the document with the same project UUID, or deliver
an old release after a new press. No stale operation commits or cancels a
newer owner's gesture. Esc preserves model/source/history and the selection.
6. Group Hide preserves semantic records and unselected child placements;
group Delete previews the union once and preserves unrelated counterparts.
Mixed nonplacement/read-only selection never becomes partial success.
7. Group movement and ordinary primary-property Apply retain surviving members;
inspector clearly edits only the primary. Right-click a selected card keeps
group actions; text-field Delete/Ctrl+A remain text actions.
These are proposed evaluators, not reported test passes. No edits to peer-owned
#16 resize or #31 traceability/navigation files are recommended by this note;
coordinate any necessary shared integration through their owners.
## Review of the parent synthesis
Reviewed `multiselect-contract-20260923.md` after its initial synthesis. Its
common selection plus family-specific planners, simultaneous geometry,
all-or-nothing Allocation, derived-order adapters and aggregate removal form a
coherent scope. The architecture planner is a useful foundation and does not
by itself establish full issue acceptance. No production implementation was
reviewed or executed in this pass.
The selected-member stationary-release policy is a reasonable choice. My
initial preference for retaining the group favored easy primary inspection;
collapse favors the ordinary single-target click convention. Neither needs a
second preference setting. The chosen policy needs these precise boundaries:
- Collapse only after a valid stationary press/release which never exceeded the
drag threshold. A drag which returns to its origin or snaps to zero has still
been a drag. A rejected destination, Escape, stale release, focus loss, or
modal entry must not reinterpret a group gesture as a singleton click.
- Freeze selection and primary for gesture authority. Refreshing the inspector
on selected-member press may change canvas bounds and cancel the gesture
through the existing geometry guards. Defer that refresh until the outcome,
or prove that a deliberately frozen pane arrangement preserves the captured
geometry. Do not weaken cancellation guards merely to mask this interaction.
- Specify whether the grabbed member becomes primary after a completed group
drag; either choice is viable, but the inspector and primary highlight must
agree while the full group remains selected.
Two capability details remain material implementation decisions:
1. Exact visual targets include ports, edges and read-only occurrences, while
the geometry planner consumes placements. The adapter must evaluate the
complete selection before calling it. For a mixed unsupported group, refuse
with an explicit reason; do not silently filter to placements, promote a
selected port to its owner, or infer an edge's endpoints as selected nodes.
A separately named nodes-only action could be specified later, but cannot
be implicit in the ordinary group drag. Arrange only has the same admission
obligation even though it preserves allocations.
2. Context Add/Remove must capture an exact target and retain the previous set.
The current singleton-select-before-menu pattern cannot run before Add or
the existing set has already been lost. Right-click a selected member should
retain its group for group actions; offering Add for an unselected target
requires retaining the prior set until that explicit command is resolved.
Therefore the blanket rule that right-click elsewhere first replaces the
selection needs an exception or an explicit two-stage menu contract. Menu
dismissal must not accidentally apply Add, Remove, or a singleton mutation.
Additional falsifiers for the final policy: press a selected member, cross the
threshold, return to origin and release (group retained, no history); drag to
an invalid allocation target (group retained, explanation, no edits); open a
context menu on an unselected function and choose Add (old members preserved);
then remove the primary (latest surviving member becomes primary). Repeat the
first press from an inspector with different field height and confirm that
inspector refresh cannot turn the gesture into a stale drop. A mixed node/port
selection must receive the declared refusal rather than a partial move.