fix: don't resolve another author's same-id revision (ROADMAP.md #75)

ROADMAP.md

@@ -36,7 +36,7 @@ Only `w:instrText` is known to the library today. A `replace()` targeting text i
 
 ### 75. Author-filtered bulk resolution must not touch other authors' revisions  [data loss; after #73 step 9]
 
-`accept_all(author=A)` / `reject_all(author=A)` list revisions by author but resolve by `w:id`, and the id lookup has no author check. With B's `<w:ins w:id="7">` before A's `<w:del w:id="7">` (duplicate ids across authors occur in real files — the corpus has them from LibreOffice), `reject_all(author="A")` deletes B's inserted text and `accept_all(author="A")` makes it permanent, both reporting 2. The `accept_all` docstring documents this and `tests/test_track_changes.py::…:1143` pins it as expected — it is not acceptable for a data-loss path. Fix: resolve the exact elements selected by the listing (element identity), not their ids; flip the pinning test. Lives in the resolution cluster, so it lands after #73 moves it (found by CodeRabbit on PR #75; verified 2026-08-30).
+`accept_all(author=A)` / `reject_all(author=A)` list revisions by author but resolve by `w:id`, and the id lookup has no author check. With B's `<w:ins w:id="7">` before A's `<w:del w:id="7">` (duplicate ids across authors occur in real files — the corpus has them from LibreOffice), `reject_all(author="A")` deletes B's inserted text and `accept_all(author="A")` makes it permanent, both reporting 2. The `accept_all` docstring documented this and a test pinned it as expected — not acceptable for a data-loss path (found by CodeRabbit on PR #75; verified 2026-08-30). **Fixed in PR #96** (the w:id index is scoped to the author being resolved; the pinning test is flipped). The per-id verbs keep the defect; that is #84.
 
 ### 79. Inserted text loses formatting after an empty split segment  [after #73 step 7]
 
@@ -54,6 +54,22 @@ Every path that decides which `rPr` a newly inserted run copies picks its bounda
 
 The #91 diagnostics caught it on their first CI failure (run 33331432527, load average 1.14 — an idle runner): the kernel wrote `kernel.json` in 0.5 s, then our `kernel_info` probe on the control channel raised inside ipykernel's ZMQStream callback — `KeyError: '_control_lock'` (traitlets) — because the control message arrived before the kernel finished initializing; the callback's exception left the control stream broken and the kernel never replied within the remaining 59.5 s. So the residual "flake" is a race our own probe triggers, not load. Fix directions, in preference order: probe on the shell channel or wait for the iopub `status: starting` message before touching control; retry with a *fresh* client (the broken stream may be permanent for that connection); check whether current ipykernel has fixed `_control_lock` initialization order and pin/upgrade. Regression test: a kernel whose control channel receives `kernel_info` immediately after `kernel.json` appears must still become ready.
 
+### 84. `accept_revision`/`reject_revision` resolve a duplicated `w:id` by position  [data loss]
+
+The per-id half of #75, which fixed only `accept_all`/`reject_all`. With B's `<w:ins w:id="7">` before A's `<w:del w:id="7">`, `list_revisions(author="A")` returns one row with id 7, and `reject_revision(7)` deletes B's inserted text and leaves A's deletion pending; `accept_revision(7)` makes B's insertion permanent. The caller cannot disambiguate — `Revision` carries no element handle — and `SKILL.md` documents this exact per-id pattern. Fix: either give the per-id verbs an author/identity selector, or refuse an ambiguous id with a teaching error. The duplicate set is already computed — `duplicate_ids` in `_reconstruct_groups` (`registry.py`) — and then discarded; retain or recompute it rather than testing `_revision_groups[id] is None`, which `_adopt_split_tail` also writes for an unambiguous split tail whose origin was ungroupable. SKILL.md carries an interim caveat. Found by the multi-review of PR #96.
+
+### 85. `_rejoin_paragraph` drops the tail paragraph's other revisions  [data loss]
+
+Rejecting a paragraph-mark insertion migrates only the mark `w:ins` off the tail paragraph and then drops that paragraph's whole `w:pPr`, so another author's paragraph-mark `w:del`/`w:moveFrom`/`w:moveTo` and their `w:pPrChange` are destroyed — uncounted, unwarned, and with the merged paragraph's break left permanently untracked. Reproduces with no author filter, and is order-dependent within a single author: the same document returns 1 or 2 depending only on whether the mark-ins id sorts above the mark-del id. The `accept_all` docstring names only the `w:pPrChange` case. Fix: migrate the whole downstream mark set onto the surviving paragraph's mark `rPr`, since they all describe the break it now owns. Found by the multi-review of PR #96.
+
+### 86. Historical marks inside change records list as live revisions
+
+`list_revisions` and `_revision_element_index` walk with `skip_change_records=False` while `_unhandled_elements` uses `True`, so a revision mark recorded *inside* a change record is offered as adjudicable. `w:pPr/w:rPr/w:rPrChange/w:rPr/w:ins` is conforming (CT_ParaRPrOriginal admits EG_ParaRPrTrackChanges) and lists as a live insertion, and resolving it mutates the other author's change record; the nonconforming `w:pPrChange`-holding-`w:rPr` shape is worse — it merges two live paragraphs. Fix: have both walks pass `skip_change_records=True` and leave recorded marks to the honesty floor. Needs its own regression test. Found by the multi-review of PR #96.
+
+### 87. An unhandled mark's `w:id` can resolve a handled revision  [data loss]
+
+`list_unhandled_revisions()` reports a numeric `w:id` when the mark has one, and ids are unique only per producer: with `<w:rPrChange w:id="7" w:author="B">` alongside `<w:ins w:id="7" w:author="A">`, `reject_revision(row.id)` returns True, deletes A's inserted text and leaves the `w:rPrChange` standing — the caller adjudicated a revision it never asked about while the one it named survives. `UnhandledRevision`'s docstring claimed this could only fail; it now says what actually happens. Not reproduced on the corpus (0 handled/unhandled id collisions across 77 files), so it is a latent shape, not an observed one. Fix: have the per-id verbs verify the located element is the one the id was read from, or refuse when a handled and an unhandled element share the id. Related to #84, which is the same hazard between two *handled* rows. Found by the multi-review of PR #96.
+
 ### 69. Return-leg reconciliation: which changesets survived?  [design]
 
 The product loop is agent proposes → human adjudicates in Word → document comes back; accepted revisions become plain text, rejected ones vanish, and there is no way to ask "which of my changesets survived?". A sent-vs-returned compare keyed on changeset `w:date` + content. Design first; large.

docs/api.md

@@ -867,6 +867,14 @@ accepting the `move_to` and rejecting the `move_from` duplicates the text, the
 inverse loses it. A lone half in a damaged file behaves as what it structurally
 is: a `move_from` alone is a deletion, a `move_to` alone an insertion.
 
+**Duplicate ids are ambiguous here.** A producer may reuse one `w:id` across
+authors, and this call takes only the id: handed a duplicated one it resolves
+whichever element comes first in the document, which may be another author's.
+Groups are no way around it — a duplicated id is barred from grouping, so both
+rows carry `group_id=None`. Two rows sharing an id in `list_revisions()` is the
+signal; resolve them with `accept_all(author=…)`/`reject_all(author=…)`, which
+is author-exact.
+
 **Parameters:**
 
 - `revision_id` (int): ID of the revision to accept
@@ -892,7 +900,8 @@ Reject a revision by ID.
 - For `property_change`: restores the paragraph's recorded previous properties. A record with none (LibreOffice writes a self-closing `w:pPrChange` for "previously no properties") clears them; the recorded style id is restored verbatim even when the document defines no such style (Word falls back to Normal for it)
 - Nested revisions: rejecting an insertion removes everything inside it — deletions another author nested inside it disappear with it
 
-Resolve both halves of a move together — see `accept_revision()`.
+Resolve both halves of a move together — see `accept_revision()`, whose
+duplicate-`w:id` caveat applies here too.
 
 **Parameters:**
 
@@ -1080,6 +1089,13 @@ marker is dropped without merging or splitting paragraphs.
 Only `word/document.xml` is inspected; headers, footers and footnotes are the
 container-parts epic (ROADMAP.md #30).
 
+An `author=` call targets that author's own elements and no one else's, even
+where a producer repeats one `w:id` across authors (LibreOffice does): the id
+lookup is scoped to the author being resolved, so another author's same-id
+revision is never a candidate. That is about which elements are *targeted*, not
+about what survives — rejecting an insertion still removes whatever another
+author nested inside it, `reject_all(author=…)` included.
+
 **Parameters:**
 
 - `author` (str, optional): If provided, only accept revisions by this author
@@ -1109,7 +1125,9 @@ Reject every listed revision.
 Resolves insertions, deletions, content moves (both halves as a unit — the
 text is back at its source exactly once) and paragraph-property changes (the
 recorded previous properties are restored); every other revision type is left
-pending and reported exactly as in `accept_all()`.
+pending and reported exactly as in `accept_all()`. `author=` targets one
+author's elements in the same way, duplicate `w:id`s across authors included —
+and carries away nested revisions in the same way too.
 
 **Parameters:**
 

docx_editor/track_changes/base.py

@@ -38,7 +38,7 @@ class _RevisionManagerBase:
     _defer_range_sweep: bool
     _range_sweep_pending: bool
 
-    def _revision_element_index(self) -> dict[str, list[Element]]:
+    def _revision_element_index(self, author: str | None = None) -> dict[str, list[Element]]:
         raise NotImplementedError
 
     def _is_in_document(self, elem) -> bool:
@@ -147,7 +147,7 @@ def changeset_id_of(self, group_id: int) -> int | None:
         raise NotImplementedError
 
     def _find_revision_element(
-        self, revision_id: int, element_index: dict[str, list[Element]] | None
+        self, revision_id: int | None, element_index: dict[str, list[Element]] | None
     ) -> Element | None:
         raise NotImplementedError
 

docx_editor/track_changes/dom.py

@@ -71,6 +71,18 @@ def _adjudicable_id(elem) -> int | None:
         return None
 
 
+def _revision_author(elem) -> str:
+    """``elem``'s ``w:author``, or ``"Unknown"`` when the attribute is absent.
+
+    The companion to ``_adjudicable_id`` for the other half of a revision's
+    identity, and shared for the same reason: the listing, the honesty floor
+    and the w:id index all filter on author, and an author-filtered
+    ``accept_all``/``reject_all`` resolves nothing at all if the index reads an
+    unattributed mark differently from the listing that selected it.
+    """
+    return elem.getAttribute("w:author") or "Unknown"
+
+
 def _nearest_revision_ancestor_id(elem) -> int | None:
     """id of the closest handled-revision ancestor with a numeric w:id, else None.