11 KiB
Routing warning selector: source inspection and bounded alternatives
Reviewed source: 092f5244b9264b53bccea884a1a4fe9d860f2ad2, branch codex/palette-canvas-956atf-20260913, worktree palette-canvas-20260913/. The completed palette-consumer commit is unchanged and remains independently cherry-pickable. This follow-up makes no production edits and no new commit. No compiler, tests, UI, or Gitea operations were run. Parent explicitly narrowed the follow-up to this design/reuse inspection after the API gaps below were identified.
What the existing APIs actually provide
All source locations below are relative to that exact worktree/revision.
crates/oa-desktop/src/diagram.rs:59–85,460–526: the authoritative displayedRoutedViewbelongs to privateDiagramCanvas::State.cache; draw and hit testing sharescene(state). Allocation preview uses a separate preview cache. Node movement, route, and port previews participate in this scene construction. The host cannot read the exact scene or its private held node/route/port/pan/connection gestures. An App-sideroute_diagramcall on every view rebuild would duplicate the router and could disagree with the preview currently displayed.crates/oa-desktop/src/views.rs:569–603: an existing 30-unit host strip preserves canvas size and gives connection/allocation, promoted-endpoint expansion, or routing guidance. It computes a projected view, not a routed view. It therefore cannot currently populate an accurate warning count/control without another interface.crates/oa-desktop/src/diagram.rs:1424–1450: the current canvas warning strip already draws warning text or an actionable-route count from the actual scene. Its selected-route branch suppresses the selected suspended custom route's warning; its fallback may still report other warnings.PromotedEndpointsis excluded from the count. At:1165, selected/highlighted edges take the TEAL branch before the amber-warning branch. The selector must therefore include selected warning routes independently of their painted color.crates/oa-desktop/src/model_menu.rs:11–90:Requesthas no warning catalog or menu purpose;Openeddoes protect document identity, document revision, input epoch, page, layer, current diagram, and originating canvas context. These protections should be reused.crates/oa-desktop/src/app.rs:1943–1965: ordinaryMenuOpenintentionally clears the connection session and advancescanvas_epoch, invalidating private canvas pointer history. It cannot simply become a host warning-open action while promising preservation of held gestures. At:1972–2008, invocation validates the token/context, then selectsrequest.objectbefore dispatching its action. Using the diagram as a catalog anchor would select it before the intended relation. At:1010, ordinary selection callsapply(). A warning chooser must reject pending drafts before reaching this path; merely disabling a painted menu row does not establish an update-handler guard.crates/oa-desktop/src/context_menu.rs:23–87,212–229: the existing popup has scrollable actions, keyboard arrows/Home/End, Enter/Space, and Escape. It has no search query or search input. Adding searchable entries is a real widget contract change, not an existing capability.crates/oa-desktop/src/forms/reference_chooser.rs:17,151andforms/reference_chooser/adapter.rs:60: the live searchable reference chooser has draft/record/property-path identity and returns staged-value Write intents.forms/reference_chooser/widget.rs:157–196is generic over the outer message but accepts that concrete form chooser state and hardcodes the reference-search placeholder. It is not a ready navigation chooser. Its historical “experiment / cfg(test)” module comment is stale;forms.rs:21and App messages actually wire it into production. Reuse by fabricating a draft/path context would hide the semantic mismatch.
Smallest materially different alternatives
| Alternative | Concrete implementation boundary | Exact acceptance limitations |
|---|---|---|
| A. Add a warning section to the existing canvas-originated context menu | At the existing canvas right-click or ContextMenu/Shift-F10 open site, take relation IDs and actionable warning text from self.scene(state). Carry a typed catalog/purpose through the existing guarded menu; choose a listed relation directly without first selecting the diagram. Reject warning-menu entry during a held gesture. Use existing scrolling/navigation and explicit relation name, alias, endpoint names, and stable identity details for ambiguous names. |
Avoids a cross-widget scene bridge and introduces no second router or search framework. It does not deliver the requested persistent host warning-count control or text search. Existing top-level Actions menus cannot safely synthesize this catalog. Long lists remain slower to browse. Pointer context-menu discoverability is weaker than a dedicated control. This is a consciously smaller interim contribution, not full acceptance. |
| B. Deliver the selected fixed-host searchable selector | Add one explicit canvas-to-host presentation/interaction interface and a typed warning-menu purpose; add opt-in query/filter behavior to the existing menu. Preserve the 30-unit host row and keep expansion/tool guidance beside or ahead of the control. Reuse existing menu context guards and direct guarded relation selection. | Meets the requested outcome only after fresh scene/gesture synchronization, input ownership, stale-query, and dismissal tests. Touches at least diagram.rs, views.rs, app.rs, model_menu.rs, and context_menu.rs plus tests, rather than the original four palette files. A cached warning list alone cannot establish held-gesture safety. This is a separate interaction change needing an explicit interface decision and independent review. |
| C. Reuse the reference chooser by extracting its navigation-neutral state/overlay | Separate catalog/search/navigation/dismissal from draft-reference compatibility and staged writes, then supply distinct host adapters. It would retain the mature searchable overlay rather than adding search to the menu. | Still needs B's authoritative scene/gesture bridge and host guards. Changes form chooser identity/focus behavior and broadens regression scope to Inspector/Review reference editing. No known second non-form consumer justifies this extraction for a bounded palette task. Reject for now; do not fake form contexts to avoid the extraction. |
Recommendation: keep 092 moving through palette integration. If the full selector remains the next priority, choose B as an explicitly scoped interaction task; A is useful only if the parent deliberately accepts its missing persistent control and search. Do not silently substitute A for the requested selector. C adds unrelated form risk. An on-route glyph remains rejected because existing label/arrow geometry provides no safe placement contract (crates/oa-core/src/routing.rs:47,1269; desktop diagram.rs:372,1208).
Minimum interface decisions before B
The canvas must publish or otherwise expose a read-only presentation snapshot, derived from its existing scene, containing originating route_edit::Context, an identity for the scene/presentation, actionable route IDs and their warning reasons, and whether a gesture prevents opening. No second routed-scene cache or model mutation is needed merely to represent this list. Count routes, not individual warning reasons; omit routes whose only warning is PromotedEndpoints. Include a route with both promotion and another actionable reason, preserving expansion guidance and excluding its promotion reason from the actionable explanation.
A delayed previously-idle snapshot is insufficient: a node/port/route press does not advance document revision immediately. An open request must be revalidated against current canvas interaction ownership, or the same ordered event path must invalidate the opening capability when any gesture begins. Specify that ordering before implementing a Redraw-only publication. Similarly, opening must not reuse the ordinary connection-clearing/epoch-advancing path until active gestures and connection/allocation tools have been rejected without changes. The control can remain visible but disabled during those states and pending edits.
Menu invocation must validate its exact token, document/view/revision/epoch, catalog identity and chosen route membership; recheck busy/pending model/source/behavior state in the handler. Only then dispatch relation selection. Query changes need a result-generation rule so an old row event cannot choose a different newly filtered entry; Escape and ordered keyboard navigation must remain usable after typing. Preserve existing dismissal-release swallowing and focus restoration. These are interface invariants, not a proposal for a new general menu framework.
Evaluators needed for a future implementation
- Produce real routed scenes with duplicate relation names, an empty relation label, a selected actionable warning route, a normal route, a promotion-only route, and mixed promotion/actionable warnings. Verify exact catalog membership, reasons, route count, disambiguation, and search fields. A test that manually creates only the proposed warning DTO would not establish same-scene correctness.
- Drive the actual canvas update path through node, port, bend/label, pan, connection, and allocation presses/previews, then request the host selector before release. Assert it cannot cancel or commit the held operation. Include a queued stale idle snapshot and pending invalid Inspector/source/behavior drafts. Compare draft bytes, document revision/history, route and port data, and session/gesture continuity before and after.
- Drive actual App/menu open, query, invoke, and close handling. Falsify old tokens and stale same-ID document replacement, diagram/page/layer change, geometry/fold/filter revision, and canvas epoch. Test an entry that ceased being warned/visible, query-then-old-row activation, and no accidental intermediate diagram selection. No forged inner
Message::Selectshould bypass membership validation. - Exercise actual popup keyboard search/navigation/Enter and Escape/outside dismissal, including queued input and mouse release. Do not use only catalog helper tests as evidence for focus or release ownership.
- Native acceptance after integration: fixed measured canvas size with zero/one/many warnings, minimum window, selected warning edge, duplicate/unlabeled routes, suspended custom route expansion guidance, long reasons, and keyboard/text focus recovery. Verify source/history/routes are unchanged by browsing and selection. Until these are run, neither discoverability nor interaction acceptance is established.
The old “select an amber line” instruction and warning rendering remain unchanged because no new control has been wired. This report supersedes only the earlier handoff's estimate that the selector was a small host/menu wiring change; it does not retract the candidate's user-facing benefit or claim the requirement complete.