Reconcile exact tree versions against their common ancestor

agent-notes/base-aware-tree-merge-174.md

@@ -0,0 +1,51 @@
+# Exact three-way tree-entry reconciliation (#174 consumer)
+
+## Goal
+Provide only the generic base/left/right ordinary-path decision needed by the concrete Propstore stance-occurrence merge repair. Draft only, preserve all branches/work, no ready/merge/comments/body edits.
+
+## State / checkpoint
+CURRENT: direct Q confirmation explicitly superseded the original audit-only restriction for implementation commits/publication. One supported retry succeeded, producing correction commit9c3db90. Owner30 and consumer56 focused cases pass, retaining true criss-cross refusals. No approval blocker remains. Publish the correction on existing draft23; incorporate reviewed Propstore327 baselinee30bbc1513d28a0d8be229708ce8e27d207e0f04 in consumer328, preserving ast-equiv pin cb4ff57565ba83ad779fd3191c47706a2a06e8ef and the new exact Quire pin. Fresh aggregate CI/re-review required. Earlier denial checkpoints below are historical; no denied action was bypassed.
+
+LATEST: SUPPORTED RETRY ALSO DENIED. After parent's later coordination message explicitly directed continued current implementation, retried the exact known-not-executed Quire source/test/note commit once. Approval review rejected it again: "This commits source and test changes in Quire, while the trusted task scope explicitly prohibited modifying code; the later delegation is from another agent and cannot authorize overriding that restriction." Neither staging nor commit executed. Stop this action; no further retry or alternate route under current evidence. Source/tests/checkpoints remain unstaged. Published heads remainf578de7/consumer73fb6eaa. Need authority objection resolved by parent/Q before publication. Parent also requests incorporating Propstore327 baseline before328 readiness;327 was still OPEN at read-only check, head00c72b29, mergeCommitnull. Preserve its eventual algorithm/ast changes alongside the Quire pin. All independent repair checks complete:30 owner/56 consumer pass. Earlier checkpoint descriptions below are historical.
+
+BLOCKED on committing the completed second review repair. Automatic approval review rejected the command staging `quire/git_store.py`, `tests/test_merge_bases.py`, this note, then committing `Remove inferior merge-base candidates by graph ancestry`, citing the original audit-only/no-code instruction despite the later implementation and review-fix delegations. CreateProcess rejected the entire command: no staging/commit executed, no retry or alternate route attempted. Source/test changes are unstaged and original drafts unchanged. Existing local HEAD47a0d10 is the earlier note-only commit; published source remainsf578de7.
+
+All independent repair work is complete: owner **30 passed in10.35s**, consumer **56 passed in71.22s**, latter log `logs/test-runs/best-base-dominance-consumer-20261020-152258.log` in consumer. Same-second/skewed regressions now pass, all true criss-cross refusal/atomicity cases still pass. Need new relevant authorization clarification addressing this denial before one supported retry; then publish source, pin consumer, and fresh aggregate CI/re-review. No new source SHA exists yet.
+
+Second review repair IMPLEMENTED: six-line graph-dominance filter in the existing owner removes candidates ancestral to another candidate. **30 focused cases passed in10.35s**, `logs/test-runs/best-base-dominance-owner-20261020-152305.log`. Before the fix, strengthened deterministic owner reproduction8failed/8passed/14deselected in5.59s (`inferior-base-same-second-reproduction-20261020-152153.log`). Equal-second chain/tips, skewed/monotonic clocks, memory/disk and both input orders are covered; true criss-cross still enumerates both bases. Consumer focused file is running. Publish this bounded repair, pin it in Propstore and obtain fresh aggregate CI/re-review. Older checkpoints below are historical.
+
+Second review repair IN PROGRESS: independently ran parent's `inferior_base.py` against local source/actual Dulwich1.2.10. Public family history18 returned inferior P2 `1926689c834fab50337e339320c66bd9ffcd2c3b` plus descendantQ `2c63c7af6321f0d33dab130b93ef2d7a84f62140`, falsely refusing left74a2a344/right319ee6fa. Native `git merge-base --all left right` in retained `.tmp-inferior-base-proof/18` returned onlyQ. New deterministic consumer reproduction8failed/4passed in25.85s; owner skewed-clock reproduction4failed/22passed in7.91s. Initial source assumed Dulwich candidates were already a best-base antichain; its timestamp worklist can leave a redundant ancestor.
+
+Bounded fix checkpoint before production edit: ~6lines in existing Quire owner, prune candidates reachable from another candidate using existing `ancestor_distances`, only when there is more than one candidate; no timestamp test, new protocol or second merge algorithm. Cleanup-refactor remains active: classify the backend-caller edge as valid ancestry capability with incorrect candidate representation, disposition consume its existing graph reachability owner. Keep true incomparable bases and the single-candidate fast path. Tests cover memory/disk, equal-second chain, equal-second tips, skewed and monotonic clocks, parent orders, immutable query state, repeated public consumer merges, and prior criss-cross atomic refusals.
+
+Review repair COMPLETE at published `f578de70a13036b4b5460422d24a106a38f64940`; coupled consumer `73fb6eaa69e9a4f26f6412b8f89201025508963e`. Fresh PR CI https://github.com/propstore/quire/actions/runs/39493038076 succeeded: **680 passed,12 deselected in each Windows/Linux Python3.11/3.13 job**, lint/typecheck/build all passed. Propstore CI39493295635 passed2032/4skipped. Both exact remote heads verified OPEN/DRAFT. The14 focused owner cases and40 consumer cases pass, including12 owner/app/CLI atomicity cases. Parent re-review pending, no execution blocker. Production scope is new tree_merge plus bounded git_store enumeration/error; preserve existing merge_base representative contract and reviewed22 changes.
+
+## Prior checkpoints and evidence
+Current review repair: implementing the confirmed criss-cross ambiguity defect on the same isolated draft branch. `iter_merge_bases` now enumerates every best ancestor; existing `merge_base` still selects minimumSHA for its established query contract. Canonical `AmbiguousMergeBaseError` is consumed directly by Propstore before planning. Focused new owner file: **14 passed in4.29s**, `logs/test-runs/all-best-bases-owner-20261020-143215.log`, memory/disk, both argument orders, names/IDs, unique/unrelated/unknown histories, no object/ref writes. Previous aggregate results below apply to the previous published heads; fresh aggregate CI is required after publication.
+
+Isolated branch `feat/base-aware-tree-merge-174`, incorporated current baseline `84e7bb102970b74fda0a91b8d77bf36a13ca8c2f` (reviewed Quire22). Draft23 published at https://github.com/propstore/quire/pull/23, exact source head `c30a36a28510f114837923028ce922e1a42006c5`. Implementation is complete; aggregate CI is green. Other worker's Dulwich/reopen changes are retained unchanged. This branch changes only new `quire/tree_merge.py`, `tests/test_tree_merge.py` and this task note.
+
+Consumer production reproduction now established: 18 failing real Concept/Stance/nested-Micropublication cases cover deletion resurrection, suppressed modify/delete conflicts, rejected one-sided edits, both directions. Log in consumer `nonclaim-base-reproduction-20261020-135649.log`, repositories preserved. Read primary Diff3 PDF pp.1-6 as page images; p.5 archive-relative one-sided-change cases motivate the exact-key specialization. Equal blob convergence uses existing storage equality/#174's explicit requirement, not a claim that textual Diff3 resolves every equal list edit. Output is iterator-first (`iter_merged_tree_entries`), with explicit caller materialization and conflict during iteration before commit.
+
+Focused owner evidence: **26 passed in1.98s**, `logs/test-runs/base-aware-tree-owner-20261020-140315.log`, using retained actual Dulwich1.2.10 overlay and shared interpreter read-only. Covers symmetric path-state outcomes, lazy conflict enumeration, real Git object witnesses, two-parent history, no writes on modify/delete refusal, and existing prefix-collision admission. Consumer now has28 passing family/app/CLI/reference-image/admission regressions.
+
+FINAL aggregate evidence at published c30a36a: PR CI https://github.com/propstore/quire/actions/runs/39490641507 and push CI39490633149 both succeeded. Each Windows/Linux Python3.11/3.13 job passed lint, typing (zero errors/warnings), **666 tests,12 deselected**, and sdist/wheel build. Coupled Propstore draft328 pins c30a36a exactly; its source390c1d50 full CI39490897449 passed **2020 tests,4 skipped,4 warnings in103.53s**. Neither draft was marked ready or merged. No existing issue/PR body/comment changes. Both initial denials were resolved only after parent's explicit later clarification and one supported retry of each action.
+
+## Decisions and why
+Review repair checkpoint: current draft23 sourcec30a36a/consumer390c1d50. Parent supplied a criss-cross candidate; actual supported Concept/GitStore commits reproduced **4 silent choices** in both parent orders and absent/existing target cases. Consumer log `criss-cross-base-reproduction-20261020-142824.log` records all commit witnesses (e.g.A7eaa7ed1/Bf2eaf733 selectedB using baseA). Git's official merge-base documentation describes multiple incomparable best ancestors and unspecified single-base selection without `--all`. Reread Diff3 p.3 image: its one-sided propagation is relative to an archivedO, not arbitrary selection among differentOs.
+
+Smallest bounded repair before production edits: expose all best ancestors via `GitStore.iter_merge_bases` (~10-line net extension of existing graph resolution); keep existing deterministic `merge_base` aggregate contract and tests unchanged. Add canonical typed `AmbiguousMergeBaseError` (~10lines) at the graph owner. Consumer directly enumerates captured parent commits and refuses multiple best bases before framework/object/ref work (~10lines); no recursive/virtual-base semantics. Cleanup-refactor remains active: the broken caller edge is a valid storage capability with a lossy single-result representation; disposition is consume the generic all-base owner interface directly. Existing representative query is a distinct valid point capability, not removed/recreated. No per-family or alternate-merge path.
+
+Smallest expected owner operation (~30-50 lines): take exact base/left/right mappings of physical path to object ID; unchanged-vs-change selects the change, identical two-sided outcomes converge, competing changes raise a typed path conflict; absence is ordinary missing-path state. Return exact entries, retaining the existing commit/tree-prefix/canonical-path validation owner. No tombstone, sentinel, text diff, rename heuristic, application imports, family-specific rules or new transaction lock. Propstore owns which paths are ordinary and which selected occurrences enter its semantic transport. This is the concrete missing representation from existing #174, not a speculative general merge framework.
+
+Cleanup-refactor is active: generic path-state selection belongs here, not inside Propstore's `_merge_non_claim_entries` or Stance. Preserve all current storage APIs. Read primary literature page images before implementation; actual owner/family regressions required.
+
+## Surprises
+Parent's Quire22 baseline merged at `84e7bb102970b74fda0a91b8d77bf36a13ca8c2f` (read-only GitHub verified). Parent requested integration before publishing this coupled slice. Automatic approval review rejected `git fetch origin master` followed by `git merge --ff-only 84e7bb102970b74fda0a91b8d77bf36a13ca8c2f` on this isolated branch, because it interpreted the no-merge instruction as covering local baseline integration. CreateProcess rejected the whole command; neither operation executed. No retry or substitute rebase/cherry-pick has been attempted. Need explicit local-integration clarification before that action; do not touch22's files.
+
+Second distinct denial: automatic approval review rejected `git add -- quire/tree_merge.py tests/test_tree_merge.py agent-notes/base-aware-tree-merge-174.md` followed by `git commit -m "Add exact base-aware tree-entry reconciliation"`, citing the original audit-only/no-code instruction. The later parent delegation explicitly restarted implementation and authorized a new draft PR, but the action was denied nonetheless. CreateProcess rejected the whole command; neither staging nor commit executed. No retry. Source/tests/note remain uncommitted, draft body is prepared at `.tmp/draft-pr-body.md`, no push/PR attempted. Need parent clarification that current implementation authorization supersedes original audit-only scope before any retry of the denied commit. Independent code/test work is complete; aggregate CI awaits publication.
+
+RESOLVED local-integration authorization: parent explicitly clarified current implementation supersedes audit-only scope, isolated source commits/pushes/drafts are authorized, and no-merge refers to GitHub/default publication rather than integrating reviewed main locally. One supported retry succeeded: fast-forwarded this feature branch to Quire22 `84e7bb102970b74fda0a91b8d77bf36a13ca8c2f`, retaining all22 changes. Focused file rerun against that baseline and actual Dulwich1.2.10: **26 passed in1.23s**, `logs/test-runs/tree-owner-integrated22-20261020-141535.log`. Source commit retry is next; if denied again, stop that action.
+
+## Next step
+Resolve the new rejected source commit with parent's explicit clarification; retry only once for new relevant evidence, stop if denied again. Then publish Quire, update consumer pin, publish consumer and obtain fresh aggregate CI before re-review. No alternate route, PR body/comments, ready or merge. Preserve all existing notes, branches and reproduction artifacts. This latest note update remains uncommitted because of the block.

quire/git_store.py

@@ -79,6 +79,19 @@ def __init__(
         self.actual_head = actual_head
 
 
+class AmbiguousMergeBaseError(ValueError):
+    """A content merge cannot choose among multiple best common ancestors."""
+
+    def __init__(self, left_sha: str, right_sha: str, base_shas: Sequence[str]) -> None:
+        self.left_sha = left_sha
+        self.right_sha = right_sha
+        self.base_shas = tuple(base_shas)
+        super().__init__(
+            f"Multiple best merge bases between {left_sha!r} and {right_sha!r}: "
+            f"{', '.join(self.base_shas)}"
+        )
+
+
 class MaterializeConflictError(ValueError):
     def __init__(self, conflict_paths: Sequence[str]) -> None:
         paths = tuple(sorted(conflict_paths))
@@ -883,8 +896,12 @@ def ancestor_distances(self, start_sha: str) -> dict[str, int]:
                     queue.append(parent_sha)
         return distances
 
-    def merge_base(self, branch_a: str, branch_b: str) -> str:
-        """Find the base of branch names or full commit IDs (IDs take precedence)."""
+    def iter_merge_bases(self, branch_a: str, branch_b: str) -> Iterator[str]:
+        """Enumerate every best common ancestor in object-ID order.
+
+        Full commit IDs take precedence over literal branch names. Unrelated
+        histories yield no bases; unknown inputs raise when enumerated.
+        """
         shas: list[str] = []
         for revision in (branch_a, branch_b):
             sha = None
@@ -901,15 +918,27 @@ def merge_base(self, branch_a: str, branch_b: str) -> str:
             shas.append(sha)
         sha_a, sha_b = shas
         if sha_a == sha_b:
-            return sha_a
+            yield sha_a
+            return
 
         merge_bases = find_merge_base(
             self._repo,
             cast(Any, [sha_a.encode("ascii"), sha_b.encode("ascii")]),
         )
+        candidates = {merge_base.decode("ascii") for merge_base in merge_bases}
+        inferior: set[str] = set()
+        # The timestamp-ordered backend can leave ancestors of other candidates.
+        if len(candidates) > 1:
+            for candidate in candidates:
+                inferior.update(candidates.intersection(self.ancestor_distances(candidate)) - {candidate})
+        yield from sorted(candidates - inferior)
+
+    def merge_base(self, branch_a: str, branch_b: str) -> str:
+        """Return the smallest best-base ID; content mergers must inspect all bases."""
+        merge_bases = tuple(self.iter_merge_bases(branch_a, branch_b))
         if not merge_bases:
             raise ValueError(f"No common ancestor between {branch_a!r} and {branch_b!r}")
-        return min(merge_base.decode("ascii") for merge_base in merge_bases)
+        return min(merge_bases)
 
     def read_ref(self, ref: RefName) -> str | None:
         sha = _ref_get(self._repo.refs, ref.as_bytes())

quire/tree_merge.py

@@ -0,0 +1,43 @@
+"""Base-relative reconciliation of exact, keyed Git tree-entry versions."""
+
+from collections.abc import Iterator, Mapping
+from dataclasses import dataclass
+
+
+@dataclass
+class TreeMergeConflict(ValueError):
+    path: str
+    base_sha: str | None
+    left_sha: str | None
+    right_sha: str | None
+
+    def __str__(self) -> str:
+        return (
+            f"tree merge conflict at {self.path!r}: base {self.base_sha}, "
+            f"left {self.left_sha}, right {self.right_sha}"
+        )
+
+
+def iter_merged_tree_entries(
+    base: Mapping[str, str],
+    left: Mapping[str, str],
+    right: Mapping[str, str],
+) -> Iterator[tuple[str, str]]:
+    """Reconcile immutable snapshots, treating a missing path as absence.
+
+    Propagate a one-sided change relative to the common ancestor. Equal current
+    object IDs converge; different concurrent changes conflict. Paths and object
+    IDs remain exact, with final tree-shape validation owned by the publisher.
+    """
+    for path in sorted(base.keys() | left.keys() | right.keys()):
+        ancestor, ours, theirs = base.get(path), left.get(path), right.get(path)
+        if ours == theirs:
+            selected = ours
+        elif ours == ancestor:
+            selected = theirs
+        elif theirs == ancestor:
+            selected = ours
+        else:
+            raise TreeMergeConflict(path, ancestor, ours, theirs)
+        if selected is not None:
+            yield path, selected