refactor: reorganize lark slides skill references - #2207
Conversation
📝 WalkthroughWalkthroughThe Slides skill now uses reorganized XML, workflow, CLI, and validation references. The change adds the SML 2.0 schema, chart and IconPark references, XML linting with tests, and compatibility stubs for legacy paths. ChangesSlides XML workflow
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.6)skills/lark-slides/references/iconpark-index.jsonFile contains syntax errors that prevent linting: Line 1: unexpected character 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 |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@ad82da4ef4f0949822cb28c69a78aac43ac19eaa🧩 Skill updatenpx skills add larksuite/cli#refactor/reorg_slides -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: 12
🧹 Nitpick comments (2)
skills/lark-slides/scripts/xml_lint.py (1)
2968-2975: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unreachable
--helpkey check.
parse_argsstrips the leading--from each token, so the options dictionary never contains the key"--help". Onlyoptions.get("help")can be true.♻️ Proposed cleanup
- if options.get("help") or options.get("--help"): + if options.get("help"):🤖 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_lint.py` around lines 2968 - 2975, Remove the unreachable options.get("--help") check from run_cli, leaving the existing options.get("help") handling and usage behavior unchanged.skills/lark-slides/scripts/xml_text_overlap_lint.py (1)
2-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the compatibility stub.
xml_text_overlap_lintis only referenced in the stub itself, while existing docs/test/workflow usages point toscripts/xml_lint.py, so the wrapper module should be removed to avoid accidental use.🤖 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 2 - 7, Remove the xml_text_overlap_lint compatibility stub entirely, including its imports and re-exports; retain the canonical xml_lint module and its run_cli and XmlLayoutLintError symbols unchanged.
🤖 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/cli/lark-slides-media-upload.md`:
- Around line 28-36: Align the media upload response example with the token
extraction commands: either nest file_token under data or update the jq paths in
the documented upload flows, including the references around lines 66-68 and
108, to read the top-level field. Ensure the copy-paste commands return the
actual token rather than null.
In `@skills/lark-slides/references/cli/lark-slides-replace-slide.md`:
- Around line 132-138: Update the `<table>` example to include explicit `width`
and `height` attributes alongside `topLeftX` and `topLeftY`, ensuring it
satisfies the XML contract described in `SKILL.md` while preserving the existing
table structure.
In
`@skills/lark-slides/references/cli/lark-slides-xml-presentation-slide-replace.md`:
- Around line 42-49: Update the direct block_replace examples in this document
to include id="bab" on each replacement root, matching the corresponding
block_id so copied commands satisfy the required replacement contract; leave all
block_insert examples unchanged.
In `@skills/lark-slides/references/iconpark-index.json`:
- Around line 1-5: The legacy references/iconpark-index.json path must remain
valid JSON for existing consumers. Replace the Markdown migration notice in
iconpark-index.json with a symlink or equivalent copy of
xml/iconpark-index.json, and place the migration notice in a separate Markdown
file.
In `@skills/lark-slides/references/lark-slides-add-slide.md`:
- Line 3: Update the migration notices in
skills/lark-slides/references/lark-slides-add-slide.md:3 and
skills/lark-slides/references/lark-slides-delete-slide.md:3 to point to existing
destination documents under skills/lark-slides, or create the referenced target
documents before linking to them.
In `@skills/lark-slides/references/lark-slides-edit-workflows.md`:
- Line 3: Fix the broken migration links in
skills/lark-slides/references/lark-slides-edit-workflows.md:3-3 and
skills/lark-slides/references/lark-slides-pptx-template-workflows.md:3-3 by
moving the referenced documents to the stated workflow/ paths or updating both
links to their actual existing locations, ensuring all migration notices and
back-references resolve correctly.
In `@skills/lark-slides/references/lark-slides-history.md`:
- Line 1: Remove the two stray backticks immediately before the Markdown heading
marker in the slides history document so the line begins directly with # and
renders as a heading.
In `@skills/lark-slides/references/slides_xml_schema_definition.xml`:
- Around line 1-5: Remove the Markdown compatibility notice from
skills/lark-slides/references/slides_xml_schema_definition.xml and update all
remaining references to use xml/slides_xml_schema_definition.xml, or replace it
with parseable XML containing the notice in an XML comment. Apply the same
treatment to skills/lark-slides/references/slides_chart_demo.xml, directing
references to xml/slides_chart_demo.xml; ensure both .xml paths remain valid for
XML parsers and xml_lint.py.
In `@skills/lark-slides/references/xml/iconpark.md`:
- Line 29: Update the icon fallback guidance to choose the nearest semantically
relevant icon when no suitable match is found, rather than selecting randomly
from frequent examples. If no semantic fallback exists, remove the icon and
reclaim its layout space, preserving the surrounding icon-search guidance.
In `@skills/lark-slides/references/xml/slides_chart_demo.xml`:
- Around line 226-229: Correct the footer page-count text in each visible footer
shape, including the instances near the referenced sections, so the seven-slide
presentation is numbered sequentially from 01 / 07 through 07 / 07. Preserve the
existing footer styling and update only the displayed pagination values.
In `@skills/lark-slides/references/xml/slides_xml_schema_definition.xml`:
- Around line 955-968: Update the shadow documentation text in the annotation
above the shadow attributes to name the attribute as align instead of
shadowAlign, matching the declared xs:attribute and avoiding
unsupported-attribute guidance.
In `@skills/lark-slides/scripts/xml_lint.py`:
- Around line 325-344: Update load_iconpark_icon_types to handle a missing or
unreadable ICONPARK_INDEX_PATH before read_text, following
iconpark_tool.load_index’s existence-check-and-fail pattern. Route the failure
through fail(...) so the CLI’s existing XmlLayoutLintError handling emits the
standard error message, while preserving the current JSON decoding and
validation behavior.
---
Nitpick comments:
In `@skills/lark-slides/scripts/xml_lint.py`:
- Around line 2968-2975: Remove the unreachable options.get("--help") check from
run_cli, leaving the existing options.get("help") handling and usage behavior
unchanged.
In `@skills/lark-slides/scripts/xml_text_overlap_lint.py`:
- Around line 2-7: Remove the xml_text_overlap_lint compatibility stub entirely,
including its imports and re-exports; retain the canonical xml_lint module and
its run_cli and XmlLayoutLintError symbols unchanged.
🪄 Autofix
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 Plus
Run ID: 4b78c468-0ff2-43dc-bdcd-7181e604cb8f
📒 Files selected for processing (36)
skills/lark-slides/SKILL.mdskills/lark-slides/references/cli/lark-slides-media-upload.mdskills/lark-slides/references/cli/lark-slides-replace-pages.mdskills/lark-slides/references/cli/lark-slides-replace-slide.mdskills/lark-slides/references/cli/lark-slides-xml-presentation-slide-get.mdskills/lark-slides/references/cli/lark-slides-xml-presentation-slide-replace.mdskills/lark-slides/references/cli/lark-slides-xml-presentations-get.mdskills/lark-slides/references/iconpark-index.jsonskills/lark-slides/references/iconpark.mdskills/lark-slides/references/lark-slides-add-slide.mdskills/lark-slides/references/lark-slides-create.mdskills/lark-slides/references/lark-slides-delete-slide.mdskills/lark-slides/references/lark-slides-edit-workflows.mdskills/lark-slides/references/lark-slides-history.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-xml-presentation-slide-get.mdskills/lark-slides/references/lark-slides-xml-presentation-slide-replace.mdskills/lark-slides/references/lark-slides-xml-presentations-get.mdskills/lark-slides/references/planning-layer.mdskills/lark-slides/references/slides_chart_demo.xmlskills/lark-slides/references/slides_xml_schema_definition.xmlskills/lark-slides/references/troubleshooting.mdskills/lark-slides/references/validation-checklist.mdskills/lark-slides/references/xml-schema-quick-ref.mdskills/lark-slides/references/xml/iconpark-index.jsonskills/lark-slides/references/xml/iconpark.mdskills/lark-slides/references/xml/slides_chart_demo.xmlskills/lark-slides/references/xml/slides_xml_schema_definition.xmlskills/lark-slides/scripts/iconpark_tool.pyskills/lark-slides/scripts/xml_lint.pyskills/lark-slides/scripts/xml_lint_test.pyskills/lark-slides/scripts/xml_text_overlap_lint.pyskills/lark-slides/scripts/xml_text_overlap_lint_test.py
| ## 返回值 | ||
|
|
||
| ```json | ||
| { | ||
| "file_token": "boxcnXXXXXXXXXXXXXXXXXXXXXX", | ||
| "file_name": "pic.png", | ||
| "size": 12345, | ||
| "presentation_id": "slidesXXXXXXXXXXXXXXXXXXXXXX" | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'media-upload|file_token|\.data\.file_token' skills/lark-slides
rg -n -C 8 'media-upload|file_token' --glob '*.py' --glob '*.js' --glob '*.ts' .Repository: larksuite/cli
Length of output: 29414
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository files matching media-upload:\n'
git ls-files | grep -F 'media-upload' || true
printf '\nSearch for media-upload handlers/tests/docs:\n'
rg -n -C 3 'media-upload|upload_all|file_token|\.data\.file_token|jq -r .data.file_token' .
printf '\nCandidate CLI/tooling files:\n'
git ls-files | grep -Ei 'lark|slides|media|upload|tool' | head -200Repository: larksuite/cli
Length of output: 50374
🌐 Web query:
Lark CLI slides +media-upload response JSON file_token data.file_token
💡 Result:
When using the Lark CLI shortcut slides +media-upload to upload a local image to a presentation, the command returns a JSON response containing the file_token [1][2]. This file_token is the identifier required to embed the image within a slide's XML content using an <img src="..."> tag [1][3]. The standard JSON response structure returned by the command is [1][4]: { "file_token": "boxcnXXXXXXXXXXXXXXXXXXXXXX", "file_name": "pic.png", "size": 12345, "presentation_id": "slidesXXXXXXXXXXXXXXXXXXXXXX" } To access the file_token data specifically in a shell script, you can pipe the output into a tool like jq [4]: TOKEN=$(lark-cli slides +media-upload --file./image.png --presentation "$PID" | jq -r '.data.file_token') Key technical details regarding this process: 1. Parent Type: The +media-upload shortcut automatically sets the API parameter parent_type to slide_file [1][5]. This is a specific requirement for Lark Slides; using other values like slide_image will result in an error [1][2][3]. 2. Limitations: The command supports images up to 20 MB [1][6]. It does not support external HTTP/HTTPS URLs; files must be local to the machine running the CLI [6]. 3. Usage: The returned file_token should be placed directly into the src attribute of an img tag within the slide XML structure (e.g., <img src="boxcn..." />) [1][4]. Attempting to use a raw URL in the src field will not render the image [6].
Citations:
- 1: https://github.com/larksuite/cli/blob/main/skills/lark-slides/references/lark-slides-media-upload.md
- 2: https://github.com/larksuite/cli/blob/HEAD/skills/lark-slides/references/lark-slides-media-upload.md
- 3: ec9e67c
- 4: https://github.com/larksuite/cli/blob/main/skills/lark-slides/references/examples.md
- 5: feat(slides): add image upload via +media-upload and @path placeholders in +create #450
- 6: https://github.com/larksuite/cli/blob/main/skills/lark-slides/SKILL.md
Align the media upload JSON example with token extraction.
The documented upload response returns file_token at the top level, but the copy-paste flow reads .data.file_token. Update the JSON example or the jq path in lines 66-68, 108, and similar references so the command returns the token value instead of null.
🤖 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/references/cli/lark-slides-media-upload.md` around lines
28 - 36, Align the media upload response example with the token extraction
commands: either nest file_token under data or update the jq paths in the
documented upload flows, including the references around lines 66-68 and 108, to
read the top-level field. Ensure the copy-paste commands return the actual token
rather than null.
| `<table>`（2×2）： | ||
| ```xml | ||
| <table topLeftX="30" topLeftY="80"> | ||
| <colgroup><col span="2" width="110"/></colgroup> | ||
| <tr><td><content><p>A</p></content></td><td><content><p>B</p></content></td></tr> | ||
| <tr><td><content><p>C</p></content></td><td><content><p>D</p></content></td></tr> | ||
| </table> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add dimensions to the table example.
SKILL.md requires every <table> to define width and height, but this example only defines topLeftX and topLeftY. Add explicit dimensions so the copy-paste snippet follows the skill’s XML contract.
Proposed fix
-<table topLeftX="30" topLeftY="80">
+<table topLeftX="30" topLeftY="80" width="220" height="100">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `<table>`（2×2）： | |
| ```xml | |
| <table topLeftX="30" topLeftY="80"> | |
| <colgroup><col span="2" width="110"/></colgroup> | |
| <tr><td><content><p>A</p></content></td><td><content><p>B</p></content></td></tr> | |
| <tr><td><content><p>C</p></content></td><td><content><p>D</p></content></td></tr> | |
| </table> | |
| `<table>`（2×2）： |
🤖 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/references/cli/lark-slides-replace-slide.md` around lines
132 - 138, Update the `<table>` example to include explicit `width` and `height`
attributes alongside `topLeftX` and `topLeftY`, ensuring it satisfies the XML
contract described in `SKILL.md` while preserving the existing table structure.
| ```json | ||
| { | ||
| "parts": [ | ||
| { "action": "block_replace", "block_id": "bab", "replacement": "<shape .../>" }, | ||
| { "action": "block_insert", "insertion": "<img .../>", "insert_before_block_id": "baa" } | ||
| ] | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add the required id to direct replacement examples.
This document states that direct block_replace calls require the replacement root to contain id="<block_id>". The examples use <shape .../> without an id, so copied commands will fail with 3350001. Add id="bab" to each replacement root. Do not add it to block_insert examples.
Proposed fix
- { "action": "block_replace", "block_id": "bab", "replacement": "<shape .../>" },
+ { "action": "block_replace", "block_id": "bab", "replacement": "<shape id=\"bab\" .../>" },
- "replacement": "<shape type=\"text\" topLeftX=\"80\" topLeftY=\"80\" width=\"800\" height=\"120\"><content textType=\"title\"><p>新标题</p></content></shape>"
+ "replacement": "<shape id=\"bab\" type=\"text\" topLeftX=\"80\" topLeftY=\"80\" width=\"800\" height=\"120\"><content textType=\"title\"><p>新标题</p></content></shape>"Also applies to: 80-91, 113-124
🤖 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/references/cli/lark-slides-xml-presentation-slide-replace.md`
around lines 42 - 49, Update the direct block_replace examples in this document
to include id="bab" on each replacement root, matching the corresponding
block_id so copied commands satisfy the required replacement contract; leave all
block_insert examples unchanged.
| @@ -1,4 +1,4 @@ | |||
| # slides history（历史版本与回滚） | |||
| ``# slides history（历史版本与回滚） | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the stray backticks from the heading.
Two backticks precede the #. Markdown does not render this line as a heading. It renders as literal text that starts with ``.
📝 Proposed fix
-``# slides history（历史版本与回滚）
+# slides history（历史版本与回滚）📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``# slides history（历史版本与回滚） | |
| # slides history（历史版本与回滚） |
🤖 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/references/lark-slides-history.md` at line 1, Remove the
two stray backticks immediately before the Markdown heading marker in the slides
history document so the line begins directly with # and renders as a heading.
| - 图标用于概念提示、步骤、状态、指标、角色和导航；不要用无关装饰图标填充版面。 | ||
| - 常用尺寸：行内状态图标 16-24px，卡片标题图标 28-40px，主视觉图标 56-96px。 | ||
| - 图标必须填充颜色并和背景有足够对比；深色背景优先放在浅色圆形/方形底上，或使用 `rgba(255, 255, 255, 1)` 作为图标填充色。 | ||
| - 查不到合适图标时，从高频示例里选择替代图标（随机选择，不要千篇一律），不留空图标位。 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a semantic fallback instead of a random icon.
This rule directs the agent to select a random frequent icon when search has no match. That can add an icon unrelated to the slide content and conflicts with line 26. Select the nearest semantic fallback. If no semantic fallback exists, remove the icon and reclaim its layout space.
🤖 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/references/xml/iconpark.md` at line 29, Update the icon
fallback guidance to choose the nearest semantically relevant icon when no
suitable match is found, rather than selecting randomly from frequent examples.
If no semantic fallback exists, remove the icon and reclaim its layout space,
preserving the surrounding icon-search guidance.
| <shape width="128" height="18" topLeftX="800" topLeftY="512" type="text"> | ||
| <content textType="caption" fontSize="10" fontFamily="思源黑体" color="rgba(148, 163, 184, 1)" textAlign="right"> | ||
| <p>03 / 12</p> | ||
| </content> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the footer page counts.
This presentation contains seven slides, but its footers show pages 03 / 12 through 10 / 12 and omit 08 / 12. The rendered demo will show incorrect pagination. Renumber the footers as 01 / 07 through 07 / 07, or add the missing slides if this file must contain twelve pages.
Also applies to: 441-444, 590-593, 814-817, 1030-1033, 1186-1189, 1406-1409
🤖 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/references/xml/slides_chart_demo.xml` around lines 226 -
229, Correct the footer page-count text in each visible footer shape, including
the instances near the referenced sections, so the seven-slide presentation is
numbered sequentially from 01 / 07 through 07 / 07. Preserve the existing footer
styling and update only the displayed pagination values.
a3e9003 to
ad82da4
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 12
🧹 Nitpick comments (2)
skills/lark-slides/scripts/xml_lint.py (1)
2968-2975: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unreachable
--helpkey check.
parse_argsstrips the leading--from each token, so the options dictionary never contains the key"--help". Onlyoptions.get("help")can be true.♻️ Proposed cleanup
- if options.get("help") or options.get("--help"): + if options.get("help"):🤖 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_lint.py` around lines 2968 - 2975, Remove the unreachable options.get("--help") check from run_cli, leaving the existing options.get("help") handling and usage behavior unchanged.skills/lark-slides/scripts/xml_text_overlap_lint.py (1)
2-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the compatibility stub.
xml_text_overlap_lintis only referenced in the stub itself, while existing docs/test/workflow usages point toscripts/xml_lint.py, so the wrapper module should be removed to avoid accidental use.🤖 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 2 - 7, Remove the xml_text_overlap_lint compatibility stub entirely, including its imports and re-exports; retain the canonical xml_lint module and its run_cli and XmlLayoutLintError symbols unchanged.
🤖 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/cli/lark-slides-media-upload.md`:
- Around line 28-36: Align the media upload response example with the token
extraction commands: either nest file_token under data or update the jq paths in
the documented upload flows, including the references around lines 66-68 and
108, to read the top-level field. Ensure the copy-paste commands return the
actual token rather than null.
In `@skills/lark-slides/references/cli/lark-slides-replace-slide.md`:
- Around line 132-138: Update the `<table>` example to include explicit `width`
and `height` attributes alongside `topLeftX` and `topLeftY`, ensuring it
satisfies the XML contract described in `SKILL.md` while preserving the existing
table structure.
In
`@skills/lark-slides/references/cli/lark-slides-xml-presentation-slide-replace.md`:
- Around line 42-49: Update the direct block_replace examples in this document
to include id="bab" on each replacement root, matching the corresponding
block_id so copied commands satisfy the required replacement contract; leave all
block_insert examples unchanged.
In `@skills/lark-slides/references/iconpark-index.json`:
- Around line 1-5: The legacy references/iconpark-index.json path must remain
valid JSON for existing consumers. Replace the Markdown migration notice in
iconpark-index.json with a symlink or equivalent copy of
xml/iconpark-index.json, and place the migration notice in a separate Markdown
file.
In `@skills/lark-slides/references/lark-slides-add-slide.md`:
- Line 3: Update the migration notices in
skills/lark-slides/references/lark-slides-add-slide.md:3 and
skills/lark-slides/references/lark-slides-delete-slide.md:3 to point to existing
destination documents under skills/lark-slides, or create the referenced target
documents before linking to them.
In `@skills/lark-slides/references/lark-slides-edit-workflows.md`:
- Line 3: Fix the broken migration links in
skills/lark-slides/references/lark-slides-edit-workflows.md:3-3 and
skills/lark-slides/references/lark-slides-pptx-template-workflows.md:3-3 by
moving the referenced documents to the stated workflow/ paths or updating both
links to their actual existing locations, ensuring all migration notices and
back-references resolve correctly.
In `@skills/lark-slides/references/lark-slides-history.md`:
- Line 1: Remove the two stray backticks immediately before the Markdown heading
marker in the slides history document so the line begins directly with # and
renders as a heading.
In `@skills/lark-slides/references/slides_xml_schema_definition.xml`:
- Around line 1-5: Remove the Markdown compatibility notice from
skills/lark-slides/references/slides_xml_schema_definition.xml and update all
remaining references to use xml/slides_xml_schema_definition.xml, or replace it
with parseable XML containing the notice in an XML comment. Apply the same
treatment to skills/lark-slides/references/slides_chart_demo.xml, directing
references to xml/slides_chart_demo.xml; ensure both .xml paths remain valid for
XML parsers and xml_lint.py.
In `@skills/lark-slides/references/xml/iconpark.md`:
- Line 29: Update the icon fallback guidance to choose the nearest semantically
relevant icon when no suitable match is found, rather than selecting randomly
from frequent examples. If no semantic fallback exists, remove the icon and
reclaim its layout space, preserving the surrounding icon-search guidance.
In `@skills/lark-slides/references/xml/slides_chart_demo.xml`:
- Around line 226-229: Correct the footer page-count text in each visible footer
shape, including the instances near the referenced sections, so the seven-slide
presentation is numbered sequentially from 01 / 07 through 07 / 07. Preserve the
existing footer styling and update only the displayed pagination values.
In `@skills/lark-slides/references/xml/slides_xml_schema_definition.xml`:
- Around line 955-968: Update the shadow documentation text in the annotation
above the shadow attributes to name the attribute as align instead of
shadowAlign, matching the declared xs:attribute and avoiding
unsupported-attribute guidance.
In `@skills/lark-slides/scripts/xml_lint.py`:
- Around line 325-344: Update load_iconpark_icon_types to handle a missing or
unreadable ICONPARK_INDEX_PATH before read_text, following
iconpark_tool.load_index’s existence-check-and-fail pattern. Route the failure
through fail(...) so the CLI’s existing XmlLayoutLintError handling emits the
standard error message, while preserving the current JSON decoding and
validation behavior.
---
Nitpick comments:
In `@skills/lark-slides/scripts/xml_lint.py`:
- Around line 2968-2975: Remove the unreachable options.get("--help") check from
run_cli, leaving the existing options.get("help") handling and usage behavior
unchanged.
In `@skills/lark-slides/scripts/xml_text_overlap_lint.py`:
- Around line 2-7: Remove the xml_text_overlap_lint compatibility stub entirely,
including its imports and re-exports; retain the canonical xml_lint module and
its run_cli and XmlLayoutLintError symbols unchanged.
🪄 Autofix
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 Plus
Run ID: 4b78c468-0ff2-43dc-bdcd-7181e604cb8f
📒 Files selected for processing (36)
skills/lark-slides/SKILL.mdskills/lark-slides/references/cli/lark-slides-media-upload.mdskills/lark-slides/references/cli/lark-slides-replace-pages.mdskills/lark-slides/references/cli/lark-slides-replace-slide.mdskills/lark-slides/references/cli/lark-slides-xml-presentation-slide-get.mdskills/lark-slides/references/cli/lark-slides-xml-presentation-slide-replace.mdskills/lark-slides/references/cli/lark-slides-xml-presentations-get.mdskills/lark-slides/references/iconpark-index.jsonskills/lark-slides/references/iconpark.mdskills/lark-slides/references/lark-slides-add-slide.mdskills/lark-slides/references/lark-slides-create.mdskills/lark-slides/references/lark-slides-delete-slide.mdskills/lark-slides/references/lark-slides-edit-workflows.mdskills/lark-slides/references/lark-slides-history.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-xml-presentation-slide-get.mdskills/lark-slides/references/lark-slides-xml-presentation-slide-replace.mdskills/lark-slides/references/lark-slides-xml-presentations-get.mdskills/lark-slides/references/planning-layer.mdskills/lark-slides/references/slides_chart_demo.xmlskills/lark-slides/references/slides_xml_schema_definition.xmlskills/lark-slides/references/troubleshooting.mdskills/lark-slides/references/validation-checklist.mdskills/lark-slides/references/xml-schema-quick-ref.mdskills/lark-slides/references/xml/iconpark-index.jsonskills/lark-slides/references/xml/iconpark.mdskills/lark-slides/references/xml/slides_chart_demo.xmlskills/lark-slides/references/xml/slides_xml_schema_definition.xmlskills/lark-slides/scripts/iconpark_tool.pyskills/lark-slides/scripts/xml_lint.pyskills/lark-slides/scripts/xml_lint_test.pyskills/lark-slides/scripts/xml_text_overlap_lint.pyskills/lark-slides/scripts/xml_text_overlap_lint_test.py
🛑 Comments failed to post (4)
skills/lark-slides/references/iconpark-index.json (1)
1-5: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep the legacy
.jsonpath valid JSON.This Markdown content is not JSON. A consumer that still opens
references/iconpark-index.jsonas the advertised compatibility path will fail during JSON parsing. Keep a valid JSON compatibility asset, such as a symlink or equivalent copy ofxml/iconpark-index.json, and move this migration notice to a Markdown file.🧰 Tools
🪛 Biome (2.5.6)
[error] 1-1: unexpected character
#(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: unexpected character
（(parse)
[error] 1-1: String values must be double quoted.
(parse)
[error] 1-1: unexpected character
）(parse)
[error] 3-3: String values must be double quoted.
(parse)
[error] 3-3: unexpected character ```
(parse)
[error] 3-3: expected
,but instead foundxml(parse)
[error] 3-3: unexpected character
/(parse)
[error] 3-3: expected
,but instead foundiconpark(parse)
[error] 3-3: Minus must be followed by a digit
(parse)
[error] 3-3: expected
,but instead foundindex(parse)
[error] 3-3: unexpected character
.(parse)
[error] 3-3: expected
,but instead foundjson(parse)
[error] 3-3: End of file expected
(parse)
[error] 3-3: unexpected character ```
(parse)
[error] 3-3: unexpected character
((parse)
[error] 3-3: String values must be double quoted.
(parse)
[error] 3-3: unexpected character
/(parse)
[error] 3-3: String values must be double quoted.
(parse)
[error] 3-3: Minus must be followed by a digit
(parse)
[error] 3-3: String values must be double quoted.
(parse)
[error] 3-3: unexpected character
.(parse)
[error] 3-3: String values must be double quoted.
(parse)
[error] 3-3: unexpected character
)(parse)
[error] 3-3: unexpected character
。(parse)
[error] 5-5: String values must be double quoted.
(parse)
[error] 5-5: unexpected character
；(parse)
[error] 5-5: String values must be double quoted.
(parse)
[error] 5-5: unexpected character
。(parse)
🤖 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/references/iconpark-index.json` around lines 1 - 5, The legacy references/iconpark-index.json path must remain valid JSON for existing consumers. Replace the Markdown migration notice in iconpark-index.json with a symlink or equivalent copy of xml/iconpark-index.json, and place the migration notice in a separate Markdown file.Source: Linters/SAST tools
skills/lark-slides/references/slides_xml_schema_definition.xml (1)
1-5: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Markdown migration notices kept under
.xmlfile extensions. Both files retain the.xmlextension while their content is now Markdown starting with#. Any XML parser, schema loader, or.xmlglob — including thexml_lint.pyscript added in this stack — fails on these paths instead of reading a notice. The shared root cause is one compatibility strategy applied to artifacts that are not Markdown.
skills/lark-slides/references/slides_xml_schema_definition.xml#L1-L5: delete the path and update the remaining references, or replace the body with a parseable XML document that carries the notice in an XML comment.skills/lark-slides/references/slides_chart_demo.xml#L1-L5: apply the same treatment, pointing toxml/slides_chart_demo.xml.📍 Affects 2 files
skills/lark-slides/references/slides_xml_schema_definition.xml#L1-L5(this comment)skills/lark-slides/references/slides_chart_demo.xml#L1-L5🤖 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/references/slides_xml_schema_definition.xml` around lines 1 - 5, Remove the Markdown compatibility notice from skills/lark-slides/references/slides_xml_schema_definition.xml and update all remaining references to use xml/slides_xml_schema_definition.xml, or replace it with parseable XML containing the notice in an XML comment. Apply the same treatment to skills/lark-slides/references/slides_chart_demo.xml, directing references to xml/slides_chart_demo.xml; ensure both .xml paths remain valid for XML parsers and xml_lint.py.skills/lark-slides/references/xml/slides_xml_schema_definition.xml (1)
955-968: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align the documentation attribute name with the declared attribute.
The documentation names the attribute
shadowAlign. The declaration on Line 968 names italign. An author who follows the documentation writes an attribute thatxml_lint.pyreports assxsd_unsupported_attr.📝 Proposed documentation fix
对齐方式: - - shadowAlign: 阴影相对于元素的对齐方式（默认为top-left左上对齐） + - align: 阴影相对于元素的对齐方式（默认为top-left左上对齐）📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.对齐方式: - align: 阴影相对于元素的对齐方式（默认为top-left左上对齐） 注意:空标签表示默认阴影效果(color=rgba(0, 0, 0, 0.25), offset=15, blur=35, angle=45) </xs:documentation> </xs:annotation> <xs:attribute name="color" type="sml:Color" use="optional" default="rgba(0, 0, 0, 0.25)"/> <xs:attribute name="offset" type="sml:Range0To200DotType" use="optional" default="15" /> <xs:attribute name="blur" type="sml:PercentageType" use="optional" default="35"/> <xs:attribute name="angle" type="sml:RotationType" use="optional" default="45"/> <xs:attribute name="hScale" type="sml:ScaleType" use="optional" default="1"/> <xs:attribute name="vScale" type="sml:ScaleType" use="optional" default="1"/> <xs:attribute name="hSkew" type="sml:SkewType" use="optional" default="0"/> <xs:attribute name="vSkew" type="sml:SkewType" use="optional" default="0"/> <xs:attribute name="align" type="sml:ShadowAlignType" use="optional" default="top-left"/>🤖 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/references/xml/slides_xml_schema_definition.xml` around lines 955 - 968, Update the shadow documentation text in the annotation above the shadow attributes to name the attribute as align instead of shadowAlign, matching the declared xs:attribute and avoiding unsupported-attribute guidance.skills/lark-slides/scripts/xml_lint.py (1)
325-344: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle a missing iconpark index file.
load_iconpark_icon_typescatches onlyjson.JSONDecodeError. Ifreferences/xml/iconpark-index.jsonis absent or unreadable,read_textraisesOSError. The__main__block catches onlyXmlLayoutLintError, so the CLI prints a traceback instead of thexml-lint error: ...message. This PR moves that file into a new directory, so a stale or wrong path is a realistic failure.
iconpark_tool.load_indexalready uses the intended pattern: check existence, then callfail(...).🛡️ Proposed fix
- try: - index_data = json.loads(ICONPARK_INDEX_PATH.read_text(encoding="utf-8")) - except json.JSONDecodeError as error: - fail(f"invalid iconpark index JSON: {error}") + try: + index_data = json.loads(ICONPARK_INDEX_PATH.read_text(encoding="utf-8")) + except OSError as error: + fail(f"iconpark index not found: {ICONPARK_INDEX_PATH} ({error})") + except json.JSONDecodeError as error: + fail(f"invalid iconpark index JSON: {error}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.def load_iconpark_icon_types() -> set[str]: global _ICONPARK_ICON_TYPES_CACHE if _ICONPARK_ICON_TYPES_CACHE is not None: return _ICONPARK_ICON_TYPES_CACHE try: index_data = json.loads(ICONPARK_INDEX_PATH.read_text(encoding="utf-8")) except OSError as error: fail(f"iconpark index not found: {ICONPARK_INDEX_PATH} ({error})") except json.JSONDecodeError as error: fail(f"invalid iconpark index JSON: {error}") icons = index_data.get("icons") if not isinstance(icons, list): fail("iconpark index must contain an icons array") icon_types = { icon["iconType"] for icon in icons if isinstance(icon, dict) and isinstance(icon.get("iconType"), str) and icon["iconType"] } _ICONPARK_ICON_TYPES_CACHE = icon_types return icon_types🤖 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_lint.py` around lines 325 - 344, Update load_iconpark_icon_types to handle a missing or unreadable ICONPARK_INDEX_PATH before read_text, following iconpark_tool.load_index’s existence-check-and-fail pattern. Route the failure through fail(...) so the CLI’s existing XmlLayoutLintError handling emits the standard error message, while preserving the current JSON decoding and validation behavior.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
1 similar comment
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
skills/lark-slides/references/cli/lark-slides-replace-pages.md (1)
5-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the blank line between the two blockquotes.
markdownlint reports MD028 (
no-blanks-blockquote) at line 6. A blank line between two>blocks can render as one merged quote in some renderers. Separate the blocks with a>continuation line, or merge them into one blockquote.♻️ Proposed fix
> **灰度期间的重要兼容说明**：当前命令的 `--help` 及输出中的 `deprecated` 字段会提示改用 `+update-slide`，但该提示依赖尚未全量的 Slide Engine 后端能力。在本参考页随 skill 发布期间，必须忽略该提示并继续使用 `+replace-pages`，不得切换到 `+update-slide`。后端全量并由新版 skill 恢复路由后，再使用 `+update-slide`。 - -> 重要：这是多步编排，不是后端原子事务。CLI 对每页执行“先创建新页到旧页前，再删除旧页”；创建失败时旧页会保留。删除失败时可能出现新旧页同时存在，需要按返回结果继续处理。 +> +> 重要：这是多步编排，不是后端原子事务。CLI 对每页执行“先创建新页到旧页前，再删除旧页”；创建失败时旧页会保留。删除失败时可能出现新旧页同时存在，需要按返回结果继续处理。🤖 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/references/cli/lark-slides-replace-pages.md` around lines 5 - 7, Remove the blank line between the two blockquotes in the reference content, either by adding a `>` continuation line or merging both paragraphs into one blockquote, so the markdown satisfies MD028.Source: Linters/SAST tools
skills/lark-slides/SKILL.md (1)
82-95: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake the document references in the Quick Reference table use consistent paths.
Rows 86 and 87 use the new
xml/prefix. The other rows still use bare file names such aslark-slides-replace-slide.md,iconpark.md,validation-xml.md, andslides_chart_demo.xml. Those files now live inreferences/cli/,references/xml/, orreferences/workflow/. A reader who resolves a bare name lands on the legacy stub instead of the canonical document. Add the same subdirectory prefix to every row.🤖 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/SKILL.md` around lines 82 - 95, Update the Quick Reference table in the document so every referenced file uses its canonical references/ subdirectory path, including entries under references/cli/, references/xml/, and references/workflow/. Apply this consistently to all rows, not only the existing xml/ references, while leaving command names and non-file references unchanged.skills/lark-slides/scripts/xml_lint_test.py (1)
862-862: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDistinguish the two chart roundtrip test names.
test_lint_xml_ignores_chart_parsed_values_roundtrip_tagsandtest_lint_xml_ignores_chart_parsed_values_roundtrip_tagdiffer only by a trailings. The bodies differ only in whetherchartDatacarriesisStaticData="true". A reader cannot tell which behavior each test pins. Rename both to state the distinguishing condition, for example..._roundtrip_tag_with_static_dataand..._roundtrip_tag_without_static_data.Also applies to: 885-885
🤖 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_lint_test.py` at line 862, Rename the two chart roundtrip tests to clearly distinguish their static-data conditions: update test_lint_xml_ignores_chart_parsed_values_roundtrip_tags and test_lint_xml_ignores_chart_parsed_values_roundtrip_tag to names such as ..._roundtrip_tag_with_static_data and ..._roundtrip_tag_without_static_data, matching each test’s chartData isStaticData="true" usage.skills/lark-slides/references/workflow/validation-xml.md (1)
55-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the remaining release-blocking codes to the table.
xml_lint.pyalso emitsduplicate_element_idandtext_may_overflow_shapeaterrorlevel. Both block release, andduplicate_element_idhas a non-obvious remediation: remove theidattribute instead of inventing a new one. Add rows for them so the table covers every code that can setrelease_ready == false.♻️ Proposed additions
| `bbox_overlap` | 文本元素的估算绘制区域明显重叠 | 拉开文本坐标、缩小文本框/字号，或改成明确的分栏/分组结构 | +| `duplicate_element_id` | 多个元素共用同一个 `id` | 新写的元素直接删除 `id` 属性；改写回读 XML 时只保留原元素上的服务端 ID，不要自造新 ID | +| `text_may_overflow_shape` | 估算文本高度超出 `<shape>` 自身内容框 | 增大 `height`、精简文字，或设置 `wrap="true" autoFit="normal-auto-fit"` | | `*_out_of_canvas` | 元素边界超出页面画布 | 根据 `measurement.overflow` 移回画布或缩小尺寸 |🤖 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/references/workflow/validation-xml.md` around lines 55 - 71, 在“常见 code 的处理方向”表中补充 duplicate_element_id 和 text_may_overflow_shape 两行，确保覆盖 xml_lint.py 以 error 级别阻止 release 的代码；明确 duplicate_element_id 的处理方式是移除 id 属性而不是创建新 ID，并为 text_may_overflow_shape 指明按 lint 提示调整文本框尺寸、字号或文本内容以避免溢出。
🤖 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/cli/lark-slides-replace-slide.md`:
- Line 258: Update the link label at
skills/lark-slides/references/cli/lark-slides-replace-slide.md:258 from
lark-slides-edit-workflows.md to slides_editing.md while preserving the target
../workflow/slides_editing.md. Also update the label at
skills/lark-slides/references/lark-slides-create.md:8 from
lark-slides-pptx-template-workflows.md to template-editing.md while preserving
the target workflow/template-editing.md.
---
Nitpick comments:
In `@skills/lark-slides/references/cli/lark-slides-replace-pages.md`:
- Around line 5-7: Remove the blank line between the two blockquotes in the
reference content, either by adding a `>` continuation line or merging both
paragraphs into one blockquote, so the markdown satisfies MD028.
In `@skills/lark-slides/references/workflow/validation-xml.md`:
- Around line 55-71: 在“常见 code 的处理方向”表中补充 duplicate_element_id 和
text_may_overflow_shape 两行，确保覆盖 xml_lint.py 以 error 级别阻止 release 的代码；明确
duplicate_element_id 的处理方式是移除 id 属性而不是创建新 ID，并为 text_may_overflow_shape 指明按 lint
提示调整文本框尺寸、字号或文本内容以避免溢出。
In `@skills/lark-slides/scripts/xml_lint_test.py`:
- Line 862: Rename the two chart roundtrip tests to clearly distinguish their
static-data conditions: update
test_lint_xml_ignores_chart_parsed_values_roundtrip_tags and
test_lint_xml_ignores_chart_parsed_values_roundtrip_tag to names such as
..._roundtrip_tag_with_static_data and ..._roundtrip_tag_without_static_data,
matching each test’s chartData isStaticData="true" usage.
In `@skills/lark-slides/SKILL.md`:
- Around line 82-95: Update the Quick Reference table in the document so every
referenced file uses its canonical references/ subdirectory path, including
entries under references/cli/, references/xml/, and references/workflow/. Apply
this consistently to all rows, not only the existing xml/ references, while
leaving command names and non-file references unchanged.
🪄 Autofix
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 Plus
Run ID: 588d31eb-c5c3-495e-923a-fd7ddf25e045
📒 Files selected for processing (43)
skills/lark-slides/SKILL.mdskills/lark-slides/references/cli/lark-slides-media-upload.mdskills/lark-slides/references/cli/lark-slides-replace-pages.mdskills/lark-slides/references/cli/lark-slides-replace-slide.mdskills/lark-slides/references/cli/lark-slides-xml-presentation-slide-get.mdskills/lark-slides/references/cli/lark-slides-xml-presentation-slide-replace.mdskills/lark-slides/references/cli/lark-slides-xml-presentations-get.mdskills/lark-slides/references/iconpark-index.jsonskills/lark-slides/references/iconpark.mdskills/lark-slides/references/lark-slides-add-slide.mdskills/lark-slides/references/lark-slides-create.mdskills/lark-slides/references/lark-slides-delete-slide.mdskills/lark-slides/references/lark-slides-edit-workflows.mdskills/lark-slides/references/lark-slides-history.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-xml-presentation-slide-get.mdskills/lark-slides/references/lark-slides-xml-presentation-slide-replace.mdskills/lark-slides/references/lark-slides-xml-presentations-get.mdskills/lark-slides/references/planning-layer.mdskills/lark-slides/references/slides_chart_demo.xmlskills/lark-slides/references/slides_xml_schema_definition.xmlskills/lark-slides/references/troubleshooting.mdskills/lark-slides/references/validation-checklist.mdskills/lark-slides/references/workflow/error-handling.mdskills/lark-slides/references/workflow/slides_editing.mdskills/lark-slides/references/workflow/template-editing.mdskills/lark-slides/references/workflow/validation-xml.mdskills/lark-slides/references/xml-schema-quick-ref.mdskills/lark-slides/references/xml/iconpark-index.jsonskills/lark-slides/references/xml/iconpark.mdskills/lark-slides/references/xml/lark-slides-add-slide.mdskills/lark-slides/references/xml/lark-slides-delete-slide.mdskills/lark-slides/references/xml/slides_chart_demo.xmlskills/lark-slides/references/xml/slides_xml_schema_definition.xmlskills/lark-slides/references/xml/xml-schema-quick-ref.mdskills/lark-slides/scripts/iconpark_tool.pyskills/lark-slides/scripts/xml_lint.pyskills/lark-slides/scripts/xml_lint_test.pyskills/lark-slides/scripts/xml_text_overlap_lint.pyskills/lark-slides/scripts/xml_text_overlap_lint_test.py
🚧 Files skipped from review as they are similar to previous changes (30)
- skills/lark-slides/references/xml-schema-quick-ref.md
- skills/lark-slides/references/slides_xml_schema_definition.xml
- skills/lark-slides/references/lark-slides-history.md
- skills/lark-slides/references/lark-slides-media-upload.md
- skills/lark-slides/references/slides_chart_demo.xml
- skills/lark-slides/references/cli/lark-slides-media-upload.md
- skills/lark-slides/references/planning-layer.md
- skills/lark-slides/references/xml/iconpark.md
- skills/lark-slides/references/cli/lark-slides-xml-presentation-slide-get.md
- skills/lark-slides/references/lark-slides-xml-presentations-get.md
- skills/lark-slides/references/lark-slides-edit-workflows.md
- skills/lark-slides/references/lark-slides-xml-presentation-slide-get.md
- skills/lark-slides/references/lark-slides-replace-pages.md
- skills/lark-slides/references/iconpark.md
- skills/lark-slides/scripts/iconpark_tool.py
- skills/lark-slides/references/lark-slides-pptx-template-workflows.md
- skills/lark-slides/references/lark-slides-replace-slide.md
- skills/lark-slides/references/xml/xml-schema-quick-ref.md
- skills/lark-slides/references/lark-slides-delete-slide.md
- skills/lark-slides/references/cli/lark-slides-xml-presentations-get.md
- skills/lark-slides/references/cli/lark-slides-xml-presentation-slide-replace.md
- skills/lark-slides/references/troubleshooting.md
- skills/lark-slides/references/lark-slides-xml-presentation-slide-replace.md
- skills/lark-slides/references/workflow/error-handling.md
- skills/lark-slides/scripts/xml_text_overlap_lint.py
- skills/lark-slides/references/xml/slides_xml_schema_definition.xml
- skills/lark-slides/references/validation-checklist.md
- skills/lark-slides/references/workflow/slides_editing.md
- skills/lark-slides/references/xml/lark-slides-add-slide.md
- skills/lark-slides/references/xml/slides_chart_demo.xml
| - [xml_presentation.slide get](lark-slides-xml-presentation-slide-get.md) — 读原页拿 `block_id` / `revision_id` | ||
| - [xml_presentation.slide replace](lark-slides-xml-presentation-slide-replace.md) — 底层 replace API 参考 | ||
| - [+media-upload](lark-slides-media-upload.md) — 上传图片拿 `file_token` | ||
| - [lark-slides-edit-workflows.md](../workflow/slides_editing.md) — 读-改-写闭环 + 决策树 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale link labels after the reference reorganization. Both links point at the new document location, but the visible label still names the legacy file that is now only a migration stub. Update each label to name the file it actually opens.
skills/lark-slides/references/cli/lark-slides-replace-slide.md#L258-L258: change the labellark-slides-edit-workflows.mdtoslides_editing.mdfor the target../workflow/slides_editing.md.skills/lark-slides/references/lark-slides-create.md#L8-L8: change the labellark-slides-pptx-template-workflows.mdtotemplate-editing.mdfor the targetworkflow/template-editing.md.
📍 Affects 2 files
skills/lark-slides/references/cli/lark-slides-replace-slide.md#L258-L258(this comment)skills/lark-slides/references/lark-slides-create.md#L8-L8
🤖 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/references/cli/lark-slides-replace-slide.md` at line 258,
Update the link label at
skills/lark-slides/references/cli/lark-slides-replace-slide.md:258 from
lark-slides-edit-workflows.md to slides_editing.md while preserving the target
../workflow/slides_editing.md. Also update the label at
skills/lark-slides/references/lark-slides-create.md:8 from
lark-slides-pptx-template-workflows.md to template-editing.md while preserving
the target workflow/template-editing.md.
| @@ -1,2984 +1,10 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
[P1] Restore the required license headers on the compatibility wrappers
This replacement dropped the copyright and SPDX header, and the required license-header check is currently failing for both xml_text_overlap_lint.py and xml_text_overlap_lint_test.py. Please keep the compatibility wrappers and add the repository-standard header after each shebang so the PR can pass the merge gate.
| SKILL_ROOT = Path(__file__).resolve().parent.parent | ||
| REFERENCES_DIR = SKILL_ROOT / "references" | ||
| DEFAULT_INDEX_PATH = REFERENCES_DIR / "iconpark-index.json" | ||
| DEFAULT_INDEX_PATH = REFERENCES_DIR / "xml" / "iconpark-index.json" |
There was a problem hiding this comment.
[P1] Preserve parseable content at legacy structured-data paths
Updating the in-tree loader makes the new path work, but references/iconpark-index.json itself now starts with Markdown, so every existing JSON consumer of the promised compatibility path fails immediately. The same regression affects references/slides_xml_schema_definition.xml and references/slides_chart_demo.xml: both legacy .xml paths now contain Markdown and are no longer well-formed XML. Machine-readable resources cannot redirect through a Markdown notice. Please keep valid JSON/XML at the legacy paths (for example, generated copies of the canonical files), or add a loader-level redirect that all existing consumers actually use.
| status = slide_status(all_errors, all_warnings) | ||
| result: dict[str, Any] = { | ||
| "schema_version": "2.0", | ||
| "tool": "xml_lint", |
There was a problem hiding this comment.
[P2] Preserve the legacy lint result contract
Calling scripts/xml_text_overlap_lint.py now delegates to this implementation, so the same input returns "tool": "xml_lint"; at the merge base the legacy entry point returned "tool": "xml_text_overlap_lint". Its --help output also names the new executable. Automation that selects or validates results by the tool identifier therefore breaks even though the old path still exists. Please let the wrapper preserve the legacy tool/usage values and add a regression test that invokes the compatibility entry point instead of only executing the new test suite.
2 participants
Summary
Reorganize the lark-slides skill reference files into logical subdirectories (cli/, xml/) and split monolithic lint scripts into focused modules. This improves discoverability and makes it easier for AI agents to navigate the skill's reference material.
Changes
Test Plan
Related Issues
Summary by CodeRabbit
New Features
Documentation