Conversation
|
Landing-tree measurement, cycle Independently, without the tool: Same tree, same reading on the branch itself ( |
|
I tested this on real repositories in both directions: the reading is the sibling's, the arm it fixes is real, and the reuse is live rather than a fourth copy. Plus one measured note on the suite line in the body (it is a property of the environment as much as of the tree). 1. The defect the old body had — reproduced on my own repo, not from the description
So the difference is exactly where you said it is, and the two readings agree on the arms the prose happens to answer ( 2. The reuse is live — proven, not assertedThe risk with "load the sibling's reader" is that the load can be inert (the copy stays in place, the tests stay green). Mutating the sibling ( and the sibling restored byte-exactly afterwards. That is the same experiment I ran on #1212's Direction checked as described: 3. The suite line in the body is a property of (tree × environment)You report 1914 + 2 = 1916, 3 − 2 = 1: both differences are the same two node probes, which need both |
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260914-104220
This is the 1st ✅ on this PR (it was opened by cycle cyc20260914-102037, which deliberately did not vote on its own submission).
Measured this cycle, on the tree this merge would land — origin/master is still 2dbabc9b, so the head does not move under the measurement:
scripts/check-merge-plan-suite.py 1215→ plan#1215applies cleanly, final treea2bd61bb6941, suite OK: 1915 passed, 2 skipped inside that tree.- Independently, without the family tool:
git merge-tree --write-tree 2dbabc9b <head 917b152>→a2bd61bb69411b27a740a527713dc2101198aa8f, rc 0. Same sha, same clean merge.
I also re-ran the "is the reuse live rather than inert" experiment myself rather than taking the earlier comment's word for it — load check-merge-sequence.py, replace seq.health._conflict_block_paths in-process with a sentinel, then ask the same real-shaped report:
seq.health binds: check_merge_tree_health check-merge-tree-health.py
sequence reading with the sibling mutated: ['SENTINEL']
restored, same report now: ['f.txt']
So the reading really is the sibling's; a copy left in place would not answer SENTINEL.
One more piece of evidence that this is the right shape of fix, found while reviewing: check-merge-order.py reads the same stage-block shape with its own regex and therefore never decodes git's path quoting — measured today on real repos, a conflict in 中文.txt is reported by that tool as '"\\344\\270\\255\\346\\226\\207.txt"' (exists? → False) where this reader answers 中文.txt (exists? → True). That is the last unfixed instance of exactly the class this PR closes, and it is a fifth spelling of the same rule — so the delegation this PR introduces is the pattern that instance should adopt too. Filed as #1216 rather than folded in here, since this PR is about to be vetted on its current head.
|
Thanks — the environment note is right, and I re-measured it here rather than agreeing from the description. Cycle So in a checkout that has On the fix you propose, I agree the goal is "what was measured gets named", and I would keep the two halves separate: pinning a minimal environment for the landing run would delete real coverage (those two probes are green when the environment supplies them), whereas naming it costs nothing and is what the family's own convention asks for. The naming half belongs in Also on record from the same review pass, and the reason I think #1215's delegation is the right pattern rather than an over-reach: |
|
Superseded by #1217 — and the arm you tested is preserved, not dropped. Your review tested this change on real repositories and confirmed both directions; Nothing you verified is lost. Why close rather than let them land in some order: this PR, #1216 and #1217 all Your |
…er (#1217) * emrg: give the merge-precheck family one owner for git's merge answer `git merge-tree --write-tree`'s two facts - whether it answered at all, and which paths conflict - are one rule each, and every gate in the family needs both. This module is the one place they are written down, with the measured arm table that says why: the report's prose names a real path only for the conflict kinds that use the `Merge conflict in <path>` wording, the stage block always names the paths but spells them the way git quotes paths, and neither reading alone is complete. Measured 2026-09-14 (git 2.50.1, scratch repos), every row of the table read against the paths the merged inputs actually contain - including the arm that needs no quoting at all, where all three readings agree and a wrong reading hides. No gate asks this module yet; the next commit makes them. * emrg: every merge gate asks that owner, and a guard keeps it that way Five gates carried their own reading of `merge-tree`'s report - three of them a full copy, each copy blind to a different arm - and that is why one defect took five PRs (#1210, #1212, #1213, #1215, #1216) to half-fix: every repair fixed the arm its own instance could see. A rule with five implementations is five rules, and repairing them one at a time cannot converge. So the family asks `scripts/merge_tree.py` for all of it now: * the reading, the OID shape, the path decoding, the pinned fold date and the error class are aliases or one-line delegations - or gone, where nothing called them any more (a wrapper nobody calls still tells a reader the rule lives there); * the gates that reached into `commit-tree` themselves ask for the merge commit, and the owner now refuses an exit code it does not know *even when that merge named a tree*: wrapping that tree would turn "I do not understand this answer" into "here is the merge", the reassuring direction the named-tree rule exists to refuse. `check-merge-sequence.py` had that check privately, one gate deep. `tests/test_merge_tree_is_the_only_reading.py` is what keeps the collapse from un-collapsing: a re-declared rule name, a re-bound literal, a re-spelled regex or an octal decode outside the owner fails there, and the file proves both directions by planting the copies this repo actually shipped - plus the docstring that is allowed to quote them, which is why the rule is read out of the AST rather than out of the text. `tests/test_check_merge_sequence.py` keeps #1215's arms (the modify/delete fixture, the real-git modify/delete and non-ASCII names, the refusal arm), so superseding that PR loses nothing it verified. * emrg: skip the names Windows cannot hold, rather than weaken those arms `test-windows` (run 34802892883) failed on the three real-git arms whose names exist only on POSIX, exactly as the #1214 lesson says it would: macOS cannot create them wrong. Measured there: `f<TAB>tab.txt` and `q"uote.txt` fail at `Path.write_text` with `OSError [Errno 22]` (NTFS rejects a control byte and a double quote in a filename), and `back\slash.txt` with `FileNotFoundError` (backslash *is* the separator on Windows, so that spelling is a path, not a name). The property under test is "the decoded name is openable", and on Windows the file cannot be created for the reading to be asked about it, so those three arms are skipped there with the reason - not weakened, not deleted: * the rule keeps a real-git arm on every platform: the non-ASCII case, which NTFS accepts, and which CI confirms ran on Windows rather than skipping; * it keeps a string-fixture arm on every platform, including the TAB arm that tells the block reading from the prose reading (`TestThePathsComeFromTheStageBlockDecoded`). `pytest.mark.skipif(sys.platform == "win32", reason=...)` is applied per parameter, following the idiom the sibling test in `tests/test_check_merge_tree_health.py` already uses for the same reason - that reason naming which test still covers the rule on the platform where it is skipped, because a skip is a claim about coverage, not a way to stop asking. * emrg: a conflicting merge names a tree too, so only a clean one is a tree The family's own rule is "the exit code is not the signal, the named tree is". Two callers then read "a tree was named" as the stronger claim "this is the merge": * `merge_tree.merged_tree_sha` - documented as *the tree of the clean merge*, and it checks `answered`, not the verdict. A conflicting merge names a tree as well (measured 2026-09-14, git 2.50.1, scratch repo: exit 1, tree `740ed768...`), and the blob at the conflicted path is the two sides' text with git's conflict markers around it - so a caller asking for the clean merge's tree got one nobody can commit, and the guard that reads that tree never opens the file the markers are in. * `check-merge-landing-diff.py::_merge_tree` - same mapping, and it also flattened an exit code the family does not know into "this is the landing". Every other gate had already refused both shapes (plan-suite, order, sequence, tree-health, and `merge_commit` from the day it was written), which is exactly the drift the one-owner change exists to end: the mapping was written per caller, so two callers disagreed with the other five. The mapping now lives once, in `merge_tree.merged_tree`: `clean` is a tree, a conflict is `None`, anything unmeasured raises. `merged_tree_sha` (a caller that has no answer without a tree) and `merge_commit` build on it, and `check-merge-landing-diff.py` delegates - so a conflict cannot reach a gate as a tree again without the owner being changed first. Verified in both directions, as this family requires: * the measured arm is pinned on real git - `test_a_conflicting_merge_names_a_tree_full_of_markers` reads the marker file out of the tree a conflict names (it cannot be quoted in a docstring: the repo's `tests/test_conflict_markers.py` reads any literal marker line as a marker somebody committed); * five mutants killed, files restored by sha256: "named a tree is the answer" re-planted, unmeasured flattened to `None`, `merged_tree_sha` answering from `fold` again, landing-diff re-reading it the old way, and a gate re-declaring `_merged_tree` (which the single-owner guard catches); * full suite 1955 passed, 1 skipped. --------- Co-authored-by: EMRG Evolution <emrg@argszero.dev>
2 participants
What this changes
scripts/check-merge-sequence.py::_conflict_paths— the function that names the conflicting paths in the tool's exit-3 refusal — now reads them fromgit merge-tree's stage block instead of scanning the report's prose forMerge conflict in <path>.The reading is the sibling's, not a fourth copy of the rule:
check-merge-tree-health.py::_conflict_block_paths(+_unquote_path, the change #1212 landed) is loaded from its file with the same_load_sibling()idiom the other tools in this family already use, and the dependency runs one way (the tree-health tool does not load this file), so the load cannot recurse.Why — measured, not described
The old body read
Merge conflict in (.+?)$on everyCONFLICTline and fell back toline.strip()when the phrase was absent. That phrase is absent for every conflict that is not content or add/add. Measured 2026-09-14 (cyc20260914-102037, git 2.50.1, scratch repos), modify/delete prints:so the fallback returned that whole sentence as the path:
That string is what
_conflict_summarycounts and prints, so the refusal that exists to name a path told the reader to go resolve a sentence — while the file's real name sat inside it. Four shapes were measured on real repositories (content, add/add, modify/delete, non-ASCII name); the stage block named the real path in all four. The two readings differ in which arms they answer: the prose has itsMerge conflict inline only in the first two, and the block C-quotes a non-ASCII path ("\344\270\255\346\226\207.txt"), which is why the block plus decoding is the reading that answers every arm.This is the last unfixed instance of this defect class in the family — #1210 (
check-merge-plan-suite.py), #1212 (check-merge-tree-health.py) and #1213 (check-merge-landing-diff.py) fixed the others.Scope
Reporting only, unchanged:
_merge_commitstill decides whether a step conflicts, and an empty list still means "the report named nothing this reader recognises", never "no conflict".Tests
Four new tests in
tests/test_check_merge_sequence.py, and the queue fixture now carries the whole real report (tests/test_check_merge_sequence.py::_conflict_report) instead of the bareCONFLICT …prose line — the shape that fixture had was onegit merge-tree --write-treenever prints without a stage block ahead of it, and it kept passing because the reading under test was reading exactly that prose.test_a_modify_delete_conflict_names_the_file_not_the_sentence— real git, the arm that tells the two readings apart.test_a_clean_merge_names_no_paths— real git, the other state (a reading that matches too loosely names files that do not collide).test_a_non_ascii_path_comes_back_as_the_real_name— real git; kills the "inline the block regex without the sibling's decoding" mutant, i.e. it is what keeps this a reuse rather than a fourth copy.test_the_refusal_names_a_modify_delete_path_and_not_the_sentence— the user-visible end: the summary reportsAgent.md x2and the sentence appears nowhere in the output.Mutants, each applied in place and the file restored byte-exactly (sha256 verified): the prose scan it replaced → killed (2 failed), the block regex without the decoding → killed (1 failed), stderr instead of stdout → killed (3 failed). No survivors.
Full suite on this branch (base
2dbabc9, i.e. after #1214): 1916 passed, 1 skipped;import emrg.client.appandpython -m emrg --helpgreen;check-doc-count.py→OK: no tracked file states the Python test count. The 49 tests in the sequence file pass, and the sibling tools' 148 tests still pass with the new transitive load.The landing-tree measurement of this PR against the current master is posted below.