Skip to content

Commit a6e4d32

Browse files
authored
feat: collision-bumped w:date stamping at changeset granularity (ISSUES.md #53) (#65)
* feat: collision-bumped w:date stamping at changeset granularity (ISSUES.md #53) One changeset (a single edit call, or one whole batch_edit/batch_rewrite call) stamps one whole-second w:date, bumped past the previous stamp when the same author's changesets collide in one real second. Parse-time group reconstruction (#46) therefore never merges two of our own changesets; same-paragraph ops of one batch call share their date and merge by design. - xml_editor: _next_changeset_timestamp() allocates collision-bumped stamps (monotonic per author, also across clock regressions); frozen_timestamp() becomes reentrant-by-reuse so nested per-op freezes join the enclosing batch changeset. - track_changes: batch_edit/batch_rewrite wrap the whole call in one frozen_timestamp() scope — one date per batch call, one group per op. - tests: TestChangesetDateStamping pins the contract (shared batch date under a ticking clock, no bump when seconds differ, bounded drift, distinct dates for two batches in one second, nested freeze reuse, w16du:dateUtc parity); the former same-second over-merge relaxation flips into test_same_paragraph_same_second_two_groups. * docs+style: scope the never-share-a-second guarantee to one open session Multi-review round 1: the collision counter is per-editor-instance and not seeded from persisted dates, so the docs (api.md, SKILL.md, Revision.group_source and RevisionManager docstrings) now state the guarantee as per-session — own writes from a previous session merge like foreign revisions on identical author + date. Also rename _next_changeset_timestamp to _get_next_changeset_timestamp to match the sibling allocators (_get_next_change_id, _get_next_comment_id) and rewrap the api.md paragraph left with an orphan line. Skipped per red-team triage: stamp-slot consumption by empty/failed changesets (accepted bounded drift, in plan), per-author stamp dict (author is immutable per editor — false positive), test docstring issue-reference form (subjective). * docs+test: round-2 review polish — session qualifier, helper reuse, regression test - frozen_timestamp docstring and TestChangesetDateStamping class docstring now carry the same per-session qualifier as every other statement of the never-share-a-second guarantee (round 1 missed these two spots). - test_nested_frozen_timestamp_reuses_outer_stamp builds its editor via the file's _make_manager helper instead of a raw DocxXMLEditor construction. - New test_clock_regression_keeps_dates_monotonic drives the settable clock strictly backwards, pinning the documented NTP-step-back behavior (dates never regress; previous+1 until the clock catches up) — previously only the equality branch was exercised. * fix: dedupe SKILL.md lines duplicated in rebase conflict resolution
1 parent 12ae649 commit a6e4d32

5 files changed

Lines changed: 284 additions & 44 deletions

File tree

‎docs/api.md‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -653,10 +653,16 @@ in the `.docx` (Word has no grouping concept and strips unknown markup), so
653653
groups are **inferred** at parse time instead
654654
(`Revision.group_source == "inferred"`): contiguous revisions in the same
655655
paragraph sharing identical `w:author` + `w:date` reconstruct as one group.
656-
That heuristic almost always matches the original logical edits — one caveat:
657-
`w:date` has second precision, so two edits to the *same paragraph* within the
658-
same second merge into one inferred group. When Word already resolved part of
659-
a former edit, the remainder reconstructs as a smaller (rump) group, and
656+
That heuristic matches this library's own edits exactly: each changeset (one
657+
edit call, or one whole `batch_edit`/`batch_rewrite` call) stamps a
658+
collision-bumped whole-second date, so within one open session two changesets
659+
by the same author never share a second; all ops of one batch call share one
660+
date (one changeset) and same-paragraph batch ops merge by design. The
661+
collision counter is per-session (not seeded from dates already in the file),
662+
so own writes from a *previous* session merge like foreign revisions: any
663+
revisions with identical `w:author` + `w:date` merge (`w:date` has second
664+
precision). When Word already resolved part of a former edit, the remainder
665+
reconstructs as a smaller (rump) group, and
660666
`accept_group()`/`reject_group()` handle it fine. Revisions missing an author
661667
or date, sitting outside any paragraph (e.g. table-row markers), or sharing a
662668
duplicated id stay ungrouped (`group_id=None`), as does the trailing half

‎docx_editor/track_changes.py‎

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -346,10 +346,14 @@ class Revision:
346346
groups created by edits through this open Document, ``"inferred"``
347347
for groups reconstructed at parse time by the heuristic (contiguous
348348
same-paragraph revisions sharing identical ``w:author`` + ``w:date``
349-
are one group). Inferred groups usually match the original logical
350-
edits, but two edits to the same paragraph within the same second
351-
merge into one group (``w:date`` has second precision). None iff
352-
``group_id`` is None.
349+
are one group). Our own writes stamp collision-bumped dates (within
350+
one open session, two changesets by one author never share a
351+
second), so inferred groups match the original changesets;
352+
same-paragraph ops of one ``batch_edit``/``batch_rewrite`` call
353+
share their date and merge by design. The counter is per-session,
354+
so own writes from a previous session merge like foreign revisions:
355+
identical author + date can still over-merge (``w:date`` has second
356+
precision). None iff ``group_id`` is None.
353357
"""
354358

355359
id: int
@@ -584,9 +588,15 @@ def _reconstruct_groups(self) -> None:
584588
585589
Known, accepted imprecisions:
586590
587-
- Two logical edits to the same paragraph within the same second
588-
(w:date has second precision) merge into one group — accepted
589-
over-merge, pinned by test.
591+
- Same-paragraph over-merge is confined to one batch call for our
592+
own writes within one open session: each changeset stamps a
593+
collision-bumped date (never reused across an author's
594+
changesets in that session), so only ops of a single
595+
batch_edit/batch_rewrite call — which share their date by
596+
design — merge. The counter is not seeded from dates already in
597+
the file, so a previous session's own writes behave like
598+
foreign ones: identical author + date (w:date has second
599+
precision) can still over-merge.
590600
- A revision inside a nested paragraph (e.g. a text box's
591601
w:txbxContent) interrupts the outer paragraph's run in document
592602
order — conservative over-split.
@@ -856,7 +866,9 @@ def batch_edit(self, operations: list[EditOperation]) -> list[int]:
856866
857867
Validates all paragraph hashes before applying any edits.
858868
Applies edits in reverse paragraph order so earlier paragraphs'
859-
hashes remain valid throughout.
869+
hashes remain valid throughout. The whole call is one changeset:
870+
every op's revisions share one ``w:date``, while each op still
871+
records its own revision group in-session.
860872
861873
Args:
862874
operations: List of EditOperation objects (each must have paragraph set)
@@ -901,15 +913,18 @@ def batch_edit(self, operations: list[EditOperation]) -> list[int]:
901913

902914
try:
903915
results = [0] * len(operations)
904-
for original_idx, _ref, op in parsed:
905-
try:
906-
# Each op is its own logical edit: one group per op, so
907-
# callers can accept one op and reject another.
908-
with self._grouped():
909-
change_id = self._apply_single_edit(op)
910-
except (ValueError, DocxEditError) as e:
911-
raise BatchOperationError(original_idx, str(e), original=e) from e
912-
results[original_idx] = change_id
916+
# One batch call = one changeset: every op's revisions share one
917+
# w:date (the per-op _grouped() freezes join this outer scope).
918+
with self.editor.frozen_timestamp():
919+
for original_idx, _ref, op in parsed:
920+
try:
921+
# Each op is its own logical edit: one group per op, so
922+
# callers can accept one op and reject another.
923+
with self._grouped():
924+
change_id = self._apply_single_edit(op)
925+
except (ValueError, DocxEditError) as e:
926+
raise BatchOperationError(original_idx, str(e), original=e) from e
927+
results[original_idx] = change_id
913928
return results
914929
except Exception:
915930
# Restore via the line-tracking parser so parse_position is preserved.
@@ -1053,6 +1068,9 @@ def _validate_single(self, op: EditOperation) -> str | None:
10531068
def batch_rewrite(self, rewrites: list[tuple[str, str]]) -> list[int | None]:
10541069
"""Rewrite multiple paragraphs with upfront hash validation.
10551070
1071+
The whole call is one changeset: all rewrites share one ``w:date``,
1072+
while each rewrite still records its own revision group in-session.
1073+
10561074
Returns:
10571075
One revision group id per rewrite, in input order (None for
10581076
rewrites that created no revisions) — each rewrite gets its own
@@ -1099,13 +1117,18 @@ def batch_rewrite(self, rewrites: list[tuple[str, str]]) -> list[int | None]:
10991117
registry_snapshot = self._registry_snapshot()
11001118

11011119
try:
1102-
# Apply rewrites in reverse paragraph order
1120+
# Apply rewrites in reverse paragraph order. One batch call = one
1121+
# changeset: all rewrites share one w:date (each rewrite still
1122+
# gets its own group; duplicate paragraphs are rejected above, so
1123+
# reconstruction keeps one group per paragraph despite the
1124+
# shared date).
11031125
group_ids: list[int | None] = [None] * len(rewrites)
1104-
for original_idx, ref, new_text in parsed:
1105-
try:
1106-
group_ids[original_idx] = self.rewrite_paragraph(f"P{ref.index}#{ref.hash}", new_text)
1107-
except (ValueError, DocxEditError) as e:
1108-
raise BatchOperationError(original_idx, str(e), original=e) from e
1126+
with self.editor.frozen_timestamp():
1127+
for original_idx, ref, new_text in parsed:
1128+
try:
1129+
group_ids[original_idx] = self.rewrite_paragraph(f"P{ref.index}#{ref.hash}", new_text)
1130+
except (ValueError, DocxEditError) as e:
1131+
raise BatchOperationError(original_idx, str(e), original=e) from e
11091132
return group_ids
11101133
except Exception:
11111134
# Restore via the line-tracking parser so parse_position is preserved.

‎docx_editor/xml_editor.py‎

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from collections.abc import Callable, Iterator
1313
from contextlib import contextmanager
1414
from dataclasses import dataclass
15-
from datetime import datetime, timezone
15+
from datetime import datetime, timedelta, timezone
1616
from pathlib import Path
1717
from typing import Literal
1818
from xml.dom.minidom import Element
@@ -1239,6 +1239,9 @@ def __init__(
12391239
self._max_change_id = -1
12401240
self._tracked_change_collector: list[Element] | None = None
12411241
self._frozen_timestamp: str | None = None
1242+
# Last stamped (author, second) — the one collision counter behind
1243+
# _get_next_changeset_timestamp.
1244+
self._last_changeset_stamp: tuple[str, datetime] | None = None
12421245

12431246
def _get_next_change_id(self) -> int:
12441247
"""Get the next available change ID by checking all tracked change elements."""
@@ -1276,6 +1279,28 @@ def collect_tracked_changes(self) -> Iterator[list[Element]]:
12761279
finally:
12771280
self._tracked_change_collector = None
12781281

1282+
def _get_next_changeset_timestamp(self) -> str:
1283+
"""Whole-second UTC stamp for one changeset, collision-bumped.
1284+
1285+
True wall clock, except when this editor's previous changeset by the
1286+
same author already occupies this (or a later) second — then previous
1287+
second + 1, keeping (author, second) unique per changeset so
1288+
parse-time group reconstruction never merges two changesets. The
1289+
same rule keeps stamps monotonic per author across a clock
1290+
regression (e.g. an NTP step back): dates never go backwards,
1291+
pinned at previous + 1 until the clock catches up. The counter is
1292+
per-editor-instance (one open session) — it is not seeded from
1293+
dates already persisted in the document, so uniqueness does not
1294+
extend across sessions.
1295+
"""
1296+
now = datetime.now(timezone.utc).replace(microsecond=0)
1297+
if self._last_changeset_stamp is not None:
1298+
last_author, last_second = self._last_changeset_stamp
1299+
if last_author == self.author and now <= last_second:
1300+
now = last_second + timedelta(seconds=1)
1301+
self._last_changeset_stamp = (self.author, now)
1302+
return now.strftime("%Y-%m-%dT%H:%M:%SZ")
1303+
12791304
@contextmanager
12801305
def frozen_timestamp(self) -> Iterator[None]:
12811306
"""Stamp every element injected in this context with ONE ``w:date``.
@@ -1285,12 +1310,20 @@ def frozen_timestamp(self) -> Iterator[None]:
12851310
several times — crossing a second boundary mid-operation would spread
12861311
the operation's revisions across two ``w:date`` values, and parse-time
12871312
group reconstruction (which keys on identical dates) would then reopen
1288-
one edit as several groups. Does not nest — same single-slot design as
1289-
``collect_tracked_changes``.
1313+
one edit as several groups.
1314+
1315+
One changeset = one outermost ``frozen_timestamp()`` scope, stamped
1316+
via ``_get_next_changeset_timestamp`` (collision-bumped so two
1317+
changesets by the same author never share a second within one open
1318+
session). Reentrant by reuse: a nested entry joins the
1319+
enclosing changeset — ``batch_edit``/``batch_rewrite`` open the scope
1320+
once for the whole call and the per-op ``_grouped()`` freezes join
1321+
it. Only the outermost entry allocates a stamp and clears it on exit.
12901322
"""
12911323
if self._frozen_timestamp is not None:
1292-
raise RuntimeError("frozen_timestamp() does not nest")
1293-
self._frozen_timestamp = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ")
1324+
yield
1325+
return
1326+
self._frozen_timestamp = self._get_next_changeset_timestamp()
12941327
try:
12951328
yield
12961329
finally:

‎skills/docx/SKILL.md‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -502,10 +502,13 @@ doc.reject_group(result.group_id) # undo the whole rewrite, or:
502502
# group (r.group_source == "inferred"; session edits are "recorded"). So
503503
# accept_group/reject_group work after reopen too — but always take the
504504
# group_id from THIS session's list_revisions()/EditResult; a stale id
505-
# from a previous session may resolve to a different group. Caveat:
506-
# w:date has second precision, so two edits to the same paragraph in the
507-
# same second merge into one inferred group. save() keeps groups alive
508-
# (the Document stays open).
505+
# from a previous session may resolve to a different group. Own edits
506+
# stamp collision-bumped whole-second dates (two changesets by one author
507+
# never share a second within one open session); all ops of one
508+
# batch_edit/batch_rewrite call share one date (one changeset). Revisions
509+
# from other sources — foreign authors, or own edits from a previous
510+
# session — still merge on identical author + date. save() keeps groups
511+
# alive (the Document stays open).
509512
# Revision ids, unlike group ids, ARE stable: they are the w:id attributes
510513
# stored in the document XML and survive save()/close()/reopen — resolving
511514
# by revision id in a later session is always safe.

0 commit comments

Comments
 (0)