Require a vanilla declaration kept by either side to survive the merge

WitcherScriptMerger.Core/CLAUDE.md

@@ -523,6 +523,31 @@ defects the original design produced on a live install):
   against the real broken output preserved in `docs/bugs/artifacts/` (gate fails it on
   exactly the orphaned accessor the game rejected) with zero false positives across
   real vanilla `r4Player.ws`/`player.ws`/`actor.ws`/`baseEffect.ws`.
+- **A vanilla declaration kept by either side must survive the whole-file merge**
+  (`ValidateWholeFileMergeOutput`'s lost-unit check). This extends the function-level
+  engine's own long-standing "a deletion never silently overrides a surviving edit"
+  principle (above) to the whole-file path, which did not previously enforce it: a unit
+  present in vanilla, kept by one input and absent from the other used to be allowed to
+  vanish, on the reading that it was "a legitimate deletion propagating". From text alone
+  that shape is **indistinguishable** from the case that actually causes damage — a mod
+  shipping a whole-file copy taken from an older game build simply has no copy of
+  declarations vanilla added since, and a three-way diff reads that absence as a deletion.
+  Observed live on a next-gen install: a pre-4.0 `r4Game.ws` erased
+  `CR4Game.OnHDRChangedEvent` (engine-called, calls `GetGuiManager().OnHDRChanged()`) from
+  the merged output; the merge reported success, the scripts compiled, and the game
+  rendered its menu background with **no main menu at all**. Two other mods in the same
+  load order did the same to `mapMenu.ws` (`OnFiltersChanged`, `SetInitialFilters`,
+  `m_fxSetInitialFilters`) and `exploration.ws` (`CheckVector`, `DoHorseKick`,
+  `OnHorseKick` plus five member variables) — in every case the declaration was present in
+  vanilla *and* in the other contributing mod. Since the two cases can't be told apart,
+  this errs toward the survivable one: the deliberate-deletion mod loses its deletion,
+  rather than an engine-called vanilla event disappearing with nothing saying so. A
+  violation doesn't skip the file — it routes to the function-level rescue first, like
+  every other violation, and that engine's own edit-survives-competing-deletion policy
+  usually keeps the unit. `ValidateWholeFileMergeOutput` takes optional
+  `oldDescription`/`newDescription` purely so the message can **name the mod that lacks
+  the declaration**, since the fix is almost always "that mod is built for an older game
+  version". Only a name *both* sides dropped may still disappear.
 - A gap that still exists between two intact units is compared as before —
   whitespace-tolerant, deliberately NOT comment-stripped — producing a `Decisions` note
   when content differs; vanilla's gap text still wins at non-insertion slots.

WitcherScriptMerger.Core/Tools/DiffPlexMergeEngine.cs

@@ -268,7 +268,8 @@ public MergeEngineResult MergeHeadless(
 				// from the ORIGINAL inputs, sidestepping the corrupted output
 				// entirely), then fall through to the conflict-marker sidecar.
 				if (FunctionLevelMergeEngine.ValidateWholeFileMergeOutput(
-					baseText, oldText, newText, result.MergedText, outputPath, out var invariantViolation))
+					baseText, oldText, newText, result.MergedText, outputPath, out var invariantViolation,
+					oldDescription ?? source1.Name, newDescription ?? source2.Name))
 				{
 					// A prior attempt at this same conflict may have left a sidecar
 					// marker file behind (see below) - if this attempt now auto-solves

WitcherScriptMerger.Core/Tools/FunctionLevelMergeEngine.cs

@@ -702,8 +702,25 @@ int At(Dictionary<string, int> counts, string name) =>
 		// Inputs that don't extract cleanly make the judgement impossible - returns
 		// true (trust the merge, as before this guard existed) - EXCEPT when the inputs
 		// extract and the OUTPUT doesn't, which is itself corruption.
+		// Names the side a violation message is talking about. Falls back to "one mod"/"the
+		// other mod" when the caller supplied no descriptions (tests, and any future caller
+		// that has none) - the message stays true, just less specific.
+		static string DescribeSide(bool isOldSide, string oldDescription, string newDescription)
+		{
+			var description = isOldSide ? oldDescription : newDescription;
+			if (!string.IsNullOrWhiteSpace(description))
+				return description;
+			return isOldSide ? "one mod" : "the other mod";
+		}
+
+		// oldDescription/newDescription are optional purely so the violation text can name
+		// the mod that lacks a vanilla declaration - the single most useful thing to tell a
+		// user here, since the fix is almost always "that mod is built for an older game
+		// version". Optional rather than required so existing callers and tests are
+		// unaffected.
 		public static bool ValidateWholeFileMergeOutput(
-			string baseText, string oldText, string newText, string mergedText, string outputPath, out string violation)
+			string baseText, string oldText, string newText, string mergedText, string outputPath, out string violation,
+			string oldDescription = null, string newDescription = null)
 		{
 			violation = null;
 			if (!FileIndex.ModFile.IsScript(outputPath))
@@ -764,17 +781,50 @@ public static bool ValidateWholeFileMergeOutput(
 				}
 			}
 
+			// Every unit present in EITHER input must survive. Only a name both sides
+			// dropped may legitimately disappear - and such a name never reaches this loop,
+			// since it iterates the two inputs' own keys.
+			//
+			// The vanilla-retention case (in base, kept by one side, absent from the other)
+			// used to be excluded here, and that exclusion caused real, silent damage. A mod
+			// shipping a whole-file copy taken from an OLDER game build simply doesn't
+			// contain declarations the current vanilla added since - which a three-way diff
+			// cannot tell apart from "this mod deleted it", so the deletion won and the
+			// merged file shipped without it. Observed live on a next-gen install: a
+			// pre-4.0 copy of r4Game.ws erased `CR4Game.OnHDRChangedEvent` (an
+			// engine-called event that calls GetGuiManager().OnHDRChanged()) from the merged
+			// output; the merge reported success, the scripts compiled, and the game then
+			// rendered its menu background with no main menu at all. Two more mods did the
+			// same thing in the same load order - mapMenu.ws lost OnFiltersChanged /
+			// SetInitialFilters / m_fxSetInitialFilters, exploration.ws lost CheckVector /
+			// DoHorseKick / OnHorseKick plus five member variables. In every case the
+			// declaration was present in vanilla AND in the other contributing mod.
+			//
+			// Treating this as a violation deliberately reclassifies a genuine
+			// "one mod deletes a vanilla function, the other keeps it" case as a conflict
+			// rather than a silent deletion. That is the right trade: whole-function
+			// deletion by a mod is rare, silently losing engine-called vanilla code is
+			// catastrophic and near-impossible to diagnose from the symptom, and a violation
+			// here doesn't skip the file outright - it routes to the function-level rescue
+			// first, exactly like every other violation.
 			foreach (var name in oldCounts.Keys.Concat(newCounts.Keys).Distinct())
 			{
 				var inOld = At(oldCounts, name) > 0;
 				var inNew = At(newCounts, name) > 0;
 				var inBase = At(baseCounts, name) > 0;
-				var required = (inOld && inNew) || (inOld && !inBase) || (inNew && !inBase);
-				if (required && At(mergedCounts, name) == 0)
-				{
-					violation = $"'{name}' is present in {(inOld && inNew ? "both inputs" : "an input that inserted it")} but missing from the merged output (lost)";
-					return false;
-				}
+				if (At(mergedCounts, name) != 0)
+					continue;
+
+				if (inOld && inNew)
+					violation = $"'{name}' is present in both inputs but missing from the merged output (lost)";
+				else if (!inBase)
+					violation = $"'{name}' is present in an input that inserted it but missing from the merged output (lost)";
+				else
+					violation =
+						$"'{name}' is declared in the vanilla file and kept by {DescribeSide(inOld, oldDescription, newDescription)}, " +
+						$"but is missing from the merged output (lost) - {DescribeSide(!inOld, oldDescription, newDescription)} has no copy of it, " +
+						"which usually means that mod ships a whole-file copy taken from an older game build";
+				return false;
 			}
 
 			// Line-level: DiffPlex's silent duplication can strike INSIDE a function

WitcherScriptMerger.Tests/CLAUDE.md

@@ -32,7 +32,11 @@ does.
   most-distinct-from-vanilla tiebreak (including its deterministic tie-break and a
   scaled-down `DiffAlgorithmException` case), edit-survives-competing-deletion, insertion
   reconciliation (including the same-name-different-body decline case), and gap-comment
-  detection.
+  detection. Also `ValidateWholeFileMergeOutput`'s lost-unit check, including the
+  deliberately reversed expectation for a vanilla declaration kept by one side and
+  absent from the other (now a violation, not "a legitimate deletion propagating" -
+  see Core's `CLAUDE.md`), that a name dropped by *both* sides is still allowed to go,
+  and that the violation message names the stale mod on whichever side it is.
 - `Inventory/FileMergerTests.cs` — `FileMerger.IsVanillaDlcBundleFolder`: known vanilla
   DLC-folder names, case-insensitivity, and non-matches (including anchoring) — see
   Core's `CLAUDE.md`'s "Vortex-fork parity fixes" section. Also covers the two-arg

WitcherScriptMerger.Tests/Tools/FunctionLevelMergeEngineTests.cs

@@ -484,7 +484,7 @@
 			var result = Merge(baseText, oldText, newText);
 
 			Assert.True(result.Applied);
 			Assert.Equal(1, System.Text.RegularExpressions.Regex.Matches(result.MergedText, @"function NEW\(").Count);
 			Assert.True(FunctionLevelMergeEngine.PassesReassemblySanityGate(result.MergedText, out _));
 		}
 
@@ -501,7 +501,7 @@
 			var result = Merge(baseText, oldText, newText);
 
 			Assert.True(result.Applied);
 			Assert.Equal(1, System.Text.RegularExpressions.Regex.Matches(result.MergedText, @"function NEW\(").Count);
 			Assert.Contains("\tn();\r\n}", result.MergedText);
 		}
 
@@ -530,7 +530,7 @@
 			var result = Merge(baseText, oldText, baseText);
 
 			Assert.True(result.Applied);
 			Assert.Equal(1, System.Text.RegularExpressions.Regex.Matches(result.MergedText, @"function NEW\(").Count);
 		}
 
 		[Fact]
@@ -554,17 +554,101 @@
 			Assert.Contains("lost", violation);
 		}
 
-		[Fact]
-		public void ValidateWholeFileMergeOutput_AllowsALegitimateDeletionPropagating()
+		// DELIBERATE REVERSAL of this test's previous expectation, which asserted that a
+		// vanilla declaration present in one input and absent from the other could legitimately
+		// vanish from the merged output ("a legitimate deletion propagating").
+		//
+		// That shape is indistinguishable, from text alone, from the failure that motivated this
+		// change: a mod shipping a whole-file copy taken from an OLDER game build has no copy of
+		// declarations vanilla added since, and a three-way diff reads that absence as a
+		// deletion. Observed live on a next-gen install - a pre-4.0 r4Game.ws erased
+		// CR4Game.OnHDRChangedEvent (engine-called, calls GetGuiManager().OnHDRChanged()) from
+		// the merged output; the merge reported success, the scripts compiled, and the game
+		// rendered its menu background with no main menu. Two other mods in the same load order
+		// did the same to mapMenu.ws and exploration.ws.
+		//
+		// Since the two cases cannot be told apart, this errs toward the survivable one. A
+		// violation does not skip the file - it routes to the function-level rescue first, which
+		// re-merges per unit and has its own edit-survives-competing-deletion policy. The cost is
+		// that a deliberate whole-function deletion no longer propagates silently; the benefit is
+		// that engine-called vanilla code can no longer disappear with nothing saying so.
+		[Fact]
+		public void ValidateWholeFileMergeOutput_VanillaDeclarationKeptByOneSideButLost_IsAViolation()
 		{
 			var baseText = Fn("A", "\ta();\r\n") + "\r\n" + Fn("B", "\tb();\r\n");
 			var oldText = baseText;
 			var newText = Fn("A", "\ta();\r\n");
 			var mergedText = Fn("A", "\ta();\r\n");
 
+			Assert.False(FunctionLevelMergeEngine.ValidateWholeFileMergeOutput(baseText, oldText, newText, mergedText, "x.ws", out var violation));
+			Assert.Contains("lost", violation);
+			Assert.Contains("older game build", violation);
+		}
+
+		// The one loss that stays legitimate: neither input has it any more, so nothing is being
+		// discarded. Such a name never even reaches the check, since it iterates the two inputs'
+		// own keys - this pins that down.
+		[Fact]
+		public void ValidateWholeFileMergeOutput_VanillaDeclarationDroppedByBothSides_IsAllowed()
+		{
+			var baseText = Fn("A", "\ta();\r\n") + "\r\n" + Fn("B", "\tb();\r\n");
+			var oldText = Fn("A", "\ta();\r\n");
+			var newText = Fn("A", "\ta();\r\n");
+			var mergedText = Fn("A", "\ta();\r\n");
+
 			Assert.True(FunctionLevelMergeEngine.ValidateWholeFileMergeOutput(baseText, oldText, newText, mergedText, "x.ws", out _));
 		}
 
+		// The most useful thing this message can carry is WHICH mod lacks the declaration,
+		// because the fix is almost always "that mod is built for an older game version".
+		[Fact]
+		public void ValidateWholeFileMergeOutput_NamesTheModThatLacksTheVanillaDeclaration()
+		{
+			var baseText = Fn("A", "\ta();\r\n") + "\r\n" + Fn("B", "\tb();\r\n");
+			var oldText = baseText;
+			var newText = Fn("A", "\ta();\r\n");
+			var mergedText = Fn("A", "\ta();\r\n");
+
+			Assert.False(FunctionLevelMergeEngine.ValidateWholeFileMergeOutput(
+				baseText, oldText, newText, mergedText, "x.ws", out var violation, "modKeepsIt", "modStale"));
+
+			Assert.Contains("kept by modKeepsIt", violation);
+			Assert.Contains("modStale has no copy of it", violation);
+		}
+
+		// Same case with the stale side reversed, so the message cannot be passing by hardcoding
+		// one side.
+		[Fact]
+		public void ValidateWholeFileMergeOutput_NamesTheStaleModWhicheverSideItIs()
+		{
+			var baseText = Fn("A", "\ta();\r\n") + "\r\n" + Fn("B", "\tb();\r\n");
+			var oldText = Fn("A", "\ta();\r\n");
+			var newText = baseText;
+			var mergedText = Fn("A", "\ta();\r\n");
+
+			Assert.False(FunctionLevelMergeEngine.ValidateWholeFileMergeOutput(
+				baseText, oldText, newText, mergedText, "x.ws", out var violation, "modStale", "modKeepsIt"));
+
+			Assert.Contains("kept by modKeepsIt", violation);
+			Assert.Contains("modStale has no copy of it", violation);
+		}
+
+		// Descriptions are optional - without them the message must still be true, just less
+		// specific.
+		[Fact]
+		public void ValidateWholeFileMergeOutput_WithoutDescriptions_StillDescribesBothSides()
+		{
+			var baseText = Fn("A", "\ta();\r\n") + "\r\n" + Fn("B", "\tb();\r\n");
+			var oldText = baseText;
+			var newText = Fn("A", "\ta();\r\n");
+			var mergedText = Fn("A", "\ta();\r\n");
+
+			Assert.False(FunctionLevelMergeEngine.ValidateWholeFileMergeOutput(baseText, oldText, newText, mergedText, "x.ws", out var violation));
+
+			Assert.Contains("kept by one mod", violation);
+			Assert.Contains("the other mod has no copy of it", violation);
+		}
+
 		[Fact]
 		public void ValidateWholeFileMergeOutput_NonScriptFiles_AlwaysTrusted()
 		{