diff --git a/scripts/unsloth/additive_merge.py b/scripts/unsloth/additive_merge.py index 3ecf17c415a1..5364dc1b7c10 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,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) diff --git a/scripts/unsloth/pr-set.json b/scripts/unsloth/pr-set.json index f52560db0968..bdca9a2dd569 100644 --- a/scripts/unsloth/pr-set.json +++ b/scripts/unsloth/pr-set.json @@ -18,19 +18,18 @@ "tag + pins), so keep its pin listed until the change lands upstream." ], "prs": [ - "https://github.com/unslothai/llama.cpp/pull/107/commits/74acc40c37ae2eb36031981feda392b793944f72", - "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/ggml-org/llama.cpp/pull/24423/commits/c6f8d604b67611b73f7965c0bd39d26e7365a489", + "https://github.com/ggml-org/llama.cpp/pull/25731/commits/36df1bf409c8b257689321a971a66973ee817ee1", + "https://github.com/unslothai/llama.cpp/pull/70/commits/883f2c9ba78f3847148454adf025da29385fff3e", + "https://github.com/unslothai/llama.cpp/pull/61/commits/46cbf0e95786fe8f5b7c0e86d57aaf8f8eceea7f", "https://github.com/unslothai/llama.cpp/pull/95/commits/3db8cb5b2e9bf291057b9f19960e8601a162da81", - "https://github.com/ggml-org/llama.cpp/pull/27754/commits/949f7efb097eb20ef36fecdb1afaebff9a4ae7ed", + "https://github.com/ggml-org/llama.cpp/pull/27754/commits/629b50552801912b3e2078f9799e4d77213197d7", "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/258345efa640eb099eb1af3c9ee8f8e6e8e7b0d3", - "https://github.com/unslothai/llama.cpp/pull/154/commits/31e432e758f8cc4b2c5f27902721500173bf39db", - "https://github.com/ggml-org/llama.cpp/pull/28133/commits/3a798bf2f3e0a5ee90c0a7bcef60fb3ef1b4b8b3" + "https://github.com/unslothai/llama.cpp/pull/144/commits/a9e9c3c5fed8a0bb5cc617532d0d16b8f59c13e0", + "https://github.com/unslothai/llama.cpp/pull/152/commits/b2b5ed9ff86427a530b762a45d3fdbd453bcd4e8", + "https://github.com/unslothai/llama.cpp/pull/176/commits/09ce1a4d2939844e211f7b4d30a296f4c1aed9a8" ] } diff --git a/scripts/unsloth/test_additive_merge.py b/scripts/unsloth/test_additive_merge.py index 0a3e535d7a51..0f91e9afd12b 100644 --- a/scripts/unsloth/test_additive_merge.py +++ b/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(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) + +# --- 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_"))