230 lines
16 KiB
Markdown
230 lines
16 KiB
Markdown
# 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.
|