Repository navigation
feat: collision-bumped w:date stamping at changeset granularity (ISSUES.md #53) - #65
Conversation
|
Warning Review limit reached
Next review available in: 4 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…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.
f25bbbc to
e3471d3
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
## 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.
Summary
Implements ISSUES.md #53 (slimmed writer-side scope, gated on #46). One changeset — a single edit call, or one whole
batch_edit/batch_rewritecall — stamps one whole-secondw: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_rewritewrap the whole call in onefrozen_timestamp()scope — one date per batch call (the per-op_grouped()freezes join it), while each op keeps its own revision group in-session.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:dateUtcalways matches the bumpedw:date. The former accepted-relaxation test flips intotest_same_paragraph_same_second_two_groups.docs/api.md,skills/docx/SKILL.md, and theRevision.group_sourcedocstring 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 skippeduv run ruff check ./ruff format --check: cleanuv run ty check: exit 0 (7 pre-existing warnings in untouched test files)