fix(templates): gate resolver HiLo-merge on both-HiLo to match flattener (#262)

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.
This commit is contained in:
Joseph Doherty
2026-06-19 01:49:25 -04:00
parent 5585d7ba51
commit a1eed1c2ab
2 changed files with 74 additions and 9 deletions
@@ -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(
@@ -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. ──