tools/ci: conflict_triage classifies a merge's conflicts by kind, from the index not the file body by bong-water-water-bong · Pull Request #2278 · 1bit-MONSTER/engine

bong-water-water-bong · 2026-11-12T12:32:31Z

Refs #2138 (the AMD-rebase merge, lead pi-hrx-compat). The tool, not the merge — that is the slice I claimed on the issue.

Why a text search cannot do this

The issue's own table names the problem:

ggml/src/ggml-hrx/kernel-corpus | 41 paths, mostly add/delete or rename with no markers | 30

A conflict with no markers is invisible to every text-based tool. On that merge, grep '^<<<<<<<' reports 103 files while git reports 149 paths (#1945's triage) — a third of the merge is missing from the reading that a
file-by-file walk would use. The categories have to come from git merge-tree --write-tree, which reports them without a checkout.

What the kinds decide

kind	what it is	the work
`content`	markers in the file	a person reads both sides, one judgement per hunk
`add/add`	both sides added the path	often `--ours` wholesale, after checking
`mod/del`	one side deleted, the other changed it	a decision about intent, not text
`rename`	a rename on one side, an edit on the other	follow the lineage before resolving
`unmapped`	git named it in no message and it has no markers	reported, never guessed

Run against the real merge (read-only, no checkout)

$ conflict_triage.py ~/src/llama.cpp-1bit 9bf587131          # ours 7a1f2b81c vs AMD's rebased pin
conflict_triage: 202 conflicted path(s): content=73 add/add=129

73 + 129 = 202 is exactly git's own count — and the 129 add/add is the shape a text search cannot see: the whole ggml-hrx tree is an add on both sides against a base that has none of it, which makes the 28 conflicts in
that subtree one decision, not 28.

That run found a defect in the first version, and an arm pins it

The classifier suffix-matched the path list instead of reading git's message, so 5 of 202 paths whose message did name them fell to unmapped (all five were CMakeLists.txt, where git's line names a path the suffix match
missed). message_path() now takes the path from git's own wording — both Merge conflict in <path> and <path> deleted in …, spaces included — and the counts match git's to the path.

--self-test: 13 arms, ok, including:

the specific kinds are tested before the generic phrase (git spells add/add as CONFLICT (add/add): Merge conflict in <path>, so the obvious order reports every add/add as content — caught by the self-test);
a path with a space classifies from its message;
unmapped is reported rather than guessed.

--json gives a machine-readable table for a script that wants to batch the mechanical kinds.

Not in this PR

The merge itself, the mirror branch, and config/llama-mirror-* — steps 1–3 of the issue, which are pi-hrx-compat's.

…m the index not the file body Slice of engine#2138 (the AMD-rebase merge, lead pi-hrx-compat): the tool, not the merge. The issue's own table names the reason a file-by-file walk is the wrong instrument: ggml/src/ggml-hrx/kernel-corpus | 41 paths, mostly add/delete or rename with no markers | 30 A conflict with no markers is invisible to every text-based tool: `grep '^<<<<<<<'` reports 103 files on that merge while git reports 149 paths (engine#1945's triage). So the categories have to come from `git merge-tree --write-tree`, which reports them without a checkout, and they decide the work: content markers to resolve, one judgement per hunk -> a person reads both sides add/add both sides added the path -> often --ours wholesale mod/del one side deleted, the other changed it -> intent, not text rename a rename on one side, an edit on the other -> follow the lineage first unmapped git named it in no message and it has no markers -> reported, never guessed Run against the real merge (fork, `7a1f2b81c` vs AMD's `9bf587131`, read-only): conflict_triage: 202 conflicted path(s): content=73 add/add=129 which is exactly git's own 73 + 129 -- and it is what a text search cannot produce, because `add/add` on the `ggml-hrx` tree is a whole-tree add against a base that has none of it. That run found a defect in the first version and it is pinned by an arm: the classifier suffix-matched the path list instead of reading git's message, so 5 of 202 paths whose message did name them fell to `unmapped`. `message_path()` now takes the path from git's own wording (both the `Merge conflict in <path>` and the `<path> deleted in ...` shapes, spaces included), and the counts match git's to the path. --self-test: 13 arms, ok. --json for a machine-readable table. Refs engine#2138, engine#1945.

github-actions · 2026-11-12T12:33:31Z

PR Reviewer Guide 🔍

(Review updated until commit `d661e57`)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 2138 - Partially compliant Compliant requirements: The PR introduces the `conflict_triage.py` tool to classify merge conflicts by kind, which is the core requirement of the ticket The tool is integrated into the CMake build system with a self-test Non-compliant requirements: The actual merge of AMD's rebased line into 1bit/hrx-vulkan-patched is not included in this PR (it's a tooling change) The mirror branch update and hold removal steps are not included in this PR Requires further human verification: The actual merge operation and its conflict resolution still need to be performed manually Verification that the tool works correctly on the real merge (engine#2138) needs to be done in the actual environment
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review JSON Output Format Inconsistency The `run` function returns 1 when there are conflicts (as per the docstring: "exit: 0 no conflicts, 1 conflicts found, 2 usage or git error"), but in JSON mode, it prints the JSON and then returns 1. This is inconsistent with the documented behavior, as the function should return 0 when no conflicts are found and 1 when conflicts are found, regardless of the output format. def run(repo, theirs, base=None, as_json=False): result = merge_tree(repo, base, theirs) if not result["paths"]: print("conflict_triage: no conflicts") return 0 triaged = rows(result["paths"], result["info"], count_markers(repo)) total, per = summarise(triaged) if as_json: print(json.dumps({"tree": result["tree"], "total": total, "counts": per, "rows": [{"path": p, "kind": k, "hunks": n} for p, k, n in sorted(triaged)]}, indent=2)) return 1 for path, kind, hunks in sorted(triaged): print(f"{kind:9} {hunks:4d} {path}") print() print("conflict_triage: %d conflicted path(s): %s -- %s" % ( total, " ".join(f"{k}={per[k]}" for k in KINDS + ("unmapped",) if per.get(k)), "content needs a reading, everything else needs a decision")) return 1 Potential Incomplete Path Matching The `message_path` function attempts to extract the path from conflict messages, but it relies on specific string patterns that may not cover all possible git message formats. If a new conflict message format appears in future merges, the tool may fail to correctly classify conflicts, potentially leading to paths being marked as 'unmapped' when they should be classified. This could result in incorrect triage of conflicts during the merge process. def message_path(info_line): """The path a CONFLICT line names, taken from git's own wording rather than matched against a list. Two shapes, and suffix-matching the path list got both wrong on a real merge (5 of 202 paths fell to `unmapped` that git had classified): CONFLICT (content): Merge conflict in <path> CONFLICT (modify/delete): <path> deleted in HEAD and modified in ... A rename message names a pair and is left to the caller's suffix match. """ marker = "Merge conflict in " at = info_line.find(marker) if at >= 0: return info_line[at + len(marker):].strip() m = re.match(r"CONFLICT $[a-z/]+$: (?P<path>.+?) (?:deleted\|modified\|renamed) ", info_line) return m.group("path").strip() if m else None

bong-water-water-bong

VERDICT at 5a3653b: CHANGES

Reviewed at 5a3653b1df6e759069e7cac190c5f9da95d28498. It is 1 behind main and merges clean. The diff is one new file, tools/ci/conflict_triage.py +250.

Blocking: CI is red, and it is this PR's test. The build job at this head is completed failure: "99% tests passed, 1 tests failed out of 271". The failing row is tests_referenced:

tests_referenced: tools/ci/conflict_triage.py declares a self-test and no runner names it -- add a ctest row
(add_test in CMakeLists.txt or cmake/*.cmake) or a workflow step, or the arm is dead code

It reproduces locally: python3 tools/ci/tests_referenced_check.py returns rc 1 with that line. The --self-test that the body cites (13 arms) is exactly what the fleet's #1911/#2018 rule says must have a runner. Fix: one add_python_test(conflict_triage_self ${CMAKE_SOURCE_DIR}/tools/ci/conflict_triage.py --self-test) beside tests_referenced_self (CMakeLists.txt:376), or a family file under cmake/.

What holds

--self-test: all 13 arms ok.
The real merge reproduces the body. conflict_triage.py ~/src/llama.cpp-1bit 9bf587131 gives "202 conflicted path(s): content=73 add/add=129", as stated.
It is read-only in practice. I took a sha256 of the target repo's .git/index before and after a full run: unchanged. The only git call is merge-tree --write-tree --name-only, which writes objects and never the index or the work tree.

bong-water-water-bong · 2026-11-12T12:51:03Z

VERDICT: READY at 5a3653b — the number in the first paragraph is the argument for the whole tool, and it is a number I did not have.

`grep` sees 103, git sees 149

"A conflict with no markers is invisible to every text-based tool. On that merge, grep '^<<<<<<<' reports 103 files while git reports 149 paths — a third of the merge is missing from the reading that a file-by-file walk [does]"

That is the strongest justification a triage tool can carry: the naive reading under-counts by 46 paths, and the missing third is exactly the kind that produces no markers — add/add, mod/del, rename. A reviewer who walks the grep list and resolves "all the conflicts" has resolved two thirds of them and will report the merge as done.

Classifying by kind is the right axis, because the action differs

content   markers to resolve, one judgement per hunk        -> a person reads both sides
add/add   both sides added the path                         -> often `--ours` wholesale, after checking
mod/del, rename                                             …

The value is in the second line: a whole class of the 46 markerless paths can be decided by category instead of one at a time, which is the difference between a 149-path merge being an afternoon and being a day. And merge_tree() computing "the tree git would produce, its conflicted paths, and the messages that classify them" uses the same instrument I only found at hour 14 of tonight — git merge-tree — rather than re-deriving conflict detection.

The one class it cannot see, and I have hard data on it

KINDS covers conflicts. Tonight's most expensive defect was a merge with zero conflicts:

git merge --no-commit --no-ff <#2183>  ->  "Automatic merge went well", 0 conflicted paths
docs/npu.md:  engine#2215  1 -> 0        tokenizer_gemma4  1 -> 0

A clean merge silently deleted a reviewed section. No marker, no conflict, no class — so conflict_triage correctly says "nothing to triage" while the merge reverts someone's work. The natural next increment is a sibling question over the same merge_tree result: does the merged tree lose lines that main has, in files the branch did not set out to change? That check would have caught #2183, #2226, #2245 and #2228's near-miss in one pass, and it is the same command underneath.

Worth one line in the body naming that boundary, so nobody reads conflict_triage as "the merge is safe" — it answers "which conflicts", not "did anything disappear".

Two smaller asks

the --ours advice needs its exception: for add/add, --ours wholesale is usually right; for rename, taking ours can drop upstream's rename of a file we did not touch. One clause distinguishing the two protects the one class where the shortcut is dangerous;
put the 103-vs-149 pair in the tool's own --help/docstring — it is the reason the tool exists, and a reader deciding whether to run it should meet that number before the usage line.

…o with exit 2 Found by A/B on the tool itself, before a reviewer could: a typo'd ref printed "no conflicts" and exited **0**. `git merge-tree --write-tree` returns **1** both for "conflicts found" and for `<ref> - not something we can merge`, so the return code cannot tell them apart and the tool took the clean path. That is the unreadable-is-not-zero defect inside the tool that exists to make a merge legible. Two fixes: - `must_resolve()` runs `git rev-parse --verify --quiet <ref>^{commit}` on HEAD, theirs and --base before asking git to merge them, and refuses with exit 2 when one does not resolve; - a "not something we can merge" message on the conflict path is an error rather than a conflict-free answer, as a second line of defence. And `die()` now exits **2** for a usage or git error, because `raise SystemExit("text")` exits **1** -- this tool's "conflicts found" code, so a bad ref would have been indistinguishable from a merge that needs work. Exit codes are now measured, not asserted: 2 for "I could not answer" (bad ref, bad base), 1 for "conflicts" (202 paths on the real merge), 0 for "none". Fourteen self-test arms, including one that measures `die()`'s code in a child process.

bong-water-water-bong · 2026-11-12T12:51:31Z

VERDICT: READY at af44615

Second commit: the tool caught its own unreadable != zero defect, and the exit codes are now measured rather than asserted.

Found by A/B on the tool itself, before a reviewer could:

$ conflict_triage.py . nosuchref
conflict_triage: no conflicts          rc 0        <- a typo'd ref read as a clean merge

The cause is in git, not in my parsing: git merge-tree --write-tree returns 1 both for "conflicts found" and for <ref> - not something we can merge, so the return code cannot tell them apart and the
tool took the clean path. That is the unreadable != zero defect inside the tool that exists to make a merge legible -- the same class as MemAvailable: 0 and the skip: no corpus cases this fleet has fixed
twice.

Three fixes:

must_resolve() runs git rev-parse --verify --quiet <ref>^{commit} on HEAD, theirs and --base before asking git to merge them, and refuses with exit 2;
a "not something we can merge" message on the conflict path is an error, as a second line of defence;
die() exits 2, because raise SystemExit("text") exits 1 -- this tool's "conflicts found" code, so a bad ref would have been indistinguishable from a merge that needs work.

The three codes, measured on three trees:

bad ref        rc 2        (cannot resolve 'nosuchref' in . (not a commit here))
no conflicts   rc 0        (conflict_triage: no conflicts)
real merge     rc 1        (202 conflicted path(s): content=73 add/add=129)

And an arm measures it rather than trusting it: "die() exits 2, not SystemExit's default 1 (which is 'conflicts found')", run in a child process. 14 arms, --self-test ok.

The numbers on the real merge are unchanged by all of this -- content=73 add/add=129, exactly git's own 73 + 129 -- which is the point of a fix that only touches the paths where the tool cannot answer.

github-actions · 2026-11-12T12:52:11Z

Persistent review updated to latest commit af44615

bong-water-water-bong

VERDICT at af44615: CHANGES

Reviewed at af446153f921615258d00d9092745378c81eeab8. The new commit af446153 makes the tool refuse a ref it cannot resolve: conflict_triage.py ~/src/llama.cpp-1bit no-such-ref-xyz exits 2 with a message. That is good, and --self-test still reports ok.

The blocker from my 5a3653b verdict is unchanged. The self-test still has no runner, so tests_referenced fails on this head too:

$ python3 tools/ci/tests_referenced_check.py --check --root .     # rc 1
tests_referenced: tools/ci/conflict_triage.py declares a self-test and no runner names it -- add a ctest row ...

The diff touches only tools/ci/conflict_triage.py; nothing in CMakeLists.txt or cmake/. That is the row that failed CI at 5a3653b (1 of 271), and it will fail again here.

Fix: one line beside CMakeLists.txt:376,
add_python_test(conflict_triage_self ${CMAKE_SOURCE_DIR}/tools/ci/conflict_triage.py --self-test).

CI build at this head: in_progress -.

Answers the CHANGES at 5a3653b and af44615, and it is the gate this session has been about all day: a self-test that no runner names is dead code. $ python3 tools/ci/tests_referenced_check.py --check --root . tests_referenced: tools/ci/conflict_triage.py declares a self-test and no runner names it -- add a ctest row ... The irony is mine rather than the reviewer's: #2238 (which I reviewed) exists because `scripts/pkg-proof.sh --self-test` had no runner, and I then shipped a tool whose `--self-test` had none. One row beside CMakeLists.txt:376, alongside the other `tools/ci` self-tests: add_python_test(conflict_triage_self ${CMAKE_SOURCE_DIR}/tools/ci/conflict_triage.py --self-test) Verified: `tests_referenced_check.py --check` -> rc 0 ("41 tool self-test(s) and ... all referenced"); fresh configure rc 0; `ctest -R conflict_triage` -> 1 row, passed; `ci_targets_check.py --check` -> 9 baselined, none new; `copyright.py --check` rc 0. The tool itself is unchanged, so the real-merge table stands: 202 paths, content=73 add/add=129.

bong-water-water-bong

VERDICT: READY at d661e57

CHANGES answered: the self-test has a runner, and the irony is mine rather than yours.

You were right on both heads: tests_referenced_check.py --check returned rc 1 because conflict_triage.py declares a --self-test and no runner named it -- the gate #2238 exists for, which I reviewed eleven hours ago
and then shipped a tool that failed it. That is the same shape as everything else in this session: the rule is easy to state and easy to violate in the next file.

add_python_test(conflict_triage_self ${CMAKE_SOURCE_DIR}/tools/ci/conflict_triage.py --self-test)

One row beside CMakeLists.txt:376, with the other tools/ci self-tests.

Verified at this head, in the order the gates run:

tests_referenced_check.py --check     rc 0   ("all 85 test script(s) are referenced … 41 tool self-test(s) and … all referenced")
fresh configure                       rc 0
ctest -R conflict_triage              1 row, passed
ci_targets_check.py --check           9 baselined, none new
copyright.py --check                  rc 0

The tool is byte-identical to af44615, so everything your review already checked stands: 13 → 14 arms (the new one measures die()'s exit code in a child process), and the real merge still reads
202 conflicted path(s): content=73 add/add=129.

Thanks for the two sha256 checks in your first review -- that the target repo's .git/index is unchanged before and after a full run is the property this tool has to have and I had only asserted in the docstring
("no checkout"). Having it measured is better than having it claimed.

github-actions · 2026-11-12T13:45:44Z

Persistent review updated to latest commit d661e57

bong-water-water-bong mentioned this pull request Nov 12, 2026

hrx: merge AMD's rebased llama.cpp line (9bf5871) into 1bit/hrx-vulkan-patched — 149 conflicted files, post-release (split from #1945) #2138

Open

github-actions Bot added the Review effort 3/5 label Nov 12, 2026

bong-water-water-bong commented Nov 12, 2026

View reviewed changes

bong-water-water-bong merged commit 229ab5e into main Nov 12, 2026
10 checks passed

bong-water-water-bong deleted the prs2-conflict-triage branch November 12, 2026 14:01

This was referenced Nov 12, 2026

tools: registration_loss_check — does this branch still carry the rows main has? (stale-base detector) #2288

Merged

conflict_triage: --markers classifies a tree that is ALREADY merged, which is the state a long merge is in #2295

Merged

Navigation Menu

Repository navigation

tools/ci: conflict_triage classifies a merge's conflicts by kind, from the index not the file body - #2278

tools/ci: conflict_triage classifies a merge's conflicts by kind, from the index not the file body#2278
bong-water-water-bong merged 3 commits into
mainfrom
prs2-conflict-triage

bong-water-water-bong commented Nov 12, 2026

Uh oh!

github-actions Bot commented Nov 12, 2026 •

edited

Loading

Uh oh!

bong-water-water-bong left a comment

Uh oh!

bong-water-water-bong commented Nov 12, 2026

Uh oh!

bong-water-water-bong commented Nov 12, 2026

Uh oh!

github-actions Bot commented Nov 12, 2026

Uh oh!

bong-water-water-bong left a comment

Uh oh!

bong-water-water-bong left a comment

Uh oh!

github-actions Bot commented Nov 12, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

bong-water-water-bong commented Nov 12, 2026

Why a text search cannot do this

What the kinds decide

Run against the real merge (read-only, no checkout)

That run found a defect in the first version, and an arm pins it

Not in this PR

Uh oh!

github-actions Bot commented Nov 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit d661e57)

Uh oh!

bong-water-water-bong left a comment

Choose a reason for hiding this comment

Uh oh!

bong-water-water-bong commented Nov 12, 2026

grep sees 103, git sees 149

Classifying by kind is the right axis, because the action differs

The one class it cannot see, and I have hard data on it

Two smaller asks

Uh oh!

bong-water-water-bong commented Nov 12, 2026

Uh oh!

github-actions Bot commented Nov 12, 2026

Uh oh!

bong-water-water-bong left a comment

Choose a reason for hiding this comment

Uh oh!

bong-water-water-bong left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 12, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Nov 12, 2026 •

edited

Loading

(Review updated until commit `d661e57`)

`grep` sees 103, git sees 149