fix(aggregates): live TimeAverage/Total leave the span before an Uncertain first value out (#821)

When the data starts inside an interval (no start bound) and its first value is Uncertain, the frozen 1.5.378.182 AverageAggregateCalculator integrated the uncovered span as a ramp from 0 and divided TimeAverage by the whole interval: GetRegionsInValueSet (ignoreBadData, sloped) overwrites the BadNoData start region's status with UncertainDataSubNormal in its one branch without a not-Bad guard. Live AggregateFilter therefore disagreed with HistoryRead and Part 13 §5.4.3.6/§5.4.3.8 ("No Start Bound: Calculate Partial Interval"); QP: Total 188.96 vs 163.96, TimeAverage 9.448 vs 10.931. SDK 2.0.1 stock does the same. Owner-approved 2026-11-11. - New app-owned CoveredTimeAverageAggregateCalculator subclasses the baseline and recomputes only TimeAverage/Total, restoring the Bad status of a region that starts at a Bad point (only the start bound can). Only such an interval's value changes; status, timestamps, type and bits are the baseline's. Stepped and TreatUncertainAsBad=true never reached the defect. CompatibilityAggregate- Factory routes exactly those two IDs; copied SDK files untouched. - Golden test: 0 fixture vectors change (every data set has a usable value at -10 s), asserted. New sweep over late-start variants of every data set (cut +5 s / +25 s, first non-Bad sample Uncertain, all directions/stepping/configs/ incremental): 150 / 142 of 384 change, each pinned to a baseline-derived oracle (untouched baseline's value with that sample Good, its status/bits on the variant itself) - never our calculator. Plus a hand-computed case. - Live-vs-HistoryRead parity (TimeWeightedAggregator) forward + reverse, no masks, with discrimination against the pre-#821 baseline; QualityMix now compares QP's TimeAverage/Total with the live oracle on both SQL backends; booted-runtime facts test expects 163.96 / 10.93. - README recorded decision, ServerAggregateConfiguration remarks, oracle remarks, docs/Historian.md and .claude/rules/historian.md updated. Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk
2026-11-11 12:30:01 -05:00
parent 233e4bab25
commit 8a0032bfdf
11 changed files with 398 additions and 53 deletions
@@ -8,14 +8,18 @@ internal static class CompatibilityAggregateFactory
 {
    /// <remarks>
    ///     PercentGood / DurationGood are served by the application-owned <see cref="GoodOnlyStatusAggregateCalculator"/>
-    ///     (#809: Uncertain time is not Good, as in Part 13 and HistoryRead); every other id is the unchanged baseline.
+    ///     (#809: Uncertain time is not Good, as in Part 13 and HistoryRead), TimeAverage / Total by
+    ///     <see cref="CoveredTimeAverageAggregateCalculator"/> (#821: no start bound ⇒ only the covered part of the interval,
+    ///     even when the first value is Uncertain); every other id is the unchanged baseline.
    /// </remarks>
    internal static IAggregateCalculator Create(NodeId id, DateTimeUtc start, DateTimeUtc end, double interval,
        bool stepped, AggregateConfiguration configuration, ITelemetryContext telemetry) =>
        new Adapter(GoodOnlyStatusAggregateCalculator.Overrides(id)
            ? new GoodOnlyStatusAggregateCalculator(id, (DateTime)start, (DateTime)end, interval, stepped, configuration, telemetry)
-            : Aggregators.CreateStandardCalculator(id, (DateTime)start, (DateTime)end, interval, stepped, configuration, telemetry)
-              ?? throw new ArgumentOutOfRangeException(nameof(id), "Unsupported baseline aggregate."));
+            : CoveredTimeAverageAggregateCalculator.Overrides(id)
+                ? new CoveredTimeAverageAggregateCalculator(id, (DateTime)start, (DateTime)end, interval, stepped, configuration, telemetry)
+                : Aggregators.CreateStandardCalculator(id, (DateTime)start, (DateTime)end, interval, stepped, configuration, telemetry)
+                  ?? throw new ArgumentOutOfRangeException(nameof(id), "Unsupported baseline aggregate."));

    /// <summary>Replaces exactly the existing standard factories; no aggregate IDs are added or removed.</summary>
    internal static async ValueTask RegisterAsync(AggregateManager manager, CancellationToken ct)
@@ -0,0 +1,115 @@
+using Opc.Ua;
+using DataValue = ZB.MOM.WW.OtOpcUa.OpcUaServer.Compatibility.Aggregates.LegacyDataValue;
+
+namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Compatibility.Aggregates;
+
+/// <summary>
+///     Application-owned override (#821) of the baseline <see cref="AverageAggregateCalculator"/> for <c>TimeAverage</c>
+///     and <c>Total</c> only: the time before the first usable value (no start bound) is never integrated, whatever the
+///     quality of that first value. Part 13 §5.4.3.6/§5.4.3.8 "No Start Bound: Calculate Partial Interval" — the rule the
+///     HistoryRead evaluator (<c>TimeWeightedAggregator</c>, SqlServer + QuestDB) already follows, and the one the baseline
+///     itself follows when the first value is Good.
+/// </summary>
+/// <remarks>
+///     <para>
+///         With no usable value before the interval, the baseline's interpolated start bound is <c>BadNoData</c> (value 0),
+///         so the first region <c>[start, first value)</c> is Bad and excluded. But <c>GetRegionsInValueSet</c>
+///         (ignoreBadData, sloped) marks the region before an Uncertain value <c>UncertainDataSubNormal</c> without the
+///         not-Bad guard every other branch has — so an <b>Uncertain</b> first value turned the Bad region non-Bad and the
+///         uncovered span was integrated as a ramp from 0 (QP: Total 188.96 for Part 13's 163.96, TimeAverage 9.448 for
+///         10.931). SDK 2.0.1's stock calculator does the same.
+///     </para>
+///     <para>
+///         This class leaves the copied file untouched and restores exactly that one overwritten status: a region that
+///         starts at a Bad point (only the interpolated start bound can — the region machinery skips Bad raw values)
+///         keeps that point's Bad status. Every other region, the values, the result status (Uncertain either way: the
+///         region is non-Good), timestamp, value type and Calculated / Partial bits are the baseline's.
+///         <c>TimeAverage2</c> / <c>Total2</c> (simple bounds, no ignoreBadData) and stepped variables never reach that
+///         branch and fall through to the baseline unchanged.
+///     </para>
+/// </remarks>
+internal sealed class CoveredTimeAverageAggregateCalculator : AverageAggregateCalculator
+{
+    /// <summary>True for the two aggregate ids this class overrides.</summary>
+    internal static bool Overrides(NodeId aggregateId) =>
+        aggregateId == ObjectIds.AggregateFunction_TimeAverage || aggregateId == ObjectIds.AggregateFunction_Total;
+
+    /// <summary>Initializes the calculator; same contract as the baseline constructor.</summary>
+    public CoveredTimeAverageAggregateCalculator(
+        NodeId aggregateId,
+        DateTime startTime,
+        DateTime endTime,
+        double processingInterval,
+        bool stepped,
+        AggregateConfiguration configuration,
+        ITelemetryContext? telemetry)
+        : base(aggregateId, startTime, endTime, processingInterval, stepped, configuration, telemetry)
+    {
+    }
+
+    /// <inheritdoc />
+    protected override DataValue ComputeValue(TimeSlice slice)
+    {
+        if (AggregateId == ObjectIds.AggregateFunction_TimeAverage) return ComputeCovered(slice, total: false);
+        if (AggregateId == ObjectIds.AggregateFunction_Total) return ComputeCovered(slice, total: true);
+        return base.ComputeValue(slice);
+    }
+
+    /// <summary>
+    ///     The baseline <c>ComputeTimeAverage(slice, useSimpleBounds: false, valueType)</c> with a region that starts at a
+    ///     Bad point kept Bad.
+    /// </summary>
+    private DataValue ComputeCovered(TimeSlice slice, bool total)
+    {
+        List<DataValue>? values = GetValuesWithInterpolatedBounds(slice);
+        if (values == null || values.Count == 0)
+        {
+            return GetNoDataValue(slice);
+        }
+
+        List<SubRegion> regions = GetRegionsInValueSet(values, true, Stepped);
+
+        double area = 0;
+        double totalDuration = 0;
+        bool nonGoodRegionsExists = false;
+
+        foreach (var region in regions)
+        {
+            // #821: the one change — undo the baseline's Bad → Uncertain overwrite of a region starting at a Bad point.
+            if (StatusCode.IsBad(region.DataPoint.StatusCode) && StatusCode.IsNotBad(region.StatusCode))
+            {
+                region.StatusCode = region.DataPoint.StatusCode;
+            }
+
+            double duration = region.Duration / 1000.0;
+
+            if (StatusCode.IsNotBad(region.StatusCode))
+            {
+                area += (region.StartValue + region.EndValue) * duration / 2;
+                totalDuration += duration;
+            }
+
+            if (StatusCode.IsNotGood(region.StatusCode))
+            {
+                nonGoodRegionsExists = true;
+            }
+        }
+
+        if (totalDuration == 0)
+        {
+            return GetNoDataValue(slice);
+        }
+
+        double result = total ? area : area / totalDuration;
+
+        var value = new DataValue
+        {
+            WrappedValue = SdkValueBoundary.FromClr(result),
+            SourceTimestamp = GetTimestamp(slice),
+            ServerTimestamp = GetTimestamp(slice),
+            StatusCode = nonGoodRegionsExists ? StatusCodes.UncertainDataSubNormal : StatusCodes.Good
+        };
+        value.StatusCode = value.StatusCode.WithAggregateBits(AggregateBits.Calculated);
+        return value;
+    }
+}
@@ -28,3 +28,12 @@ The baseline `StatusAggregateCalculator.ComputeDurationGoodBad` counts Uncertain
 - `GoodOnlyStatusAggregateCalculator.cs` (application code, not a baseline copy) subclasses the baseline `StatusAggregateCalculator` and replaces only the PercentGood / DurationGood computation: the same slice values, simple bounds, stepped regions, empty-slice `BadNoData`, timestamps, value type and Calculated bit, with a region counted as Good only when its status is Good — whatever `TreatUncertainAsBad` says (when true, Uncertain is Bad, so it was never Good). `CompatibilityAggregateFactory.Create` routes exactly those two IDs to it; the other 35 IDs, including DurationBad / PercentBad / WorstQuality, are the baseline unchanged.
 - Golden test: the 64 vectors this changes (38 PercentGood + 26 DurationGood, all with an effective `TreatUncertainAsBad = false` over data with Uncertain regions) are exempted **exactly** — the PercentGood / DurationGood vectors whose baseline output differs from the same case's baseline vector with `TreatUncertainAsBad = true`, where 1.5.378.182 already counted Good regions only — and each is pinned to that twin's output. The pin is therefore baseline evidence, never regenerated from the application's calculator; any further drift in any of the 6,144 cases fails. `Sdk2AggregateCompatibilityTests.Issue809_*` also pins a hand-computed case.
 - Live-vs-history parity: `SdkAggregateParityTests.Live_AggregateFilter_PercentGood_and_DurationGood_equal_HistoryRead_over_Uncertain_samples` compares the registered calculator with `TimeWeightedAggregator` over Good / Uncertain / Bad mixes with no masks, and checks that the data discriminates against the pre-#809 baseline.
+
+### #821 — TimeAverage / Total integrate only the covered part when there is no start bound (owner-approved 2026-11-11)
+
+When the data starts inside an interval (no usable value before it) the baseline's interpolated start bound is `BadNoData`, so the region before the first value is Bad and left out — Part 13 §5.4.3.6/§5.4.3.8 "No Start Bound: Calculate Partial Interval", and what the HistoryRead evaluator (`TimeWeightedAggregator`, SqlServer + QuestDB) does. But when that first value is **Uncertain**, `AggregateCalculator.GetRegionsInValueSet` (ignoreBadData, sloped) overwrites the region's Bad status with `UncertainDataSubNormal` (the one branch without the not-Bad guard), so `AverageAggregateCalculator` integrated the uncovered span as a ramp from 0 and divided TimeAverage by the whole interval (QP: Total 188.96 for 163.96, TimeAverage 9.448 for 10.931). SDK 2.0.1's stock calculator does the same. ScadaBridge uses neither AggregateFilter nor these aggregates.
+
+- The copied upstream files above are **unchanged** (their SHA-256s still match `baseline-sources.json`).
+- `CoveredTimeAverageAggregateCalculator.cs` (application code) subclasses the baseline `AverageAggregateCalculator` and recomputes only TimeAverage / Total: the baseline's interpolated-bounds values and regions, with a region that starts at a Bad point (only the start bound can) kept Bad. Values change only for such an interval; status (Uncertain either way, except `BadNoData` when nothing at all is covered), timestamps, value type and Calculated / Partial bits are the baseline's. Stepped variables and `TreatUncertainAsBad = true` never reached the defect and are unchanged. `CompatibilityAggregateFactory.Create` routes exactly those two IDs to it; TimeAverage2 / Total2 and the other 31 IDs are the baseline unchanged.
+- Golden test: **no** fixture vector changes — every fixture data set has a usable value at −10 s, before every window, so no fixture interval lacks a start bound; all 384 TimeAverage / Total vectors stay held to their own output (`Issue821FixtureExemptions = 0`). The change is pinned over late-start variants of every data set (cut at +5 s and +25 s, first remaining non-Bad sample made Uncertain, every direction / stepping / configuration / incremental mode): 150 and 142 of the 384 change, each held to a baseline-derived oracle — the untouched baseline's value on the same variant with that sample Good, its status / timestamps / bits on the variant itself. `Sdk2AggregateCompatibilityTests.Issue821_*` also pins a hand-computed case.
+- Live-vs-history parity: `SdkAggregateParityTests.Live_AggregateFilter_TimeAverage_and_Total_equal_HistoryRead_over_an_Uncertain_first_late_start` compares the registered calculator with `TimeWeightedAggregator`, forward and reverse, with no masks, and checks that the data discriminates against the pre-#821 baseline; `QualityMixConformance` now includes TimeAverage / Total over QP on both SQL backends.
@@ -31,8 +31,12 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer;
 ///         (owner-approved 2026-11-11) overrides exactly those two (<c>GoodOnlyStatusAggregateCalculator</c>), so live and
 ///         HistoryRead now agree on them — pinned by
 ///         <c>SdkAggregateParityTests.Live_AggregateFilter_PercentGood_and_DurationGood_equal_HistoryRead_over_Uncertain_samples</c>.
-///         Other known live-vs-HistoryRead differences (e.g. #821, an Uncertain first sample in a late-start interval) are
-///         documented in <c>docs/Historian.md</c>.
+///         Likewise the frozen <c>AverageAggregateCalculator</c> integrated the span before an <b>Uncertain</b> first value
+///         as a ramp from 0 when the data starts inside an interval (no start bound) — live TimeAverage / Total disagreed
+///         with HistoryRead and Part 13 ("No Start Bound: Calculate Partial Interval"). #821 (owner-approved 2026-11-11)
+///         overrides exactly those two (<c>CoveredTimeAverageAggregateCalculator</c>) — pinned by
+///         <c>SdkAggregateParityTests.Live_AggregateFilter_TimeAverage_and_Total_equal_HistoryRead_over_an_Uncertain_first_late_start</c>.
+///         The remaining documented live-vs-HistoryRead differences are listed in <c>docs/Historian.md</c>.
 ///     </para>
 ///     <para>
 ///         A request asking for <c>UseServerCapabilitiesDefaults</c>, or spelling out exactly these values, is served;