classify-conflict.py advises KEEP BOTH for the same doc line at two revisions, and the remedy duplicates it #1183

Activity

1. how2how2how2-arch commented on Sep 13, 2026

   

   how2how2how2-arch

   on Sep 13, 2026

   Collaborator

   More actions

   I ran the differential this issue asks for on 409 real conflict blocks, and implemented the candidate rule. Result: it changes exactly 2 of them (disjoint → overlapping), both are genuinely this shape, no code block is affected, and the existing suite stays green — which is also the finding, because the suite is green in the defect state too.

   Contributor measurement work from a separate checkout (read-only instance; no PR possible here, so the patch is inline below and verified).

   Reproduction of this issue's block, from the real trees

   #1182's pre-rebase head is 3b5f72c9, and the merge you re-applied was onto c9a7d8a, so a real three-way merge of those two reproduces the block exactly (git merge-file on Agent.md, merge-base 3f5889eb):

   block: ours = 3 line(s), theirs = 1 line(s)
     shipped   : disjoint   "the two sides share no content line - KEEP BOTH (concatenate)"
     candidate : overlapping
     evidence  : 1093 vs 1280 chars, common prefix 1091, common suffix 2  (delta 187)
     concatenation line count: 4   <- from 3 + 1, i.e. one line duplicated
   

   The prefix 1091 and the delta 187 are the same numbers as the offsets in this issue, measured from the two real Agent.md revisions rather than restated. Under the shipped rule this block reaches rc 0 — the verdict, not a warning.

   The differential (the gate the issue asks to be measured)

   Corpus: 409 real blocks — 10 from git merge-file on the live queue (every open PR that conflicts with master, all paths), plus 399 from the last 400 commits touching Agent.md, built from each hunk's pre/post images (the method the count-revision docstring cites). Classified through the real classify() entry point, so _content_lines normalisation applies.

   shipped    : overlapping 68   disjoint 11   duplicate 1   count-line 329
   candidate  : overlapping 70   disjoint  9   duplicate 1   count-line 329
   changes    : 2, both disjoint -> overlapping
   

   Both changed blocks are the same shape as this issue, and both go through the no-shared-line fallback (no shared line, no declared symbols):

   history:166b5394:Agent.md
     ours  : '# Additional models for /model switching (optional)'                  (51)
     theirs: '# Additional models for /model switching (optional — add or remove as…' (77)
     evidence: prefix 50, suffix 1     -> a contiguous insertion before the final ')'
   
   history:d1963e41:Agent.md
     ours  : '  - SIGWINCH handler for terminal resize'                             (40)
     theirs: '  - SIGWINCH handler for real-time terminal resize'                   (50)
     evidence: prefix 25, suffix 16    -> 'real-time ' inserted mid-line
   

   For both, KEEP BOTH concatenates a stale and a current copy of the same documentation line — the same failure as this issue's block, found twice more in history. That is the strongest argument for the generalisation: this is not a one-off.

   Code is not affected: 0 of the changed blocks are in .py/.js/.ts/.sh files. The rule only runs in the no-shared-line fallback, and symbol-declaring hunks take the symbol path before it.

   The candidate rule as specified

   Exactly the issue's formulation — maximal common prefix p, maximal common suffix s, require a[:p] + a[len(a)-s:] == b (or symmetrically). Verified in both directions on the patched module:

   prefix case (the shipped rule's case):  _looks_like_a_revision(['abc\n'], ['abcdef\n'])  -> True
   unrelated lines:                        _looks_like_a_revision(['hello world\n'], ['goodbye moon\n']) -> False
   this issue's shape:                     classify(...) -> overlapping
   

   The bit that matters more than the rule: nothing pins it today

   tests/test_classify_conflict.py, shipped rule   : 55 passed, 2 skipped
   tests/test_classify_conflict.py, candidate rule : 55 passed, 2 skipped   <- identical
   

   So the candidate rule is compatible, and — separately — the suite is green in the defect state, which is why this stayed green. Whatever rule lands, the test has to assert a block of this shape; the mutation check is that reverting the predicate turns that assertion red (measured: identical block, disjoint under the shipped predicate, overlapping under the candidate one).

   Full suite with the patch applied: 1631 passed, 7 skipped (7 deselected — CRLF/conflict-marker tests and the two git ls-files-based doc tests need a real .git, which a git ls-tree-materialised tree does not have; identical on the unpatched tree, so not caused by the rule).

   One dead end, reported so it isn't re-explored

   Reading the rule literally, _one_contiguous_edit('', b) is True for any b (b[:0] + b[len(b):] == ''), so I expected an over-fire on blocks whose only pair-relationship is "blank line vs non-blank line", and started writing a non-blank guard. Measured: not needed — _content_lines already drops blank lines and rstrips before the rule ever sees them, and a constructed block that relates only via a blank line classifies disjoint under shipped, literal and guarded alike. So the guard would be redundant code. (My first differential printout showed len 0 vs 50 pairs; that was my harness re-splitting the raw block text, not the classifier's input.)

   Ready-to-apply patch

   Mechanical change only; keep the existing docstring and append this issue's measurement to it rather than replacing it with the one-liner below.

   --- a/scripts/classify-conflict.py
   +++ b/scripts/classify-conflict.py
   @@ -231,32 +231,40 @@
   +def _common_prefix(a: str, b: str) -> int:
   +    n, i = min(len(a), len(b)), 0
   +    while i < n and a[i] == b[i]:
   +        i += 1
   +    return i
   +
   +
   +def _common_suffix(a: str, b: str) -> int:
   +    n, i = min(len(a), len(b)), 0
   +    while i < n and a[len(a) - 1 - i] == b[len(b) - 1 - i]:
   +        i += 1
   +    return i
   +
   +
   +def _one_contiguous_edit(a: str, b: str) -> bool:
   +    """True when b is a with exactly one contiguous block deleted, or vice versa.
   +
   +    A prefix relation (one side continues the other) is the special case where the
   +    edit sits at the end; #1183 is the general case, where the inserted sentence is
   +    mid-line, so neither side is a prefix of the other.
   +    """
   +    if a == b:
   +        return False
   +    p, s = _common_prefix(a, b), _common_suffix(a, b)
   +    if p + s >= min(len(a), len(b)):
   +        return True
   +    return a[:p] + a[len(a) - s:] == b or b[:p] + b[len(b) - s:] == a
   +
   +
    def _looks_like_a_revision(ours: list[str], theirs: list[str]) -> bool:
        for a in ours:
            for b in theirs:
   -            if a != b and (b.startswith(a) or a.startswith(b)):
   +            if a != b and _one_contiguous_edit(a, b):
                    return True
        return False

   No verdict from me — the merge call is yours.
2. argszero commented on Sep 13, 2026

   

   argszero

   on Sep 13, 2026

   OwnerAuthor

   More actions

   Fixed in #1189 — the predicate now asks for exactly one contiguous insertion/deletion anywhere (maximal common prefix/suffix stripped, bounded so the middles cannot overlap, one middle required to be empty), with the old prefix relation retained as the empty-suffix case. It escalates to overlapping rather than picking a side, per the asymmetry this file already records.

   Measured as the issue asked for, on a differential over real merges rather than fixtures: merging the real heads of the recent PR queue into the current master in a scratch worktree (16 of them conflict) yields 67 genuine conflict blocks, and the candidate rule moves 4 of 67 — all disjoint -> overlapping, none in the other direction. Each of the four was checked as a true positive; the two documented-sentence cases (Merge sequence:, 1280 vs 1093 chars; Release bump:, 782 vs 519) repeat a line head when concatenated, i.e. the state tests/test_doc_counts.py::_duplicated_count_line_kinds rejects. The other two are a code comment and a wrapped prose fragment, the latter being the deletion direction (ours = theirs minus a run).

   False-escalation bound on a negative corpus of 200,000 pairs of unrelated real content lines: the old prefix rule fires on 1 pair, the generalised one on 10 (0.005%), all of them rename/indentation pairs.

   Mutation-verified in the defect state before trusting the verdict: reverting to prefix-only turns 3 new tests red; degrading the rule to "the common prefix/suffix is long enough" turns the negative control red (so the rule cannot drift into "the lines look alike"). The exact live block from this issue now prints block 1/1 -> overlapping with exit=1.
3. argszero

   added 2 commits that reference this issue on Sep 13, 2026

   emrg: rebuild #1183's branch on master, resolving the Agent.md confli…

   ...

   509a3a0

   emrg: classify a mid-line revision as a revision, not a disjoint addi…

   ...

   9b755ec
4. argszero

   closed this as completedin #1189on Sep 13, 2026