From acc6a64b264c5e7f329a43ecaa03140ddfcf19d6 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 18 Jun 2026 13:31:10 -0400 Subject: [PATCH] docs(plan): implementation plan + tasks for F10b surgical tag-attribute writes --- ...6-18-f10b-surgical-tag-attribute-writes.md | 138 ++++++++++++++++++ ...urgical-tag-attribute-writes.md.tasks.json | 10 ++ 2 files changed, 148 insertions(+) create mode 100644 docs/plans/2026-06-18-f10b-surgical-tag-attribute-writes.md create mode 100644 docs/plans/2026-06-18-f10b-surgical-tag-attribute-writes.md.tasks.json diff --git a/docs/plans/2026-06-18-f10b-surgical-tag-attribute-writes.md b/docs/plans/2026-06-18-f10b-surgical-tag-attribute-writes.md new file mode 100644 index 00000000..7bc53788 --- /dev/null +++ b/docs/plans/2026-06-18-f10b-surgical-tag-attribute-writes.md @@ -0,0 +1,138 @@ +# F10b surgical in-place tag-attribute writes — Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development to implement this plan task-by-task. + +**Goal:** Surgical in-place updates for equipment-tag `Writable` + `IsHistorized`/`HistorianTagname` toggles, avoiding a full `RebuildAddressSpace` (preserving client subscriptions). DataType/array keep rebuild; alarm severity dropped. + +**Architecture:** A new optional `ISurgicalAddressSpaceSink` capability (Commons) backed by `OtOpcUaNodeManager.UpdateTagAttributes` (mirrors `EnsureVariable`'s access/Historizing/handler/historian-map logic). `Phase7Applier` detects surgical-eligible tag deltas (only Writable/IsHistorized/HistorianTagname differ, no alarm, identity stable) and applies them in place, falling back to a full rebuild for anything else or if the sink lacks the capability / a node is missing. + +**Tech Stack:** .NET 10, OPC UA SDK (BaseDataVariableState), Akka.NET, xUnit + Shouldly. + +**Design:** `docs/plans/2026-06-18-f10b-surgical-tag-attribute-writes-design.md` (committed `4fe88519`). + +**Standing constraints:** NO Commons wire/proto change · NO **breaking** interface change (ADD a new interface, do NOT modify `IOpcUaAddressSpaceSink`) · NO EF migration · NO bUnit · stage by explicit path (never `git add .`) · never stage the never-stage files · no force-push / no `--no-verify` · `dangerouslyDisableSandbox:true` for build/test/rig · finish = merge + push. + +--- + +### Task 1: Surgical-write capability — interface + node-manager + sink + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** none + +**Files:** +- Create: `src/Core/ZB.MOM.WW.OtOpcUa.Commons/OpcUa/ISurgicalAddressSpaceSink.cs` +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs` (add `UpdateTagAttributes` + extract `ComposeAccessLevel`) +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/SdkAddressSpaceSink.cs` (implement the new interface) + +**Context:** `EnsureVariable` (`OtOpcUaNodeManager.cs:1318-1370`) is the source of truth for tag node attributes: +`historized = historianTagname is not null`; `access = writable ? (byte)(CurrentRead|CurrentWrite) : CurrentRead`, then `if (historized) access = (byte)(access | HistoryRead)`; `Historizing = historized`; `if (writable) variable.OnWriteValue = OnEquipmentTagWrite`; `if (historized) _historizedTagnames[nodeId] = historianTagname!`. `_historizedTagnames` is a `ConcurrentDictionary` (`:50`). The node lookup is `_variables.TryGetValue` under `lock (Lock)`. The existing in-place pattern is `WriteValue` (`:256-275`): mutate the node + `ClearChangeMasks(SystemContext, includeChildren: false)`. + +**Steps:** +1. **Interface** `ISurgicalAddressSpaceSink.cs` (namespace `ZB.MOM.WW.OtOpcUa.Commons.OpcUa`) — exactly as in the design: + ```csharp + public interface ISurgicalAddressSpaceSink + { + /// Update an existing variable node's Writable (AccessLevel + inbound-write handler) and + /// Historizing (+ historian-tagname binding) IN PLACE, notifying subscribers without a rebuild. + /// Returns false if the node does not exist (caller should fall back to a full rebuild). + bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname); + } + ``` +2. **`OtOpcUaNodeManager`:** extract a `private static byte ComposeAccessLevel(bool writable, bool historized)` matching the EnsureVariable byte logic exactly, and use it IN `EnsureVariable` (replace the two inline `access` lines with a `ComposeAccessLevel(writable, historized)` call — verify the resulting byte is identical). Then add: + ```csharp + public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname) + { + ArgumentException.ThrowIfNullOrEmpty(variableNodeId); + lock (Lock) + { + if (!_variables.TryGetValue(variableNodeId, out var v)) return false; + var historized = historianTagname is not null; + var access = ComposeAccessLevel(writable, historized); + v.AccessLevel = access; + v.UserAccessLevel = access; + v.Historizing = historized; + v.OnWriteValue = writable ? OnEquipmentTagWrite : null; + if (historized) _historizedTagnames[variableNodeId] = historianTagname!; + else _historizedTagnames.TryRemove(variableNodeId, out _); + v.ClearChangeMasks(SystemContext, includeChildren: false); + return true; + } + } + ``` + Add an XML doc summary explaining this is the surgical counterpart of `EnsureVariable` for F10b (Writable/Historizing toggles), mutating the live node so client subscriptions survive. +3. **`SdkAddressSpaceSink`:** add `, ISurgicalAddressSpaceSink` to the class declaration and `public bool UpdateTagAttributes(string variableNodeId, bool writable, string? historianTagname) => _nodeManager.UpdateTagAttributes(variableNodeId, writable, historianTagname);` (match the field name for the node manager — read the file). +4. **Build:** `dotnet build src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer` (dangerouslyDisableSandbox:true) → 0 errors. (Commons builds transitively.) +5. **Commit** (explicit paths): the new interface + the two modified files. + +**Acceptance:** the interface exists; `UpdateTagAttributes` mirrors `EnsureVariable` via the shared `ComposeAccessLevel`; SdkAddressSpaceSink delegates; build clean. NO change to `IOpcUaAddressSpaceSink`. + +--- + +### Task 2: Phase7Applier surgical-eligibility + apply/rebuild-fallback + tests + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** none (needs T1's interface) + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Applier.cs` +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7ApplierTests.cs` + +**Context:** `Phase7Applier.Apply` (`:48-115`) computes `needsRebuild` (the just-shipped vtag-skip uses `plan.ChangedEquipmentVirtualTags.Any(d => !VtagDeltaIsNodeIrrelevant(d))`). `EquipmentTagPlan` (`Phase7Composer.cs:92-115`) = `(TagId, EquipmentId, DriverInstanceId, FolderPath, Name, DataType, FullName, Writable, EquipmentTagAlarmInfo? Alarm, IsHistorized=false, HistorianTagname=null, IsArray=false, ArrayLength=null)` with AUTO record equality. `Phase7Plan.EquipmentTagDelta(Previous, Current)`. `MaterialiseEquipmentTags` (`:194-220`) computes `writable = tag.Writable && !tag.IsArray` and `historianTagname = tag.IsHistorized ? (blank HistorianTagname ? tag.FullName : tag.HistorianTagname) : null`, node id `EquipmentNodeIds.Variable(tag.EquipmentId, tag.FolderPath, tag.Name)`. Read `Phase7ApplierTests.cs` for the `RecordingSink` + `CompositionWithVirtualTags`-style helpers + the just-added vtag-skip tests to mirror. + +**Steps:** +1. Add the eligibility helper (mirror the vtag helper's whitelist-of-may-differ style): + ```csharp + // F10b: a CHANGED equipment tag whose ONLY differences are Writable / IsHistorized / HistorianTagname + // (a plain value variable — no alarm condition node) can be updated IN PLACE on the existing node via + // ISurgicalAddressSpaceSink.UpdateTagAttributes, avoiding a full rebuild (preserving subscriptions). + // DataType / IsArray / ArrayLength / FullName / identity / alarm differences fall through to a rebuild + // (the override-unequal default also covers any future field). + private static bool TagDeltaIsSurgicalEligible(Phase7Plan.EquipmentTagDelta d) => + d.Previous.Alarm is null && d.Current.Alarm is null && + (d.Previous with + { + Writable = d.Current.Writable, + IsHistorized = d.Current.IsHistorized, + HistorianTagname = d.Current.HistorianTagname, + }).Equals(d.Current); + ``` +2. Change the `needsRebuild` `ChangedEquipmentTags.Count > 0` term to `plan.ChangedEquipmentTags.Any(d => !TagDeltaIsSurgicalEligible(d))` (keep Added/Removed tag terms forcing rebuild). Rename the local to `structuralRebuild` if clearer. +3. Replace the `if (needsRebuild) { _sink.RebuildAddressSpace(); }` block with the apply/fallback control flow from the design §"Control flow in Apply": if structural ⇒ rebuild; else if there are surgical-eligible tag deltas ⇒ if `_sink is ISurgicalAddressSpaceSink surgical`, loop them computing `nodeId`/`writable`/`historian` exactly as `MaterialiseEquipmentTags` does and call `surgical.UpdateTagAttributes(...)`; if any returns false ⇒ `_sink.RebuildAddressSpace()` (fallback); if the sink lacks the capability ⇒ rebuild. Track a `rebuilt` bool and return it as `Phase7ApplyOutcome.RebuildCalled`. Keep `changedCount`/`ChangedNodes` unchanged. Preserve the existing try/catch around `RebuildAddressSpace` + the log line (extend the log to note surgical vs rebuild if cheap). +4. Update the `needsRebuild` comment block to document the surgical-tag path alongside the vtag-skip. +5. **Tests** in `Phase7ApplierTests.cs`: + - Extend `RecordingSink` to ALSO implement `ISurgicalAddressSpaceSink`, recording each `UpdateTagAttributes(nodeId, writable, historianTagname)` into a queue + a counter; return `true`. Add a second fake (or a flag) for the "node missing ⇒ returns false" + the "sink without the capability" cases. + - Facts: Writable-only ⇒ no rebuild + 1 surgical call (writable=new); IsHistorized toggle ⇒ no rebuild + surgical(historian = FullName-or-override / null); HistorianTagname-only (historized) ⇒ no rebuild + surgical(historian=new); DataType change ⇒ rebuild; IsArray change ⇒ rebuild; FullName change ⇒ rebuild; Name change ⇒ rebuild; Alarm-presence change ⇒ rebuild; surgical-eligible mixed-with-another-change ⇒ rebuild; sink-without-capability + surgical delta ⇒ rebuild (fallback); surgical returns false (node missing) ⇒ rebuild (fallback). Assert `outcome.RebuildCalled` + `sink.RebuildCalls` + the recorded surgical calls + `ChangedNodes`. +6. **Build + test:** `dotnet build src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer` + `dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests --filter "FullyQualifiedName~Phase7Applier"` → all green. +7. **Commit** (explicit paths): `Phase7Applier.cs` + `Phase7ApplierTests.cs`. + +**Acceptance:** surgical-eligible tag deltas apply in place (no rebuild); everything else + the fallbacks rebuild; tests green. + +--- + +### Task 3: Reconcile + live `/run` + finish (merge + push) + +**Classification:** small +**Estimated implement time:** ~5 min +**Parallelizable with:** none (last) + +**Files:** +- Modify (NEVER staged): `stillpending.md` (#11 — note the surgical Writable/Historize slice) +- Modify: memory `project_stillpending_backlog.md` + `MEMORY.md` index line + +**Steps:** +1. Full solution build: `dotnet build ZB.MOM.WW.OtOpcUa.slnx` — 0 errors. +2. Targeted tests: `OpcUaServer.Tests` (Phase7Applier) green; report counts. +3. **Live `/run` (confirmatory — the surgical path needs the real SDK sink, which runs on docker-dev):** rebuild BOTH central nodes (`docker compose -f docker-dev/docker-compose.yml build central-1 central-2` then `up -d --no-deps --force-recreate central-1 central-2`); via the AdminUI `/uns` toggle one equipment tag's Historize (or Writable), Deploy current configuration, and confirm the server log shows `rebuild=False` for that deploy + the server stays healthy + the node remains readable (a Client.CLI read still works). If observing the live toggle is impractical in the time budget, record that the unit tests pin the logic and the live gate is the rebuild=False log line; do NOT fake it. +4. `stillpending.md` (never staged): update #11 — the surgical Writable/IsHistorized/HistorianTagname in-place slice SHIPPED (alarm severity dropped as live-shadowed; DataType/array kept-rebuild as dirty — both documented). +5. Memory: top marker in `project_stillpending_backlog.md` + concise `MEMORY.md` index update (keep under the soft cap). +6. Finish (superpowers-extended-cc:finishing-a-development-branch): verify tests → ff-merge `feat/f10b-surgical-tag-attribute-writes` → master → push → delete branch → confirm `local master = origin/master`. + +**Acceptance:** build clean, tests green, live `/run` shows rebuild=False on a Writable/Historize toggle (or the documented confirmatory evidence), backlog/memory reconciled, merged + pushed. + +--- + +## Dependency graph + +`T1 → T2 → T3` (T1/T2 coupled by the new interface; T3 last). diff --git a/docs/plans/2026-06-18-f10b-surgical-tag-attribute-writes.md.tasks.json b/docs/plans/2026-06-18-f10b-surgical-tag-attribute-writes.md.tasks.json new file mode 100644 index 00000000..0392a23d --- /dev/null +++ b/docs/plans/2026-06-18-f10b-surgical-tag-attribute-writes.md.tasks.json @@ -0,0 +1,10 @@ +{ + "planPath": "docs/plans/2026-06-18-f10b-surgical-tag-attribute-writes.md", + "executionState": "PENDING", + "tasks": [ + {"id": 1, "subject": "Task 1: ISurgicalAddressSpaceSink + OtOpcUaNodeManager.UpdateTagAttributes + SdkAddressSpaceSink impl", "classification": "standard", "status": "pending"}, + {"id": 2, "subject": "Task 2: Phase7Applier surgical-eligibility + apply/rebuild-fallback + tests", "classification": "standard", "status": "pending", "blockedBy": [1]}, + {"id": 3, "subject": "Task 3: Reconcile + live /run + finish (merge + push)", "classification": "small", "status": "pending", "blockedBy": [1, 2]} + ], + "lastUpdated": "2026-06-18" +}