Repository navigation
docs: full SKILL/docs audit — changeset tier, dry_run, occurrence, exception sections (ISSUES.md #55) - #69
Conversation
…ception sections (ISSUES.md #55) Sync README, docs/{api,index,quickstart}, and skills/docx/SKILL to the current API surface: - Revision Management described at three granularities (revision/group/changeset); EditResult and Revision carry group_id and changeset_id. - Document accept_changeset/reject_changeset with recorded-vs-inferred semantics. - occurrence= disambiguation on replace/delete/insert_after/insert_before/add_comment. - batch_edit dry_run=True returning EditValidationResult (index/valid/error). - count_matches; session `status` subcommand; machine-path-stripped eval traceback. - New api.md exception sections: HashMismatchError, ParagraphIndexError, InvalidDocumentError; retarget stale #exceptions anchors to per-exception ids. - Revision table gains paragraph_ref/occurrence/nested_under/contains_ids. - quickstart word-trim example corrected (30 days -> 60 days tracks 30/60).
📝 WalkthroughWalkthroughDocumentation updates cover tracked-change hierarchy, occurrence-based editing, dry-run validation, API error references, revision metadata, and session-mode commands and output. ChangesEditing and revision-management documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/api.md`:
- Around line 1168-1175: Update the HashMismatchError documentation to describe
paragraph_preview as a bounded preview of the paragraph’s current text, noting
that it may be truncated to 80 characters with an appended ellipsis; leave the
other structured field descriptions unchanged.
- Around line 1186-1200: Update the ParagraphIndexError recovery example to
handle total_paragraphs == 0 before applying the clamp and retry. For an empty
document, use the established empty-document recovery guidance instead; only
call get_paragraph with the clamped index when total_paragraphs is greater than
zero.
In `@docs/quickstart.md`:
- Line 88: Update the Markdown example near the replace() explanation so the `
days` code span no longer contains leading whitespace; move the space into
surrounding prose or use a trimmed code span while preserving the intended
unchanged-word explanation.
In `@README.md`:
- Around line 158-160: Update the dry-run example in README.md to define the ops
variable before calling doc.batch_edit(ops, dry_run=True), reusing the operation
list shown in the preceding example or assigning it explicitly so the example is
self-contained.
- Around line 26-27: Update the changeset documentation consistently across
README.md lines 26-27, docs/index.md line 14, and docs/quickstart.md lines
218-232: define a changeset as covering every complete edit call, including
single replace() and rewrite_paragraph() operations, while noting that
batch_edit() and batch_rewrite() are examples that may contain multiple groups;
explicitly state that ordinary edit results expose usable changeset_id values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7977158e-20d2-4fe8-97fd-428be00bc761
📒 Files selected for processing (5)
README.mddocs/api.mddocs/index.mddocs/quickstart.mdskills/docx/SKILL.md
| ### `HashMismatchError` | ||
|
|
||
| Raised when a `paragraph` ref's hash no longer matches the paragraph's current | ||
| content — the paragraph changed (usually by an earlier edit) since the ref was | ||
| listed, so the ref is stale. Structured fields: `paragraph_index` (1-based), | ||
| `expected_hash` (the hash in the stale ref), `actual_hash` (the paragraph's | ||
| current hash), and `paragraph_preview` (the current text). Recover by retrying | ||
| with the fresh ref `P{paragraph_index}#{actual_hash}`, or re-list paragraphs. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document that paragraph_preview may be truncated.
The implementation limits this field to the first 80 characters and appends ... for longer paragraphs, so calling it “the current text” is misleading. State that it is a bounded preview.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/api.md` around lines 1168 - 1175, Update the HashMismatchError
documentation to describe paragraph_preview as a bounded preview of the
paragraph’s current text, noting that it may be truncated to 80 characters with
an appended ellipsis; leave the other structured field descriptions unchanged.
| ### `ParagraphIndexError` | ||
|
|
||
| Raised when a paragraph index is out of range — `< 1` or greater than | ||
| `paragraph_count()` (for example `get_paragraph(0)`). Structured fields: | ||
| `index` (the offending index) and `total_paragraphs` (how many the document | ||
| has). Clamp to `1..total_paragraphs` and retry, or call `list_paragraphs()` | ||
| to pick a valid ref. | ||
|
|
||
| ```python | ||
| from docx_editor.exceptions import ParagraphIndexError | ||
|
|
||
| try: | ||
| para = doc.get_paragraph(index) | ||
| except ParagraphIndexError as e: | ||
| para = doc.get_paragraph(max(1, min(e.index, e.total_paragraphs))) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard the empty-document recovery case.
When total_paragraphs == 0, max(1, min(...)) produces index 1, which is still invalid and causes the retry to fail. Handle the empty document before clamping, as the corresponding recovery guidance elsewhere already does.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/api.md` around lines 1186 - 1200, Update the ParagraphIndexError
recovery example to handle total_paragraphs == 0 before applying the clamp and
retry. For an empty document, use the established empty-document recovery
guidance instead; only call get_paragraph with the clamped index when
total_paragraphs is greater than zero.
| ``` | ||
|
|
||
| This creates a tracked deletion of `30 days` and a tracked insertion of `60 days`. | ||
| This creates a tracked deletion of `30` and a tracked insertion of `60`. `replace()` trims the common prefix and suffix and tracks only the differing words, so the unchanged surrounding word (` days`) is left intact rather than deleted and re-inserted. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the Markdown code span spacing.
The ` days` span violates MD038 because it contains a leading space. Rephrase it as text such as " days" outside the code span, or use a code span without surrounding whitespace.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 88-88: Spaces inside code span elements
(MD038, no-space-in-code)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/quickstart.md` at line 88, Update the Markdown example near the
replace() explanation so the ` days` code span no longer contains leading
whitespace; move the space into surrounding prose or use a trimmed code span
while preserving the intended unchanged-word explanation.
Source: Linters/SAST tools
| - **Revision Management**: List, accept, and reject tracked changes at three granularities — individual revisions, groups (one logical edit), and changesets (one whole `batch_edit`/`batch_rewrite` call); `EditResult` and `Revision` objects carry `group_id` and `changeset_id` | ||
| - **Session Mode**: Optional persistent kernel (`docx-session start/exec/eval/status/stop`) keeps documents open across many small commands — ideal for AI agents (`pip install "docx-editor[session]"`) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Document the complete changeset scope consistently.
The API creates a changeset for every whole call, including a single edit; batch_edit()/batch_rewrite() are only examples that can contain multiple groups. Update all descriptions so users understand that ordinary edit results also carry usable changeset_id values.
README.md#L26-L27: broaden the changeset definition beyond batch calls.docs/index.md#L14-L14: mention single edit calls in the Revision Management feature description.docs/quickstart.md#L218-L232: explain that a singlereplace()orrewrite_paragraph()also forms a changeset, while batches may span multiple groups.
📍 Affects 3 files
README.md#L26-L27(this comment)docs/index.md#L14-L14docs/quickstart.md#L218-L232
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 26 - 27, Update the changeset documentation
consistently across README.md lines 26-27, docs/index.md line 14, and
docs/quickstart.md lines 218-232: define a changeset as covering every complete
edit call, including single replace() and rewrite_paragraph() operations, while
noting that batch_edit() and batch_rewrite() are examples that may contain
multiple groups; explicitly state that ordinary edit results expose usable
changeset_id values.
| Pass `dry_run=True` to validate every operation up front without touching the | ||
| document — `doc.batch_edit(ops, dry_run=True)` returns a list of | ||
| `EditValidationResult` instead of applying the edits. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Define ops before using it in the dry-run example.
The preceding example passes an inline operation list, so doc.batch_edit(ops, dry_run=True) references an undefined variable. Assign the operations to ops or repeat the list in this example.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 158 - 160, Update the dry-run example in README.md to
define the ops variable before calling doc.batch_edit(ops, dry_run=True),
reusing the operation list shown in the preceding example or assigning it
explicitly so the example is self-contained.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #69 +/- ##
=====================================
Coverage 95.8% 95.9%
=====================================
Files 11 11
Lines 3984 4057 +73
Branches 798 803 +5
=====================================
+ Hits 3818 3891 +73
Misses 77 77
Partials 89 89 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…atch_edit result naming Multi-reviewer PR review (3 Claude subagents + codex + red-team meta-review): - quickstart changeset example (MEDIUM, flagged by 3 independent reviewers): rewrote P2#f3c1 then reused the now-stale ref in the following batch_edit, which raises BatchOperationError(HashMismatchError) as written and searched for "30 days" the rewrite had already replaced. Move the rewrite to an untouched paragraph (P3#b2c4) so the batch refs stay fresh. - api.md InvalidDocumentError (MEDIUM): "malformed OOXML" → "malformed XML". open() only checks XML well-formedness via expat, never OOXML schema validity; matches SKILL.md and the source docstring. - naming consistency (LOW): quickstart `refs` → `new_refs` (same-file convention); SKILL `batch` → `results` (matches api.md). Skipped (intentional): README vs index.md feature-bullet wording divergence — README is the detailed surface, index the terser one.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/quickstart.md`:
- Around line 229-233: Rename the batch_edit result variable from new_refs to
results in the quickstart example, and update its use in the reject_changeset
call to results[0].changeset_id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 60b2b8c6-a9cd-4226-9932-4451a7d89c7b
📒 Files selected for processing (3)
docs/api.mddocs/quickstart.mdskills/docx/SKILL.md
| new_refs = doc.batch_edit([ | ||
| EditOperation.replace("30 days", "60 days", paragraph="P2#f3c1"), | ||
| EditOperation.delete("obsolete clause", paragraph="P5#d4e5"), | ||
| ]) | ||
| doc.reject_changeset(new_refs[0].changeset_id) # undo the whole batch in one call |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Name the batch result as results, not new_refs.
batch_edit() returns result objects here because the next line reads new_refs[0].changeset_id; the current name suggests paragraph-reference strings and conflicts with the documented EditResult contract. Rename this variable consistently to avoid misleading users.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/quickstart.md` around lines 229 - 233, Rename the batch_edit result
variable from new_refs to results in the quickstart example, and update its use
in the reject_changeset call to results[0].changeset_id.
1 participant
Summary
Pre-0.7.0 documentation audit (ISSUES.md #55). Syncs
README.md,docs/{api,index,quickstart}.md, andskills/docx/SKILL.mdto the current APIsurface. Docs-only — no Python changed.
Every added claim was verified against the source:
(revision / group / changeset).
EditResultandRevisioncarrygroup_idand
changeset_id; newaccept_changeset/reject_changesetwithrecorded-vs-inferred semantics.
replace/delete/insert_after/insert_before/add_comment(the five methods that accept it).batch_editreturningEditValidationResult(
index/valid/error).statussubcommand, and the machine-path-strippedeval traceback (relative
docx_editor/...frames,<docx-session eval>line).HashMismatchError,ParagraphIndexError,InvalidDocumentError— and retargeting of stale#exceptionsanchors toper-exception ids.
paragraph_ref/occurrence/nested_under/contains_ids.30 days→60 daystracks30/60).Verification
docx_editor/source._trim_replace_affixes→30/60).docs/api.mdresolve.mkdocs build --strictpasses.Summary by CodeRabbit
occurrenceparameter, including how ambiguity is handled.dry_run=Truedocumentation for batch editing to validate operations upfront and return validation results without altering documents.