Repository navigation
docs: SKILL/docs round-3 sync — python-docx redline caveats, get_paragraph, WorkspaceLockedError, session EditResult transit (ISSUES.md #49) - #64
Conversation
…graph, WorkspaceLockedError, session EditResult transit (ISSUES.md #49) SKILL.md: - Decision tree + structure-navigation warning: python-docx is blind to tracked changes (paragraph.text silently drops w:ins/w:del content) and its paragraph numbering diverges from P{n} refs at the first table cell - Creation snippet: List Bullet / List Number style examples - Basic Usage exit comment: replaced the false workspace-preservation claim — the kept workspace holds only state flushed by save(); unsaved edits are lost; rescue idiom documented incl. the WorkspaceSyncError it leaves on the next open() of the source - Track-changes walkthrough: get_paragraph() snippet; error table gains WorkspaceLockedError row and get_paragraph recovery hint - Revision groups: revision ids are w:id attributes and survive reopen - Session mode: eval of an edit returns EditResult as a bare JSON string (group_id/revision_ids lost in transit; use a kernel-side projection); status is a point-in-time snapshot - Editing in parallel / Limitations: advisory lock semantics replace the stale "share one workspace and overwrite each other" claim docs/api.md: - New get_paragraph(index) entry - workspace_path: preserved workspace holds last save() flush only
|
Warning Review limit reached
Next review available in: 22 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 (2)
📝 WalkthroughWalkthroughDocumentation updates clarify workspace persistence, add the ChangesDocumentation contracts
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: 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 `@skills/docx/SKILL.md`:
- Line 754: Update the ParagraphIndexError recovery guidance in the error table
to return a usable paragraph reference by recommending doc.get_paragraph(i).ref,
including an explicit empty-document case, or list_paragraphs(start=i, limit=1)
so indexes beyond the default 200-item window can be found.
🪄 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: 10280559-f0a3-4621-b13b-7a8c287e5b05
📒 Files selected for processing (2)
docs/api.mdskills/docx/SKILL.md
| | `TextNotFoundError` | `search_text`, `paragraph_ref`, `paragraph_preview`, `occurrence`, `total_occurrences` | Use `paragraph_preview` to pick a substring that actually appears; if `total_occurrences` is set, retry with an `occurrence` < `total_occurrences`. | | ||
| | `AmbiguousTextError` | `search_text`, `paragraph_ref`, `paragraph_preview`, `total_occurrences` | The target matched more than once with no `occurrence` given. Enumerate with `find_all()` and pick, or pass an explicit `occurrence` (0-based). | | ||
| | `ParagraphIndexError` | `index`, `total_paragraphs` | Clamp to `1..total_paragraphs` or call `list_paragraphs()` to pick a valid ref. | | ||
| | `ParagraphIndexError` | `index`, `total_paragraphs` | Clamp to `1..total_paragraphs` and retry with `get_paragraph(i)`, or call `list_paragraphs()` to pick a valid ref. | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make ParagraphIndexError recovery return a usable paragraph reference.
At Line 754, get_paragraph(i) returns ParagraphInfo, while edit/search methods generally require its .ref; additionally, the default list_paragraphs() window cannot locate indexes above 200. Recommend doc.get_paragraph(i).ref (with an explicit empty-document case) or list_paragraphs(start=i, limit=1).
🧰 Tools
🪛 SkillSpector (2.3.11)
[warning] 749: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 749: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 751: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 759: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 760: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 761: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 975: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 981: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
[warning] 1066: [PE2] Sudo/Root Execution: Commands invoke sudo or root privileges. Verify this elevated access is necessary and justified.
Remediation: Avoid sudo/root unless strictly required. Prefer least-privilege patterns. If elevation is needed, document the justification and scope.
(Privilege Escalation (PE2))
[warning] 1068: [PE2] Sudo/Root Execution: Commands invoke sudo or root privileges. Verify this elevated access is necessary and justified.
Remediation: Avoid sudo/root unless strictly required. Prefer least-privilege patterns. If elevation is needed, document the justification and scope.
(Privilege Escalation (PE2))
[warning] 1069: [PE2] Sudo/Root Execution: Commands invoke sudo or root privileges. Verify this elevated access is necessary and justified.
Remediation: Avoid sudo/root unless strictly required. Prefer least-privilege patterns. If elevation is needed, document the justification and scope.
(Privilege Escalation (PE2))
🤖 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 `@skills/docx/SKILL.md` at line 754, Update the ParagraphIndexError recovery
guidance in the error table to return a usable paragraph reference by
recommending doc.get_paragraph(i).ref, including an explicit empty-document
case, or list_paragraphs(start=i, limit=1) so indexes beyond the default
200-item window can be found.
…ck-aware session bullet, P{i} placeholder
Multi-reviewer findings on PR #64:
- The rescue-idiom parenthetical wrongly claimed the catch-inside flow
leaves a diverged workspace; verified by execution that a swallowed
exception exits the block normally (workspace cleaned up, next open()
clean) while a re-raise after the rescue save keeps it flagged and the
next open() raises WorkspaceSyncError — text now describes both paths
- Session-mode --session-file bullet still said the same document
'shares one workspace'; now states the second open raises
WorkspaceLockedError, matching the rewritten Editing-in-parallel section
- P{n} placeholder aligned to the house P{i} convention
Skipped (red-team verdict LOW/optional): callout blockquote format
…aming, docx_editor prose form - The 'unsaved edits are LOST, next open() succeeds' claim was false for a document's first add_comment: comment-part scaffolding is copied into the workspace and the dirty flag set before any save(), so the next open() of the source raises WorkspaceSyncError instead (verified by execution: unsaved comment + exception → WorkspaceSyncError; unsaved replace + exception → clean open). Scoped the claim to tracked-change edits and documented the comment caveat in SKILL.md and the api.md workspace_path entry - rescue.docx → rescued.docx, matching the established example filename for this recovery scenario (SKILL.md error table, session bullet, api.md exception example) - docx-editor → docx_editor for the library-as-actor prose form
…ocs-round-3-sync--works # Conflicts: # skills/docx/SKILL.md
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #64 +/- ##
=====================================
Coverage 95.8% 95.8%
=====================================
Files 11 11
Lines 3898 3898
Branches 776 776
=====================================
Hits 3736 3736
Misses 75 75
Partials 87 87 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…#76) * feat: report revision types accept_all cannot resolve (ISSUES.md #64) accept_all/reject_all walk w:ins/w:del only, so a redline whose revisions are format changes or moves returned a bare 0 and read as "there was nothing to accept". They now return a ResolveResult (an int subclass carrying .unhandled/.unhandled_types), warn with UnhandledRevisionWarning when adjudication is incomplete, and list_unhandled_revisions() names the marks that are still pending. Adds count_revision_elements()/RevisionCensus and a corpus-harness --census mode recording which revision types real producers emit. * fix: don't count marks recorded inside a change record as pending A w:tcPrChange records the cell's previous w:tcPr, which is CT_TcPrInner and so may legally hold a w:cellIns/w:cellDel/w:cellMerge. Those describe state that is already gone, but the unhandled scan descended into the record and reported them as extra pending revisions. iter_revision_elements now takes skip_change_records, which _unhandled_elements uses; count_revision_elements stays a raw inventory and documents the difference. Also: extract _parse_w_date instead of a third copy of the w:date parse; correct the "excluded from every filtered call" author rule, which the author="Unknown" test contradicted; move the SKILL.md honesty check next to the call it inspects, since it read a result four calls stale; drop the incorrect ECMA sub-numbers; label the census error count by part rather than file and stop truncating it silently. * fix: correct the change-record schema note and pin the handled-path gap The rationale for CHANGE_RECORD_TAGS claimed only w:tcPrChange's recorded subtree can hold revision marks. Two more can: a paragraph-mark w:rPrChange records CT_ParaRPrOriginal, which opens with EG_ParaRPrTrackChanges, and w:pPrChange records CT_PPrBase, whose w:numPr admits w:ins/w:numberingChange. Only the pending-revision path skips those subtrees; list_revisions and accept_all still walk every w:ins/w:del, so a historical w:del recorded in a change record is listed and resolved like a live one. That is pre-existing, and is now pinned by an xfail(strict=True) test so ISSUES.md #68 inherits it rather than rediscovering it. Also: guard CHANGE_RECORD_TAGS against drifting out of the property-change family; group w:trPrChange with the property changes in the warning docstring; stop labelling every w:rPr context as a paragraph mark, since a change record's recorded rPr shares the tag; report files whose census never ran instead of counting them as revision-free; build the location context only when list_unhandled_revisions has rows; name the author scope in the warning.
Summary
Round-3 documentation sync (ISSUES.md #49, referenced as plain text on purpose — repo-local issue list, not a GitHub issue): audits and fixes false/stale claims in
skills/docx/SKILL.mdanddocs/api.md, and documents APIs that shipped without docs. Docs-only diff; no library code touched.SKILL.md
paragraph.textsilently drops everything insidew:ins/w:del(a redlined doc reads as mangled prose with no error); read redlined docs with docx_editor orpandoc --track-changes=all. Also: never carry paragraph indexes between libraries — python-docx excludes table-cell paragraphs,P{n}refs include them, so numbering diverges at the first table.save(); unsaved edits live in memory and are lost. Rescue idiom documented (catch inside thewithblock,doc.save("rescue.docx")) including its side effect: the rescue save flags the workspace, so the nextopen()of the source raisesWorkspaceSyncError.WorkspaceLockedErrorwithpid/lock_path, same-process detection, stale-lock reclaim,force_recreate=Truetakeover) instead of the stale "two processes share one workspace and overwrite each other" claim.get_paragraph(): walkthrough snippet +ParagraphIndexErrorrecovery hint.w:idattributes stored in the XML — stable across save/close/reopen (groups are not).evalof an edit returnsEditResultas a bare JSON string (strsubclass) —group_id/revision_idslost in transit; keep the result kernel-side and eval a dict projection.statusis a point-in-time snapshot.List Bullet/List Number).docs/api.md
get_paragraph(index)reference entry (house format, cross-linked).workspace_path: preserved workspace holds the lastsave()flush only.Verification
Every changed claim was verified by execution against this worktree's library (probe scripts: redline blindness output, P-count divergence 4 vs 2 on a table doc,
get_paragraphparity + bounds errors,WorkspaceLockedErrorwith live pid, revision-id stability across reopen, exception-path workspace contents, rescue-idiomWorkspaceSyncErrorfollow-up, session envelope for an eval'd edit).Gates:
pytest1200 passed / 2 skipped,ruff check+ruff format --checkclean,ty checkexit 0 (7 pre-existing tests/-only warnings),mkdocs build --strictclean.Summary by CodeRabbit
Document.get_paragraph(index)API, including usage, return values, and errors.