22 KiB
Independent critique: freely editable behavior canvases
2026-09-12. Reviewer /root/diagram_usage_workflows, delegated by Codex-ytr83r. Read-only review of current issues #2/#20/#22, the proposal in free-behavior-canvas-design-20260912.md, and primary code in .local/diagram-usages-ytr83r-20260912, branch codex/diagram-usages-ytr83r-20260912, base 7aa007b0777aa182a99f46e90eeb9b2558f3ad2a plus root's uncommitted navigation changes. No production edit, build, GUI operation, or Gitea mutation. This is design evidence, not native acceptance. Root owns synthesis; peers retain #16 resize and #21 activation rendering ownership.
Actionable conclusions
- Conditionally adopt the existing Diagram identity/lifecycle, not the entire proposed implementation shape at once. Owner-bound Process and Scenario projections fit saved views and avoid a second navigation/source/API aggregate. Use a single tagged behavior-layout payload, or an equally strict validated representation, rather than unrelated optional process/scenario blocks. Preserve the existing architecture/state storage contract. Do not make moving every desktop geometry algorithm into core a prerequisite for the first useful slice.
- Resolve the projection/gesture contract with a bounded numeric experiment before committing to the scenario schema. Absolute desired anchors are a credible candidate; stored and resolved geometry legitimately differ already in architecture/state. What remains undefined is conflict resolution, the inverse from a requested drag/resize to authored changes, and what a text-only user sees after reorder. The table below makes this uncertainty falsifiable. This is a policy prerequisite, not a categorical rejection of desired coordinates or permission to postpone the feature indefinitely.
- Include legacy ordered process paths in the route contract. Requiring an explicit SequenceLink before bending an already drawn legacy arrow fails the user's bend request for those existing models. Add a typed derived-path address and a defined lifecycle. Never manufacture semantic links as an elbow-drag side effect.
- Retain relation identity in qualified previews before saving their routes. Current preview links retain only endpoint pairs. Parallel links would collide if those pairs became the persistence key. A qualified route must identify both the full caller path and the relation. The same leaf reached through different paths must remain independently editable.
- Do not implement first-edit capture as a screenshot-to-model copy. It must seed valid authored inputs whose projected result equals the preview, preserve omitted/invisible records, and allocate view/placement identities once. Peer #16 already supplies a concrete counterexample to copying rendered dimensions back as authored dimensions. Complete capture also needs a size/performance bound and a stable policy for missing or collapsed appearances.
- Specify reference transformations and source comment keys before UI fan-out. Owner deletion/change, reidentify, caller-target changes, source reorder, legacy-to-explicit conversion, and hidden routes are not incidental cleanup. They determine whether manual work survives normal editing. Two views of one owner must be part of the first identity tests.
Requirements and verified evidence
The current user clarification in #2 comment608 rejects the camera-repair milestone as satisfying free behavior editing. #20 requires spreading lifelines to make call names readable, moving arrows, resizing, and replacing strange controls. #22 comment606 specifically adds process connection bends. Earlier sequence-constraint discussion does not erase these requirements. Cosmetic manipulation should preserve semantics; graphical/text/API parity follows the user-owned project specification. Always-present canvas and ordinary Properties are the agreed direction, not permission for canvas keyboard actions behind a focused text field.
Verified local constraints:
- model.rs:1431: ProjectionKind is Copy and currently has only a StateMachine owner variant. Diagram contains common metadata, view options, ID-keyed NodePlacements and relation/trace EdgeRoutes. Keeping owner IDs in ProjectionKind while storing layout separately avoids a needless Copy-breaking change.
- validation.rs:645: routes must refer to existing relations/traces, are unique per edge, have bounded finite waypoints and normalized label fractions. validation.rs:774 enforces one placement per semantic element per diagram. Repeated qualified appearances therefore cannot simply be extra ordinary nodes referencing the same leaf.
- topology.rs:914: GraphNode previews carry OccurrenceEndpoints; preview_links are only endpoint pairs. OrderedPath explicitly has source/target/points and no relation ID. topology.rs:1128 drops each preview relation's ID while constructing those pairs.
- topology.rs:1000: Graph rebuilds a TopologyIndex, derives rank from explicit links plus ordered adjacency, then constructs geometry. The rank traversal filters the adjacency list for each queued node. This is a concrete existing repeated-scan risk; adding stored layouts should not multiply this by every saved view on every pointer move.
- behavior.rs:1393: scenario rows follow steps/branches and recursively accumulate geometry. Fragment and branch boxes have different anchor conventions. A message y, frame rectangle and branch extent need a deliberate canonical relationship.
- The separate #21 read-only activation analysis verifies self-message receive geometry below its row anchor and existing lifetime/branch attachment concerns. This foundation must expose send/receive anchors to the activation owner; it must not implement a competing activation engine.
Storage alternatives against the actual criteria
| Candidate | Manual freedom and identity | Source/API/lifecycle cost | Main falsifier | Recommendation |
|---|---|---|---|---|
| Shared Diagram plus independent optional process_layout/scenario_layout | Expressive; keeps saved-view identity | Small additive syntax, but many invalid combinations and duplicated authority | Scenario projection with process layout or generic unrelated nodes passes unnoticed; two records claim one direct appearance | Avoid this public contract unless strict cross-field validation is explicit and exhaustive |
| Shared Diagram plus tagged BehaviorLayout, projection retains owner ID | Expressive; common metadata/navigation/history retained; domain records typed | Moderate schema/parser/walker work; owner/tag compatibility still needs validation | Tag/owner mismatch, same address in generic and qualified records, incompatible fields retained after projection change | Preferred bounded shape; constructor/accessor invariants plus parser/API validation |
| Behavior layout stored inside a payload-carrying projection enum | Stronger type coupling possible | Changes Copy assumptions and a widely matched enum; can make common projection queries expensive | Large ripple before first persisted move; geometry cloning during inexpensive kind checks | Viable only if implementation evidence makes it simpler; not needed merely for type purity |
| Separate top-level BehaviorView collection | Clean discriminated domain structure | Duplicates naming, identities, references, source ownership, REST inventory, selection, recovery, where-used and view deletion | A process view cannot participate in ordinary diagram navigation/export/history without a second branch everywhere | Keep as fallback if common Diagram validation becomes unmanageable, not first choice |
| Session-only map or spacing sliders | Quick visible result | Cannot provide durable source/REST/reopen parity | Save/reopen loses edits; an individual bend or position remains impossible | Rejected as production feature; candidate preview remains transient |
Minimal recommended boundary: common Diagram ID/metadata; Process/Scenario owner tags; ordinary direct-process nodes and explicit relation routes; typed extra addresses only where the existing representation cannot express identity. A tagged Process payload can hold qualified placements and derived adjacency routes. A tagged Scenario payload can hold lifeline/event/frame/message geometry. Empty ordinary nodes/routes should be required where they have no defined scenario meaning. Do not relax the global one-element-per-diagram invariant just to fit qualified appearances.
Define whether an empty qualified path is illegal or canonicalizes to the ordinary direct occurrence. Allowing both forms creates two geometry authorities. Prefer rejecting the empty path in the qualified-only collection. Presentation records keyed by semantic addresses need not invent global UUIDs, but any new placement UUIDs must join identity/reidentify/alias/serialization walkers consistently. Neither indexes nor displayed labels are persistent addresses.
Numeric experiment for constrained scenario geometry
These are deliberately simplified independent examples, not production metric claims. Let origin=0, minimum gap=10, A and B each have height20, order=[A,B], displayed tops A=100/B=200. Compare (D) absolute desired tops plus the illustrative monotone projection resolved[i] = max(desired[i], previous_bottom + 10) and (G) nonnegative gap-before stored on each occurrence, initially gap(A)=100/gap(B)=80. Neither example algorithm is adopted by this report.
| Operation | D with no inverse adjustment | G with no inverse adjustment | What the experiment must decide/prove |
|---|---|---|---|
| Move A to50 | A50/B200 | A50/B150 | Ordinary move should be able to preserve B at200; G needs gap(B)=130. Test a planner that changes only necessary dependent records. |
| Move A to250, preserve order | A250/B280; B's desired200 remains displaced | A250/B350 | Both preserve semantics, but select different whitespace policies. Preview must show B's actual final position. A feasible requested layout A250/B280 must be expressible in both (G gap(B)=10). |
| Move B upward to110 | B resolves130; stored110 differs | Requested negative/too-small gap needs defined clamp or rejection | Report a concrete constraint and preview actual B130; never commit one pixel result and reopen another. Explicitly distinguish desired values from resolved inspector values. |
| Increase A height20→80 | A100/B200 remains feasible | A100/B260 | Resize must be able to preserve B at200 when feasible (G gap(B)=20). Inactive dimensions and unaffected coordinates should not drift accidentally. |
| Increase A height20→120 | A100/B230 | A100/B300 | Minimum-gap conflict now exists. Show which rows move and why; preserve authored inactive axes. A source/API resize must produce the same projection as the GUI candidate. |
| Source reorder to[B,A] | B200/A230, A desired100 displaced | B80/A200 if each gap travels with its occurrence | Both are deterministic yet materially different. Specify whether whitespace belongs to an occurrence, a predecessor pair, or a sequence boundary. No passive rewrite or semantics-from-y. |
| Delete A | B200 | B80 | This is a lifecycle choice, not a numerical accident. Preserve surviving absolute position or deliberately remove preceding whitespace; GUI/source/API must agree. |
The test harness should enumerate small feasible integer layouts and check every requested feasible result has an authored representation and a stable projector result. Test repeat projection idempotence, no hidden model/source mutation, and planner preview = committed projection = source round trip = JSON/REST round trip. Measure changed records as well as output positions; equality from calling the same solver twice is insufficient. Independently assert the expected selected coordinate, semantic order and unchanged unaffected positions.
Additional discriminating probes:
- Nested fragment F spans y80..400, operand starts120, A160 and B260. Resize only F's bottom to300, then to220. Define header, padding, operand and child containment constraints. The first feasible result must preserve authored children. The second must report/preview specific container growth, constrained bound or chosen child movement; silently clipping children or reparenting them is invalid. Repeat moving F with and without an explicitly named group-move action, specifying absolute versus translated child coordinates.
- Self-call send at200/receive at224, reply later at320: move the call to250 and change its loop height independently. Send, receive, activation start and reply attachment must consume the same anchor outputs. A single
message.yis sufficient only if the receive-offset convention is explicitly part of projection/route geometry. Interior waypoints must not accidentally change the semantic receiver or activation interval. - A delayed-created lifeline must attach its header to its creation receive anchor after message movement; deletion must end the lane at the deletion receive anchor. Moving a header horizontally never changes sender/receiver. Hidden branch paths must retain stored values without entering a contradictory visible ordering.
- Repeated messages with one exchange and distinct occurrence IDs get independent y/bends/label offsets. An opaque ScenarioUse does not create layout for callee events merely because bindings refer to them.
- Text reorder with conflicting desired anchors must return a deterministic inspectable layout diagnostic containing the affected occurrence addresses and reason. Decide whether the UI displays desired and resolved values, offers an explicit Normalize Layout transaction, or exposes the conflict otherwise. Valid semantics must not be silently rewritten to match pixels.
Recommendation: implement both tiny pure candidate projectors/inverse planners outside production architecture and run these probes. Start with absolute anchors as the likely better fit for exact coordinates and preserving unaffected positions, but select it only after the nested/lifetime probes and inverse-resize counterexample pass. Gap-before remains a meaningful comparison, not an automatic-only caricature: a planner can translate requested absolute positions into gaps. Record measured complexity and source diff size. Bound this experiment to the policy uncertainty rather than broad renderer extraction.
Reference lifecycle and source contract that cannot remain implicit
| Case | Required independent oracle |
|---|---|
| Two saved views of one owner | Change only view V1's layout; V2 and semantics remain byte-for-byte equal. All context stamps, camera caches, navigation and recovery include diagram ID. |
| Reuse same callee twice; nested reuse twice | Route/resize [caller1,nested1]/leaf only; caller2 and the callee definition remain unchanged. Parallel callee relations retain separate routes despite equal endpoints. |
| Delete/reidentify caller or occurrence | Reidentify transforms every address component and source comment key; delete removes or explicitly invalidates dependent records in the same validated transaction. Never retarget by name. Undo restores exact keys and geometry. |
| Change a caller's referenced process | Reject incompatible old paths or remove them through an explicit reported transformation. Reuse of a UUID elsewhere must not silently activate an unrelated stale record. |
| Legacy order A,B,C → A,C,B | Derived route(A,B) stops applying; route(A,C) is a different key. Define whether obsolete records are removed, retained dormant, or diagnosed. If retained, state whether reintroducing adjacency restores them. The decision must be bounded and tested. |
| Convert legacy paths to explicit links | Define an atomic presentation route migration by exact endpoint correspondence, with ambiguity handling for parallel links. No automatic semantic conversion during bend edits. |
| Filter/collapse temporarily hides a route/node | Preserve authored records. Partial current visibility must never become a destructive replacement snapshot. Reopen/reexpand restores the same geometry. |
| Change diagram owner/projection/layer | Reject incompatible retained fields, or require an explicit complete conversion in one transaction. No silent geometry loss or unrelated node acceptance. |
| Invalid source/API payload | Reject duplicate typed keys, owner/tag mismatch, scenario event of another owner, empty/invalid qualified path, wrong route subject kind, NaN/infinity/out-of-bounds geometry, and over-budget path/point collections with precise field diagnostics. |
The current version7 strict parsing/serde and validation gates require a coordinated schema version decision, not unversioned optional-field accretion. Old sources should open clean without synthesized diagrams or formatted rewrites. An older reader must reject unsupported new constructs rather than discard manual geometry. Source format/parse, JSON, REST conditional transactions, reference walkers, copy/transition, multi-file ownership and comments all need compatible changes.
Nested comments must be matched by the full typed key: a scenario occurrence, qualified endpoint, qualified relation, or derived ordered-path pair plus its domain. Array index and terminal occurrence alone are insufficient after reorder/reidentify. Same-named aliases in included files must resolve by IDs. Explicit reset of one geometry record must not discard sibling comments or materialize invisible records.
First edit, scalability, and proportionate implementation
First accepted gesture should create the view and required authored records in one normal transaction. Pointer-down/preview may reserve deterministic candidate IDs, but render, selection and cancel create no source changes. Undo removes the first view creation; redo restores the same IDs and geometry. If a first resize begins from derived geometry, preserve the authored-versus-resolved contract established by #16 instead of serializing every projected width/height.
Complete direct-surface capture protects untouched objects against later seed-layout movement, but has costs: an O(model-size) first edit, larger source diffs, new identity allocation and potential dependence on transient filters. Sparse capture is smaller but leaves untouched nodes dependent on changing ranks/row generation. A bounded compromise is a stable initial placement plan plus explicit overrides, provided that this does not introduce two unexplained coordinate authorities. The first slice should compare total records and diff size on a realistic long model before selecting capture policy. Do not claim that “currently rendered” means a stable semantic set until filter/expansion/default-view rules are fixed.
Keep stored geometry projection pure and reuse geometry primitives. It is not necessary to relocate the entire Scene/Graph renderer before proving persistence. An adapter can resolve typed authored records into current Scene/Graph positions, attachment anchors and routed paths; both preview and committed render must consume that result. Any behavior constraint or inverse planner needed by REST should be UI-free. Do not duplicate a second projector for source/API or retain competing automatic geometry paths that overwrite saved positions late in rendering.
The existing process rank scan can approach V×E work; repeated TopologyIndex rebuilding and qualification resolution add cost. Cache immutable owner/topology indexes per model revision, rebuild affected geometry once per candidate, and bound visible expansion separately from stored-record validation. Validate stored keys without enumerating every possible recursive appearance. Limit total records, aggregate path length/depth and points as well as points per route. A saved view must not force eager expansion of the Cartesian product of nested uses. Stress tests should record first-edit latency, per-pointer candidate work, lookup/reopen work and source size on the same fixture, with an explicit target budget agreed for the actual hardware rather than invented acceptance milliseconds.
Minimum independent acceptance before broad adoption
- One owner-bound process view moves a direct occurrence off its rank column, resizes it, bends both an explicit link and an existing legacy path, and moves a label. Links, steps and represented work do not change. Preview/commit/reopen are numerically equal and one Undo restores source/model.
- Two-lifeline scenario independently spaces headers and positions/routes two occurrences of the same exchange, preserving sender/receiver, message kind, reply_to, steps and lifetimes. Long labels become inspectable through ordinary controls. The numeric source-reorder/resize policy probes above pass.
- First edit, cancel, zero movement, stale revision, document/view switch, dirty-source navigation, focus loss and modal entry have explicit nonmutation or atomic commit outcomes. Actual App input paths must be exercised later, not only pure command constructors.
- Two saved views and two qualified appearances retain independent geometry, full target identity and source comments across save/reopen, REST, reidentify, collapse, delete and Undo. An exact usage jump reaches the saved appearance without changing geometry.
- Nested fragments, operands, self messages and delayed creation/deletion use explicit attachment outputs. Peer #21 activation rendering is reviewed against those outputs; this task neither overwrites its fix nor claims its native acceptance.
- Actual native workflows at the minimum supported window demonstrate immediate canvas manipulation, visible resize/bend handles, Properties focus, discoverable semantic Reorder, and stable viewport origin. A screenshot or small fixture alone does not establish all process/scenario kinds or large-model usability.
Adopt the common saved-view direction and a bounded end-to-end slice now; settle the numeric projection/inverse and address-lifecycle rules before multiplying UI controls and schema cases. Revisit the common Diagram choice only if the typed payload/validation slice demonstrates more duplication than a separate aggregate, not because domain-specific geometry exists.