Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 84 additions & 1 deletion WitcherScriptMerger.Core/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ still external dependencies rather than an in-process replacement.
(UTF-16LE+BOM normalization — see "Text-merge input encoding" below),
`DiffPlexMergeEngine.cs` (see below), `FileOpener.cs` (portable "open in the OS's
default associated app" helper; the only call site is
`DiffPlexMergeEngine.MergeHeadless`, opening a genuine conflict's marker sidecar).
`DiffPlexMergeEngine.MergeHeadless`, opening a genuine conflict's marker sidecar),
`ScriptUnitExtractor.cs`/`UnitAligner.cs`/`FunctionLevelMergeEngine.cs` (the
function-level merge fallback — see "Function-level merge engine" below).
- `Cli/` — `MergeOperations.cs`: the scan-then-merge sequence shared by both hosts'
`merge` CLI verb and by the MCP tools (see "CLI & MCP orchestration" below).
- `Mcp/` — `WsmMcpTools.cs`: the MCP server's tool implementations (see below and
Expand Down Expand Up @@ -375,6 +377,87 @@ switching chunkers** — the bug reproduces under DiffPlex's own default/tested
`LineChunker` too, just at a somewhat lower rate, so switching would trade a real,
working byte-for-byte line-ending-preservation property for no actual safety gain.

## Function-level merge engine

`Tools/ScriptUnitExtractor.cs`, `Tools/UnitAligner.cs`, and
`Tools/FunctionLevelMergeEngine.cs` are a **fallback**, activated only from inside
`DiffPlexMergeEngine.MergeHeadless` at the two points where the whole-file merge has
already failed for a given pairwise chain step (the `DiffAlgorithmException` catch and
the `HasConflicts` branch) — never a parallel code path, so every conflict that already
auto-solves via the whole-file engine is unaffected. The idea: split vanilla and both
sides of a pairwise merge into individual function/field units, resolve each
independently, then reassemble — most of a real `.ws` file's line-level "conflict"
surface comes from whitespace/comment noise around a handful of actually-edited
functions, not from genuine overlapping logic changes, and per-function merging sidesteps
that noise entirely (and, as a side effect, mitigates `DiffAlgorithmException`'s
worse-at-small-inputs failure rate above by attempting DiffPlex's inline 3-way merge at
function granularity only when both sides changed a given function differently, not on
the whole file at once).

**Validated empirically before being built, not just unit-tested.** A throwaway
measurement against a real, live Witcher 3 install's `actor.ws` conflict (vanilla +
6 real overhaul mods) found only 6 of 395 functions were genuine two-mod collisions
(both sides edit the same function differently) — confirming per-function decomposition
was worth building rather than just relocating the same conflict into a smaller,
statistically more `DiffAlgorithmException`-prone box. A follow-up chain-step replay
against the same real install's 5 currently-unresolved conflicts found that a
naively insertion-only alignment (tolerating a mod adding new functions, but declining
outright the moment any mod deleted a vanilla function) would rescue only 2 of the 5 —
one real mod in that install deletes several vanilla functions outright, and that
deletion was found to persist through the merge chain into later steps even where the
deleting mod isn't a direct input, since an earlier *clean* whole-file merge step
faithfully propagates a one-sided deletion into the accumulated text. `UnitAligner`
handles insertions and deletions symmetrically (an LCS alignment of each side's unit
names against vanilla's) for this reason; final validation against the same 5 real
files, run against an isolated copy (never the live install directly), got all 5 to
merge successfully, including `actor.ws` itself.

**Function identity is name-only** (`(scope-free) name`, no parameter signature) —
confirmed empirically against several large real vanilla files (`actor.ws`, `player.ws`,
`npc.ws`) that WitcherScript function names don't collide within a file in practice, so
overload-aware identity wasn't needed.

**Extraction (`ScriptUnitExtractor`)** is a brace/paren-matching tokenizer, not a full
parser or a binding to the third-party `tree-sitter-witcherscript` grammar — confirmed
via direct research into WitcherScript's grammar that class/state/struct/enum
declarations are top-level only (never nested) and the language has no nested
function-like constructs at all (no lambdas, local functions, or closures), so a
function body only ever gains brace depth from control flow, never another function
declaration. That structural simplicity is what makes plain brace/paren counting
sufficient, as long as it's string/comment-aware (a single masking pass shared by both
the brace-safe extraction path and the public `StripComments` helper) so a brace or
paren inside a string literal or comment can never be mistaken for real syntax. Reuses
`Tools/FileEncoding.cs` for all file I/O — mod files are inconsistently encoded even
though vanilla is always UTF-16LE+BOM (see "Text-merge input encoding" below), the exact
same hazard this class's own callers already have to account for.

**Per-function resolution (`FunctionLevelMergeEngine.TryMerge`)** tries cheap one-sided
shortcuts (unchanged, only-one-side-edited, both-sides-made-the-identical-edit) before
ever calling `DiffPlexMergeEngine.BuildMerge` — only a function genuinely edited
differently on both sides reaches a real per-function 3-way merge attempt, falling back
to a whole-function tiebreak (**most distinct from vanilla wins**, scored via
`DiffPlex.Differ`'s plain 2-way line diff over comment-stripped, whitespace-ignored text
— deliberately not the buggy `ThreeWayDiffer`) if that merge attempt itself conflicts or
throws `DiffAlgorithmException`. A vanilla function deleted on one side and edited on the
other resolves as **edit wins** (a deletion never silently overrides a surviving edit —
losing code silently is unrecoverable if the deletion was wrong, keeping an unwanted
edit is not) — every non-mechanical resolution (a tiebreak, an edit surviving a
competing deletion, a mod's gap comment not making it into the output) is recorded in a
`Decisions` audit trail, never applied silently. That trail is threaded all the way out:
`DiffPlexMergeEngine.LastFunctionLevelDecisions` → `FileMerger`'s
`HeadlessMergeSummary.FunctionLevelDecisions` → both hosts' CLI output and the MCP
`merge_conflicts` tool's `functionLevelDecisions` field. Not yet surfaced in the WinForms
GUI's `MergeReportForm` — a deliberate deferral, not an oversight, since that's UI work
on the host side rather than engine work here.

**Scope note on non-function content ("gaps"):** a gap is only ever compared, and only
ever produces a `Decisions` note (never a decline), when it sits between two vanilla
units both sides kept and neither side inserted anything at that slot — comparison uses
the same whitespace-tolerant spirit as `IsWhitespaceOnlyDifference` above, deliberately
NOT comment-stripped (unlike the extraction masking pass) since the whole point is to
detect when comment *content* differs, not just formatting. Reassembly always keeps
vanilla's own gap text verbatim regardless.

## Text-merge input encoding

Vanilla `.ws` files are UTF-16LE with a BOM; mod authors' files are often plain
Expand Down
43 changes: 40 additions & 3 deletions WitcherScriptMerger.Core/Inventory/FileMerger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ public class HeadlessMergeSummary
{
public List<string> Merged { get; } = new List<string>();
public List<string> Skipped { get; } = new List<string>();
// Audit notes from FunctionLevelMergeEngine (see DiffPlexMergeEngine.
// TryFunctionLevelRescue), each prefixed with the file it belongs to -
// empty whenever no conflict in this run needed the function-level
// fallback at all, which is the common case.
public List<string> FunctionLevelDecisions { get; } = new List<string>();
}

// One file's interactive merge request, extracted by the host project's
Expand Down Expand Up @@ -123,6 +128,15 @@ public class MergeReportData
bool _bundleChanged;
List<Merge> _pendingBundleMerges = new List<Merge>();

// Drained into HeadlessMergeSummary.FunctionLevelDecisions at the end of
// MergeConflictsHeadless. Appended to (not overwritten) right after every
// MergeTextHeadless call, since _mergeEngine.LastFunctionLevelDecisions only
// reflects the single most recent pairwise MergeHeadless call - a multi-mod
// chain can trigger the function-level rescue at more than one step, and each
// one's decisions would otherwise be lost the moment the next chain step's
// MergeHeadless call resets that property back to empty.
List<string> _functionLevelDecisions = new List<string>();

// Anchored at BOTH ends ("^...$") - see IsVanillaDlcBundleFolder's own comment
// below for why this matters: it's matched against just the extracted folder-name
// segment, not the full path, so a full-string match is required, not merely a
Expand Down Expand Up @@ -534,6 +548,8 @@ public HeadlessMergeSummary MergeConflictsHeadless(
}
}

summary.FunctionLevelDecisions.AddRange(_functionLevelDecisions);

CleanUpTempFiles();
CleanUpEmptyDirectories();

Expand Down Expand Up @@ -571,15 +587,28 @@ bool MergeFlatConflictHeadless(ModFile conflict, Merge merge, string mergedModNa

_vanillaFile = new FileInfo(conflict.GetVanillaFile());

// Tracked locally, independent of merge.Mods (which can carry stale entries
// from a previous run when merge is a re-merge pulled from _inventory.Merges
// rather than freshly created) - this is only ever the real mod names folded
// into source1 so far within THIS chain, for FunctionLevelMergeEngine's
// Decisions[] audit text (see DiffPlexMergeEngine.TryFunctionLevelRescue's
// own comment on why source1.Name alone is misleading past the first step).
var accumulatedModNames = new List<string> { orderedNames[0] };

for (int i = 1; i < orderedNames.Length; ++i)
{
var hash = conflict.Mods.First(h => h.Name.EqualsIgnoreCase(orderedNames[i]));
var source2 = MergeSource.FromFlatFile(new FileInfo(conflict.GetModFile(orderedNames[i])), hash);

var mergedFile = MergeTextHeadless(merge, source1, source2, dryRun);
var oldDescription = accumulatedModNames.Count > 1
? "accumulated merge (" + string.Join(", ", accumulatedModNames) + ")"
: accumulatedModNames[0];

var mergedFile = MergeTextHeadless(merge, source1, source2, dryRun, oldDescription, orderedNames[i]);
if (mergedFile == null)
return false;
source1 = MergeSource.FromFlatFile(mergedFile, null);
accumulatedModNames.Add(orderedNames[i]);
}
return true;
}
Expand Down Expand Up @@ -700,7 +729,7 @@ string[] ResolveMergeOrder(ModFile conflict, string mergedModName, IReadOnlyDict
.ToArray();
}

FileInfo MergeTextHeadless(Merge merge, MergeSource source1, MergeSource source2, bool dryRun)
FileInfo MergeTextHeadless(Merge merge, MergeSource source1, MergeSource source2, bool dryRun, string oldDescription = null, string newDescription = null)
{
ProgressInfo.CurrentAction = $"Merging {source1.Name} && {source2.Name}";

Expand All @@ -711,7 +740,15 @@ FileInfo MergeTextHeadless(Merge merge, MergeSource source1, MergeSource source2
// this, MergeConflictsHeadless(dryRun: true) against a mods folder with N
// genuine conflicts would pop open N editor windows - a real bug caught in
// review before it shipped (see docs/decisions/kdiff3-retirement.md).
var result = _mergeEngine.MergeHeadless(source1, source2, _vanillaFile, _outputPath, openConflictMarkers: !dryRun);
var result = _mergeEngine.MergeHeadless(
source1, source2, _vanillaFile, _outputPath, openConflictMarkers: !dryRun,
oldDescription: oldDescription, newDescription: newDescription);

if (_mergeEngine.LastFunctionLevelDecisions.Count > 0)
{
foreach (var decision in _mergeEngine.LastFunctionLevelDecisions)
_functionLevelDecisions.Add(merge.RelativePath + ": " + decision);
}

if (result != MergeEngineResult.AutoSolved)
return null;
Expand Down
9 changes: 7 additions & 2 deletions WitcherScriptMerger.Core/Mcp/WsmMcpTools.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,12 @@ public static object ScanConflicts()
[McpServerTool(Name = "merge_conflicts"), Description(
"Merges detected conflicts headlessly; conflicts that can't be auto-solved are skipped " +
"and reported, not merged - a conflict-marker sidecar file is written and opened in the " +
"default editor for manual review instead. Restrict to specific files with " +
"default editor for manual review instead. A conflict a whole-file merge can't resolve " +
"may still auto-solve via a function-level fallback (splits vanilla/mod .ws files into " +
"individual functions and resolves each independently); any such case is reported in " +
"functionLevelDecisions, one line per resolved function explaining which side's version " +
"was kept and why - always worth surfacing to the user, not just the merged/skipped " +
"counts. Restrict to specific files with " +
"relativePaths (default: every detected conflict); override a file's mod merge order " +
"with orderOverrides (default merge order otherwise matches the game's own load order). " +
"Set dryRun to preview which conflicts would auto-solve without writing any merged " +
Expand Down Expand Up @@ -143,7 +148,7 @@ public static object MergeConflicts(
if (!dryRun)
AppState.Inventory.Save();

return new { merged = summary.Merged, skipped = summary.Skipped, unmatched, dryRun };
return new { merged = summary.Merged, skipped = summary.Skipped, unmatched, dryRun, functionLevelDecisions = summary.FunctionLevelDecisions };
}
}

Expand Down
98 changes: 95 additions & 3 deletions WitcherScriptMerger.Core/Tools/DiffPlexMergeEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,22 @@ public DiffAlgorithmException(string message) : base(message) { }

#endregion

// Set (or reset to empty) at the start of every MergeHeadless call - see that
// method's own comment. A single instance of this class is reused for a whole
// headless run (FileMerger constructs one field), so this must never be left
// holding a previous file's decisions when the current file never reaches the
// function-level engine at all.
public IReadOnlyList<string> LastFunctionLevelDecisions { get; private set; } = Array.Empty<string>();

public MergeEngineResult Merge(
FileMerger.MergeSource source1,
FileMerger.MergeSource source2,
FileInfo vanillaFile,
string outputPath)
string outputPath,
string oldDescription = null,
string newDescription = null)
{
var result = MergeHeadless(source1, source2, vanillaFile, outputPath);
var result = MergeHeadless(source1, source2, vanillaFile, outputPath, oldDescription: oldDescription, newDescription: newDescription);
return result == MergeEngineResult.NeedsManualResolution ? MergeEngineResult.Failed : result;
}

Expand All @@ -147,8 +156,12 @@ public MergeEngineResult MergeHeadless(
FileMerger.MergeSource source2,
FileInfo vanillaFile,
string outputPath,
bool openConflictMarkers = true)
bool openConflictMarkers = true,
string oldDescription = null,
string newDescription = null)
{
LastFunctionLevelDecisions = Array.Empty<string>();

var hasVanillaVersion = vanillaFile != null && vanillaFile.Exists;

// A 3-way merge is meaningless without a base to diff against - confirmed
Expand Down Expand Up @@ -213,6 +226,15 @@ public MergeEngineResult MergeHeadless(
}
catch (DiffAlgorithmException ex)
{
// Before giving up on the whole file: the function-level engine catches
// this same exception per-function (see FunctionLevelMergeEngine) and
// falls back to a whole-function tiebreak instead, so a file can still
// merge even when the whole-file 3-way diff hits this bug. Only
// attempted for .ws files - the extractor is WitcherScript-specific and
// has no notion of XML structure.
if (TryFunctionLevelRescue(baseText, oldText, newText, source1, source2, outputPath, oldDescription, newDescription))
return MergeEngineResult.AutoSolved;

// DiffPlex's own diff algorithm produced output it isn't safe to trust
// (see BuildMerge's comment) - don't write anything, including a sidecar:
// the "conflict marker" content itself would have been built from the
Expand Down Expand Up @@ -242,6 +264,15 @@ public MergeEngineResult MergeHeadless(
return MergeEngineResult.AutoSolved;
}

// Before falling back to conflict markers: same function-level rescue
// attempt as the DiffAlgorithmException catch above, for the "produced
// output, but with a real conflict block" case. See
// FunctionLevelMergeEngine's own comment for why this is a fallback that
// only ever activates where the whole-file merge has already failed, never
// a parallel code path for merges that would have succeeded anyway.
if (TryFunctionLevelRescue(baseText, oldText, newText, source1, source2, outputPath, oldDescription, newDescription))
return MergeEngineResult.AutoSolved;

// Never write conflict markers to outputPath itself: FileMerger's headless
// callers (MergeFlatConflictHeadless/MergeBundleConflictHeadless) check
// `File.Exists(_outputPath)` BEFORE attempting a merge and, if it exists,
Expand Down Expand Up @@ -366,6 +397,67 @@ static void DeleteIfExists(string path)
try { if (File.Exists(path)) File.Delete(path); } catch { }
}

// The function-level fallback (see FunctionLevelMergeEngine's own header
// comment). Called from both of MergeHeadless's give-up points, never from
// anywhere else - this method's whole job is "try to do better than the
// failure that's already about to happen," so every early return here means
// "the caller proceeds exactly as if this method didn't exist," never a worse
// outcome than today's baseline.
//
// oldDescription/newDescription default to source1.Name/source2.Name (the
// pre-existing marker-label convention) when not supplied - accurate for a
// merge chain's first pairwise step, but source1.Name resolves to the
// accumulated-merge output's own mod name (e.g. the configured merged-mod
// folder) from the second step onward, since FileMerger.MergeFlatConflictHeadless
// reassigns source1 to the prior step's output file. FileMerger passes a
// richer "accumulated merge (modA, modB)" description once it has more than
// one real mod recorded for this chain (see MergeTextHeadless/
// MergeTextInteractive) so a Decisions[] note reads sensibly past the first
// step; this method doesn't know or care which case it's in.
bool TryFunctionLevelRescue(
string baseText, string oldText, string newText,
FileMerger.MergeSource source1, FileMerger.MergeSource source2, string outputPath,
string oldDescription, string newDescription)
{
if (!Path.GetExtension(outputPath).EqualsIgnoreCase(".ws"))
return false;

FunctionLevelMergeResult result;
try
{
result = FunctionLevelMergeEngine.TryMerge(
baseText, oldText, newText,
source1.Name, source2.Name,
oldDescription ?? source1.Name, newDescription ?? source2.Name);
}
catch
{
// A latent bug in the new engine must never regress this method below
// its pre-existing behavior - the caller falls through to whatever it
// was already about to do (write a sidecar, or report the
// DiffAlgorithmException as-is) exactly as if this rescue attempt had
// declined outright.
return false;
}

if (!result.Applied)
return false;

DeleteIfExists(GetConflictMarkerPath(outputPath));
FileEncoding.WriteUtf16(outputPath, result.MergedText);
LastFunctionLevelDecisions = result.Decisions;

if (result.Decisions.Count > 0)
{
AppState.Notifier.ShowMessage(
$"Merged {source1.Name} + {source2.Name} at the function level after the whole-file merge " +
$"couldn't auto-solve it:\n\n" + string.Join("\n", result.Decisions),
"Merged (function-level)", NotifyButtons.OK, DialogIcon.Information);
}

return true;
}

#region Merge algorithm

// The actual 3-way merge, factored out as a public static method (independent of
Expand Down
Loading
Loading