9.8 KiB
Allocation label checkpoint review — 2026-09-12
Reviewer: /root/allocation_checkpoint_review. Read-only source review of the saved 6b9cc77 allocation change, initially at 94b5eec; the parent subsequently formatted it at d9b3f0dc91e8882e6efc262c2e8d971998d2694e. Read applicable AGENTS/PROJECT_GOAL, the v2 contract, prior repair review, planner, App, rendering and tests. Root owns remote/shared coordination. No compiler, tests, native UI, Gitea or Git/source mutations were performed; this report is the sole write. Findings are source-derived, not runtime observations.
[P2] Joint-label refusal removes every destination point for an ordinary collapsed-performer drag
Primary location: crates/oa-desktop/src/allocation_drag.rs:593 (new rejection at 597–602). Related: app/allocation_drag.rs:98 (label geometry), :120 (collapsed hit-region height), :188 (frozen resolver), :554 (pointer-relative requested placement).
Concrete OA/SA counterexample using app/allocation_drag_tests.rs:28 fixture:
- Rename A to
Receiving subsystem(19 characters), and set its existing placement'scollapsedflag. Keep A at(100,100)with default224×94size, Work at(100,500)with the same default size, and zoom 1/zero pan. No ports or other content are needed. The default size is defined atoa-core/src/model.rs:1944; neither the collapsed projection nor port minimum-size logic expands the width for this name (projection.rs:314,projection/port_layout.rs:131). - Press the work name at
(140,535), the normal fixture grab offset(40,35)(app/allocation_drag_tests.rs:95). Attempt to move/release anywhere in A's entire exposed header. - A's frozen targetable rectangle is half-open
[100,324) × [100,162). Its frozen label is[110,308) × [131,153)(19 characters × 10 + 8 = 198 wide). - For every pointer in that targetable rectangle, requested work coordinates are
(pointer.x−40,pointer.y−35). Grid snapping gives work-header left in{60,70,…,280}and top in{70,80,…,130}. The header remains224×62: the work is a leaf and only its new performer ancestor expands (oa-core/src/projection.rs:541). - Every such header has left < 308 and right ≥ 284 > 110, plus top ≤ 130 < 153 and bottom ≥ 132 > 131. Thus every position has positive-area overlap with the label and the new check refuses it. Moving below the label as instructed leaves the collapsed hit rectangle; this resolves to OutsidePerformers and cannot allocate/unfold A. Moving beside cannot clear the label while the pointer remains in A for this grab offset.
The attempted assignment is otherwise semantically valid and the candidate can unfold A (allocation_drag.rs:557). This makes successful ordinary collapsed-target assignment depend on knowing to restart with a different grab offset or manually expand the target. It contradicts the adopted collapsed-header workflow and makes the refusal's “Aim below or beside” recovery impossible within this held gesture. It is a regression caused by composing the new readability gate with existing frozen target geometry, not a request to weaken collision safety.
The existing collapsed-target resolver test (app/allocation_drag_tests.rs:758) checks only the header hit boundary. Planner success tests (allocation_drag/tests.rs:986, :1491) directly supply a destination and requested position; they do not prove an actual pointer inside a collapsed target can produce that position. They therefore miss this counterexample.
What the saved fix does establish
- The original
Receivercounterexample is rejected: selected projected header(120,110)224×62intersects the actual frozen(110,131)88×22label. Preview and release shareplace, andfinishrepeats it (allocation_drag.rs:633). Failure occurs before source execution (app/allocation_drag.rs:563onward). Existing model/source/history should consequently remain untouched on this path. - The new check distinguishes label occlusion from legitimate performer enclosure; it does not remove the ancestor/descendant exemption from general card collision checks. Nearby ordinary expanded-target placement
(120,160)remains valid by the inspected geometry. - Existing packet test destinations changed from y140 to y190 while their semantic, identity, source and history assertions remain. This repairs formerly unreadable test inputs; it does not weaken those assertions. The new overlap test retains the old input as an explicit refusal and adds transformed OA/SA, preserved redo, exact nearby placement, semantic-cache reuse and one-step commit/Undo/Redo coverage (
app/allocation_drag_tests.rs:958). - The same-performer test preserves a readable snapped no-op and allocation identity (
allocation_drag/tests.rs:1115). It does not independently cover same-performer occlusion, although that path shares the new check.
Remedies compared against the actual workflow
The specification should distinguish frozen hit geometry from readable destination identity presentation. The user's requirements need clear nesting, readable work/destination and exact preview; they do not require a duplicate opaque target name to stay at its old world coordinates. Resolve the source contract before encoding further refusals.
| Candidate | Ordinary drag / frozen target / exact placement / readability | Tradeoff |
|---|---|---|
| Recommended: retain the frozen target outline; present its identity in a dedicated, reliably visible toolbar/status slot outside the canvas; remove its opaque duplicate name overlay. | Preserves all destination hit rules and pointer placement, including collapsed targets; removes the overlay collision without moving work or target. | Amend the contract's frozen-label-in-header requirement and corresponding planner context. The identity slot must remain visible at narrow widths and with long names/refusal text; do not rely on arbitrary clipped status prose to preserve the name. Existing toolbar infrastructure at views.rs:543 provides a starting point. |
| Bounded alternative proposed by root: use the frozen badge if readable; otherwise use the same target's candidate projected header. | Preserves frozen hit resolution, exact pointer placement and model geometry. Solves both the Receiver and collapsed long-name counterexamples at the fixture camera. | Store exact chosen badge bounds/text in PlacementPlan and consume those bounds in both admission and renderer; change the old requirement that the badge remain within the frozen accepted header. A toolbar fallback is still needed to guarantee reachability when the candidate header is outside the viewport. |
| Keep an in-canvas name badge but place it in a collision-free viewport location, separately from the frozen hit outline. | Can preserve frozen target resolution and exact work placement while keeping both labels readable. | Requires a deterministic badge-placement policy, explicit relation to its target, and fallback when no space exists. More geometry and crowded-view cases than the toolbar approach; changing badge position must never change target resolution. |
| Offset/pack the work below the frozen label, expand the hit zone, or merely ask the user to expand/regrab. | Packing breaks exact pointer placement; enlarging hidden target area breaks visible frozen resolution; expand/regrab leaves this ordinary valid gesture unsupported. | Reject as the default remedy for this finding. These alter the interaction contract or transfer the failure to users rather than resolving the presentation conflict. |
The candidate-header alternative has a useful source-level invariant: for a direct performer ancestor, candidate target top is at most work top minus 64 (projection.rs:568). A badge at target top + 31 with height 22 therefore ends at least 11 units above the work header. The Receiver root y110 gives candidate badge y77–99; the collapsed example roots y70–130 give badge tops y37–97. Both clear the selected header. Indirect ancestors through coherently projected work parents retain additional header separation. Keep the frozen name/size or explicitly derive and store any changed truncation so geometry and actual text do not diverge.
Its unresolved viewport case is concrete: use the same collapsed long-name fixture with pan y−100 so the target starts at the canvas top (world viewport top=100). Every reachable header still intersects the frozen label. Only work roots y100–130 satisfy selected-header viewport containment, while candidate badge tops are y67–97, all above the viewport. Frozen-first/candidate-second therefore still admits no point in the collapsed target for that grab. A named viewport refusal would be truthful, but would retain the reachability limitation; a toolbar destination fallback avoids it without moving the camera or work. This should be an explicit decision and regression, rather than silently claiming the candidate-header fallback solves all valid collapsed drops.
Regression evidence still needed
A full-tree regression is warranted because the defect is a composition of pointer admission, frozen targets, projection and a separately stacked feedback canvas (views.rs:462). Exercise OA and SA with the long-name collapsed target above and the ordinary (40,35) grab; require a successful exact-placement assignment/unfold, readable selected work and destination identity, and single-step Undo/Redo. Pair it with the original Receiver geometry and transformed camera. Assert the actual visible presentation appropriate to the chosen remedy, not just planner acceptance or unchanged pixels when unrelated toolbar text varies.
The current full-tree pixel test (app/allocation_drag_full_tree_tests.rs:252) proves toolbar isolation by changing only feedback prose. It cannot establish joint label readability or collapsed-target reachability. Minimum-window layout and packaged native acceptance remain root-owned gates; this review alone does not establish them.