From e4169d0296208af9d4b9a5d00c3db4db32da9e28 Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Thu, 3 Sep 2026 08:33:48 +0000 Subject: [PATCH 1/3] unsloth: compare add/add conflicts on content, not on the braces around it additive_merge.py refuses a conflict when both sides add a line the other also adds, on the grounds that one construct added twice would be duplicated by a union. Two independent case arms in the same switch always share their scaffolding, so that check fires on `{` and `} break;` and refuses exactly the add/add it exists to resolve. That is what stopped the 09-02 nightly on its last pin: refused tools/mtmd/clip.cpp: both sides add the same line(s), so this is one change made twice: {, } break; where one side added a PROJECTOR_TYPE_KIMIK3 arm and the other a PROJECTOR_TYPE_DEEPSEEK4V one, with no line of actual content in common. Compare the sides on their identifying lines instead: braces, brackets, parens, semicolons and commas around at most one bare block-terminating keyword are scaffolding and carry no identity. `break;` is scaffolding, `return true;` is not, and anything naming a type, constant or function is not. Two arms that really are the same change still share their case label and their body, so a genuine duplicate is still refused. A side whose addition is nothing but scaffolding now refuses too: with the scaffolding discounted there is no content left to tell the two additions apart, so unioning would be a guess. --- scripts/unsloth/additive_merge.py | 39 +++++++++++++++++++++++++- scripts/unsloth/test_additive_merge.py | 36 ++++++++++++++++++++++-- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/scripts/unsloth/additive_merge.py b/scripts/unsloth/additive_merge.py index 3ecf17c415a1..0265b00172f8 100644 --- a/scripts/unsloth/additive_merge.py +++ b/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,26 @@ 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)} + + 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 +127,24 @@ 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)) + 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) diff --git a/scripts/unsloth/test_additive_merge.py b/scripts/unsloth/test_additive_merge.py index 0a3e535d7a51..561b1a24dc8e 100644 --- a/scripts/unsloth/test_additive_merge.py +++ b/scripts/unsloth/test_additive_merge.py @@ -86,12 +86,42 @@ 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\ncase FOO:\n log(\"same\");\n break;\nz\n" +theirs = "a\ncase BAR:\n log(\"same\");\n break;\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", + 'log("same");' in reason and "break;" not in reason, 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(ctx, img);\n" + " } break;\n}\n") +theirs = ("switch (t) {\n case PROJECTOR_TYPE_DEEPSEEK4V:\n {\n" + " builder = std::make_unique(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) + +# --- 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_")) From 71d6b39f65162350e366f05d35c72d8293855921 Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Thu, 3 Sep 2026 08:38:02 +0000 Subject: [PATCH 2/3] unsloth: let two case arms with different labels share a body line Discounting the braces is not enough on its own. Once upstream landed DEEPSEEK4V, the KIMIK3 arm and the DEEPSEEK4V arm of the same switch both set `hparams.rope_theta = 10000.0f;`, so the shared-line check refuses on a coincidence: conflict tools/mtmd/clip.cpp: both sides add the same line(s), so this is one change made twice: hparams.rope_theta = 10000.0f; Two arms of one switch labelled differently are two constructs, whatever lines their bodies have in common, so when both sides add case arms and no label appears on both sides, the union is the resolution. The labels are the proof, and they are also what keeps the duplicate check working: the same change made twice keeps its label, so it never reaches this branch and is still refused. A duplicated label would not compile. --- scripts/unsloth/additive_merge.py | 32 ++++++++++++++++ scripts/unsloth/pr-set.json | 14 +++---- scripts/unsloth/test_additive_merge.py | 51 ++++++++++++++++++++++++-- 3 files changed, 87 insertions(+), 10 deletions(-) diff --git a/scripts/unsloth/additive_merge.py b/scripts/unsloth/additive_merge.py index 0265b00172f8..5364dc1b7c10 100644 --- a/scripts/unsloth/additive_merge.py +++ b/scripts/unsloth/additive_merge.py @@ -114,6 +114,24 @@ def identifying(lines: list[str]) -> set[str]: 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): @@ -127,6 +145,20 @@ 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) + 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, diff --git a/scripts/unsloth/pr-set.json b/scripts/unsloth/pr-set.json index f52560db0968..f3765de7fa28 100644 --- a/scripts/unsloth/pr-set.json +++ b/scripts/unsloth/pr-set.json @@ -22,14 +22,14 @@ "https://github.com/ggml-org/llama.cpp/pull/25731/commits/44eb88e9aba218b24c0f374f2ec1c4d7d7920877", "https://github.com/unslothai/llama.cpp/pull/70/commits/edfd4c1a3b7a653303a85257ddac2a1f3ce39a2f", "https://github.com/unslothai/llama.cpp/pull/91/commits/c86ed269986f2dced6325c5c58bda966a2e2ead1", - "https://github.com/unslothai/llama.cpp/pull/95/commits/3db8cb5b2e9bf291057b9f19960e8601a162da81", - "https://github.com/ggml-org/llama.cpp/pull/27754/commits/949f7efb097eb20ef36fecdb1afaebff9a4ae7ed", - "https://github.com/unslothai/llama.cpp/pull/137/commits/4e1865e34ec5f6ca39403215c89129c13731be70", - "https://github.com/unslothai/llama.cpp/pull/158/commits/abfc45b9cb21eae4848cb82196e659f42c9a8341", - "https://github.com/unslothai/llama.cpp/pull/157/commits/6c6da89266ba7839d825c9997782af4f4d26b81b", - "https://github.com/unslothai/llama.cpp/pull/149/commits/b65a2dce12c14a489e19a059cb6ee59112f1b733", + "https://github.com/unslothai/llama.cpp/pull/95/commits/db908313fc679541e9ac88b8e3effd136dc125c7", + "https://github.com/ggml-org/llama.cpp/pull/27754/commits/bf550e99d3316a8c0e0280de6422f85f1a557190", + "https://github.com/unslothai/llama.cpp/pull/137/commits/8378dcec2b032c054cafc8e7a79584b621cd0348", + "https://github.com/unslothai/llama.cpp/pull/158/commits/384ddf4a249ece277237553019aeb3a688284ad4", + "https://github.com/unslothai/llama.cpp/pull/157/commits/01fbc1dede48cbe1728fee05057a7263db007aef", + "https://github.com/unslothai/llama.cpp/pull/149/commits/c7dce133d745f9af9acf5839c65141fe2d705394", "https://github.com/unslothai/llama.cpp/pull/144/commits/5a08a717da20caa6c5c4dfaa85024adf6fc4e7fa", - "https://github.com/unslothai/llama.cpp/pull/152/commits/258345efa640eb099eb1af3c9ee8f8e6e8e7b0d3", + "https://github.com/unslothai/llama.cpp/pull/152/commits/6f9c19833ac849eb70feb332784485a648f048a5", "https://github.com/unslothai/llama.cpp/pull/154/commits/31e432e758f8cc4b2c5f27902721500173bf39db", "https://github.com/ggml-org/llama.cpp/pull/28133/commits/3a798bf2f3e0a5ee90c0a7bcef60fb3ef1b4b8b3" ] diff --git a/scripts/unsloth/test_additive_merge.py b/scripts/unsloth/test_additive_merge.py index 561b1a24dc8e..0f91e9afd12b 100644 --- a/scripts/unsloth/test_additive_merge.py +++ b/scripts/unsloth/test_additive_merge.py @@ -86,15 +86,15 @@ 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 log(\"same\");\n break;\nz\n" -theirs = "a\ncase BAR:\n log(\"same\");\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 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", - 'log("same");' in reason and "break;" not in reason, reason) + 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 @@ -114,6 +114,51 @@ def run(repo, *extra): 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" From 184488ad3477340c562faa713ddf695eb54b51fe Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Thu, 3 Sep 2026 11:35:46 +0000 Subject: [PATCH 3/3] Revert the pin file to master repin.py writes its new pins back into scripts/unsloth/pr-set.json, and running it here to reproduce the failing conflict left that file modified. It has nothing to do with this change, and the pins it wrote point at local merge commits that were never pushed, so the lint correctly rejected them. The pin changes live in their own PR. --- scripts/unsloth/pr-set.json | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/scripts/unsloth/pr-set.json b/scripts/unsloth/pr-set.json index f3765de7fa28..f52560db0968 100644 --- a/scripts/unsloth/pr-set.json +++ b/scripts/unsloth/pr-set.json @@ -22,14 +22,14 @@ "https://github.com/ggml-org/llama.cpp/pull/25731/commits/44eb88e9aba218b24c0f374f2ec1c4d7d7920877", "https://github.com/unslothai/llama.cpp/pull/70/commits/edfd4c1a3b7a653303a85257ddac2a1f3ce39a2f", "https://github.com/unslothai/llama.cpp/pull/91/commits/c86ed269986f2dced6325c5c58bda966a2e2ead1", - "https://github.com/unslothai/llama.cpp/pull/95/commits/db908313fc679541e9ac88b8e3effd136dc125c7", - "https://github.com/ggml-org/llama.cpp/pull/27754/commits/bf550e99d3316a8c0e0280de6422f85f1a557190", - "https://github.com/unslothai/llama.cpp/pull/137/commits/8378dcec2b032c054cafc8e7a79584b621cd0348", - "https://github.com/unslothai/llama.cpp/pull/158/commits/384ddf4a249ece277237553019aeb3a688284ad4", - "https://github.com/unslothai/llama.cpp/pull/157/commits/01fbc1dede48cbe1728fee05057a7263db007aef", - "https://github.com/unslothai/llama.cpp/pull/149/commits/c7dce133d745f9af9acf5839c65141fe2d705394", + "https://github.com/unslothai/llama.cpp/pull/95/commits/3db8cb5b2e9bf291057b9f19960e8601a162da81", + "https://github.com/ggml-org/llama.cpp/pull/27754/commits/949f7efb097eb20ef36fecdb1afaebff9a4ae7ed", + "https://github.com/unslothai/llama.cpp/pull/137/commits/4e1865e34ec5f6ca39403215c89129c13731be70", + "https://github.com/unslothai/llama.cpp/pull/158/commits/abfc45b9cb21eae4848cb82196e659f42c9a8341", + "https://github.com/unslothai/llama.cpp/pull/157/commits/6c6da89266ba7839d825c9997782af4f4d26b81b", + "https://github.com/unslothai/llama.cpp/pull/149/commits/b65a2dce12c14a489e19a059cb6ee59112f1b733", "https://github.com/unslothai/llama.cpp/pull/144/commits/5a08a717da20caa6c5c4dfaa85024adf6fc4e7fa", - "https://github.com/unslothai/llama.cpp/pull/152/commits/6f9c19833ac849eb70feb332784485a648f048a5", + "https://github.com/unslothai/llama.cpp/pull/152/commits/258345efa640eb099eb1af3c9ee8f8e6e8e7b0d3", "https://github.com/unslothai/llama.cpp/pull/154/commits/31e432e758f8cc4b2c5f27902721500173bf39db", "https://github.com/ggml-org/llama.cpp/pull/28133/commits/3a798bf2f3e0a5ee90c0a7bcef60fb3ef1b4b8b3" ]