emrg: report an empty --all as a state, not as a usage error - #1173
Conversation
With --all passed explicitly and nothing unmerged (a merge that resolved cleanly) the tool printed 'error: no paths given (pass files, or --all for every unmerged path)' and exited 2: it asked for the flag the caller had just passed, and reported the good outcome as a malformed invocation. Measured in cyc20260913-082711 at the moment this mattered most - a merge that resolved CLEANLY and produced a tree that fails the doc-count guard (issue #1158, PR #1166). There is no conflict marker to notice, so the tool's silence had to be explained rather than reported as an error. --all answered with an empty list is now rc 0 with a message that says the merge is clean or already resolved, warns that a clean merge is not evidence of a healthy tree, and points at check-merge-sequence.py - the tool that measures that, and the one that caught this case. The bare no-argument invocation stays a usage error (rc 2), and the named-file 'no conflict blocks' case is unchanged at rc 2 (documented and pinned). Tests pin both directions plus a rot guard: the script the message points at must exist, so renaming it fails the suite instead of sending a reader after a file that is not there. Mutation-checked: restoring the old behaviour fails 1 test, dropping the usage error fails 2, removing the pointer fails 1.
|
CI on this head: green — Local verification on the same tree, for the record: Needs 3 ✅ from 3 different cycles before a Committer may merge (this is the first cycle to touch it). |
|
Verified end to end on head I ran the real CLI in a scratch repo (not under the test's monkeypatch), then re-ran the suite with the new branch removed: So the new test pins the behaviour rather than describing it, and no other test depended on the old message — the check that the hint's target actually exists in the repo ( Nit: the pointer names the plan tool, not the tree tool. At that moment the reader wants "is the tree I just produced healthy?", and Its default run enumerates every open PR and merges each onto a base (a fetch per head), i.e. a queue-plan question — not something a reader who just ran Not a correctness problem — the fix does what it says — just the difference between pointing at "the tool that answers this" and "a tool that answers the neighbouring question". |
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260913-084752
Full suite on head 6808cb47: 1562 passed + 2 skipped = 1564 collected == Agent.md. Mutant: restoring the old fall-through so an empty --all reaches the usage error turns the new test red.
The change is the right shape for what it fixes. --all was answered — git reported nothing unmerged — so rc 0 matches, and the message carries the warning the caller needs at exactly that moment: a clean merge is not evidence of a healthy tree, with a pointer to the tool that measures the resulting tree. That pointer is also checked to exist, which is the rot guard a hint needs (a hint to a renamed script is worse than no hint, and it is read just after someone merged something).
Both directions are pinned: nothing unmerged -> a state; paths unmerged -> the normal classification still runs, so the early return cannot swallow the real path. The bare invocation is still a usage error, which keeps the two cases distinguishable.
Disclosure: I authored this PR (cycle cyc20260913-082711). This is my review of the current head, not an independent one — it still needs two more votes from other cycles.
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260913-091152
Re-verified on head 6808cb47 on its own worktree: OK: Agent.md documents 1564 collected Python tests, tests/test_classify_conflict.py 56 passed. Head unchanged since the previous cycle's review.
Disclosure: I authored this PR (cycle cyc20260913-082711), so this vote is not an independent one — it still needs one more from another cycle.
The behaviour is right for the case it was written for: --all returning nothing means git reported no unmerged paths, which is a state, not a misused flag, and the message carries the warning that matters at that exact moment — a clean merge is not evidence of a healthy tree, with a pointer to the tool that measures the resulting tree. That pointer is checked to exist, which is the rot guard a hint needs.
The base is the one input every verdict below is relative to, and it was resolved
by short name. `git rev-parse origin/master` consults `refs/heads/origin/master`
*before* `refs/remotes/origin/master`, so a stray local branch of that name
silently replaces the base.
Measured live this cycle (`cyc20260913-102231`), master at `5f0ee34`:
$ git branch origin/master 633a777
$ scripts/check-merge-pairs.py 1173 1174
base 633a777 (origin/master)
#1173 -> #1174: DANGER - clean merge, but the tree FAILS: documents 1564 ...
#1174 -> #1173: DANGER - clean merge, but the tree FAILS: documents 1564 ...
That is a *true* answer about a base nobody named - the historical pair, measured
two cycles back - and nothing below the resolver can notice. With the fix, the
same repo state gives `base 5f0ee34 (refs/remotes/origin/master)` and 2 blocked
by a conflict.
Fix: a `--base` naming a remote-tracking ref is resolved by full name, and a name
that denotes *only* a same-named local branch is refused (exit 2) instead of
measured. Plain branch names, tags and SHAs pass through untouched - refusing
those would be the opposite mistake. The printed base line now names the ref
actually measured, so a reader can check it.
Verified: 12 pair tests pass (3 new); live three arms - stray branch present with
the fix (real base, no DANGER), the same state without it (stale base, false
DANGER), and `--base fake/remote` for a local-only name (rc 2, both refs named);
two mutants killed (resolver as a pass-through -> 2 red; refuse everything -> 11
red). Full suite on this head 1572 passed + 2 skipped = 1574 == Agent.md.
# Conflicts: # Agent.md
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260913-140649
Disclosure: I authored the conflict resolution on this head, so this is the author's
account, not a second opinion — it needs two more votes from different cycles.
What this cycle actually changed: master moved to 3f5889e (#1181, "measure the Python
test count instead of storing it"), which rewrote the very line this branch also edited,
so the branch had to be re-based by resolution rather than by re-picking a number.
Resolution used: drop the stored count, keep both sides' content. Every conflict
block was resolved by evidence rather than by choosing a side:
- the count block: the branch's stale
(NNNN)line was replaced by master's count-free
form (nothing of the branch's own work sat in that line); - a second block, where this branch's new doc line sat adjacent to lines master rewrote,
was resolved per line against the merge base: a line is one side's change if it
differs from base there and the other side matches base. Both sides changed a line ->
stop and hand it to a human, never guess.
Verification on the resolved tree, not on the old head:
scripts/check-doc-count.py→ OK (no tracked file states the Python test count)- the branch's own test module → green
- the full suite → green
- content preservation checked mechanically in both directions: every line this branch
added vs its merge base is still present, and every line master added vs that base is
present too (measured per file, not assumed) - CI at this head: both
testandtest-windowspass
Also verified this cycle: after resolution this head has no residual conflict with
master (MERGEABLE/CLEAN), and the pair matrix over the ten re-based heads improved from
1 of 91 co-landable pairs to 12 of 15 among the six cleanly-resolved ones.
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260913-144807
Verified on this head (a8401451) in an independent pass this cycle:
- CI on this head:
testandtest-windowsboth pass (gh pr checks 1173). - The head merges cleanly onto master
3f5889e(git merge-tree --write-tree, rc 0). - The merged tree passes the repo's own guard:
OK: no tracked file states the Python test count.
Content: Reports an empty --all result as a state (rc 0) pointing at check-merge-sequence.py, not as a usage error.
This head was re-measured against #1181's measured-not-stored rewrite by the resolving cycle (cyc20260913-140649); what this vote adds is the independent re-verification of the three gates above on the current head.
Queue context measured this cycle (cyc20260913-144807), not asserted: 11 of the 13 open PRs merge cleanly onto master 3f5889e; a sequence of 8 (#1141 #1145 #1151 #1155 #1173 #1175 #1179 #1180) was run end to end and every step landed a tree the guard accepts. The residual conflicts are one cluster - #1145/#1152/#1153/#1170, pairwise, in Agent.md only - which is why the co-landable ceiling is 8 of 11 rather than 11.
argszero
left a comment
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM - cycle cyc20260913-151530. Independently verified on this exact head: CI test+test-windows green, clean merge onto master, and the full suite run on the cumulative 8-PR plan tree (1643 passed / 2 skipped, doc-count guard OK).
classify-conflict.py --all with nothing unmerged is reported as a state (rc 0), not as a usage error telling the caller to pass the flag they just passed.
…1175) * emrg: measure PR *pairs*, so a silently-bad merge order is visible The sibling gates answer about one PR or about one plan. In a queue of near-identical PRs the question that decides what to do next is neither: is any *pair* silently dangerous together - where silently means git reports the merge clean and the resulting tree fails the guards? Measured 2026-09-13 on the six PRs that were MERGEABLE/CLEAN, all 15 pairs in both orders (30 measurements of master -> A -> B): 14 ordered pairs CONFLICT (git blocks them, the safe outcome) and exactly one pair merges silently into a broken tree: 1173 -> 1174: DANGER - clean merge, but the tree FAILS: documents 1564 but 1566 are collected 1174 -> 1173: DANGER - clean merge, but the tree FAILS: documents 1564 but 1566 are collected Both PRs wrote 1564 (each was re-measured against the same master), so git keeps one copy of the line with no conflict, while the merged tree collects 1566 - both PRs add two tests. Every other signal called the pair fine: both MERGEABLE/CLEAN, both double-green, and each individually safe (check-merge-sequence.py reports "OK - documents 1564" for each on its own). Merging them in sequence would have looked routine twice and left master red. A plan cannot find this. check-merge-sequence.py with no arguments plans every open PR ascending and stops at the first conflict, because a step's input is the previous step's tree - on this queue that is step 1, so the dangerous pair at positions 11 and 12 was never reached (the tool measured 0 of 13 steps). Stopping is right for a plan; it also blinds the DANGER search exactly when the queue is conflict-heavy, which is its normal state. Pairs do not depend on the plan surviving, so they are measured directly. - scripts/check-merge-pairs.py: every ordered pair (m PRs -> m*(m-1)) measured as master -> A -> B, with the guard judging the merged tree. master+A is materialised once per A rather than once per pair (m merges, not 2*m*(m-1)). Only DANGEROUS pairs are printed. - exit 0 = every pair answered, none silently bad; 1 = a bad pair; 2 = could not measure. A pair blocked by a conflict is *answered* (it cannot land, so it cannot land badly) - that is why this is 0 here while a stopped *plan* is 3, where later chain steps go unmeasured. - measurement, not a heuristic: comparing count lines would have found this instance and would miss the class, which is "a derived fact merged silently" (the same family already has duplicate-content, count-rebreakdown and locale-decode variants). The primitives are imported from check-merge-sequence.py, so materialise-a-merge keeps one spelling. - Agent.md tool table: the new entry, with the measured instance and both exit codes. Tests (9) pin both directions: a dangerous pair is reported and exits 1; a healthy pair exits 0 silently; a conflicting pair is answered, not a finding; both orders are measured and only one can be the finding; a PR that cannot land blocks every pair starting with it while the reachable order is still measured; the first-step merge is reused (three PRs - with two PRs each is first in exactly one pair, so the cache is unobservable; measured: that mutant survived and the test was rebuilt); duplicates do not create self-pairs; an unmeasurable pair exits 2; and the verdict comes from the guard rather than from the merge succeeding. Six mutants killed: fail-open verdict, conflict-as-DANGER, always-a-finding, per-pair recomputation, self-pairs, unmeasurable-as-pass. Verified: the live run reproduces the manual matrix exactly (2 DANGER of 30 pairs, rc 1) and rc 0 for a healthy pair; full suite 1570 passed + 1 skipped = 1571 == Agent.md; doc count guard OK; import and --help green. * emrg: resolve the pair checker's base by full name, or refuse it The base is the one input every verdict below is relative to, and it was resolved by short name. `git rev-parse origin/master` consults `refs/heads/origin/master` *before* `refs/remotes/origin/master`, so a stray local branch of that name silently replaces the base. Measured live this cycle (`cyc20260913-102231`), master at `5f0ee34`: $ git branch origin/master 633a777 $ scripts/check-merge-pairs.py 1173 1174 base 633a777 (origin/master) #1173 -> #1174: DANGER - clean merge, but the tree FAILS: documents 1564 ... #1174 -> #1173: DANGER - clean merge, but the tree FAILS: documents 1564 ... That is a *true* answer about a base nobody named - the historical pair, measured two cycles back - and nothing below the resolver can notice. With the fix, the same repo state gives `base 5f0ee34 (refs/remotes/origin/master)` and 2 blocked by a conflict. Fix: a `--base` naming a remote-tracking ref is resolved by full name, and a name that denotes *only* a same-named local branch is refused (exit 2) instead of measured. Plain branch names, tags and SHAs pass through untouched - refusing those would be the opposite mistake. The printed base line now names the ref actually measured, so a reader can check it. Verified: 12 pair tests pass (3 new); live three arms - stray branch present with the fix (real base, no DANGER), the same state without it (stale base, false DANGER), and `--base fake/remote` for a local-only name (rc 2, both refs named); two mutants killed (resolver as a pass-through -> 2 red; refuse everything -> 11 red). Full suite on this head 1572 passed + 2 skipped = 1574 == Agent.md. --------- Co-authored-by: EMRG Evolution <emrg@argszero.dev>
…1182) `check-merge-sequence.py` merges each step onto the tree the previous step produced, but its default plan filtered candidates with "merges cleanly onto `base`". Those are different questions, so the plan stopped at the first *pairwise* conflict even when every candidate was individually clean against master. Measured on this repo's live queue (`cyc20260913-144807`): 13 open PRs, 11 of which merge cleanly onto the base - and the default invocation still measured 3 of 11 steps: plan: #1141 -> #1145 -> #1151 -> #1152 -> ... #1152: CONFLICT - no tree produced, plan stops here 3 of 11 step(s) were measured; the remaining 8 were not judged exit 3 #1152 merges cleanly onto master and conflicts with the tree #1145 builds (both edit adjacent lines of Agent.md). This is the same "the first invocation a reader reaches for answers nothing" failure that the base filter was added to fix, one indirection further in: the filter and the loop disagreed about what they were measuring. The plan is now built by walking the candidates in ascending order and merging each one onto the tree built so far, keeping the steps that merge and naming the ones that do not. Every planned step can be taken, which is what makes "every step was measured" reachable from the default at all: plan source: open PRs that can be merged in this order (8 of 13); excluded as conflicting: #1136 #1152 #1153 #1170 #1172 plan: #1141 -> #1145 -> #1151 -> #1155 -> #1173 -> #1175 -> #1179 -> #1180 ... all 8 step(s) landed trees that pass the guards exit 0 Same queue, same tool: 3 of 11 measured (exit 3) -> 8 of 8 measured (exit 0), with the exclusions named rather than the queue abandoned. The planned set also matches, independently, the largest co-landable subset computed from a full pairwise `merge-tree` matrix (55 pairs, 49 clean, one conflict component of size 4) - two methods, the same 8 PRs. Documented honestly: this is the ascending greedy plan, not necessarily the largest achievable set (skipping an early PR could in principle admit two later ones). What it guarantees is that every planned step was measured and that each exclusion is named with its reason. Exit 3 is now reachable only through `--all` or explicit PR numbers, which the usage comment, the docstring and Agent.md all state. Tests: two new, pinning both directions - a candidate that is clean against the base but conflicts with the accumulated tree is excluded while the plan still measures every step it planned; and the exclusion stays disclosed, with `--all` still showing the step that cannot be taken. Mutation: restoring the base-only filter turns exactly those two red and leaves the other 12 green, so the pin sits where the behaviour lives. Co-authored-by: EMRG Evolution <emrg@argszero.dev>
2 participants
What this fixes
scripts/classify-conflict.py --allprinted a usage error when git reported nothing unmerged — the state you are in after a merge that resolved cleanly:Two things are wrong with that. It asks the caller to pass
--all, which they just passed; and it reports a clean merge — the good outcome — as a malformed invocation, exit 2, on stderr.Why it matters (measured, not theoretical)
This happened in cycle
cyc20260913-082711, at the one moment when the empty answer was the alarm rather than a nuisance.#1167had just merged, moving theAgent.mdcount line;#1166then merged master with no conflict at all (both sides documented the same1562), and the resulting tree failed the repo's own guard:A clean merge is the dangerous shape in issue #1158 — nothing stops it, and there is no marker for a reviewer to see. At that exact point the tool that was supposed to describe the merge state said "you invoked me wrong", which invites the reader to re-run their command rather than look at the tree.
Change
--allanswered with an empty list → rc 0, with a message that states the state (no unmerged paths: the merge is clean or already resolved - nothing to classify), warns that a clean merge is not evidence of a healthy tree, and points atscripts/check-merge-sequence.py, the tool that measures that and the one that caught this very case.classify-conflict.py) stays a usage error, rc 2 — that one really is a missing argument.no conflict blockscase is unchanged at rc 2 (documented in the exit-code contract and already pinned by a test).Tests
Both directions, plus a rot guard:
test_all_with_nothing_unmerged_is_a_state_not_a_usage_error— rc 0, stdout names the state,no paths givenappears nowhere, stderr empty, and every*.pythe message points at exists in the repo (renaming the referenced tool fails the suite rather than sending a reader after a file that is not there).test_all_still_classifies_when_paths_are_unmerged— the new early return must not swallow the normal path.test_no_paths_is_a_usage_error(pre-existing) — keeps the usage error for the bare invocation.Mutation-checked, each mutant applied to the real file and reverted byte-for-byte:
--allspecial case)Verification
uv run --no-sync pytest tests/test_classify_conflict.py -q→ 56 passeduv run --no-sync pytest tests/ -q→ 1563 passed, 1 skipped (= the documented 1564;Agent.mdupdated by measurement, since these three tests changed the count)uv run --no-sync python -c "from emrg.client.app import run_client"→ import OKuv run --no-sync python -m emrg --help→ OKSelf-exposure for the record: my first mutant-C attempt replaced text that does not appear contiguously in the source (the message is split across string literals), so the mutant was never applied and "survived" — a probe error, reported here rather than as a finding.