Add function-level merge fallback for whole-file merge failures - #18
Merged
Conversation
DiffPlexMergeEngine.MergeHeadless merges whole files line-by-line, which means a handful of real, incidental whitespace/comment differences (or a confirmed upstream DiffPlex bug that gets worse as edit density rises) can block an entire file from auto-solving even when the actual overlapping logic changes are confined to one or two functions. Adds a fallback, activated only at the two points where the whole-file merge has already failed for a given pairwise step: split vanilla and both sides into function/field units (ScriptUnitExtractor, a brace/paren-matching tokenizer - WitcherScript has no nested functions, so this doesn't need a full parser), align each side against vanilla by name (UnitAligner, LCS-based, handling both insertions and deletions), and resolve each function independently (FunctionLevelMergeEngine) - cheap shortcuts for untouched or single-sided edits, a real 3-way merge for non-overlapping changes, and a most-distinct-from-vanilla tiebreak for genuine collisions. An edit always wins over a competing deletion. Every non-mechanical resolution is recorded in an audit trail (DiffPlexMergeEngine.LastFunctionLevelDecisions -> FileMerger.HeadlessMergeSummary.FunctionLevelDecisions -> both hosts' CLI output and the MCP merge_conflicts tool's functionLevelDecisions field), never applied silently. Validated empirically before and after building: a real live install's actor.ws conflict (6 real overhaul mods) showed only 6 of 395 functions were genuine two-mod collisions, confirming per-function decomposition was worth building. A chain-step replay against that same install's 5 real-world unresolved conflicts found an insertion-only alignment would rescue only 2 of 5 (one mod deletes several vanilla functions outright, and that deletion persists through the merge chain even at steps where the deleting mod isn't a direct input) - UnitAligner's symmetric insert/delete handling was added specifically to cover this. Final end-to-end validation against an isolated copy of the same 5 real files got all 5 to merge successfully, including actor.ws itself, with well-formed output and no conflict markers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DiffPlexMergeEngine.MergeHeadlessmerges whole.wsfiles line-by-line, which means real, incidental whitespace/comment differences (or a confirmed upstream DiffPlex bug that gets worse as edit density rises — seeWitcherScriptMerger.Core/CLAUDE.md's "Compatibility constraint" section) can block an entire file from auto-solving even when the actual overlapping logic changes are confined to one or two functions.This adds a fallback, activated only at the two points where the whole-file merge has already failed for a given pairwise chain step — never a parallel code path, so every conflict that already auto-solves today is unaffected:
ScriptUnitExtractor— splits a.wsfile into function/event/@addField-field units via a string/comment-aware brace-matching tokenizer. Not a full parser: confirmed via direct research into WitcherScript's grammar thatclass/state/struct/enumdeclarations are top-level only and the language has no nested function-like constructs at all, so plain brace/paren counting is sufficient.UnitAligner— aligns each side's units against vanilla's by name (LCS-based), handling insertions and deletions symmetrically.FunctionLevelMergeEngine— resolves each vanilla function independently: cheap shortcuts for untouched/single-sided edits, a real per-function 3-way merge for non-overlapping changes, and a most-distinct-from-vanilla tiebreak (viaDiffPlex.Differ's 2-way line diff, not the buggyThreeWayDiffer) for genuine collisions. An edit always wins over a competing deletion. Every non-mechanical resolution is recorded in an audit trail — surfaced viaDiffPlexMergeEngine.LastFunctionLevelDecisions→FileMerger.HeadlessMergeSummary.FunctionLevelDecisions→ both hosts' CLI output and the MCPmerge_conflictstool's newfunctionLevelDecisionsfield — never applied silently.Full design rationale is in
WitcherScriptMerger.Core/CLAUDE.md's new "Function-level merge engine" section.AI-assisted development disclosure (per
CONTRIBUTING.md): this PR was developed with Claude Code, including the design research, empirical validation, and implementation.Why this design
Before writing any code, I measured a real, live Witcher 3 install's
actor.wsconflict (vanilla + 6 real overhaul mods): only 6 of 395 functions were genuine two-mod collisions, confirming per-function decomposition was worth building rather than just relocating the same conflict into a smaller, statistically moreDiffAlgorithmException-prone box.Mid-implementation, a chain-step replay against the real install's 5 currently-unresolved conflicts found that an insertion-only alignment (the originally-scoped design) 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 even at steps where the deleting mod isn't a direct input (an earlier clean whole-file merge step faithfully propagates a one-sided deletion into the accumulated text).
UnitAligner's symmetric insert/delete handling exists specifically to cover this — confirmed as the right call before building it further.Test plan
dotnet build WitcherScriptMerger.sln --configuration Release— clean, 0 errors (pre-existing unrelatedCA1823warnings only)dotnet format whitespace WitcherScriptMerger.sln --verify-no-changes— cleandotnet test WitcherScriptMerger.sln— 125/125 passing, including 3 new test files (ScriptUnitExtractorTests,UnitAlignerTests,FunctionLevelMergeEngineTests) covering round-trip extraction fidelity, alignment (matches/insertions/deletions/both), every one-sided shortcut, the tiebreak (including its deterministic tie-break and a scaled-downDiffAlgorithmExceptioncase), edit-survives-competing-deletion, insertion reconciliation, and gap-comment detectionmerge_conflictstool against it end-to-end. All 5 files merged successfully, includingactor.wsitself (the flagship case that motivated this feature) — well-formed UTF-16LE+BOM output, no conflict markers, itemizedfunctionLevelDecisionsexplaining every resolution. Never run against the live install directly (dry-run predicted the "output already exists" guard correctly with no writes).🤖 Generated with Claude Code
https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah