feat(slides): refine slide XML workflows - #1814
Open
ethan-zhx wants to merge 11 commits into
Open
Conversation
📝 WalkthroughWalkthroughThis PR caps ChangesSlides shortcut behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Lark-Slides documentation and guidance
Estimated code review effort: 2 (Simple) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1814 +/- ##
=========================================
Coverage 74.43% 74.43%
=========================================
Files 854 860 +6
Lines 88490 89891 +1401
=========================================
+ Hits 65867 66910 +1043
- Misses 17554 17791 +237
- Partials 5069 5190 +121 ☔ 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@6776eec77f6841fb5b681acada53dbc09f9812b8🧩 Skill updatenpx skills add larksuite/cli#feat/slide_0708 -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: 8
🧹 Nitpick comments (2)
shortcuts/slides/slides_screenshot.go (1)
284-288: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winError message hardcodes "10" alongside the
%dformat specifier.The format string at line 286 says
"request at most 10 pages at a time"while also interpolatingmaxSlidesPerScreenshotvia%d. If the constant ever changes, the hardcoded "10" in the message will be stale and misleading.♻️ Proposed fix
func validateSlidesScreenshotSelectorLimit(count int) error { if count > maxSlidesPerScreenshot { - return slidesScreenshotFlagErrorf("too many slide selectors: got %d, maximum is %d; request at most 10 pages at a time", count, maxSlidesPerScreenshot) + return slidesScreenshotFlagErrorf("too many slide selectors: got %d, maximum is %d; request at most %d pages at a time", count, maxSlidesPerScreenshot, maxSlidesPerScreenshot) } return nil }🤖 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 `@shortcuts/slides/slides_screenshot.go` around lines 284 - 288, The error text in validateSlidesScreenshotSelectorLimit hardcodes “10” while also formatting maxSlidesPerScreenshot, so update the slidesScreenshotFlagErrorf message to use the constant consistently instead of a literal. Keep the check in validateSlidesScreenshotSelectorLimit unchanged, but replace the stale wording so the limit shown in the message always matches maxSlidesPerScreenshot.shortcuts/slides/slides_xml_get_test.go (1)
94-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for
--rawvalidation error paths.The three new tests cover the happy paths (JSON envelope, jq filter, raw stdout), but the
--rawincompatibility validations (lines 53-62 inslides_xml_get.go) are untested. Per coding guidelines, every behavior change needs a test alongside the change.Consider adding tests for:
--raw+--output→ validation error withparam: --raw--raw+--jq→ validation error withparam: --raw--raw+--format(non-json) → validation error withparam: --raw🧪 Example test scaffold
func TestSlidesXMLGetRawIncompatibleWithOutput(t *testing.T) { dir := t.TempDir() withSlidesTestWorkingDir(t, dir) f, _, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) err := runSlidesShortcut(t, f, io.Discard, SlidesXMLGet, []string{ "+xml-get", "--presentation", "pres_abc", "--output", "out.xml", "--raw", "--as", "user", }) if err == nil { t.Fatal("expected validation error for --raw + --output, got nil") } var ve *errs.ValidationError if !errors.As(err, &ve) || ve.Param != "--raw" { t.Fatalf("expected ValidationError with Param --raw, got %v", err) } }🤖 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 `@shortcuts/slides/slides_xml_get_test.go` around lines 94 - 198, The new SlidesXMLGet tests cover only successful behavior, but the --raw incompatibility checks in SlidesXMLGet need coverage too. Add focused tests around runSlidesShortcut for the validation paths in slides_xml_get.go, using the existing TestFactory/setup helpers and asserting a ValidationError via errors.As with Param set to --raw for each case: --raw with --output, --raw with --jq, and --raw with a non-json --format. Keep the assertions aligned with the existing TestSlidesXMLGet* naming and shortcut invocation style so the validation behavior is exercised alongside the happy paths.Source: Coding guidelines
🤖 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 274-299: The test for SlidesScreenshot selector limits currently
only checks the error text, but it should also verify the structured error
metadata. Update TestSlidesScreenshotListRejectsMoreThanTenSelectors to assert
the returned error with errs.ProblemOf, checking the invalid-argument subtype
(SubtypeInvalidArgument) and preserving the cause, while keeping the existing
selector-limit scenario in runSlidesShortcut and SlidesScreenshot intact.
In `@skills/lark-slides/references/lark-slides-replace-pages.md`:
- Line 92: The backup instruction for the replace-pages flow is too implicit:
update the `slides +xml-get` step to explicitly write the current XML to a file
on disk using `--output`, so it cannot be mistaken for raw stdout streaming.
Keep the existing pre-edit backup intent in `lark-slides-replace-pages.md`, and
mention that the saved file should be created before recording the target page
`slide_id` for replacement.
In `@skills/lark-slides/references/lark-slides-whiteboard.md`:
- Around line 17-23: The guidance in the whiteboard reference is inconsistent:
non-native data visuals like scatter and funnel are being suggested for Mermaid
in the model-based branch, but the chart selection table already routes them to
SVG. Update the Mermaid fallback wording in the lark-slides-whiteboard.md
guidance so it only applies to topology-style diagrams, and keep unsupported
data visuals directed to `<whiteboard>` SVG. Use the existing `<chart>` /
`<whiteboard>` recommendation structure and the surrounding chart-type examples
to ensure the rules stay aligned.
In `@skills/lark-slides/references/lark-slides-xml-get.md`:
- Around line 19-23: The `lark-slides-xml-get.md` docs are inconsistent about
stdout behavior: `xml_get` currently implies omitting `--output` prints raw XML,
but the actual raw-XML stdout path is controlled by `--raw`. Update the
parameter table and the stdout example to describe `--raw` as the raw-XML
switch, and clarify that without `--output` the command returns the JSON
envelope with `xml_presentation.content`; also note in the `xmlGet`/`xml_get`
usage section that `--raw` cannot be combined with `--output` or `--jq`.
In `@skills/lark-slides/references/lark-slides-xml-presentation-slide-delete.md`:
- Around line 52-55: The subsection under the slide delete workflow is
misleading because the example no longer uses jq even though the heading still
says “使用 jq”. Update the subsection in the slide-delete reference by either
renaming it to match the new XML/JSON-only flow or adding a jq inspection step
back into the example, and keep the wording aligned with the command sequence
shown in the delete workflow.
In `@skills/lark-slides/references/planning-layer.md`:
- Line 222: The inline code in the planning-layer reference uses a
single-backtick span around text that already contains backtick-wrapped <chart>
markup, which breaks Markdown rendering; update that quoted fallback text to use
a higher-level code span delimiter so the inner <chart> tokens can remain
literal. Keep the change limited to the referenced markdown entry and preserve
the existing wording while fixing the nested code span formatting.
In `@skills/lark-slides/references/xml-schema-quick-ref.md`:
- Around line 187-203: The img XML example is misleading because
`src="file_token_or_url"` suggests unsupported http(s) URLs, which conflicts
with the documented rule that `src` only accepts a `file_token` or an `@<本地路径>`
placeholder. Update the example in the XML schema quick reference to use a valid
`src` value pattern that matches the `img` / `slides +media-upload` flow, and
keep the surrounding note aligned with the `img` and `src` rules so readers can
copy a valid snippet.
In `@skills/lark-slides/SKILL.md`:
- Around line 262-267: Update the `+xml-get` shortcut description in the skill
guide so it no longer implies XML is always saved to disk; clarify that file
output only happens when `--output` is provided, while the default behavior is
printing to stdout (JSON envelope or `--raw`). Keep the wording aligned with the
`+xml-get` command contract and the surrounding shortcut table so the guidance
matches the actual output flow.
---
Nitpick comments:
In `@shortcuts/slides/slides_screenshot.go`:
- Around line 284-288: The error text in validateSlidesScreenshotSelectorLimit
hardcodes “10” while also formatting maxSlidesPerScreenshot, so update the
slidesScreenshotFlagErrorf message to use the constant consistently instead of a
literal. Keep the check in validateSlidesScreenshotSelectorLimit unchanged, but
replace the stale wording so the limit shown in the message always matches
maxSlidesPerScreenshot.
In `@shortcuts/slides/slides_xml_get_test.go`:
- Around line 94-198: The new SlidesXMLGet tests cover only successful behavior,
but the --raw incompatibility checks in SlidesXMLGet need coverage too. Add
focused tests around runSlidesShortcut for the validation paths in
slides_xml_get.go, using the existing TestFactory/setup helpers and asserting a
ValidationError via errors.As with Param set to --raw for each case: --raw with
--output, --raw with --jq, and --raw with a non-json --format. Keep the
assertions aligned with the existing TestSlidesXMLGet* naming and shortcut
invocation style so the validation behavior is exercised alongside the happy
paths.
🪄 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: ea192629-b896-49fa-b8ab-f6816c7a8b1b
📒 Files selected for processing (31)
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-whiteboard.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/slides_chart_demo.xmlskills/lark-slides/references/slides_demo.xmlskills/lark-slides/references/slides_xml_schema_definition.xmlskills/lark-slides/references/troubleshooting.mdskills/lark-slides/references/validation-checklist.mdskills/lark-slides/references/visual-planning.mdskills/lark-slides/references/visual-styles.mdskills/lark-slides/references/xml-format-guide.mdskills/lark-slides/references/xml-schema-quick-ref.mdskills/lark-slides/scripts/xml_text_overlap_lint_test.py
💤 Files with no reviewable changes (3)
- skills/lark-slides/references/xml-format-guide.md
- skills/lark-slides/references/slides_demo.xml
- skills/lark-slides/references/examples.md
Comment on lines
+274
to
+299
| func TestSlidesScreenshotListRejectsMoreThanTenSelectors(t *testing.T) { | ||
| f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) | ||
|
|
||
| err := runSlidesShortcut(t, f, stdout, SlidesScreenshot, []string{ | ||
| "+screenshot", | ||
| "--presentation", "pres_abc", | ||
| "--slide-number", "1", | ||
| "--slide-number", "2", | ||
| "--slide-number", "3", | ||
| "--slide-number", "4", | ||
| "--slide-number", "5", | ||
| "--slide-number", "6", | ||
| "--slide-number", "7", | ||
| "--slide-number", "8", | ||
| "--slide-number", "9", | ||
| "--slide-number", "10", | ||
| "--slide-number", "11", | ||
| "--as", "user", | ||
| }) | ||
| if err == nil { | ||
| t.Fatal("expected error") | ||
| } | ||
| if !strings.Contains(err.Error(), "request at most 10 pages at a time") { | ||
| t.Fatalf("error = %v, want max 10 pages guidance", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert typed error metadata, not just message substring.
Per coding guidelines for **/*_test.go, error-path tests must assert typed metadata via errs.ProblemOf (category/subtype/param) and cause preservation, not message substrings alone. This test only checks strings.Contains(err.Error(), ...). Add assertions for the error subtype (SubtypeInvalidArgument) to guard against error-type regressions.
💚 Proposed fix to add typed error assertion
func TestSlidesScreenshotListRejectsMoreThanTenSelectors(t *testing.T) {
f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, ""))
err := runSlidesShortcut(t, f, stdout, SlidesScreenshot, []string{
"+screenshot",
"--presentation", "pres_abc",
"--slide-number", "1",
"--slide-number", "2",
"--slide-number", "3",
"--slide-number", "4",
"--slide-number", "5",
"--slide-number", "6",
"--slide-number", "7",
"--slide-number", "8",
"--slide-number", "9",
"--slide-number", "10",
"--slide-number", "11",
"--as", "user",
})
if err == nil {
t.Fatal("expected error")
}
- if !strings.Contains(err.Error(), "request at most 10 pages at a time") {
- t.Fatalf("error = %v, want max 10 pages guidance", err)
+ problem, ok := errs.ProblemOf(err)
+ if !ok {
+ t.Fatalf("expected typed error, got %v", err)
+ }
+ if problem.Subtype != errs.SubtypeInvalidArgument {
+ t.Fatalf("subtype = %v, want %v", problem.Subtype, errs.SubtypeInvalidArgument)
+ }
+ if !strings.Contains(err.Error(), "request at most 10 pages at a time") {
+ t.Fatalf("error = %v, want max 10 pages guidance", err)
}
}📝 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.
Suggested change
| func TestSlidesScreenshotListRejectsMoreThanTenSelectors(t *testing.T) { | |
| f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) | |
| err := runSlidesShortcut(t, f, stdout, SlidesScreenshot, []string{ | |
| "+screenshot", | |
| "--presentation", "pres_abc", | |
| "--slide-number", "1", | |
| "--slide-number", "2", | |
| "--slide-number", "3", | |
| "--slide-number", "4", | |
| "--slide-number", "5", | |
| "--slide-number", "6", | |
| "--slide-number", "7", | |
| "--slide-number", "8", | |
| "--slide-number", "9", | |
| "--slide-number", "10", | |
| "--slide-number", "11", | |
| "--as", "user", | |
| }) | |
| if err == nil { | |
| t.Fatal("expected error") | |
| } | |
| if !strings.Contains(err.Error(), "request at most 10 pages at a time") { | |
| t.Fatalf("error = %v, want max 10 pages guidance", err) | |
| } | |
| } | |
| func TestSlidesScreenshotListRejectsMoreThanTenSelectors(t *testing.T) { | |
| f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, "")) | |
| err := runSlidesShortcut(t, f, stdout, SlidesScreenshot, []string{ | |
| "+screenshot", | |
| "--presentation", "pres_abc", | |
| "--slide-number", "1", | |
| "--slide-number", "2", | |
| "--slide-number", "3", | |
| "--slide-number", "4", | |
| "--slide-number", "5", | |
| "--slide-number", "6", | |
| "--slide-number", "7", | |
| "--slide-number", "8", | |
| "--slide-number", "9", | |
| "--slide-number", "10", | |
| "--slide-number", "11", | |
| "--as", "user", | |
| }) | |
| if err == nil { | |
| t.Fatal("expected error") | |
| } | |
| problem, ok := errs.ProblemOf(err) | |
| if !ok { | |
| t.Fatalf("expected typed error, got %v", err) | |
| } | |
| if problem.Subtype != errs.SubtypeInvalidArgument { | |
| t.Fatalf("subtype = %v, want %v", problem.Subtype, errs.SubtypeInvalidArgument) | |
| } | |
| if !strings.Contains(err.Error(), "request at most 10 pages at a time") { | |
| t.Fatalf("error = %v, want max 10 pages guidance", err) | |
| } | |
| } |
🤖 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 `@shortcuts/slides/slides_screenshot_test.go` around lines 274 - 299, The test
for SlidesScreenshot selector limits currently only checks the error text, but
it should also verify the structured error metadata. Update
TestSlidesScreenshotListRejectsMoreThanTenSelectors to assert the returned error
with errs.ProblemOf, checking the invalid-argument subtype
(SubtypeInvalidArgument) and preserving the cause, while keeping the existing
selector-limit scenario in runSlidesShortcut and SlidesScreenshot intact.
Source: Coding guidelines
| ## 使用建议 | ||
|
|
||
| 1. 大幅改写前先 `xml_presentations.get` 保存当前 XML，并记录要替换页面的 `slide_id`。 | ||
| 1. 大幅改写前先 `slides +xml-get` 保存当前 XML，并记录要替换页面的 `slide_id`。 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the backup step explicitly save to disk.
slides +xml-get now has a raw stdout mode, so this instruction can still stream XML instead of preserving a reusable backup unless --output is spelled out. Please show the file path here to keep the pre-edit backup contract unambiguous.
Proposed fix
-1. 大幅改写前先 `slides +xml-get` 保存当前 XML，并记录要替换页面的 `slide_id`。
+1. 大幅改写前先 `slides +xml-get --output .lark-slides/plan/<deck-id>/readback.xml` 保存当前 XML，并记录要替换页面的 `slide_id`。📝 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.
Suggested change
| 1. 大幅改写前先 `slides +xml-get` 保存当前 XML，并记录要替换页面的 `slide_id`。 | |
| 1. 大幅改写前先 `slides +xml-get --output .lark-slides/plan/<deck-id>/readback.xml` 保存当前 XML，并记录要替换页面的 `slide_id`。 |
🤖 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-replace-pages.md` at line 92, The
backup instruction for the replace-pages flow is too implicit: update the
`slides +xml-get` step to explicitly write the current XML to a file on disk
using `--output`, so it cannot be mistaken for raw stdout streaming. Keep the
existing pre-edit backup intent in `lark-slides-replace-pages.md`, and mention
that the saved file should be created before recording the target page
`slide_id` for replacement.
| - `{"asset_type":"architecture_diagram","purpose":"Explain component relationships.","suggested_query":"service architecture diagram","fallback_if_missing":"Draw a component diagram with grouped boxes, connector arrows, and short labels."}` | ||
| - `{"asset_type":"logo","purpose":"Identify the customer context.","suggested_query":"customer logo","fallback_if_missing":"Use a text label in a small badge."}` | ||
| - `{"asset_type":"chart","purpose":"Show adoption trend.","suggested_query":"monthly adoption trend chart","fallback_if_missing":"Draw a simple trend line chart with axis labels and data points."}` | ||
| - `{"asset_type":"chart","purpose":"Show adoption trend.","suggested_query":"monthly adoption trend chart","fallback_if_missing":"Render a native `<chart>` using the provided series when available; otherwise render a native `<chart>` with mock placeholder values and label it as 模拟数据，仅占位，待替换真实数据."}` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix nested code span formatting on line 222.
The inline code span contains nested backtick-wrapped `<chart>` elements, which breaks Markdown rendering — the outer single-backtick span terminates at the first inner backtick. markdownlint flags this as MD038 (no-space-in-code). Use double backticks for the outer span so inner single backticks render correctly.
📝 Proposed fix
-- `{"asset_type":"chart","purpose":"Show adoption trend.","suggested_query":"monthly adoption trend chart","fallback_if_missing":"Render a native `<chart>` using the provided series when available; otherwise render a native `<chart>` with mock placeholder values and label it as 模拟数据，仅占位，待替换真实数据."}`
+- ``{"asset_type":"chart","purpose":"Show adoption trend.","suggested_query":"monthly adoption trend chart","fallback_if_missing":"Render a native `<chart>` using the provided series when available; otherwise render a native `<chart>` with mock placeholder values and label it as 模拟数据，仅占位，待替换真实数据."}``📝 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.
Suggested change
| - `{"asset_type":"chart","purpose":"Show adoption trend.","suggested_query":"monthly adoption trend chart","fallback_if_missing":"Render a native `<chart>` using the provided series when available; otherwise render a native `<chart>` with mock placeholder values and label it as 模拟数据，仅占位，待替换真实数据."}` | |
| ``{"asset_type":"chart","purpose":"Show adoption trend.","suggested_query":"monthly adoption trend chart","fallback_if_missing":"Render a native `<chart>` using the provided series when available; otherwise render a native `<chart>` with mock placeholder values and label it as 模拟数据，仅占位，待替换真实数据."}`` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 222-222: Spaces inside code span elements
(MD038, no-space-in-code)
[warning] 222-222: Spaces inside code span elements
(MD038, no-space-in-code)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/lark-slides/references/planning-layer.md` at line 222, The inline code
in the planning-layer reference uses a single-backtick span around text that
already contains backtick-wrapped <chart> markup, which breaks Markdown
rendering; update that quoted fallback text to use a higher-level code span
delimiter so the inner <chart> tokens can remain literal. Keep the change
limited to the referenced markdown entry and preserve the existing wording while
fixing the nested code span formatting.
Source: Linters/SAST tools
Comment on lines
+262
to
+267
| | `+xml-get` | 读取全文 XML 并保存到本地文件，避免终端输出被截断 | | ||
| | [`+media-upload`](references/lark-slides-media-upload.md) | 上传本地图片到指定演示文稿，返回 `file_token`（用作 `<img src="...">`），最大 20 MB | | ||
| | [`+replace-slide`](references/lark-slides-replace-slide.md) | 对已有幻灯片页面进行块级替换/插入（`block_replace` / `block_insert`），自动注入 id 和 `<content/>`，不改变页序 | | ||
| | [`+replace-pages`](references/lark-slides-replace-pages.md) | 在原演示文稿内批量重建多个页面：先创建新页到旧页前，再删除旧页；适合已有 Slides 的多页大改，不新建链接 | | ||
|
|
||
| 没有 Shortcut 覆盖时使用原生 API。高频资源：`xml_presentations.get` 读取全文；`xml_presentation.slide.create/delete/get/replace` 管理单页。 | ||
| 没有 Shortcut 覆盖时使用原生 API。高频资源：`slides +xml-get` 读取全文；`xml_presentation.slide.create/delete/get/replace` 管理单页。 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clarify the +xml-get output flow.
This row still reads like +xml-get always saves XML to disk, but the command now only writes a file when --output is set and otherwise prints to stdout (JSON envelope or --raw). Rewording this will keep the shortcut guidance aligned with the command contract.
♻️ Suggested wording
-| `+xml-get` | 读取全文 XML 并保存到本地文件，避免终端输出被截断 |
+| `+xml-get` | 读取全文 XML；需要落盘时配合 `--output`，否则默认输出到 stdout |📝 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.
Suggested change
| | `+xml-get` | 读取全文 XML 并保存到本地文件，避免终端输出被截断 | | |
| | [`+media-upload`](references/lark-slides-media-upload.md) | 上传本地图片到指定演示文稿，返回 `file_token`（用作 `<img src="...">`），最大 20 MB | | |
| | [`+replace-slide`](references/lark-slides-replace-slide.md) | 对已有幻灯片页面进行块级替换/插入（`block_replace` / `block_insert`），自动注入 id 和 `<content/>`，不改变页序 | | |
| | [`+replace-pages`](references/lark-slides-replace-pages.md) | 在原演示文稿内批量重建多个页面：先创建新页到旧页前，再删除旧页；适合已有 Slides 的多页大改，不新建链接 | | |
| 没有 Shortcut 覆盖时使用原生 API。高频资源：`xml_presentations.get` 读取全文；`xml_presentation.slide.create/delete/get/replace` 管理单页。 | |
| 没有 Shortcut 覆盖时使用原生 API。高频资源：`slides +xml-get` 读取全文；`xml_presentation.slide.create/delete/get/replace` 管理单页。 | |
| | `+xml-get` | 读取全文 XML；需要落盘时配合 `--output`，否则默认输出到 stdout | | |
| | [`+media-upload`](references/lark-slides-media-upload.md) | 上传本地图片到指定演示文稿，返回 `file_token`（用作 `<img src="...">`），最大 20 MB | | |
| | [`+replace-slide`](references/lark-slides-replace-slide.md) | 对已有幻灯片页面进行块级替换/插入（`block_replace` / `block_insert`），自动注入 id 和 `<content/>`，不改变页序 | | |
| | [`+replace-pages`](references/lark-slides-replace-pages.md) | 在原演示文稿内批量重建多个页面：先创建新页到旧页前，再删除旧页；适合已有 Slides 的多页大改，不新建链接 | | |
| 没有 Shortcut 覆盖时使用原生 API。高频资源：`slides +xml-get` 读取全文；`xml_presentation.slide.create/delete/get/replace` 管理单页。 |
🤖 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 262 - 267, Update the `+xml-get`
shortcut description in the skill guide so it no longer implies XML is always
saved to disk; clarify that file output only happens when `--output` is
provided, while the default behavior is printing to stdout (JSON envelope or
`--raw`). Keep the wording aligned with the `+xml-get` command contract and the
surrounding shortcut table so the guidance matches the actual output flow.
ethan-zhx
force-pushed
the
feat/slide_0708
branch
from
July 9, 2026 13:11
6776eec to
86f4306
Compare
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/references/slides_demo.xml`:
- Line 1: The demo uses self-closing whiteboard elements that do not satisfy the
schema’s required mermaid/svg child choice. Update the whiteboard examples in
slides_demo.xml by either supplying one valid child element (mermaid or svg) for
each whiteboard, or adjust the whiteboard definition in
slides_xml_schema_definition.xml so the choice is optional if empty placeholders
are intended. Use the whiteboard element and the related schema choice to locate
the fix.
🪄 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: 181cc324-c800-4d92-b67b-3f59e802d98e
📒 Files selected for processing (31)
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-whiteboard.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/slides_chart_demo.xmlskills/lark-slides/references/slides_demo.xmlskills/lark-slides/references/slides_xml_schema_definition.xmlskills/lark-slides/references/troubleshooting.mdskills/lark-slides/references/validation-checklist.mdskills/lark-slides/references/visual-planning.mdskills/lark-slides/references/visual-styles.mdskills/lark-slides/references/xml-format-guide.mdskills/lark-slides/references/xml-schema-quick-ref.mdskills/lark-slides/scripts/xml_text_overlap_lint_test.py
💤 Files with no reviewable changes (2)
- skills/lark-slides/references/xml-format-guide.md
- skills/lark-slides/references/examples.md
✅ Files skipped from review due to trivial changes (14)
- skills/lark-slides/references/lark-slides-replace-pages.md
- skills/lark-slides/references/iconpark.md
- skills/lark-slides/references/validation-checklist.md
- skills/lark-slides/references/lark-slides-xml-get.md
- skills/lark-slides/references/visual-planning.md
- skills/lark-slides/references/lark-slides-xml-presentations-get.md
- skills/lark-slides/references/lark-slides-xml-presentation-slide-get.md
- skills/lark-slides/references/lark-slides-media-upload.md
- skills/lark-slides/references/lark-slides-replace-slide.md
- skills/lark-slides/references/troubleshooting.md
- skills/lark-slides/references/lark-slides-xml-presentation-slide-create.md
- skills/lark-slides/references/lark-slides-xml-presentation-slide-delete.md
- skills/lark-slides/references/lark-slides-create.md
- skills/lark-slides/references/lark-slides-screenshot.md
🚧 Files skipped from review as they are similar to previous changes (8)
- shortcuts/slides/slides_screenshot_test.go
- skills/lark-slides/scripts/xml_text_overlap_lint_test.py
- shortcuts/slides/slides_screenshot.go
- shortcuts/slides/slides_xml_get_test.go
- skills/lark-slides/references/slides_chart_demo.xml
- skills/lark-slides/references/asset-planning.md
- shortcuts/slides/slides_xml_get.go
- skills/lark-slides/references/xml-schema-quick-ref.md
| </note> | ||
| </slide> | ||
| </presentation> | ||
| <presentation xmlns="/sml/2.0" width="960" height="540"><title>增长飞轮 · 从线性增长到复利增长</title><theme><textStyles><headline fontColor="rgba(31, 35, 41, 1)"/><sub-headline fontColor="rgba(31, 35, 41, 1)"/><body fontColor="rgba(31, 35, 41, 1)"/><caption fontColor="rgba(155, 158, 162, 1)" fontSize="14"/></textStyles></theme><slide><style><fill><fillColor color="rgba(245, 241, 232, 1)"/></fill></style><data><shape width="6" height="540" topLeftX="0" topLeftY="0" presetHandlers="0" type="rect"><fill><fillColor color="rgba(198, 159, 90, 1)"/></fill><content fontSize="16" fontFamily="思源黑体" color="rgba(31, 35, 41, 1)"/></shape><shape width="230" height="20" topLeftX="700" topLeftY="34" type="text"><content fontSize="10" fontFamily="思源黑体" color="rgba(198, 159, 90, 1)" bold="true" textAlign="right"><p>02 · FLYWHEEL MECHANISM</p></content></shape><shape width="680" height="46" topLeftX="40" topLeftY="60" type="text"><content fontSize="30" fontFamily="思源黑体" color="rgba(15, 30, 61, 1)" bold="true"><p>增长飞轮的四大环节</p></content></shape><shape width="880" height="26" topLeftX="40" topLeftY="112" type="text"><content fontSize="14" fontFamily="思源黑体" color="rgba(76, 96, 128, 1)"><p>四个环节顺时针相互驱动，任一环节的效率提升都会被飞轮系统性放大。</p></content></shape><line startX="40" startY="150" endX="920.0005681816348" endY="150"><border color="rgba(198, 159, 90, 0.35)" width="1"/></line><whiteboard width="440" height="360" topLeftX="30" topLeftY="160"/><shape width="60" height="34" topLeftX="490" topLeftY="175" type="text"><content fontSize="22" fontFamily="思源黑体" color="rgba(198, 159, 90, 1)" bold="true"><p>01</p></content></shape><shape width="370" height="28" topLeftX="550" topLeftY="177" type="text"><content fontSize="17" fontFamily="思源黑体" color="rgba(15, 30, 61, 1)" bold="true"><p>获取 · Acquire</p></content></shape><shape width="380" height="36" topLeftX="550" topLeftY="206" type="text"><content fontSize="12" fontFamily="思源黑体" color="rgba(76, 96, 128, 1)"><p>以精准渠道触达目标客户，把陌生流量转化为可识别的潜在用户。</p></content></shape><shape width="60" height="34" topLeftX="490" topLeftY="253" type="text"><content fontSize="22" fontFamily="思源黑体" color="rgba(198, 159, 90, 1)" bold="true"><p>02</p></content></shape><shape width="370" height="28" topLeftX="550" topLeftY="255" type="text"><content fontSize="17" fontFamily="思源黑体" color="rgba(15, 30, 61, 1)" bold="true"><p>激活 · Activate</p></content></shape><shape width="380" height="36" topLeftX="550" topLeftY="284" type="text"><content fontSize="12" fontFamily="思源黑体" color="rgba(76, 96, 128, 1)"><p>用第一次价值体验降低认知成本，让用户在关键动作中完成价值感知。</p></content></shape><shape width="60" height="34" topLeftX="490" topLeftY="331" type="text"><content fontSize="22" fontFamily="思源黑体" color="rgba(198, 159, 90, 1)" bold="true"><p>03</p></content></shape><shape width="370" height="28" topLeftX="550" topLeftY="333" type="text"><content fontSize="17" fontFamily="思源黑体" color="rgba(15, 30, 61, 1)" bold="true"><p>留存 · Retain</p></content></shape><shape width="380" height="36" topLeftX="550" topLeftY="362" type="text"><content fontSize="12" fontFamily="思源黑体" color="rgba(76, 96, 128, 1)"><p>通过高频价值场景与产品化机制，把一次性使用者变成长期活跃客户。</p></content></shape><shape width="60" height="34" topLeftX="490" topLeftY="409" type="text"><content fontSize="22" fontFamily="思源黑体" color="rgba(198, 159, 90, 1)" bold="true"><p>04</p></content></shape><shape width="370" height="28" topLeftX="550" topLeftY="411" type="text"><content fontSize="17" fontFamily="思源黑体" color="rgba(15, 30, 61, 1)" bold="true"><p>推荐 · Refer</p></content></shape><shape width="380" height="36" topLeftX="550" topLeftY="440" type="text"><content fontSize="12" fontFamily="思源黑体" color="rgba(76, 96, 128, 1)"><p>把满意客户变成新一轮获取的来源，让飞轮进入自我强化的复利阶段。</p></content></shape></data><note><content/></note></slide><slide><style><fill><fillColor color="rgba(245, 241, 232, 1)"/></fill></style><data><shape width="6" height="540" topLeftX="0" topLeftY="0" presetHandlers="0" type="rect"><fill><fillColor color="rgba(198, 159, 90, 1)"/></fill><content fontSize="16" fontFamily="思源黑体" color="rgba(31, 35, 41, 1)"/></shape><shape width="230" height="18" topLeftX="700" topLeftY="20" type="text"><content fontSize="10" fontFamily="思源黑体" color="rgba(198, 159, 90, 1)" bold="true" textAlign="right"><p>05 · CASE / SAAS APPLICATION</p></content></shape><shape width="960" height="40" topLeftX="0" topLeftY="40" type="text"><content fontSize="28" fontFamily="思源黑体" color="rgba(15, 30, 61, 1)" bold="true" textAlign="center"><p>SaaS 增长飞轮</p></content></shape><shape width="960" height="22" topLeftX="0" topLeftY="82" type="text"><content fontSize="13" fontFamily="思源黑体" color="rgba(76, 96, 128, 1)" textAlign="center"><p>每个阶段的提升都会加速下一阶段，形成正向循环</p></content></shape><shape width="240" height="66" topLeftX="40" topLeftY="115" presetHandlers="0" type="rect"><fill><fillColor color="rgba(255, 253, 246, 1)"/></fill><border color="rgba(198, 159, 90, 1)" width="1"/><content fontSize="16" fontFamily="思源黑体" color="rgba(31, 35, 41, 1)"/></shape><shape width="240" height="22" topLeftX="40" topLeftY="124" type="text"><content fontSize="12" fontFamily="思源黑体" color="rgba(15, 30, 61, 1)" bold="true" textAlign="center"><p>关键指标</p></content></shape><shape width="240" height="22" topLeftX="40" topLeftY="150" type="text"><content fontSize="11" fontFamily="思源黑体" color="rgba(76, 96, 128, 1)" textAlign="center"><p>NPS · LTV · 留存 · 自然增长</p></content></shape><shape width="240" height="66" topLeftX="680" topLeftY="115" presetHandlers="0" type="rect"><fill><fillColor color="rgba(15, 30, 61, 1)"/></fill><content fontSize="16" fontFamily="思源黑体" color="rgba(31, 35, 41, 1)"/></shape><shape width="240" height="22" topLeftX="680" topLeftY="124" type="text"><content fontSize="12" fontFamily="思源黑体" color="rgba(238, 232, 218, 1)" bold="true" textAlign="center"><p>飞轮启动期</p></content></shape><shape width="240" height="22" topLeftX="680" topLeftY="150" type="text"><content fontSize="11" fontFamily="思源黑体" color="rgba(198, 159, 90, 1)" textAlign="center"><p>初始动能源自首批种子用户</p></content></shape><shape width="200" height="86" topLeftX="380" topLeftY="105" presetHandlers="0" type="rect"><fill><fillColor color="rgba(255, 253, 246, 1)"/></fill><border color="rgba(28, 45, 80, 1)"/><content fontSize="16" fontFamily="思源黑体" color="rgba(31, 35, 41, 1)"/></shape><shape width="200" height="24" topLeftX="380" topLeftY="115" type="text"><content fontSize="14" fontFamily="思源黑体" color="rgba(28, 45, 80, 1)" bold="true" textAlign="center"><p>① 用户增长</p></content></shape><shape width="180" height="20" topLeftX="390" topLeftY="142" type="text"><content fontSize="11" fontFamily="思源黑体" color="rgba(76, 96, 128, 1)" textAlign="center"><p>免费试用 · 推荐裂变</p></content></shape><shape width="180" height="20" topLeftX="390" topLeftY="162" type="text"><content fontSize="11" fontFamily="思源黑体" color="rgba(76, 96, 128, 1)" textAlign="center"><p>内容营销 · 品牌信任</p></content></shape><line startX="480" startY="191" endX="480" endY="200.0553851381374"><border color="rgba(28, 45, 80, 0.65)" width="1"/></line><whiteboard width="240" height="240" topLeftX="360" topLeftY="200"/><line startX="600" startY="320" endX="680.0062497558785" endY="320"><border color="rgba(76, 96, 128, 0.7)" width="1"/></line><shape width="240" height="116" topLeftX="680" topLeftY="262" presetHandlers="0" type="rect"><fill><fillColor color="rgba(255, 253, 246, 1)"/></fill><border color="rgba(76, 96, 128, 1)"/><content fontSize="16" fontFamily="思源黑体" color="rgba(31, 35, 41, 1)"/></shape><shape width="240" height="26" topLeftX="680" topLeftY="276" type="text"><content fontSize="15" fontFamily="思源黑体" color="rgba(76, 96, 128, 1)" bold="true" textAlign="center"><p>② 数据沉淀</p></content></shape><shape width="220" height="22" topLeftX="690" topLeftY="312" type="text"><content fontSize="11" fontFamily="思源黑体" color="rgba(15, 30, 61, 1)" textAlign="center"><p>行为日志 · 业务数据</p></content></shape><shape width="220" height="22" topLeftX="690" topLeftY="336" type="text"><content fontSize="11" fontFamily="思源黑体" color="rgba(15, 30, 61, 1)" textAlign="center"><p>反馈样本 · 场景理解</p></content></shape><line startX="480" startY="440" endX="480" endY="450.0498756211209"><border color="rgba(146, 116, 68, 0.85)" width="1"/></line><shape width="200" height="82" topLeftX="380" topLeftY="450" presetHandlers="0" type="rect"><fill><fillColor color="rgba(255, 253, 246, 1)"/></fill><border color="rgba(198, 159, 90, 1)"/><content fontSize="16" fontFamily="思源黑体" color="rgba(31, 35, 41, 1)"/></shape><shape width="200" height="24" topLeftX="380" topLeftY="460" type="text"><content fontSize="14" fontFamily="思源黑体" color="rgba(146, 116, 68, 1)" bold="true" textAlign="center"><p>③ 产品迭代</p></content></shape><shape width="180" height="20" topLeftX="390" topLeftY="486" type="text"><content fontSize="11" fontFamily="思源黑体" color="rgba(76, 96, 128, 1)" textAlign="center"><p>AI 模型优化 · 新功能</p></content></shape><shape width="180" height="20" topLeftX="390" topLeftY="506" type="text"><content fontSize="11" fontFamily="思源黑体" color="rgba(76, 96, 128, 1)" textAlign="center"><p>智能化推荐 · 工作流</p></content></shape><line startX="360" startY="320" endX="279.99375024412154" endY="320"><border color="rgba(178, 101, 78, 0.7)" width="1"/></line><shape width="240" height="116" topLeftX="40" topLeftY="262" presetHandlers="0" type="rect"><fill><fillColor color="rgba(255, 253, 246, 1)"/></fill><border color="rgba(178, 101, 78, 1)"/><content fontSize="16" fontFamily="思源黑体" color="rgba(31, 35, 41, 1)"/></shape><shape width="240" height="26" topLeftX="40" topLeftY="276" type="text"><content fontSize="15" fontFamily="思源黑体" color="rgba(178, 101, 78, 1)" bold="true" textAlign="center"><p>④ 体验提升</p></content></shape><shape width="220" height="22" topLeftX="50" topLeftY="312" type="text"><content fontSize="11" fontFamily="思源黑体" color="rgba(15, 30, 61, 1)" textAlign="center"><p>响应更快 · 结果更准</p></content></shape><shape width="220" height="22" topLeftX="50" topLeftY="336" type="text"><content fontSize="11" fontFamily="思源黑体" color="rgba(15, 30, 61, 1)" textAlign="center"><p>个性化 · 留存提升</p></content></shape></data><note><content/></note></slide></presentation> No newline at end of file |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Self-closing <whiteboard> elements violate the schema's required mermaid/svg choice.
The demo contains two self-closing whiteboard elements (<whiteboard width="440" height="360" topLeftX="30" topLeftY="160"/> and <whiteboard width="240" height="240" topLeftX="360" topLeftY="200"/>), but the schema in slides_xml_schema_definition.xml (lines 3044-3050) defines whiteboard with an <xs:choice> that has no minOccurs="0", requiring exactly one of mermaid or svg as a child. Since this file is referenced as a canonical example in xml-schema-quick-ref.md, users who copy the self-closing whiteboard pattern will produce schema-invalid presentations.
Either add minOccurs="0" to the xs:choice in the schema to allow empty whiteboards (if they serve as visual placeholder areas), or provide mermaid/svg content in the demo's whiteboard elements.
🔧 Schema fix option (allow empty whiteboards)
<xs:sequence>
- <xs:choice>
+ <xs:choice minOccurs="0">
<xs:element name="mermaid" type="xs:string" />
<xs:any namespace="http://www.w3.org/2000/svg" processContents="skip"/>
</xs:choice>
<xs:element name="border" type="sml:BorderType" minOccurs="0"/>
</xs:sequence>🤖 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_demo.xml` at line 1, The demo uses
self-closing whiteboard elements that do not satisfy the schema’s required
mermaid/svg child choice. Update the whiteboard examples in slides_demo.xml by
either supplying one valid child element (mermaid or svg) for each whiteboard,
or adjust the whiteboard definition in slides_xml_schema_definition.xml so the
choice is optional if empty placeholders are intended. Use the whiteboard
element and the related schema choice to locate the fix.
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
1 participant
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
Refines the slides XML and screenshot workflows, including request handling fixes and focused tests. Reorganizes the lark-slides skill references so XML readback, schema guidance, screenshot usage, and PPTX template workflows are easier for agents to follow.
Changes
Test Plan
lark-cli <domain> <command>flow works as expected (not run in this turn due local machine safety instruction)Related Issues
Summary by CodeRabbit
slides +xml-getwith an optional--outputand a new--rawmode to either print a structured JSON envelope or raw XML to stdout.slides +screenshotrequest, with clearer validation and updated help text.+xml-getdocs and updated workflow/constraints, visual styles, asset planning, and troubleshooting.+xml-getstdout behaviors.