unsloth: compare add/add conflicts on content, not on the braces around it

scripts/unsloth/additive_merge.py

@@ -13,6 +13,12 @@
 the merge base is non-empty means at least one side *edited* shared text, and
 picking a side or unioning them is a guess. This script never guesses.
 
+The two additions are compared on their CONTENT, not on the braces around it.
+A case arm is `case X:`, a body, and `} break;`, and two arms for different
+architectures share that last part whatever they do. Treating the scaffolding
+as evidence that the same change was made twice refuses exactly the conflict
+this script exists for; see STRUCTURAL below.
+
 Reads a conflicted work tree, writes resolutions in place, exits 0 if every
 conflict in every file was resolved and 1 otherwise. `--report` emits JSON
 describing what it did for the caller to quote in a PR body.
@@ -22,6 +28,7 @@
 
 import argparse
 import json
+import re
 import subprocess
 import sys
 from pathlib import Path
@@ -87,6 +94,44 @@ def nonblank(lines: list[str]) -> list[str]:
     return [ln.strip() for ln in lines if ln.strip()]
 
 
+# A line that closes or opens a block and nothing else. Two INDEPENDENT case
+# arms in the same switch share these by construction -- `{`, `} break;`, `}`
+# are what a case arm is made of, not what makes it that case arm -- so finding
+# them on both sides says nothing about whether the two sides added the same
+# construct. Matching them as "shared" is what refused the real add/add of
+# PROJECTOR_TYPE_KIMIK3 next to PROJECTOR_TYPE_DEEPSEEK4V in tools/mtmd/clip.cpp
+# with "one change made twice: {, } break;", when the two arms had no line of
+# actual content in common.
+#
+# Deliberately narrow: braces, brackets, parens, semicolons and commas, around
+# at most one bare block-terminating keyword. `break;` matches, `return true;`
+# does not, and anything naming a type, a constant or a function does not.
+STRUCTURAL = re.compile(r"^[\s{}()\[\];,]*(?:break|continue|return|pass)?[\s{}()\[\];,]*$")
+
+
+def identifying(lines: list[str]) -> set[str]:
+    """The lines that say WHICH construct this is, ignoring block scaffolding."""
+    return {ln for ln in nonblank(lines) if not STRUCTURAL.match(ln)}
+
+
+# `case FOO:`, `case FOO :`, `default:`. A fallthrough label may carry no body
+# at all, which is the shape the nightly hits most often.
+CASE_LABEL = re.compile(r"^(?:case\s+[^:]+|default\s*):")
+
+
+def case_arms(lines: list[str]) -> set[str] | None:
+    """The case labels this side adds, or None if it is not a run of case arms.
+
+    None, not an empty set: "adds no case arm" and "adds case arms, none of
+    which the other side adds" have to be told apart, and only the second one
+    licenses the union below.
+    """
+    ident = [ln for ln in nonblank(lines) if not STRUCTURAL.match(ln)]
+    if not ident or not CASE_LABEL.match(ident[0]):
+        return None
+    return {ln for ln in ident if CASE_LABEL.match(ln)}
+
+
 def resolve_region(ours: list[str], base: list[str], theirs: list[str]) -> list[str]:
     """Return the union, or raise if this region is not a pure add/add."""
     if nonblank(base):
@@ -100,14 +145,38 @@ def resolve_region(ours: list[str], base: list[str], theirs: list[str]) -> list[
     if ours == theirs:
         # Both sides added byte-identical text; one copy is the resolution.
         return list(ours)
-    shared = set(nonblank(ours)) & set(nonblank(theirs))
+    ours_arms, theirs_arms = case_arms(ours), case_arms(theirs)
+    if ours_arms and theirs_arms and ours_arms.isdisjoint(theirs_arms):
+        # Both sides added case arms, and not one label is on both sides. Two
+        # arms of the same switch labelled differently are two constructs, so
+        # any line they happen to share is body text, not a duplicate: the real
+        # tools/mtmd/clip.cpp collision has a KIMIK3 arm and a DEEPSEEK4V arm
+        # that both set `hparams.rope_theta = 10000.0f;`, and refusing on that
+        # coincidence is what the shared-line check is for, backwards.
+        #
+        # The same change made twice would keep its label, so it lands in the
+        # check below instead. This is the one place where a shared line is
+        # allowed, and it is allowed because the labels prove the arms are
+        # distinct -- a duplicated label would not even compile.
+        return list(theirs) + list(ours)
+    shared = identifying(ours) & identifying(theirs)
     if shared:
         # Overlapping content is the signature of one construct added twice,
         # not two independent additions. Unioning it would duplicate code.
+        # Scaffolding lines are excluded above, so what is left is content both
+        # sides genuinely wrote, which is the thing that makes this a duplicate.
         raise Unresolvable(
             "both sides add the same line(s), so this is one change made twice: "
             + ", ".join(sorted(shared)[:3])
         )
+    if not identifying(ours) or not identifying(theirs):
+        # Everything one side added is scaffolding, so there is no content to
+        # tell the two additions apart and the exclusion above has nothing left
+        # to work with. Refuse rather than union braces onto braces.
+        raise Unresolvable(
+            "one side adds only block scaffolding, so the two additions cannot "
+            "be told apart"
+        )
     # Upstream first, then ours: the same order a human repin produces.
     return list(theirs) + list(ours)
 

scripts/unsloth/test_additive_merge.py

@@ -86,12 +86,87 @@ def run(repo, *extra):
 check("identical add/add is not a conflict at all", rc == 1 and "no conflicted files" in json.dumps(rep))
 
 base = "a\nz\n"
-ours = "a\ncase FOO:\n    break;\nz\n"
-theirs = "a\ncase BAR:\n    break;\nz\n"
+ours = "a\nstatic void helper() {\n    log(\"same\");\n}\nz\n"
+theirs = "a\nstatic void helper2() {\n    log(\"same\");\n}\nz\n"
 repo, f = make_conflict(base, ours, theirs)
 rc, rep = run(repo)
-check("overlapping add/add refuses (shared 'break;')",
+check("overlapping add/add refuses on shared CONTENT",
       rc == 1 and "made twice" in json.dumps(rep), rep)
+reason = rep["refused"][0]["reason"] if rep.get("refused") else ""
+check("overlapping add/add names the content line, not the braces",
+      reason.endswith('twice: log("same");'), reason)
+
+# --- 3b. two independent case arms: braces are shared, content is not -------
+# The real tools/mtmd/clip.cpp shape. Refusing this on `{` and `} break;` is
+# what took the 09-02 nightly's last pin down.
+base = "switch (t) {\n}\n"
+ours = ("switch (t) {\n    case PROJECTOR_TYPE_KIMIK3:\n        {\n"
+        "            builder = std::make_unique<clip_graph_kimik3>(ctx, img);\n"
+        "        } break;\n}\n")
+theirs = ("switch (t) {\n    case PROJECTOR_TYPE_DEEPSEEK4V:\n        {\n"
+          "            builder = std::make_unique<clip_graph_deepseek4v>(ctx, img);\n"
+          "        } break;\n}\n")
+repo, f = make_conflict(base, ours, theirs)
+rc, rep = run(repo)
+txt = f.read_text()
+check("independent case arms resolve despite shared braces", rc == 0 and rep["ok"], rep)
+check("independent case arms keep both", "KIMIK3" in txt and "DEEPSEEK4V" in txt and "<<<<" not in txt, txt)
+check("independent case arms keep both bodies once",
+      txt.count("} break;") == 2 and txt.count("clip_graph_kimik3") == 1, txt)
+
+# --- 3b2. two case arms that share a body line, which is a coincidence ------
+# The clip.cpp shape after upstream landed DEEPSEEK4V: both arms set the same
+# rope_theta, and refusing on that is the shared-line check backwards.
+base = "switch (t) {\n}\n"
+ours = ("switch (t) {\n    case PROJECTOR_TYPE_KIMIK3:\n        {\n"
+        "            hparams.image_resize_algo = RESIZE_ALGO_BILINEAR;\n"
+        "            hparams.rope_theta = 10000.0f;\n        } break;\n}\n")
+theirs = ("switch (t) {\n    case PROJECTOR_TYPE_DEEPSEEK4V:\n        {\n"
+          "            hparams.image_resize_algo = RESIZE_ALGO_BICUBIC;\n"
+          "            hparams.rope_theta = 10000.0f;\n        } break;\n}\n")
+repo, f = make_conflict(base, ours, theirs)
+rc, rep = run(repo)
+txt = f.read_text()
+check("case arms with a coincidentally shared body line resolve", rc == 0 and rep["ok"], rep)
+check("case arms with a shared body line keep both arms",
+      txt.count("rope_theta") == 2 and "KIMIK3" in txt and "DEEPSEEK4V" in txt, txt)
+
+# --- 3b3. the SAME arm added twice keeps its label, so it still refuses -----
+base = "switch (t) {\n}\n"
+ours = ("switch (t) {\n    case PROJECTOR_TYPE_KIMIK3:\n        {\n"
+        "            hparams.rope_theta = 10000.0f;\n        } break;\n}\n")
+theirs = ("switch (t) {\n    case PROJECTOR_TYPE_KIMIK3:\n        {\n"
+          "            hparams.rope_theta = 50000.0f;\n        } break;\n}\n")
+repo, f = make_conflict(base, ours, theirs)
+rc, rep = run(repo)
+check("the same case label on both sides still refuses",
+      rc == 1 and "made twice" in json.dumps(rep), rep)
+
+# --- 3b4. only one side is case arms: no label proof, ordinary rules apply --
+base = "a\nz\n"
+ours = "a\ncase FOO:\n    f(1);\n    break;\nz\n"
+theirs = "a\nstatic void helper() { f(1); }\nz\n"
+repo, f = make_conflict(base, ours, theirs)
+rc, rep = run(repo)
+check("one side not a case arm falls back to the shared-line check",
+      rc == 0 and rep["ok"], rep)
+
+base = "a\nz\n"
+ours = "a\ncase FOO:\n    f(1);\n    break;\nz\n"
+theirs = "a\nstatic void helper();\n    f(1);\nz\n"
+repo, f = make_conflict(base, ours, theirs)
+rc, rep = run(repo)
+check("one side not a case arm still refuses on a shared content line",
+      rc == 1 and "made twice" in json.dumps(rep), rep)
+
+# --- 3c. one side adds only scaffolding: nothing distinguishes the two ------
+base = "a\nz\n"
+ours = "a\n}\nz\n"
+theirs = "a\ncase BAR:\n    break;\nz\n"
+repo, f = make_conflict(base, ours, theirs)
+rc, rep = run(repo)
+check("scaffolding-only addition refuses",
+      rc == 1 and "scaffolding" in json.dumps(rep), rep)
 
 # --- 4. one file good, one file bad: refuse the whole merge ----------------
 d = Path(tempfile.mkdtemp(prefix="am_"))