refactor(slides): streamline create workflow and validation#1887
Merged
Conversation
📝 WalkthroughWalkthroughSlides shortcuts now support bounded screenshot batches and flexible full- or single-slide XML retrieval. The XML linter adds schema, namespace, icon, text-overlap, and whiteboard checks, while Slides guidance documents revised creation, validation, readback, and template workflows. ChangesSlides shortcut behavior
XML linting and Slides guidance
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant SlidesXMLGet
participant SlidesAPI
participant OutputWriter
CLI->>SlidesXMLGet: provide presentation and optional slide selector
SlidesXMLGet->>SlidesAPI: request full or single-slide XML
SlidesAPI-->>SlidesXMLGet: return XML content
SlidesXMLGet->>OutputWriter: emit JSON, raw XML, or file metadata
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1887 +/- ##
==========================================
+ Coverage 74.66% 74.68% +0.01%
==========================================
Files 878 878
Lines 91764 91894 +130
==========================================
+ Hits 68520 68633 +113
- Misses 17930 17941 +11
- Partials 5314 5320 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@87c055d32e06b7fc5a55ff7521664f4238eafa0a🧩 Skill updatenpx skills add larksuite/cli#refactor/slides-create-workflow-validation -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 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 `@shortcuts/slides/slides_screenshot_test.go`:
- Around line 293-298: Update the error assertion in the slide screenshot test
to inspect typed metadata through errs.ProblemOf instead of matching the error
message substring. Assert the expected category, subtype, and parameter metadata
for the maximum-pages validation while retaining the existing requirement that
an error is returned.
In `@shortcuts/slides/slides_screenshot.go`:
- Around line 284-288: Update validateSlidesScreenshotSelectorLimit so the
validation error message contains only the limit violation details, and attach
the “request at most 10 pages at a time” recovery guidance via WithHint.
Construct the error directly with errs.NewValidationError and preserve the
existing nil return when count is within maxSlidesPerScreenshot.
In `@shortcuts/slides/slides_xml_get_test.go`:
- Around line 387-406: Update TestSlidesXMLGetRejectsConflictingSlideSelectors,
TestSlidesXMLGetRejectsEmptySlideID, and
TestSlidesXMLGetRejectsRemoveAttrIDForSingleSlide to validate errors through
errs.ProblemOf(err), matching TestSlidesXMLGetRejectsUnsafeOutputPath. Assert
the expected category, subtype, and parameter metadata, and verify the
underlying cause is preserved instead of only checking ValidationError.Param.
In `@skills/lark-slides/references/examples.md`:
- Around line 10-11: Update the two table-of-contents links for 示例 7 and 示例 8 to
use fragments generated from their exact example headings, including the correct
Markdown anchor normalization. Verify both targets resolve successfully and
preserve the existing link text.
- Around line 57-71: Update the presentation creation flow around
PRESENTATION_ID so the lark-cli slides +create exit status is captured
separately from writing create.json, preserving failures instead of allowing the
pipeline to report jq’s status. Extract .data.xml_presentation_id from
create.json with jq -e and validate that it is a non-empty string before saving
it for readback.
In `@skills/lark-slides/references/troubleshooting.md`:
- Line 13: Update the two-step creation guidance in the troubleshooting
documentation to use the registered command syntax `xml_presentation.slide
create` instead of `xml_presentation.slide.create`, while preserving the
surrounding recovery flow.
In `@skills/lark-slides/references/validation-checklist.md`:
- Line 92: Update the whiteboard coordinate validation guidance in the checklist
to include lower-bound checks: topLeftX and topLeftY must be at least zero, and
width and height must be non-negative, alongside the existing right and bottom
upper bounds.
In `@skills/lark-slides/references/xml-schema-quick-ref.md`:
- Line 149: 更新该 XML schema 快速参考中关于 iconType 的说明，明确要求其必须存在于离线 IconPark 索引
iconpark-index.json 中，而非仅使用“已验证路径”的表述；保持 fillColor 和 iconpark.md 的相关规则不变。
In `@skills/lark-slides/scripts/xml_text_overlap_lint.py`:
- Around line 500-536: Update the shape text extraction in the element-building
loop so paragraph boundaries from separate <p> elements are preserved before
line-width estimation. Replace the strip_xml(content) path used for the
element’s “text” value with paragraph-aware normalization that emits newline
separators between paragraphs, and apply the same handling to the related text
extraction around the later reference. Keep existing tag removal and whitespace
behavior for content within each paragraph.
- Around line 733-734: Update lint_slide and the should_flag_overlap flow so
should_flag_horizontal_text_overflow is evaluated before the
declared-bounding-box intersection gate, allowing overflow across gaps to be
detected. Preserve intersection-based handling for other overlap cases, and add
a regression test covering adjacent text boxes with non-intersecting declared
boxes.
- Around line 408-462: Update validate_sml_tag_prefixes to create the Expat
parser with a namespace separator so StartNamespaceDeclHandler is enabled and
namespace_map is populated before prefixed-tag validation. Preserve the existing
declaration scoping and SML_NAMESPACE checks.
In `@skills/lark-slides/SKILL.md`:
- Line 13: Update the global hard constraint in the slide guidance to treat
960x540 as the default canvas size rather than an invariant. Instruct
implementations to preserve and use the declared dimensions of imported or
existing presentations, keeping all content within that presentation’s actual
page boundaries.
- Line 183: Replace the bare Chinese text under the XML <data> element with a
valid XML comment, preserving its guidance that shape, line, table, and chart
elements belong there.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4d4fef28-2413-438b-b56e-985b09dcabb7
📒 Files selected for processing (25)
shortcuts/slides/slides_screenshot.goshortcuts/slides/slides_screenshot_test.goshortcuts/slides/slides_xml_get.goshortcuts/slides/slides_xml_get_test.goskills/lark-slides/SKILL.mdskills/lark-slides/references/examples.mdskills/lark-slides/references/iconpark.mdskills/lark-slides/references/lark-slides-create.mdskills/lark-slides/references/lark-slides-media-upload.mdskills/lark-slides/references/lark-slides-pptx-template-workflows.mdskills/lark-slides/references/lark-slides-replace-pages.mdskills/lark-slides/references/lark-slides-replace-slide.mdskills/lark-slides/references/lark-slides-screenshot.mdskills/lark-slides/references/lark-slides-xml-get.mdskills/lark-slides/references/lark-slides-xml-presentation-slide-create.mdskills/lark-slides/references/lark-slides-xml-presentation-slide-delete.mdskills/lark-slides/references/lark-slides-xml-presentation-slide-get.mdskills/lark-slides/references/lark-slides-xml-presentations-get.mdskills/lark-slides/references/planning-layer.mdskills/lark-slides/references/troubleshooting.mdskills/lark-slides/references/validation-checklist.mdskills/lark-slides/references/visual-planning.mdskills/lark-slides/references/xml-schema-quick-ref.mdskills/lark-slides/scripts/xml_text_overlap_lint.pyskills/lark-slides/scripts/xml_text_overlap_lint_test.py
💤 Files with no reviewable changes (2)
- skills/lark-slides/references/lark-slides-xml-presentation-slide-create.md
- skills/lark-slides/references/lark-slides-media-upload.md
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/lark-slides/scripts/xml_text_overlap_lint_test.py`:
- Around line 595-596: Update the test assertions for the horizontal-overflow
case to also verify that the reported issue code is exactly bbox_overlap, while
preserving the existing error-count and element-pair assertions.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a49e8e2-b4c4-4815-9a40-9316053055f6
📒 Files selected for processing (10)
shortcuts/slides/slides_screenshot.goshortcuts/slides/slides_screenshot_test.goshortcuts/slides/slides_xml_get.goshortcuts/slides/slides_xml_get_test.goskills/lark-slides/SKILL.mdskills/lark-slides/references/asset-planning.mdskills/lark-slides/references/examples.mdskills/lark-slides/references/troubleshooting.mdskills/lark-slides/scripts/xml_text_overlap_lint.pyskills/lark-slides/scripts/xml_text_overlap_lint_test.py
💤 Files with no reviewable changes (1)
- skills/lark-slides/references/asset-planning.md
🚧 Files skipped from review as they are similar to previous changes (7)
- shortcuts/slides/slides_screenshot_test.go
- skills/lark-slides/references/examples.md
- shortcuts/slides/slides_xml_get_test.go
- skills/lark-slides/SKILL.md
- shortcuts/slides/slides_screenshot.go
- shortcuts/slides/slides_xml_get.go
- skills/lark-slides/scripts/xml_text_overlap_lint.py
ethan-zhx
force-pushed
the
refactor/slides-create-workflow-validation
branch
from
July 15, 2026 03:47
6d90cb3 to
fba8d5e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
skills/lark-slides/scripts/xml_text_overlap_lint.py (2)
246-296: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
pathdoesn't disambiguate siblings, so multi-slide validation output is ambiguous.
validate_sxsd_tag_attributesandvalidate_iconpark_icon_typesboth run once against the full<presentation>root (line 905-906), but the recursivevisit()never records a child's ordinal position among its siblings. For a presentation with multiple<slide>elements (a supported, tested scenario pertest_lint_xml_preserves_presentation_canvas_and_slide_order), an unsupported tag/attr or icon issue on slide 2 gets the exact samepathstring (e.g."presentation/slide/data/shape") as the same issue would get on slide 1, making it hard to locate which slide actually has the problem.♻️ Suggested fix: thread a sibling index into the path
- def visit(element: ET.Element, ancestors: list[str], path: str) -> None: + def visit(element: ET.Element, ancestors: list[str], path: str, position: int | None = None) -> None: if should_skip_sxsd_subtree(element, ancestors): return tag_name = xml_local_name(element.tag) - current_path = f"{path}/{tag_name}" if path else tag_name + segment = f"{tag_name}[{position}]" if position is not None else tag_name + current_path = f"{path}/{segment}" if path else segment ... - for child in element: - visit(child, [*ancestors, tag_name], current_path) + for index, child in enumerate(element): + visit(child, [*ancestors, tag_name], current_path, index) - visit(root, [], "") + visit(root, [], "")The same pattern applies to
validate_iconpark_icon_types'svisit()(lines 340-381). None of the provided tests assert on exactpathstrings, so this is safe to land independently.Also applies to: 309-384
🤖 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/lark-slides/scripts/xml_text_overlap_lint.py` around lines 246 - 296, Update the recursive visit functions in validate_sxsd_tag_attributes and validate_iconpark_icon_types to include each element’s ordinal sibling index in the generated path, while preserving existing tag names and traversal behavior. Thread the index through recursion so repeated slide paths, such as slide 1 versus slide 2, are distinct in all reported issues.
481-495: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSwitch
parse_xml_roottodefusedxml
ET.fromstring(xml)parses arbitrary slide XML here;defusedxml.ElementTree.fromstringis a safer drop-in that blocks DTD/entity-expansion payloads.🤖 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/lark-slides/scripts/xml_text_overlap_lint.py` around lines 481 - 495, Update parse_xml_root to use defusedxml.ElementTree.fromstring instead of the standard ET.fromstring, while preserving the existing ParseError handling and root validation behavior. Ensure the imported defusedxml parser exposes the exception type caught by the current error handling.
🤖 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/lark-slides/references/examples.md`:
- Line 5: Update the command reference in the slide-creation guidance to use the
registered syntax “xml_presentation.slide create” instead of the dotted
“xml_presentation.slide.create” form, while preserving the surrounding
instructions about adding slides incrementally.
In `@skills/lark-slides/references/lark-slides-create.md`:
- Around line 8-10: Rephrase the guidance in the create slides documentation to
recommend using directly generated single-slide XML and avoiding payloads split
from presentation XML, rather than stating that such input is prohibited or
rejected. Preserve the behavior that slides +create accepts a JSON array and
submits each element as a slide.
---
Nitpick comments:
In `@skills/lark-slides/scripts/xml_text_overlap_lint.py`:
- Around line 246-296: Update the recursive visit functions in
validate_sxsd_tag_attributes and validate_iconpark_icon_types to include each
element’s ordinal sibling index in the generated path, while preserving existing
tag names and traversal behavior. Thread the index through recursion so repeated
slide paths, such as slide 1 versus slide 2, are distinct in all reported
issues.
- Around line 481-495: Update parse_xml_root to use
defusedxml.ElementTree.fromstring instead of the standard ET.fromstring, while
preserving the existing ParseError handling and root validation behavior. Ensure
the imported defusedxml parser exposes the exception type caught by the current
error handling.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d86534e1-7518-4c5c-8b7c-a936d709df7c
📒 Files selected for processing (26)
shortcuts/slides/slides_screenshot.goshortcuts/slides/slides_screenshot_test.goshortcuts/slides/slides_xml_get.goshortcuts/slides/slides_xml_get_test.goskills/lark-slides/SKILL.mdskills/lark-slides/references/asset-planning.mdskills/lark-slides/references/examples.mdskills/lark-slides/references/iconpark.mdskills/lark-slides/references/lark-slides-create.mdskills/lark-slides/references/lark-slides-media-upload.mdskills/lark-slides/references/lark-slides-pptx-template-workflows.mdskills/lark-slides/references/lark-slides-replace-pages.mdskills/lark-slides/references/lark-slides-replace-slide.mdskills/lark-slides/references/lark-slides-screenshot.mdskills/lark-slides/references/lark-slides-xml-get.mdskills/lark-slides/references/lark-slides-xml-presentation-slide-create.mdskills/lark-slides/references/lark-slides-xml-presentation-slide-delete.mdskills/lark-slides/references/lark-slides-xml-presentation-slide-get.mdskills/lark-slides/references/lark-slides-xml-presentations-get.mdskills/lark-slides/references/planning-layer.mdskills/lark-slides/references/troubleshooting.mdskills/lark-slides/references/validation-checklist.mdskills/lark-slides/references/visual-planning.mdskills/lark-slides/references/xml-schema-quick-ref.mdskills/lark-slides/scripts/xml_text_overlap_lint.pyskills/lark-slides/scripts/xml_text_overlap_lint_test.py
💤 Files with no reviewable changes (3)
- skills/lark-slides/references/lark-slides-xml-presentation-slide-create.md
- skills/lark-slides/references/asset-planning.md
- skills/lark-slides/references/lark-slides-media-upload.md
🚧 Files skipped from review as they are similar to previous changes (16)
- skills/lark-slides/references/lark-slides-replace-pages.md
- skills/lark-slides/references/visual-planning.md
- skills/lark-slides/references/lark-slides-xml-presentation-slide-get.md
- skills/lark-slides/references/planning-layer.md
- shortcuts/slides/slides_screenshot.go
- shortcuts/slides/slides_screenshot_test.go
- skills/lark-slides/references/lark-slides-xml-presentation-slide-delete.md
- skills/lark-slides/references/lark-slides-xml-get.md
- skills/lark-slides/references/lark-slides-screenshot.md
- skills/lark-slides/references/xml-schema-quick-ref.md
- skills/lark-slides/references/lark-slides-replace-slide.md
- shortcuts/slides/slides_xml_get.go
- skills/lark-slides/references/validation-checklist.md
- shortcuts/slides/slides_xml_get_test.go
- skills/lark-slides/references/lark-slides-xml-presentations-get.md
- skills/lark-slides/SKILL.md
ethan-zhx
force-pushed
the
refactor/slides-create-workflow-validation
branch
from
July 15, 2026 04:38
fba8d5e to
87c055d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/lark-slides/scripts/xml_text_overlap_lint.py`:
- Around line 713-719: Update the sibling filtering logic around
is_text_element, has_text_content, is_template_text_stack, and
is_similar_text_overlay so only the left element must be a text element with
content; allow non-text right siblings as overflow targets. Apply the text-stack
and similar-overlay exclusions only when the right element is also text, and add
a regression case covering text overflowing into an image.
- Around line 908-918: Update the pre-parse validation alongside the
namespace_issues handling so fatal structural SXSD issues, including
sxsd_unsupported_tag, return the existing structured error response before
parse_presentation is called. Alternatively, make slide parsing consume the
already validated root; preserve the machine-readable output shape and existing
handling for valid presentations.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 045c9b0c-1686-481c-b83f-e5f0cf901065
📒 Files selected for processing (10)
shortcuts/slides/slides_screenshot.goshortcuts/slides/slides_screenshot_test.goshortcuts/slides/slides_xml_get.goshortcuts/slides/slides_xml_get_test.goskills/lark-slides/SKILL.mdskills/lark-slides/references/asset-planning.mdskills/lark-slides/references/examples.mdskills/lark-slides/references/troubleshooting.mdskills/lark-slides/scripts/xml_text_overlap_lint.pyskills/lark-slides/scripts/xml_text_overlap_lint_test.py
💤 Files with no reviewable changes (1)
- skills/lark-slides/references/asset-planning.md
🚧 Files skipped from review as they are similar to previous changes (7)
- shortcuts/slides/slides_screenshot_test.go
- shortcuts/slides/slides_screenshot.go
- skills/lark-slides/references/examples.md
- shortcuts/slides/slides_xml_get.go
- skills/lark-slides/SKILL.md
- shortcuts/slides/slides_xml_get_test.go
- skills/lark-slides/scripts/xml_text_overlap_lint_test.py
fangshuyu-768
approved these changes
Jul 15, 2026
This was referenced Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Streamline the Lark Slides create/edit workflow and add validation coverage for XML, namespaces, icon types, text overlap, and screenshot requests.
Changes
Test Plan
Related Issues
Summary by CodeRabbit
slides +xml-getcan now fetch full presentation XML or a single slide XML (by ID/number), with optional--output, default JSON envelope, and--rawstdout mode.slides +screenshotenforces max 10 pages per request when using slide selectors.slides +xml-getreadback/verification flow and usage guidance.