From 5dc8d3447276b321ca45f336596131ea53ccf789 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:56:16 -0400 Subject: [PATCH] docs: the stale-Good quality contract, stated once (R3-02) --- CLAUDE.md | 27 +++++++- .../plans/R3-02-stale-good-family-plan.md | 10 +++ ...R3-02-stale-good-family-plan.md.tasks.json | 2 +- docs/DriverLifecycle.md | 66 ++++++++++++++++++- docs/Raw.md | 11 ++++ docs/Uns.md | 11 ++++ 6 files changed, 123 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ff22131d..fb620717 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -297,7 +297,32 @@ non-`Healthy` state **and** its `LastSuccessfulRead` has been frozen for 90 s. T system — it is a local `GetHealth()` read, and only reaches the network once a driver has already stopped reading. -See `docs/DriverLifecycle.md` §"The comms-loss watchdog". +- **What comms loss now does downstream — one contract, six seams (archreview R3-02).** "A value consumer + never reports a quality better than its source's" is stated once in `docs/DriverLifecycle.md` §"The + quality contract" and implemented by `Core.Abstractions.InputQuality` (`WorstOf`/`IsUsable`). Three + things changed with it, all client-visible: + - ⚠️ **Value nodes of a comms-lost driver now go Bad.** `DriverHostActor.FanCommsLossToValueNodes` + stamps `BadNoCommunication` on the raw NodeId **and every referencing UNS NodeId**, **value retained**, + via the new status-only sink primitive `IOpcUaAddressSpaceSink.WriteStatuses` (the host holds no value + cache, so only the node can keep the value). #481 had wired comms loss to the mux and to Part 9 + conditions and stopped one seam short of the external data plane. A consumer that treats Bad as "drop + the tag" (ScadaBridge) will see more Bads — that is the fix working. + - **Nothing Good is written on reconnect** — recovery is the driver's own re-publish (the poll engine's + `forceRaise`, MQTT's new initial push). Instead the host **audits**: refs still latched 30 s after a + reconnect log a Warning and increment `otopcua.driver.comms_latched_refs`. Non-zero on that counter + means a driver reconnects without re-publishing. + - **Every dedup on the recovery path is keyed on (value, status), not value.** A device that comes back + usually comes back holding the same reading, so a value-only dedup swallows the recovery and the node + stays Bad forever — the v3-gate trap, now closed in the Calculation driver, the virtual-tag actor and + (via a connection-loss stale) MQTT's plain-mode suppression. **Any new consumer of driver values must + repeat this**; a regression test for it must replay the IDENTICAL value or it proves nothing. + - Consequences per seam: MQTT/Sparkplug stale on connection loss and push cached snapshots on every + (re)subscribe (`docs/drivers/Mqtt.md`); the Calculation driver and virtual tags **hold** while any + input is Bad and inherit the worst input's sub-code, evaluate through Uncertain, and hand scripts the + input's real `StatusCode`/`SourceTimestampUtc` (`docs/drivers/Calculation.md`, `docs/ScriptEditor.md`) + — so a script that relied on receiving `null` for a dead input no longer runs at all while it is Bad. + +See `docs/DriverLifecycle.md` §"The comms-loss watchdog" and §"The quality contract". ## mxaccessgw diff --git a/archreview/plans/R3-02-stale-good-family-plan.md b/archreview/plans/R3-02-stale-good-family-plan.md index ffcff20e..7cdc46d9 100644 --- a/archreview/plans/R3-02-stale-good-family-plan.md +++ b/archreview/plans/R3-02-stale-good-family-plan.md @@ -806,3 +806,13 @@ test could only pass once the production change landed. - `_lastPublishedBad` became `_hasPublished` + `_lastPublishedStatus`, which is also what makes `ReassertValue` re-emit `BadNoCommunication` after a deploy instead of guessing Good. - Docs: `docs/ScriptEditor.md` §"Input quality: what a script sees, and when it runs at all". + +### T17 — Docs: the contract stated once + +- `docs/DriverLifecycle.md` gains §"The quality contract" (the contract quoted verbatim + a table of the + six seams that implement it + the consumer-visible warning) and a new sub-section under the comms-loss + fan-out covering the value-node fan, `WriteStatuses`, and the recovery audit. +- Pointers added, each linking back rather than restating: `docs/Raw.md`, `docs/Uns.md`, + `docs/ScriptEditor.md` (T16), `docs/drivers/Calculation.md` (T14), `docs/drivers/Mqtt.md` (T5). +- `CLAUDE.md`: one bullet under the comms-loss watchdog section — value nodes go Bad, nothing Good on + reconnect (+ the audit meter), and the **(value, status)** dedup rule any future consumer must repeat. diff --git a/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json b/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json index 25cc7158..2e05fd07 100644 --- a/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json +++ b/archreview/plans/R3-02-stale-good-family-plan.md.tasks.json @@ -125,7 +125,7 @@ { "id": "T17", "subject": "Docs: the quality contract stated once (DriverLifecycle.md) + Raw/Uns/ScriptEditor/Calculation pointers + CLAUDE.md bullet", - "status": "pending", + "status": "done", "blockedBy": [] }, { diff --git a/docs/DriverLifecycle.md b/docs/DriverLifecycle.md index 3e4519db..196d55ae 100644 --- a/docs/DriverLifecycle.md +++ b/docs/DriverLifecycle.md @@ -219,6 +219,37 @@ So a driver whose `InitializeAsync` disposes anything on its failure path must b `MTConnectDriver` does today, because it is the only one that tears down inside a `catch`. The shutdown paths deliberately keep the single trailing write. +### The quality contract + +> **A value consumer never reports a quality better than its source's.** A node derived from a driver +> value — the raw node, every UNS mirror, a Calculation output, a virtual tag, a scripted condition's +> quality — carries, at worst, the worst status among its inputs: a Bad input makes the derived node Bad +> **with that input's sub-code and the last-known value**; an Uncertain input makes it Uncertain; a source +> that has stopped communicating is a Bad input (`BadNoCommunication`) for everything downstream of it, +> **including the external OPC UA data plane**. Recovery is a *publish*, never an assumption: the first +> post-recovery value must land even when it equals the value held before the outage, so every dedup on +> the path — driver caches, the node's own status, a consumer's last-published memory — must be keyed on +> **(value, status)**, not value alone. + +This is one rule with one implementation of its comparison, +`ZB.MOM.WW.OtOpcUa.Core.Abstractions.InputQuality` (`WorstOf` / `IsUsable`), used by every derived-value +seam. It was written down because the same lie had been fixed four times in four places and re-opened in +a fifth (archreview round 3, theme "the stale-Good family"). The seams that implement it: + +| Seam | What it does | Where | +|---|---|---| +| Driver comms loss → **value nodes** | one status-only batch stamps `BadNoCommunication` on the driver's raw NodeIds and every referencing UNS NodeId, **value retained** | `DriverHostActor.FanCommsLossToValueNodes` (below) | +| Driver comms loss → dependency mux | synthetic Bad per ref, so scripts and alarms see the source die | `DriverHostActor.FanCommsLossToDependencyMux` | +| Driver comms loss → Part 9 conditions | condition quality annotated out of band | `DriverHostActor.OnDriverConnectivityChanged` | +| MQTT / Sparkplug | connection loss stales observed refs; every (re)subscribe pushes the cached snapshot | [`docs/drivers/Mqtt.md`](drivers/Mqtt.md#quality-on-connection-loss-and-re-subscribe) | +| Calculation driver | holds on Bad, propagates the worst input status | [`docs/drivers/Calculation.md`](drivers/Calculation.md#input-quality) | +| Virtual tags | holds on Bad, inherits the worst input status, reassert carries it | [`docs/ScriptEditor.md`](ScriptEditor.md#input-quality-what-a-script-sees-and-when-it-runs-at-all) | +| Scripted alarms | worst-of-input quality annotated out of band; Bad inputs hold the condition | [`docs/ScriptedAlarms.md`](ScriptedAlarms.md) | + +⚠️ **Consumer-visible:** value nodes of a comms-lost driver now go **Bad**, where they used to serve +their last value at Good. A consumer that treats a Bad value as "drop the tag" (ScadaBridge's data +connection layer, say) will see more Bads than before — that is the fix working, not a regression. + ### The comms-loss watchdog (Gitea #527) — `GetHealth()` is now load-bearing `GetHealth()` used to be **reporting-only**: `DriverInstanceActor.PublishHealthSnapshot` read it every 30 s @@ -283,8 +314,8 @@ on it still gets its initialise attempt. #### The fan-out that watchdog triggers: indexed per driver, and one batch `DisconnectObserved` raises `ConnectivityChanged`, and `DriverHostActor` answers it by pushing -`BadNoCommunication` at every ref the flapping driver owns — the value fan to the dependency mux, and the -alarm fan to the publish actor's condition quality. Both used to answer "which refs are this driver's?" +`BadNoCommunication` at every ref the flapping driver owns — **three** fans: the value nodes (the +external OPC UA surface), the dependency mux, and the publish actor's condition quality. Both used to answer "which refs are this driver's?" by **scanning the node's entire ref map and filtering** — the maps are keyed by the composite `(DriverInstanceId, RawPath)`, so one driver's flap walked all ~50k keys on a large node to find its own few thousand. Each now has a secondary index, `DriverInstanceId → [(RawPath, node set)]` @@ -310,6 +341,37 @@ its last one — everything on the subscription path was already wrapped in `Att `DependencyMuxActor` still registers the per-value receive, but in production it is dead code; it stays for tests and direct senders. +#### The third fan: the value nodes (archreview 03/S15) + +The two fans above reach *internal* consumers and *conditions*. Until archreview round 3 the driver's raw +value variables and their UNS mirrors — what an OPC UA client actually subscribes to — got nothing, so a +client saw the last value at **Good** quality for the whole outage. That was invisible on Modbus (which +degrades per tag within one poll interval on its own) and total on every driver whose failure shape is +"the reader stops producing": a coalesced-read throw, a dropped push session, or simply a driver whose +last act was a Good publish. + +`FanCommsLossToValueNodes` closes it with **one status-only batch** — a new sink primitive, +`IOpcUaAddressSpaceSink.WriteStatuses` → `OtOpcUaNodeManager.WriteStatuses` — that changes each node's +StatusCode and Timestamp and **leaves its Value alone**. Status-only is not an optimisation: the host +holds no value cache (v3's single-source design deliberately removed the independent buffer), so only the +node itself can retain the last known value, and a Bad node still carrying its last reading is the OPC UA +Part 8 convention a SCADA consumer expects. Two details: + +- It is **ungated by redundancy role**, like the condition annotation: a Secondary keeps its address space + honest for failover. +- An unmaterialised NodeId is **skipped, never created** — a status carries nothing to create a node from, + and the per-driver index can legitimately name a node a surgical remove has just dropped. + +**Nothing is written on reconnect.** Publishing Good there would assert a quality the host has not +observed. Recovery is the driver's own re-publish: for a poll driver the watchdog's `DetachSubscription` +forces a fresh `PollGroupEngine` subscription whose initial poll runs `forceRaise: true`; for MQTT it is +the initial push added by 05/STAB-18. What the host does instead is **audit** it — `CommsRecoveryAudit` +fires `DefaultCommsRecoveryAuditDelay` (30 s) after the driver reconnects and reports any ref still +latched Bad, as a Warning naming the driver plus the `otopcua.driver.comms_latched_refs` counter (tagged +`driver`, `driver_type`). That counter going non-zero means a driver reconnected without re-publishing — +a push driver missing its initial push, or a value-only dedup swallowing an unchanged value. It is a +tripwire, not a repair: nothing downstream depends on it. + ### The connect path feeds the circuit breaker (R2-10 T10 closure) `IDriver.InitializeAsync` is routed through the capability invoker as diff --git a/docs/Raw.md b/docs/Raw.md index 144ba49f..022451bc 100644 --- a/docs/Raw.md +++ b/docs/Raw.md @@ -123,6 +123,17 @@ the full design. Highlights: rejects an A→B→A oscillation loop naming the members. Compile is not hard-gated (VirtualTag parity); a runtime compile/throw/timeout lands as Bad quality + a `script-logs` entry. +## Quality: a raw tag's node when its driver stops communicating + +A raw tag's node — and every UNS mirror of it — goes **`BadNoCommunication` with its last value +retained** when the owning driver is declared comms-lost, and clears when the driver re-publishes. It is +never cleared by the server on reconnect: recovery is a value, never an assumption. The rule and the +seams that implement it are stated once in +[`docs/DriverLifecycle.md` §"The quality contract"](DriverLifecycle.md#the-quality-contract). + +Calc tags authored here inherit the same rule from their inputs — see +[`docs/drivers/Calculation.md` §"Input quality"](drivers/Calculation.md#input-quality). + ## Retirement note The routed `/clusters/{id}/drivers` driver-authoring flow (`DriverTypePicker`, diff --git a/docs/Uns.md b/docs/Uns.md index 7ccc5238..89651195 100644 --- a/docs/Uns.md +++ b/docs/Uns.md @@ -476,6 +476,17 @@ not — clients pick those up on their next browse. And a **degraded** non-rebui (some node failed to materialise) deliberately does not record its composition as applied, so the next deploy re-diffs and repairs it. +## Quality on a UNS node + +A UNS reference variable mirrors its backing raw tag exactly — value, status and timestamps — so when +that tag's driver stops communicating the mirror goes **`BadNoCommunication` with its last value +retained**, in the same fan-out message as the raw node, and clears only when the driver re-publishes. +A VirtualTag on the same equipment follows the same rule from its own inputs: it holds while any input +is Bad and reports that input's sub-code. The contract, and every seam that implements it, is stated +once in [`docs/DriverLifecycle.md` §"The quality contract"](DriverLifecycle.md#the-quality-contract); +what a script sees is in +[`docs/ScriptEditor.md`](ScriptEditor.md#input-quality-what-a-script-sees-and-when-it-runs-at-all). + ## See also - [Configuration.md](Configuration.md) — the underlying config entities.