From a1eed1c2abab3463c371c5f5f51a7d9c1886a67a Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Fri, 19 Jun 2026 01:49:25 -0400
Subject: [PATCH] fix(templates): gate resolver HiLo-merge on both-HiLo to
 match flattener (#262)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Change `mergeEffective` delegate in `ResolveWinners<T>` from `Func<string?, T, string?>`
to `Func<Winner<T>, T, string?>` so the alarm hook can inspect the existing winner's row.
Gate the per-setpoint merge in `ResolveAlarmWinners` on both sides being HiLo
(`existingWinner.Row.TriggerType == HiLo && derived.TriggerType == HiLo`), matching
`FlatteningService.ResolveInheritedAlarms` exactly. Base non-HiLo + derived HiLo now
falls through to whole-replace (derived config verbatim) — the same path the flattener
takes. Preview-only fix; the deploy path is unchanged.

Add test: `Resolve_BaseNonHiLo_DerivedHiLo_DerivedConfigWinsVerbatim` — asserts
resolver and flattener agree when base is ValueMatch and derived overrides to HiLo.
---
 .../TemplateInheritanceResolver.cs            | 23 ++++---
 .../TemplateInheritanceResolverTests.cs       | 60 +++++++++++++++++++
 2 files changed, 74 insertions(+), 9 deletions(-)

diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/TemplateInheritanceResolver.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/TemplateInheritanceResolver.cs
index 013f8114..9e05f106 100644
--- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/TemplateInheritanceResolver.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/TemplateInheritanceResolver.cs
@@ -111,8 +111,10 @@ public static class TemplateInheritanceResolver
     /// effective value when a derived row OVERRIDES an existing winner -- for HiLo
     /// alarms this reuses <see cref="FlatteningService.MergeHiLoConfig"/>
     /// so a partial override (e.g. just <c>hi</c>) inherits the rest of the
-    /// ancestor config, exactly as deploy does. When null, the winning row's own
-    /// value is used verbatim (whole-replace).
+    /// ancestor config, exactly as deploy does. The full existing <see cref="Winner{T}"/>
+    /// is passed so the hook can inspect the inherited row's fields (e.g.
+    /// <c>TriggerType</c>) before deciding whether to merge or whole-replace.
+    /// When null, the winning row's own value is used verbatim (whole-replace).
     /// </para>
     /// </summary>
     private static Dictionary<string, Winner<T>> ResolveWinners<T>(
@@ -123,7 +125,7 @@ public static class TemplateInheritanceResolver
         Func<T, bool> isLocked,
         Func<T, bool> lockedInDerived,
         Func<T, string?> valueOf,
-        Func<string?, T, string?>? mergeEffective = null)
+        Func<Winner<T>, T, string?>? mergeEffective = null)
     {
         var result = new Dictionary<string, Winner<T>>(StringComparer.Ordinal);
 
@@ -158,7 +160,7 @@ public static class TemplateInheritanceResolver
                 // hook a chance to fuse the two effective values (HiLo per-setpoint);
                 // otherwise the overriding row's own value wins (whole-replace).
                 var effective = existing != null && mergeEffective != null
-                    ? mergeEffective(existing.EffectiveValue, row)
+                    ? mergeEffective(existing, row)
                     : valueOf(row);
                 result[name] = new Winner<T>(row, template, baseLocked, effective);
             }
@@ -183,8 +185,11 @@ public static class TemplateInheritanceResolver
         // Winner.EffectiveValue carries the PER-SETPOINT MERGED trigger config so
         // a partial HiLo override (e.g. just `hi`) inherits the rest from the
         // ancestor -- reusing FlatteningService.MergeHiLoConfig so the resolver
-        // preview equals deploy. Other trigger types whole-replace (mergeEffective
-        // returns the derived row's own config unchanged).
+        // preview equals deploy. The merge is gated on BOTH the existing winner
+        // AND the derived row being HiLo, matching FlatteningService.ResolveInheritedAlarms
+        // (lines: alarm.TriggerType == HiLo && existing.TriggerType == nameof(HiLo)).
+        // When only the derived side is HiLo (base is non-HiLo), derived config wins
+        // verbatim — the same whole-replace path the flattener takes in that case.
         ResolveWinners(
             chain,
             t => t.Alarms,
@@ -193,9 +198,9 @@ public static class TemplateInheritanceResolver
             a => a.IsLocked,
             a => a.LockedInDerived,
             a => a.TriggerConfiguration,
-            (existingTrigger, derived) =>
-                derived.TriggerType == AlarmTriggerType.HiLo
-                    ? FlatteningService.MergeHiLoConfig(existingTrigger, derived.TriggerConfiguration)
+            (existingWinner, derived) =>
+                existingWinner.Row.TriggerType == AlarmTriggerType.HiLo && derived.TriggerType == AlarmTriggerType.HiLo
+                    ? FlatteningService.MergeHiLoConfig(existingWinner.EffectiveValue, derived.TriggerConfiguration)
                     : derived.TriggerConfiguration);
 
     private static Dictionary<string, Winner<TemplateScript>> ResolveScriptWinners(
diff --git a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/TemplateInheritanceResolverTests.cs b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/TemplateInheritanceResolverTests.cs
index 443ccf17..be39c767 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/TemplateInheritanceResolverTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/TemplateInheritanceResolverTests.cs
@@ -355,6 +355,66 @@ public class TemplateInheritanceResolverTests
         Assert.Contains("\"hi\":95", resolvedAlarm.EffectiveTriggerConfiguration);
     }
 
+    // ── I1b: base NON-HiLo alarm overridden by a derived HiLo alarm — the resolver
+    //    must NOT attempt the per-setpoint merge (guard matches the flattener's
+    //    both-HiLo gate). Derived config must win verbatim (whole-replace). ──
+
+    [Fact]
+    public void Resolve_BaseNonHiLo_DerivedHiLo_DerivedConfigWinsVerbatim()
+    {
+        // Base uses a non-HiLo trigger type (ValueMatch, JSON-object config).
+        // Derived overrides to HiLo. The resolver must take the same path the
+        // flattener takes: derived config wins verbatim (no per-setpoint merge).
+        var baseT = new Template("Base") { Id = 1 };
+        baseT.Alarms.Add(new TemplateAlarm("Level")
+        {
+            Id = 10,
+            TemplateId = 1,
+            PriorityLevel = 5,
+            TriggerType = AlarmTriggerType.ValueMatch,
+            TriggerConfiguration = "{\"value\":42}"
+        });
+
+        var derived = new Template("Derived") { Id = 2, ParentTemplateId = 1 };
+        derived.Alarms.Add(new TemplateAlarm("Level")
+        {
+            Id = 20,
+            TemplateId = 2,
+            PriorityLevel = 5,
+            TriggerType = AlarmTriggerType.HiLo,
+            TriggerConfiguration = "{\"hi\":90,\"lo\":10}",
+            IsInherited = false
+        });
+
+        var all = new[] { baseT, derived };
+
+        // Resolver view.
+        var resolved = TemplateInheritanceResolver.Resolve(2, all);
+        var resolvedAlarm = Assert.Single(resolved.Alarms, m => m.Name == "Level");
+
+        // Flattener view: no merge expected (base non-HiLo), derived config verbatim.
+        var instance = new Instance("inst") { Id = 1, TemplateId = 2, SiteId = 1 };
+        var chain = new List<Template> { derived, baseT }; // most-derived first
+        var flat = new FlatteningService().Flatten(
+            instance,
+            chain,
+            new Dictionary<int, IReadOnlyList<TemplateComposition>>(),
+            new Dictionary<int, IReadOnlyList<Template>>(),
+            new Dictionary<int, DataConnection>());
+
+        Assert.True(flat.IsSuccess, flat.IsFailure ? flat.Error : null);
+        var flatAlarm = Assert.Single(flat.Value.Alarms, a => a.CanonicalName == "Level");
+
+        // The resolver's effective TriggerConfiguration MUST equal the flattener's
+        // result — derived config verbatim, not a merge of base-keys + derived-keys.
+        Assert.Equal(flatAlarm.TriggerConfiguration, resolvedAlarm.EffectiveTriggerConfiguration);
+
+        // Sanity: the derived config came through unchanged; the base config key
+        // ("value") must NOT appear (it would if the merge hook fired incorrectly).
+        Assert.Equal("{\"hi\":90,\"lo\":10}", resolvedAlarm.EffectiveTriggerConfiguration);
+        Assert.DoesNotContain("\"value\"", resolvedAlarm.EffectiveTriggerConfiguration ?? "");
+    }
+
     // ── I2: staleness must flag drift on the fields that actually matter, not
     //    just one scalar per type. ──