From cc9b845f993159ba1f32e9dfe127c883466c42df Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 10:45:11 -0400 Subject: [PATCH] feat(engines): IVirtualTagEvaluator snapshot overload; Roslyn read cache stops fabricating Good (02/S14) --- .../plans/R3-02-stale-good-family-plan.md | 10 ++ ...R3-02-stale-good-family-plan.md.tasks.json | 2 +- .../Engines/IVirtualTagEvaluator.cs | 30 ++++++ .../Engines/RoslynVirtualTagEvaluator.cs | 50 ++++++---- .../RoslynVirtualTagEvaluatorTests.cs | 93 +++++++++++++++++++ 5 files changed, 166 insertions(+), 19 deletions(-) diff --git a/archreview/plans/R3-02-stale-good-family-plan.md b/archreview/plans/R3-02-stale-good-family-plan.md index a5e2c0cb..2fbebca4 100644 --- a/archreview/plans/R3-02-stale-good-family-plan.md +++ b/archreview/plans/R3-02-stale-good-family-plan.md @@ -770,3 +770,13 @@ test could only pass once the production change landed. xmldoc says so and points at the snapshot overload. - Docs: `docs/drivers/Calculation.md` gains an "Input quality" section (the policy table + the behaviour change for null-tolerant scripts) and the dedup bullet now says **(value, status)**. + +### T15 — `IVirtualTagEvaluator` snapshot overload + Roslyn read-cache pass-through + +- RED: `--filter "…RoslynVirtualTagEvaluatorTests"` ⇒ **build failure**, `CS1503: cannot convert from + 'Dictionary' to 'IReadOnlyDictionary'` (×3). +- GREEN: 24/24 in that suite; full solution build clean (0 errors), so **all five existing + `IVirtualTagEvaluator` fakes compile untouched** — which is the point of default-implementing the + overload. A test pins that fallback explicitly (`ValueOnlyEvaluator`). +- `RoslynVirtualTagEvaluator.BuildReadCache` is **deleted**; the snapshots are the read cache. The + value-only overload now projects up into the snapshot one, so there is a single evaluation body. 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 0d0ef866..2bd803a6 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 @@ -110,7 +110,7 @@ { "id": "T15", "subject": "IVirtualTagEvaluator snapshot overload (default-implemented) + RoslynVirtualTagEvaluator.BuildReadCache pass-through (02/S14)", - "status": "pending", + "status": "done", "blockedBy": [] }, { diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Engines/IVirtualTagEvaluator.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Engines/IVirtualTagEvaluator.cs index 7864246c..0dc8825a 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Engines/IVirtualTagEvaluator.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Engines/IVirtualTagEvaluator.cs @@ -1,3 +1,5 @@ +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + namespace ZB.MOM.WW.OtOpcUa.Commons.Engines; /// @@ -18,6 +20,34 @@ public interface IVirtualTagEvaluator /// Read-only dictionary of variable names to values for expression evaluation. /// Result containing success flag, evaluated value, and optional failure reason. VirtualTagEvalResult Evaluate(string virtualTagId, string expression, IReadOnlyDictionary dependencies); + + /// + /// Evaluate against dependency snapshots — value, status + /// code and source timestamp per input — so a script's ctx.GetTag(path).StatusCode reports + /// what the source actually said (archreview 02/S14). + /// + /// + /// Default-implemented on purpose. It projects to values and calls the older overload, so + /// every existing implementation — the Roslyn evaluator aside, that is the null evaluator and the + /// test fakes — compiles and behaves unchanged; an implementation that can do better (the Roslyn + /// one) overrides it. Making it abstract would have been a breaking change for a fix whose whole + /// point is that it changes nothing on the Good path. + /// The default necessarily discards the status codes it was given, which is exactly the + /// laundering this overload exists to end — so a consumer that cares must call an implementation + /// that overrides it. + /// + /// The unique identifier of the virtual tag being evaluated. + /// The expression string to evaluate. + /// Read-only dictionary of dependency snapshots, keyed by tag path. + /// Result containing success flag, evaluated value, and optional failure reason. + VirtualTagEvalResult Evaluate( + string virtualTagId, string expression, IReadOnlyDictionary dependencies) + { + ArgumentNullException.ThrowIfNull(dependencies); + var values = new Dictionary(dependencies.Count, StringComparer.Ordinal); + foreach (var kv in dependencies) values[kv.Key] = kv.Value?.Value; + return Evaluate(virtualTagId, expression, values); + } } /// Result of one virtual-tag expression eval. Stash a Reason on every Failure so diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs index 2e028c28..a0fc339d 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs @@ -59,7 +59,34 @@ public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IScriptCac } /// + /// + /// Value-only overload. Every dependency reaches the script stamped Good with an + /// evaluation-time timestamp — bare values carry nothing better. The runtime calls the snapshot + /// overload instead (archreview 02/S14); this one remains for callers that genuinely hold only + /// values. + /// public VirtualTagEvalResult Evaluate(string virtualTagId, string expression, IReadOnlyDictionary dependencies) + { + ArgumentNullException.ThrowIfNull(dependencies); + var nowUtc = DateTime.UtcNow; + var snapshots = new Dictionary(dependencies.Count, StringComparer.Ordinal); + foreach (var kv in dependencies) + snapshots[kv.Key] = new DataValueSnapshot(kv.Value, StatusCode: 0u, nowUtc, nowUtc); + + return Evaluate(virtualTagId, expression, snapshots); + } + + /// + /// + /// Overrides the interface's default (which projects to values and loses the status codes): the + /// snapshots pass through as the script's read cache, so + /// ctx.GetTag(path).StatusCode and .SourceTimestampUtc are the input's own rather + /// than a fabricated Good stamped at evaluation time (archreview 02/S14). The virtual-tag + /// actor holds evaluation entirely while any input is Bad, so what a script observes here is Good + /// or Uncertain. + /// + public VirtualTagEvalResult Evaluate( + string virtualTagId, string expression, IReadOnlyDictionary dependencies) { if (_disposed) return VirtualTagEvalResult.Failure("evaluator disposed"); if (string.IsNullOrWhiteSpace(expression)) return VirtualTagEvalResult.Failure("empty expression"); @@ -71,12 +98,14 @@ public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IScriptCac // so the script returns null — Ok(null) here too. if (PassthroughScript.TryMatch(expression, out var passthroughRef)) { - return dependencies.TryGetValue(passthroughRef, out var ptValue) - ? VirtualTagEvalResult.Ok(ptValue) + // The VALUE only — the caller stamps the result with the worst input status, which for a + // single-dependency passthrough IS this input's status. + return dependencies.TryGetValue(passthroughRef, out var ptSnapshot) + ? VirtualTagEvalResult.Ok(ptSnapshot?.Value) : VirtualTagEvalResult.Ok(null); } - var readCache = BuildReadCache(dependencies); + var readCache = dependencies; // Per-evaluation script logger: bind both ScriptId and VirtualTagId from the virtual-tag id // (in the live path the script id equals the virtual-tag id) so the Script-log page can // attribute each line. EquipmentId stays unbound for now. @@ -162,21 +191,6 @@ public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IScriptCac /// public void ClearCompiledScripts() => _cache.Clear(); - private static IReadOnlyDictionary BuildReadCache( - IReadOnlyDictionary deps) - { - // VirtualTagContext.GetTag returns a DataValueSnapshot — we wrap each raw dep value - // as Good-quality so the script's `(int)ctx.GetTag("a").Value` pattern works. Null - // values stay null; the script can null-check via GetTag(path).Value. - var nowUtc = DateTime.UtcNow; - var cache = new Dictionary(StringComparer.Ordinal); - foreach (var kv in deps) - { - cache[kv.Key] = new DataValueSnapshot(kv.Value, StatusCode: 0u, SourceTimestampUtc: nowUtc, ServerTimestampUtc: nowUtc); - } - return cache; - } - /// Disposes the evaluator and cleans up all cached script evaluators. public void Dispose() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/RoslynVirtualTagEvaluatorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/RoslynVirtualTagEvaluatorTests.cs index c25bbdb9..4fd78970 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/RoslynVirtualTagEvaluatorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/RoslynVirtualTagEvaluatorTests.cs @@ -7,6 +7,7 @@ using Shouldly; using Xunit; using ZB.MOM.WW.OtOpcUa.Commons.Engines; using ZB.MOM.WW.OtOpcUa.Commons.Messages.Logging; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Core.Scripting; using ZB.MOM.WW.OtOpcUa.Host.Engines; @@ -539,6 +540,98 @@ public sealed class RoslynVirtualTagEvaluatorTests $"{starved}/{totalEvaluations} evaluations missed the 5s budget"); } + // ── input quality reaches the script (archreview 02/S14) ──────────────────────────────── + + /// + /// A script reading ctx.GetTag(path).StatusCode sees the dependency's real status. + /// The read cache used to stamp every input Good, so a script could not detect a degraded + /// input at all — "broken input looks Good", the other door onto the same lie 02/S13 closed for + /// broken scripts. + /// + [Fact] + public void Evaluate_read_cache_carries_the_inputs_real_status_code() + { + const uint uncertain = 0x40000000u; + using var sut = new RoslynVirtualTagEvaluator(NullLogger.Instance, NoOpScriptRoot()); + var stamp = new DateTime(2026, 8, 20, 9, 0, 0, DateTimeKind.Utc); + + var result = sut.Evaluate( + virtualTagId: "vt-quality", + expression: "return (long)ctx.GetTag(\"a\").StatusCode;", + dependencies: new Dictionary + { + ["a"] = new(7, uncertain, stamp, stamp), + }); + + result.Success.ShouldBeTrue(result.Reason); + result.Value.ShouldBe((long)uncertain); + } + + /// The dependency's own source timestamp reaches the script, not evaluation time. + [Fact] + public void Evaluate_read_cache_carries_the_inputs_real_source_timestamp() + { + using var sut = new RoslynVirtualTagEvaluator(NullLogger.Instance, NoOpScriptRoot()); + var stamp = new DateTime(2026, 8, 20, 9, 0, 0, DateTimeKind.Utc); + + var result = sut.Evaluate( + virtualTagId: "vt-stamp", + expression: "return ctx.GetTag(\"a\").SourceTimestampUtc;", + dependencies: new Dictionary + { + ["a"] = new(7, 0u, stamp, DateTime.UtcNow), + }); + + result.Success.ShouldBeTrue(result.Reason); + result.Value.ShouldBe(stamp); + } + + /// + /// The value-only overload keeps working unchanged — it is what the interface's default + /// implementation and the five test fakes still use, which is the whole reason the new overload is + /// default-implemented rather than abstract. + /// + [Fact] + public void The_value_only_overload_still_evaluates() + { + using var sut = new RoslynVirtualTagEvaluator(NullLogger.Instance, NoOpScriptRoot()); + + var result = sut.Evaluate( + virtualTagId: "vt-values", + expression: "return (int)ctx.GetTag(\"a\").Value + 1;", + dependencies: new Dictionary { ["a"] = 41 }); + + result.Success.ShouldBeTrue(result.Reason); + result.Value.ShouldBe(42); + } + + /// + /// A fake implementing ONLY the value-only member still answers the snapshot overload, through the + /// interface's default projection. Without that default, adding the overload would have been a + /// breaking change for every existing implementation. + /// + [Fact] + public void The_snapshot_overload_falls_back_to_a_value_only_implementation() + { + IVirtualTagEvaluator fake = new ValueOnlyEvaluator(); + + var result = fake.Evaluate("vt-fake", "ignored", new Dictionary + { + ["a"] = new(41, 0u, DateTime.UtcNow, DateTime.UtcNow), + }); + + result.Success.ShouldBeTrue(); + result.Value.ShouldBe(41); + } + + /// An evaluator predating the snapshot overload: implements the value-only member only. + private sealed class ValueOnlyEvaluator : IVirtualTagEvaluator + { + public VirtualTagEvalResult Evaluate( + string virtualTagId, string expression, IReadOnlyDictionary dependencies) + => VirtualTagEvalResult.Ok(dependencies["a"]); + } + /// Reads the count of the private compiled-script cache via reflection. The cache is a /// CompiledScriptCache<,> which exposes a Count property (not ICollection). private static int CompiledCacheCount(RoslynVirtualTagEvaluator sut)