API hygiene: typed EditOperation constructors + public SearchResult#40
Conversation
…_all__ Add EditOperation.replace/.delete/.insert_after/.insert_before classmethods that validate at construction time with the same rules batch_edit applies (field-specific ValueError messages), mirroring the Document method signatures 1:1. The raw EditOperation(action=..., ...) form remains supported (#25). Document.find_text now returns a public SearchResult (start/end offsets in the paragraph's visible text, matched text, hash-anchored paragraph_ref usable in follow-up edits, spans_revision flag) instead of leaking the internal TextMapMatch. Remove TextMap, TextMapMatch, TextPosition, build_text_map and find_in_text_map from the top-level public API; a module __getattr__ shim keeps them importable with a DeprecationWarning for one release, and they remain available from docx_editor.xml_editor (#7).
|
Warning Review limit reached
Next review available in: 10 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 (12)
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #40 +/- ##
=======================================
+ Coverage 93.5% 94.1% +0.5%
=======================================
Files 11 11
Lines 2950 2910 -40
Branches 634 607 -27
=======================================
- Hits 2761 2740 -21
+ Misses 84 76 -8
+ Partials 105 94 -11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Multi-reviewer findings: find_text's occurrence counts matches document-wide, while edit methods count within one paragraph, so chaining paragraph_ref into a follow-up edit could silently target a different match when the text repeats inside the found paragraph. SearchResult now carries paragraph_occurrence (the within-paragraph index, same text-map counting _find_in_paragraph uses) to pass as the follow-up occurrence=. _find_across_boundaries_located returns a _LocatedMatch dataclass instead of a growing tuple. Also from review: docs/quickstart.md batch example now uses the typed constructors, and the api.md Raises summary no longer implies empty replace/insert payloads are rejected. Skipped deliberately: constructor error-message prefix divergence from _resolve_action_target (field-specific messages are intentional) and a spans_boundary compat alias (would re-introduce the deprecated name).
Review iteration 2: README's Batch Editing section and batch_edit()'s own docstring example still showed the raw EditOperation(action=...) form the PR steers users away from.
…rence, refs, and batch elements (ISSUES.md #40) Fixes the three ISSUES.md #40 input-validation bugs plus the same-class type hardening the 5-iteration multi-reviewer loop surfaced: - Empty/None search text: rejected up front (was: silent edit loss with explicit occurrence, meaningless AmbiguousTextError without) - Non-EditOperation batch elements: BatchOperationError with op index (was: raw AttributeError); validate_batch returns invalid rows - Non-string paragraph refs: ValueError from ParagraphRef.parse and the four Document edit methods (was: raw TypeError / silent doc-wide fallback) - Truthy non-string search text and payloads: ValueError at every boundary (was: raw TypeError/AttributeError escaping the batch contracts; dry-run reported such ops valid) - occurrence type-leak: shared _require_valid_occurrence guard at all six boundaries (was: raw TypeError for str/float, bool silently misread) - add_comment: non-string anchor and comment rejected with CommentError before any mutation (was: raw TypeError; non-string comment corrupted document.xml with orphaned range markers) 27 new TDD-first tests; docstrings and SKILL.md error contract updated. Known follow-up: reply_to_comment has the same non-string payload gap.
1 participant
Implements ISSUES.md items 25 (typed EditOperation constructors) and 7 (internal types exported as public API) — the repo-local backlog file, not GitHub issues.
What
Typed
EditOperationconstructors (#25).EditOperation.replace/.delete/.insert_after/.insert_beforeclassmethods validate arguments at construction time with the same rulesbatch_edit()applies, raisingValueErrorwith field-specific messages. Signatures mirror the correspondingDocumentmethods 1:1, sodoc.replace(...)translates mechanically toEditOperation.replace(...). The rawEditOperation(action=..., ...)form remains supported.Public
SearchResult+ sealed__all__(#7).Document.find_text()now returns aSearchResult—start/endoffsets in the containing paragraph's visible text, matchedtext, a hash-anchoredparagraph_refdirectly usable as theparagraph=of follow-up edits, and aspans_revisionflag. The text-map internals (TextMap,TextMapMatch,TextPosition,build_text_map,find_in_text_map) are removed from__all__; a module__getattr__shim keeps top-level access working with aDeprecationWarningfor one release, and the internals remain importable fromdocx_editor.xml_editor.Breaking change
Document.find_text()no longer returns the internalTextMapMatch: code reading.spans_boundaryor.positionsoff its result must migrate toSearchResult.spans_revision/paragraph_ref, or use the internals viadocx_editor.xml_editor. All in-repo consumers (tests, docs, README, skill) are migrated in this PR.Validation parity
Constructor checks exactly mirror apply-time
_resolve_action_targetsemantics — including the deliberate allowances (replace_with=""and inserttext=""are valid tracked edits). A dry-run drift-guard test pins the parity so future divergence fails loudly.Testing
SearchResultshape + ref chaining into a follow-up edit, deprecation shim warns/forwards,__all__surface checksruff check/ruff format --checkclean