Skip to content

feat: collision-bumped w:date stamping at changeset granularity (ISSUES.md #53) - #65

Merged
pablospe merged 4 commits into
mainfrom
task/33618b36-Collision-bumped-w-date-stampi
Jul 22, 2026
Merged

pablospe merged 4 commits into
mainfrom
task/33618b36-Collision-bumped-w-date-stampi

Conversation

@pablospe

Copy link
Copy Markdown
Owner

Summary

Implements ISSUES.md #53 (slimmed writer-side scope, gated on #46). One changeset — a single edit call, or one whole batch_edit/batch_rewrite call — stamps one whole-second w:date, collision-bumped past the previous stamp when the same author's changesets land in one real second. Parse-time group reconstruction (#46) therefore never merges two of this library's own changesets after save/reopen; same-paragraph ops of one batch call share their date and merge by design (changeset granularity, pinned as a contract).

No public API changes, no version bump.

Changes

  • xml_editor.py: new _next_changeset_timestamp() — true wall clock, except on collision with the editor's previous stamp by the same author, then previous+1. Monotonic per author, including across clock regressions (NTP step back). frozen_timestamp() becomes reentrant-by-reuse: a nested entry joins the enclosing changeset; only the outermost entry allocates/clears the stamp.
  • track_changes.py: batch_edit/batch_rewrite wrap the whole call in one frozen_timestamp() scope — one date per batch call (the per-op _grouped() freezes join it), while each op keeps its own revision group in-session.
  • Tests: new TestChangesetDateStamping (7 tests) pins the contract: batch ops share one date under an adversarial ticking clock, no bump when seconds genuinely differ, drift bounded by changesets-per-second and collapsing once real time catches up, two batches in one second get distinct dates, nested freeze reuses the outer stamp, w16du:dateUtc always matches the bumped w:date. The former accepted-relaxation test flips into test_same_paragraph_same_second_two_groups.
  • Docs: docs/api.md, skills/docx/SKILL.md, and the Revision.group_source docstring updated — own edits never over-merge across changesets; foreign revisions with identical author+date still can.

Known limitation

The collision counter is in-memory, per open Document: save/close/reopen within one real second can still stamp the same (author, second) and merge on the next reopen. Out of this slimmed scope (no seeding from persisted dates).

Verification

  • uv run pytest: 1223 passed, 2 skipped
  • uv run ruff check . / ruff format --check: clean
  • uv run ty check: exit 0 (7 pre-existing warnings in untouched test files)

@coderabbitai

coderabbitai Bot commented Jul 22, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

@pablospe, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 4 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6d1df474-b0a6-49e8-8f52-64b6e90a19a4

📥 Commits

Reviewing files that changed from the base of the PR and between 12ae649 and e3471d3.

📒 Files selected for processing (5)
  • docs/api.md
  • docx_editor/track_changes.py
  • docx_editor/xml_editor.py
  • skills/docx/SKILL.md
  • tests/test_revision_groups.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task/33618b36-Collision-bumped-w-date-stampi

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

pablospe added 4 commits July 22, 2026 19:36
…ES.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.
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).
…egression 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.
@pablospe
pablospe force-pushed the task/33618b36-Collision-bumped-w-date-stampi branch from f25bbbc to e3471d3 Compare July 22, 2026 17:37
@pablospe
pablospe merged commit a6e4d32 into main Jul 22, 2026
8 checks passed
@pablospe
pablospe deleted the task/33618b36-Collision-bumped-w-date-stampi branch July 22, 2026 17:50
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.9%. Comparing base (12ae649) to head (e3471d3).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main     #65   +/-   ##
=====================================
  Coverage   95.8%   95.9%           
=====================================
  Files         11      11           
  Lines       3898    3910   +12     
  Branches     776     778    +2     
=====================================
+ Hits        3736    3750   +14     
+ Misses        75      74    -1     
+ Partials      87      86    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pablospe added a commit that referenced this pull request Aug 29, 2026
## What

A drawing's `w:txbxContent` is no longer part of the document's addressable
surface. Before: a text box's paragraphs were enumerated as document
paragraphs and their text leaked into the host paragraph's text map. After:
box paragraphs appear in no listing, their text in no view, no search and no
paragraph hash, and no ref — paragraph or table — addresses one. A new
`Document.has_textbox_content` says whether a file carries content this
exclusion hides.

## Why

Word stores one text box twice — an `mc:Choice` (`wps:txbx`) copy and an
`mc:Fallback` (`v:textbox`) copy — so a single box leaked its text four times:
twice inline in the host paragraph and twice more as paragraphs of its own.
Those paragraphs were addressable, so an edit through such a ref could only
ever reach one of the two copies and desynchronize the pair.

## How

`body_paragraphs()` and `_inside_textbox()` in `xml_editor.py` are the two
primitives; `_body_tables()` does the same for `w:tbl` so the paragraph and
table index spaces close together. `build_text_map()` excludes box content in
both views, bounded by the element being examined — that bound is what lets
`_parse_revision` report a `w:del` inside a box while excluding the box from a
host `w:del`. The 23 document-wide `w:p` enumerations in `document.py`,
`track_changes.py` and `comments.py` move together: a partial migration would
have index `N` name different paragraphs in different methods.
`get_markup_text()` and `Revision` text/location follow the same rule, so a
boxed revision lists with `paragraph_ref`/`occurrence` `None`.

Contract changes for callers: paragraph indexes shift in documents containing
boxes (a stale ref fails loud with `HashMismatchError`), and a search crossing
into a box now raises `TextNotFoundError` instead of editing boxed text. A box
revision is listed once per copy, so only `accept_all`/`reject_all` resolve
both — documented, and pinned by a test.

## Testing

`tests/test_textbox_exclusion.py` — 40 cases over a Word-shaped twin fixture:
enumeration, all three text views, search, hashing across save/reopen, comment
anchoring, revisions inside a box, the table index space, `has_textbox_content`,
and the placement variants (table cell, inside `w:ins`, box-only paragraph,
nested, empty). `TestIndexSpacesAgree` is the partial-migration guard.
`TestBuildTextMapTextBox` in `test_text_map.py` pins the bounded check. Two
tests in `test_nested_ins.py` are rewritten to encode the contract change.

1478 passed, 2 skipped (serial and memory-capped). `ruff check`, `ruff format
--check`, `ty check` and `mkdocs build -s` clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant