Files
ScadaBridge/docs/plans/2026-09-08-instance-picker.md
T

262 lines
21 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Instance Picker Implementation Plan
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:executing-plans to implement this plan task-by-task.
**Goal:** Replace the flat instance `<select>` on Flow Debugger and Debug View with a shared combobox whose dropdown is the site's collapsed area tree, searchable by typing.
**Architecture:** A pure `InstanceTreeBuilder` turns `Area`/`Instance` lists (+ filter) into `InstanceTreeNode` forests; a new Bootstrap-only `InstancePicker` component wraps a text field + the existing `TreeView` in an absolutely-positioned panel with a transparent backdrop (no JS). Both pages keep their selection state and only swap the control. Design: `docs/plans/2026-09-08-instance-picker-design.md`.
**Tech Stack:** Blazor Server, Bootstrap 5, bUnit + NSubstitute + xUnit, Playwright (rig).
**Branch:** `feat/instance-picker` (from `main` @ `859922ba`). Worktree agents branch from `main`, so every implementer starts with `git reset --hard <current feature sha>` (see memory `agent-worktree-isolation-base`).
**Conventions:** `docs/plans/...` rules in `CLAUDE.md`; tests run per project (`dotnet test tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests --filter ...`); commit trailer `Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29`. Never block on a Task from a Blazor lifecycle/event path.
---
### Task 1: `InstanceTreeNode` + `InstanceTreeBuilder` (pure) with tests
**Classification:** standard
**Estimated implement time:** ~4 min
**Parallelizable with:** none (everything else depends on it)
**Files:**
- Create: `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstanceTreeNode.cs`
- Create: `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstanceTreeBuilder.cs`
- Test: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Shared/InstanceTreeBuilderTests.cs`
**Step 1: Write the failing tests** (`InstanceTreeBuilderTests`, namespace `ZB.MOM.WW.ScadaBridge.CentralUI.Tests.Shared`; `Area` is `ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites.Area` — check the namespace with grep — `new Area("Name") { Id, SiteId, ParentAreaId }`; `Instance` is `Commons.Entities.Instances.Instance`, `new Instance("Name") { Id, SiteId, AreaId }`):
- `Build_NestsAreasByParent_AndPutsRootInstancesAfterRootAreas`: areas Line1 (1), Cell A (2, parent 1); instances P1 (area 2), Zulu (no area), Alpha (no area) → roots = [`a:1`, `i:<Alpha>`, `i:<Zulu>`]; `a:1` → [`a:2`] → [`i:P1`]. Labels = names; `IsInstance` true only on instance nodes.
- `Build_SortsAreasAndInstancesCaseInsensitively`: "beta", "Alpha" areas → Alpha first; same for instances.
- `Build_OrphanedArea_BecomesARoot`: area with `ParentAreaId = 99` (absent) appears at root, its instances under it.
- `Build_Filter_KeepsMatchingInstancesWithAncestors_DropsEmptyBranches`: filter "p1" (case-insensitive) keeps `a:1 → a:2 → i:P1`, drops Zulu/Alpha and any area with no match; `Build_Filter_WhitespaceMeansNoFilter`.
- `Build_NullOrEmptyInputs_ReturnEmpty` (null areas / null instances tolerated).
- `Build_AreaWithNoInstancesAndNoFilter_IsStillShown` (empty areas are shown when unfiltered).
**Step 2: Run** `dotnet test tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests --filter FullyQualifiedName~InstanceTreeBuilderTests` → build errors (types missing).
**Step 3: Implement**
```csharp
// InstanceTreeNode.cs
namespace ZB.MOM.WW.ScadaBridge.CentralUI.Components.Shared;
/// <summary>One row of the <see cref="InstancePicker"/> tree: an area branch or an instance leaf. Keys are a:{areaId} / i:{instanceId}.</summary>
public sealed record InstanceTreeNode(string Key, string Label, Area? Area, Instance? Instance)
{
public List<InstanceTreeNode> Children { get; } = new();
public bool IsInstance => Instance is not null;
public bool HasChildren => Children.Count > 0;
}
```
```csharp
// InstanceTreeBuilder.cs — pure, no Blazor/DI.
public static class InstanceTreeBuilder
{
public static IReadOnlyList<InstanceTreeNode> Build(IReadOnlyList<Area>? areas, IReadOnlyList<Instance>? instances, string? filter)
// 1. areas ??= []; instances ??= []; needle = filter?.Trim(); hasFilter = !string.IsNullOrEmpty(needle)
// 2. known = set of area ids; children-of lookup: parentId → areas (null parent OR parent not in known ⇒ root)
// 3. instancesByArea: AreaId (null ⇒ root) → instances, each sorted OrdinalIgnoreCase by UniqueName
// 4. BuildLevel(parentId): child areas sorted by Name OrdinalIgnoreCase, then instances of that level;
// with a filter an instance is kept iff UniqueName.Contains(needle, OrdinalIgnoreCase),
// an area is kept iff it has any kept child (area or instance); without a filter everything is kept.
// 5. return roots
}
```
**Step 4: Run the tests → PASS. Step 5: Commit** `feat(central-ui): InstanceTreeBuilder — area/instance forest for the instance picker`.
---
### Task 2: `InstancePicker` component — field, panel, select, outside-click
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** none
**Files:**
- Create: `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor`
- Create: `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor.cs`
- Create: `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor.css`
- Test: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Shared/InstancePickerTests.cs`
Read first: `TreeView.razor` (parameters at ~line 165–190, `ExpandAll`/`CollapseAll`/`RevealNode` at ~600–680, the context-menu overlay markup at ~line 41 for the backdrop pattern), `TemplatePickerDialog.razor` (search box + tree host + `NodeContent` with `tv-glyph`/`tv-label`), `TemplatePickerDialogTests.cs` (bUnit style; `JSInterop.Mode = JSRuntimeMode.Loose`).
**Step 1: Failing tests** (`InstancePickerTests : BunitContext`, Loose JS interop). Fixture: areas Line1 (1) → Cell A (2); instances P1 (id 7, area 2), Zulu (id 8, root). Render `InstancePicker` with `Areas`, `Instances`, `DataTestPrefix="t"`, `SelectedInstanceIdChanged` capturing into a local.
- `Closed_ShowsNothingButTheField`: no `[data-test='t-panel']`; input `[data-test='t-input']` has `role="combobox"`, `aria-expanded="false"`.
- `Toggle_OpensPanel_WithAreasCollapsed`: click `[data-test='t-toggle']` → panel present; rows `[data-test='t-row']` visible = Line1 (area) and Zulu (root instance) only — P1 is under a collapsed area, so no `[data-instance-id='7']`.
- `ClickingAnArea_ExpandsIt`: click the `tv-toggle` of Line1 then Cell A → `[data-instance-id='7']` appears.
- `ClickingAnInstance_Selects_Closes_AndRaisesTheCallback`: after expanding, click `[data-instance-id='7']` → callback got 7, panel gone, input value `"P1"`.
- `Disabled_NeverOpens`: `Disabled=true` → input disabled, toggle disabled, clicking does nothing.
- `Backdrop_ClickClosesThePanel_WithoutChangingSelection`.
- `ParentDrivenSelection_UpdatesTheFieldText`: re-render with `SelectedInstanceId=8` → input value "Zulu"; `SelectedInstanceId=0` → "".
- `ItemLabel_IsUsedForRowsAndField`: `ItemLabel = i => i.UniqueName + " (x)"`.
**Step 2: run → build errors. Step 3: Implement.**
`InstancePicker.razor` (sketch — Bootstrap only):
```razor
@namespace ZB.MOM.WW.ScadaBridge.CentralUI.Components.Shared
@using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances
@using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites
<div class="ip-wrap position-relative" data-test="@($"{DataTestPrefix}-picker")">
<div class="input-group input-group-sm">
<input type="text" class="form-control" role="combobox" autocomplete="off"
id="@_inputId" aria-autocomplete="list" aria-haspopup="tree"
aria-expanded="@(_open ? "true" : "false")" aria-controls="@_panelId"
placeholder="@Placeholder" disabled="@Disabled"
data-test="@($"{DataTestPrefix}-input")"
value="@_text" @oninput="OnInput" @onfocus="OpenAsync" @onkeydown="OnInputKeyDown" />
<button type="button" class="btn btn-outline-secondary" tabindex="-1" aria-label="Browse instances"
disabled="@Disabled" data-test="@($"{DataTestPrefix}-toggle")" @onclick="ToggleAsync">
<i class="bi @(_open ? "bi-chevron-up" : "bi-chevron-down")"></i>
</button>
</div>
@if (_open)
{
<div class="ip-backdrop" @onclick="CloseAsync"></div>
<div class="ip-panel border rounded bg-body shadow-sm" id="@_panelId" data-test="@($"{DataTestPrefix}-panel")">
<TreeView @ref="_tree" TItem="InstanceTreeNode" Items="_roots"
ChildrenSelector="n => n.Children" HasChildrenSelector="n => n.HasChildren"
KeySelector="n => n.Key" IndentPx="16">
<NodeContent Context="node">
<span class="tv-glyph"><i class="bi @(node.IsInstance ? "bi-box" : "bi-diagram-3")"></i></span>
<span class="tv-label @(node.IsInstance ? "ip-instance" : "ip-area")" title="@node.Label"
data-test="@($"{DataTestPrefix}-row")" data-instance-id="@(node.Instance?.Id)"
@onclick="() => OnRowClickAsync(node)">@node.Label</span>
</NodeContent>
<EmptyContent><span class="text-muted fst-italic small px-2">@(_hasFilter ? "No instances match." : "No instances at this site.")</span></EmptyContent>
</TreeView>
</div>
}
</div>
```
Code-behind essentials:
- Parameters per design §2 table (`Areas`, `Instances`, `SelectedInstanceId`, `SelectedInstanceIdChanged`, `Disabled`, `ItemLabel`, `DataTestPrefix` (default `"instance"`), `Placeholder` (default `"Search or browse instances…"`)).
- State: `_open`, `_text`, `_filter`, `_roots`, `_tree` (`TreeView<InstanceTreeNode>?`), `_inputId`/`_panelId` from a `Guid` suffix.
- `OnParametersSet`: if `Areas`/`Instances` references changed → rebuild `_roots` with the current filter and (if the selection is no longer in `Instances`) clear text; if `SelectedInstanceId` changed and the panel is closed → `_text = LabelOf(selected)`.
- `Rebuild()` → `InstanceTreeBuilder.Build(Areas, Instances, _filter)` — the label used for matching is `UniqueName` (the builder's contract), rows display `ItemLabel`.
- `OnRowClickAsync(node)`: area → `_tree.ToggleExpand`? (`TreeView` has no public toggle — expose one: add `public void ToggleExpanded(object key)` to `TreeView.razor` next to `IsExpanded`, a one-line wrapper around the existing private `ToggleExpand`); instance → `SelectAsync(node.Instance)`.
- `SelectAsync(inst)`: `_text = LabelOf(inst)`, `_filter = ""`, close, `await SelectedInstanceIdChanged.InvokeAsync(inst.Id)`.
- `CloseAsync`: `_open = false`, `_filter = ""`, `_text = LabelOf(current)`.
- Guard: when `Disabled`, `OpenAsync`/`ToggleAsync` no-op.
CSS (`InstancePicker.razor.css`): `.ip-backdrop { position: fixed; inset: 0; z-index: 1040; background: transparent; }` `.ip-panel { position: absolute; left: 0; right: 0; top: 100%; z-index: 1041; max-height: 40vh; overflow-y: auto; min-width: 18rem; padding: .25rem; }` `.ip-instance { cursor: pointer; }` `.ip-area { cursor: pointer; }`. Rows render inside `TreeView`, whose `tv-*` classes are already global to that component — check `TreeView.razor.css` whether `tv-label`/`tv-glyph` are `::deep`-reachable from a consumer (TemplatePickerDialog uses them the same way, so follow that).
**Step 4: tests → PASS. Step 5: Commit** `feat(central-ui): InstancePicker — combobox over the site's area tree`.
---
### Task 3: `InstancePicker` — search filter + keyboard
**Classification:** standard
**Estimated implement time:** ~4 min
**Parallelizable with:** none
**Files:**
- Modify: `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor` / `.razor.cs`
- Test: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Shared/InstancePickerTests.cs`
**Step 1: Failing tests**
- `Typing_Filters_AndExpandsToTheMatches`: open, `Input("p1")` on the input → `[data-instance-id='7']` is rendered (ancestors expanded), Zulu row absent, Line1 row present.
- `Clearing_RestoresTheFullTree_Collapsed`: after the above, `Input("")` → Zulu back, P1 hidden again (Line1 collapsed).
- `Typing_OpensTheClosedPanel`.
- `Enter_PicksTheFirstVisibleMatch`: type "u" (matches Zulu only) + `KeyDown(Key="Enter")` → callback 8, closed, text "Zulu"; with no match Enter does nothing (still open).
- `Escape_ClosesAndRestoresTheSelectedText`: select P1, open, type "zzz", Escape → closed, text "P1".
- `ArrowDown_MovesFocusIntoTheTree`: after ArrowDown the first visible `[role='treeitem']` has `tabindex="0"` (the tree's roving tabindex target) — assert via the `TreeView` markup; focus itself is JS (Loose mode), so assert the call happened through `JSInterop` if `TreeView` uses `FocusAsync` (an `ElementReference.FocusAsync` shows up as a `Blazor._internal.domWrapper.focus` invocation in bUnit — assert `JSInterop.Invocations` contains it, or expose a `public Task FocusFirstVisibleAsync()` on `TreeView` and assert it doesn't throw).
**Step 2: run → fail. Step 3: Implement**
- `OnInput(ChangeEventArgs e)`: `_text = e.Value?.ToString() ?? ""`, `_filter = _text`, `_open = true`, `Rebuild()`; after render, if `_hasFilter` → `_tree?.ExpandAll()` else `_tree?.CollapseAll()` (do it in `OnAfterRender` guarded by a `_pendingExpand` flag so the tree instance exists).
- `OnInputKeyDown(KeyboardEventArgs e)`: `Enter` → first instance node in document order of the current (filtered) `_roots` → `SelectAsync`; `Escape` → `CloseAsync`; `ArrowDown` → `_open = true` and focus the tree's first visible item (`TreeView`: add `public async Task FocusFirstVisibleAsync()` that sets `_focusedKey` to the first visible node key and applies focus via the existing `_focusNeedsApply` mechanism).
- Typing on a closed picker opens it.
**Step 4: PASS. Step 5: Commit** `feat(central-ui): InstancePicker search filter + keyboard (Enter/Escape/ArrowDown)`.
---
### Task 4: Wire Flow Debugger to the picker
**Classification:** standard
**Estimated implement time:** ~4 min
**Parallelizable with:** Task 5, Task 6
**Files:**
- Modify: `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor` (~lines 62–71, the instance `<select>`)
- Modify: `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/FlowDebugger.razor.cs` (`_instances` ~114, `LoadInstancesAsync` ~412)
- Test: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/FlowDebuggerPageTests.cs` (five `select[data-test='flow-instance-select']` call sites ~623/637/656/675/717; `WireInstance` ~608)
**Steps:** (1) Add a test helper `static void PickInstance(IRenderedComponent<FlowDebuggerPage> cut, int id)` = click `[data-test='flow-instance-toggle']`, click `[data-test='flow-instance-row'][data-instance-id='{id}']` (the fixture instance has no area, so it is a root row); replace the five `.Change("7")` lines; make `_templates.GetAreasBySiteIdAsync(1, …)` return `[]` in `WireInstance`. Add `ScriptEntry_InstancePicker_ShowsAreasCollapsed` (areas fixture with the instance under an area: only the area row is visible until expanded). (2) Run the class → fails. (3) Page: `private List<Area> _areas = [];` loaded in `LoadInstancesAsync` alongside instances (`GetAreasBySiteIdAsync`), cleared on site 0; markup:
```razor
<InstancePicker Areas="_areas" Instances="_instances"
SelectedInstanceId="_selectedInstanceId" SelectedInstanceIdChanged="OnInstancePickedAsync"
Disabled="@(_running || _selectedSiteId == 0)" DataTestPrefix="flow-instance" />
```
with `private async Task OnInstancePickedAsync(int id) { _selectedInstanceId = id; await OnInstanceChangedAsync(); }`. Everything else (deep link, localStorage, scope) unchanged. (4) `FlowDebuggerPageTests` all pass. (5) Commit `feat(central-ui): Flow Debugger picks the instance through InstancePicker`.
---
### Task 5: Wire Debug View to the picker
**Classification:** standard
**Estimated implement time:** ~4 min
**Parallelizable with:** Task 4, Task 6
**Files:**
- Modify: `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/DebugView.razor` (instance `<select>` ~lines 83–92; `_siteInstances` ~259; `LoadInstancesForSite` ~430–452)
- Test: create `tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Deployment/DebugViewInstancePickerTests.cs` (copy the service registration from `DebugViewDisposalTests.cs`)
**Steps:** (1) Test: substitute repo returns one site, instances [Enabled "Sim01" id 7 in area 2] + areas [Line1 (1) → Cell A (2)]; render `DebugView`, `Change` the site select to the site, open `[data-test='debug-instance-toggle']` → only the Line1 row; expand twice → row `[data-instance-id='7']` text `"Sim01 (Enabled)"`; click it → *Connect* button enabled. (2) fails. (3) Page: `_siteAreas` list loaded in `LoadInstancesForSite` (same try/catch), picker markup with `ItemLabel="@(i => $"{i.UniqueName} ({i.State})")"`, `SelectedInstanceId="_selectedInstanceId" SelectedInstanceIdChanged="id => { _selectedInstanceId = id; OnInstanceSelectionChanged(); }"`, `Disabled="@_connected"`, `DataTestPrefix="debug-instance"`. (4) `DebugView*` tests pass. (5) Commit `feat(central-ui): Debug View picks the instance through InstancePicker`.
---
### Task 6: Playwright tests drive the picker
**Classification:** small
**Estimated implement time:** ~3 min
**Parallelizable with:** Task 4, Task 5
**Files:**
- Modify: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/Deployment/FlowDebuggerTests.cs` (~lines 209–213)
- Modify: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/Deployment/DebugViewTests.cs` (~68, ~93–97)
- Modify: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/Deployment/DebugViewTreeTests.cs` (~79–83)
Replace `<select>` + `SelectOptionAsync` with: `FillAsync` the `[data-test='{prefix}-input']` with the instance name, then `Expect(row [data-test='{prefix}-row'][data-instance-id='{id}']).ToBeVisibleAsync()` and `ClickAsync` it (Flow Debugger knows the name, not the id → click `[data-test='flow-instance-row']:has-text("<name>")` exactly; `Skip.If` no row appears within 10 s, same skip messages). `DebugViewTests` line 68 asserts the picker container `[data-test='debug-instance-picker']` count 1. Not runnable without the rig — build the project (`dotnet build tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests`) and commit `test(playwright): drive the instance picker on Flow Debugger and Debug View`. The controller runs them on the rig in the gate.
---
### Task 7: Docs
**Classification:** trivial
**Estimated implement time:** ~2 min
**Parallelizable with:** Task 4, Task 5, Task 6
**Files:**
- Modify: `docs/requirements/Component-CentralUI.md` (Flow Debugger v1.3 paragraph + the Debug View section — grep `debug-instance-select` / "Debug View")
- Modify: `CLAUDE.md` (Flow Debugger bullet, v1.3 sentence — append one sentence)
Text: "The instance is picked through the shared **`InstancePicker`** combobox (2026-09-08, `docs/plans/2026-09-08-instance-picker-design.md`): a text field whose dropdown is the site's area tree (`InstanceTreeBuilder`, all areas collapsed, un-areaed instances at the root), typed text filters by instance name and expands to the matches, Enter picks the first match; Debug View uses the same component with a `(State)` suffix." Commit `docs: instance picker on Flow Debugger + Debug View`.
---
### Task 8: Gate (controller)
Build solution (0 warnings), run `CentralUI.Tests` + `Commons.Tests`, `bash docker/deploy.sh`, Playwright `FlowDebuggerTests` + `DebugViewTests` + `DebugViewTreeTests`, browser check of both pages (collapsed areas, typing filter, Enter, Escape, deep link `?instanceId=`), then append gate evidence to this plan and finish via `finishing-a-development-branch`.
## Gate evidence (2026-09-08, branch `feat/instance-picker`)
- Solution build: 0 warnings. `CentralUI.Tests`: 1335 passed (picker 30 + builder 12 + Flow Debugger page 29 + Debug View picker 7 among them). Playwright project builds.
- Rig rebuilt (`docker/deploy.sh`) twice. Playwright `FlowDebuggerTests` + `DebugViewTests` + `DebugViewTreeTests`: **7/7, 0 skipped** on the second build. The first build failed two of them and skipped two: the tests typed into the picker immediately after choosing a site, while the site's instances were still loading; the loaded lists then arrived as a new reference and the picker reset (its "site changed" rule), so the row never appeared. Fixed by keeping the picker **disabled until the load completes** (`_instancesLoading` on both pages, pinned by TCS-based bUnit tests) — a real race for an operator on a slow database, not just a test artefact.
- Browser (Claude-in-Chrome, `localhost:9000`): Flow Debugger deep link `?instanceId=3687` shows `DelmiaSim01` in the field without opening the tree; typing `Delmia` opens the panel with the single matching row; Escape restores the selection text and closes. Debug View: after picking Test Plant A the panel lists the three `zztest-area-*` areas first (collapsed) and the Enabled instances after them with the `(State)` suffix.
- Whole-branch review findings closed: keyboard activation now routes through `TreeView` selection (mouse + keyboard one handler; `SelectedKey` deliberately unbound to avoid the TreeView reset loop); `aria-label`/`InputId`; `aria-controls` only while open; Debug View's localStorage auto-reconnect guards a stored instance that is no longer Enabled; select-era `OrderBy` dropped; deep-link-in-area test added; design doc corrected (tab-out does not close — needs JS `relatedTarget`; explicit handlers instead of `@bind`).
- Untested on the rig: keyboard activation inside the tree in a real browser (pinned by bUnit only); `overflow: hidden` clipping (neither page has such an ancestor today; CSS comment warns integrators).