Three final-review findings on the two picker pages:
1. DebugView's localStorage auto-reconnect restored the stored instance id with
no membership check, where the ?instanceId= path has one — a stored instance
that is no longer Enabled (or was deleted, or moved sites) is filtered out of
the list the picker is handed, so the field rendered blank while the page held
a non-zero id and auto-connected to it anyway. The guard now mirrors the
deep-link path: forget the stored instance id, tell the operator, and leave the
site selected and loaded so they can pick again.
2. FlowDebugger's _instances kept an OrderBy(UniqueName) from the flat <select>
era; InstanceTreeBuilder sorts areas and instances itself.
3. The deep-link test is joined by one where the instance sits two areas deep, so
the field text is pinned to resolve from the flat list with the panel never
opened.
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
ArrowDown moved focus into the tree, but the picker passed no Selectable /
SelectedKeyChanged, so Enter/Space on a treeitem only moved the roving tabindex —
the pick lived on the label span's @onclick, i.e. mouse only. The tree now runs in
Selectable mode and BOTH gestures arrive at one handler (OnTreeKeyActivatedAsync)
through TreeView's SelectedKeyChanged: an `i:{id}` key selects that instance, an
`a:{id}` key toggles the branch. The label's own @onclick is gone — the click
already bubbles into TreeView's content span, so a second handler there would fire
the callback twice. SelectedKey stays unbound (a combobox shows its pick in the
field, not as a row highlight, and binding it would arm TreeView's
selected-key-not-in-tree reset on every render while the pick is filtered out);
SelectedCssClass is emptied to match.
a11y: the field gains an accessible name via a new AriaLabel parameter (default
"Instance") and an optional InputId so a page can point a <label for> at it;
aria-controls is rendered only while the panel exists, instead of always pointing
at an id that is absent when closed.
Also drops the unused @using of Commons.Entities.Instances from the .razor.
Tests: keyboard activation on an instance row and on an area row, a
one-selection-per-gesture guard across mouse and keyboard, and the labelling
assertions. CentralUI suite 1331 passed / 0 failed; build 0 warnings.
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
Both Flow Debugger and Debug View enable the shared InstancePicker as soon as
_selectedSiteId lands, but Blazor renders at the site-change handler's first
await — at which point the picker is still holding the PREVIOUS site's Areas /
Instances lists. Typing into the field in that window (an operator, or
Playwright's FillAsync, which waits only for "enabled") was silently lost: the
picker compares both lists by reference, so the freshly-assigned lists read as
"the site changed" and it clears the field text and closes the panel.
Both pages now hold an _instancesLoading flag across the whole load — set before
the first await, cleared in a finally so a fault or a site-scope refusal cannot
leave the picker disabled forever — and fold it into the picker's Disabled
expression. InstancePicker itself is unchanged.
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
Pins the site-change behaviour the picker's reference contract exists for: after
picking an instance, switching the site select empties both the page's
_selectedInstanceId and the picker's field text (the new site hands it a
different instance list), and leaves the script select on its placeholder.
Layout: the Instance column goes col-md-2 → col-md-3 so a full instance name
fits — the script-entry branch still sums to 8 of 12 (site 2 + instance 3 +
script 3), and Method never renders beside them — with the placeholder
shortened to "Search instances…" for the narrower field.
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
Task 5 review follow-ups:
- New DebugViewInstancePickerTests.WhileConnected_ThePickerIsDisabled pins that
a live connection locks the picker: both the input and the chevron carry
`disabled`. It marks the page connected by reflection, the way the other
DebugView page tests do (DebugViewAlarmTableTests) — driving the real gRPC
stream is not available to a bUnit render.
- OnInstanceSelectionChanged() was already a no-op and its comment described the
@bind that the picker replaced; deleted, and OnInstancePicked now just records
the id (XML-documented).
- The ItemLabel lambda is hoisted to a private static InstanceLabel(Instance),
passed as a method group.
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
The instance row locator used Locator.GetByText scoped to the panel, which
bypasses the [data-test='flow-instance-row'] contract InstancePicker rows
are documented to carry. Match through the data-test attribute instead,
with an exact-text regex since the test only knows the instance name (not
its id, unlike Debug View's tests).
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
ArrowDown on a picker that has never been opened pins the deferred path where
_tree is still null when the keydown is handled: the panel opens in that render
pass and the roving tab stop lands on the first row in the next one. Escape on a
closed picker is pinned as a no-op.
Two notes, no behaviour change: FirstVisibleInstance records WHY it may trust
TreeView.IsExpanded (Blazor Server's single renderer sync context runs the render
plus the pending ExpandAll/CollapseAll before the next keydown is dispatched — an
await that genuinely yields inside OnAfterRenderAsync would break that), and the
stylesheet warns integrators that the absolutely-positioned panel is clipped by
any ancestor with overflow: hidden/auto.
The ArrowDown focus interop assertion is kept — it was stable across repeated
runs — with a comment marking the tabindex as the behavioural check.
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
The script entry's flat instance <select> is replaced by the shared
InstancePicker combobox (design 2026-09-08 §3): the site's areas load
alongside its instances in LoadInstancesAsync and are held in fields, so the
picker sees one stable list reference per site and only closes its panel on a
real site change. Picking a row raises the id, which OnInstancePickedAsync
applies before running the unchanged OnInstanceChangedAsync — the deep link,
the localStorage restore and the site-scope gate are untouched.
Tests: the five instance-select .Change("7") call sites go through a
PickInstance helper (open the panel, click the data-instance-id row), plus a
new test that areas open collapsed and only reveal the instance once both
levels are expanded, and one that ?instanceId= shows the instance name in the
picker field.
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
The flat instance <select> on Debug View is replaced by the shared
InstancePicker area-tree combobox (design 2026-09-08 §3): the page now loads
the site's areas alongside its Enabled instances in LoadInstancesForSite and
hands both lists to the picker with ItemLabel = "UniqueName (State)",
Disabled="@_connected" and DataTestPrefix="debug-instance".
Both lists are now REPLACED rather than cleared in place wherever the site
changes (LoadInstancesForSite, StartFresh) — the picker compares Areas and
Instances by reference and a new object is what tells it the site changed.
The ?siteId=/?instanceId= deep link and the localStorage reconnect keep
setting _selectedInstanceId directly; the picker reads it on parameter set,
so both paths are unchanged.
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
Flow Debugger's script-entry instance selection and Debug View's instance
selection both go through the shared InstancePicker combobox now instead of
a flat <select>. Update the tests to type into the {prefix}-input, wait for
the matching {prefix}-row, and click it; keep the <select>-polling helper
for the site select, which is unchanged.
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
Three Task 2 review findings:
1. The outside-click backdrop is a fixed full-viewport layer at z-index 1040,
but the input-group was unpositioned — so once the panel was open a click on
the field or the chevron hit the backdrop and closed the panel instead of
letting the operator keep typing. The field now carries .ip-field
(position:relative, z-index 1042), stacking backdrop < panel < field. Pinned
in bUnit by the class (the z-index itself is scoped CSS, verified visually).
2. OnParametersSet closes the panel on ANY lists change, not only when the
selection disappeared: an open panel is showing the previous site's tree,
which is wrong even when the selected instance happens to exist in both.
The reference-equality contract on Areas/Instances is now stated on both
parameters — pass a stable reference, replace it only when the set changed.
3. Tests: the chevron closes an open panel; two pickers mint distinct panel ids
(and matching aria-controls); an unrelated parameter change with the SAME
Instances reference leaves a half-typed filter and the open panel alone.
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
Typing in the field is the filter: the forest is rebuilt to the matching
instances plus their ancestor areas and force-expanded so every match is on
screen; clearing the text restores the full forest, collapsed. Both bulk
expansions are deferred to OnAfterRenderAsync, because the TreeView only exists
once the panel has rendered.
Enter picks the first instance the operator can actually SEE — document order,
descending into a branch only when the tree reports it expanded — so with areas
collapsed it picks the first root instance, and with a filter (every branch
expanded) it degrades to plain document order. Escape closes and restores the
field text from the current selection; ArrowDown hands keyboard navigation to
the tree via a new additive TreeView.FocusFirstVisibleAsync(), which reuses the
existing roving-tabindex _focusNeedsApply path rather than adding a second
focus mechanism.
No preventDefault: Blazor's flag is all-or-nothing per element, and neither
default is harmful (Enter in a text input outside a form does nothing,
ArrowDown just moves the caret) while suppressing Tab would trap the control.
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
A shared Bootstrap-only combobox that replaces the flat instance <select> on
Flow Debugger and Debug View (design docs/plans/2026-09-08-instance-picker-design.md,
plan task 2): a text field over a dropdown panel hosting TreeView across the
InstanceTreeBuilder forest, every area collapsed on open. Clicking an area row
toggles it, clicking an instance row selects it, closes the panel and raises
SelectedInstanceIdChanged. Outside clicks close through a transparent fixed
backdrop — the same pattern TreeView uses for its context menu, so no JavaScript.
A site change (new Areas/Instances references) closes the panel and clears the
field when the selection no longer resolves; a parent-driven SelectedInstanceId
change re-derives the field text while closed. ItemLabel customises row and field
text (Debug View wants a "(State)" suffix) while search still matches UniqueName.
TreeView gains one additive public ToggleExpanded(key) — the chevron's existing
private toggle, exposed so a consumer whose own NodeContent owns the label click
can toggle the branch.
Search filtering and keyboard handling land in task 3; _filter/Rebuild() are in
place for them.
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
Review follow-ups on f12ba2f9:
- An instance whose AreaId points at an area outside the supplied list was
silently dropped; it now surfaces at the root, mirroring the orphaned-area
rule, and stays filterable.
- InstanceTreeNode.Children is an IReadOnlyList over a private backing list,
composed by the builder through an internal AddChild — the record no longer
hands callers a mutable list.
- Extra tests: unknown-area instance (root + filterable), 3-level nesting,
a ParentAreaId cycle that must neither loop nor throw, and a filter that
matches instance names only (an area name match alone does not keep it).
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
Task 1 of the Instance Picker plan (docs/plans/2026-09-08-instance-picker.md).
Pure, DI-free builder that nests a site's areas by ParentAreaId and hangs each
area's instances beneath it, mirroring Topology's ordering (child areas first,
then that level's instances; OrdinalIgnoreCase sorts) with unparented instances
at the root after the root areas. Orphaned areas become roots, a case-insensitive
filter keeps matching instances plus their ancestors and drops empty branches,
and a ParentAreaId cycle can never recurse forever.
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
The trigger-cascade (Spawn) edge used a lightning-bolt marker IN PLACE of the arrowhead. On the upward
Tags→Site hop the bolt sat on top of the riser and read as a broken arrow (rig screenshot 2026-09-08).
Every connector now ends in an ordinary arrowhead; the bolt is a badge (circle + bolt) centred on the
connector's vertical run (FlowGraphView.SpawnBadgePoint, kept in step with OrthogonalPath/AsyncPath).
Same screenshot, second defect: the cascade spawned by "RecipeDownloadFlag = false" was drawn from the
LAST write of the burst (RecipeProcessedFlag) because all four writes share a millisecond and the
spawner rule was time-only. FlowGraphBuilder now prefers the parent's last AttributeWrite of the attribute
the child's triggerAttribute names (at or before the child, else the last such write); the time/position
rules remain the fallback when the child names no attribute or the parent never wrote it.
Verified live on the rebuilt rig (DelmiaSim01 / DelmiaReceiverSimulator): both spawn edges show an
arrowhead + badge, the second one originates from the RecipeDownloadFlag write. bUnit + builder tests
updated/added; Playwright FlowDebuggerTests 4/4.
Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29
Joseph Doherty