fix(slides): restore update-slide skill guidance - #2227
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe Slides workflow replaces full-page ChangesSlides full-page editing workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Test as Live E2E test
participant CLI as slides +update-slide
participant Slides as Slides backend
participant Read as slides +xml-get
participant Drive as drive +delete
Test->>CLI: Submit updated slide XML
CLI->>Slides: Update slide in place
Slides-->>CLI: Return preserved slide ID
Test->>Read: Read presentation and slide XML
Read-->>Test: Return updated markers
Test->>Drive: Delete test presentation
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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/lark-slides-update-slide.md`:
- Around line 103-117: Update the 成功输出 example in the update-slide documentation
to match the CLI response shape consumed by the E2E: either wrap the fields in
the full data envelope or explicitly label the JSON as the data payload. Ensure
callers can locate slide_id and revision_id under data.
🪄 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: 22a43a0d-d959-49e3-b13a-3975dbf9bc3a
📒 Files selected for processing (4)
skills/lark-slides/SKILL.mdskills/lark-slides/references/lark-slides-edit-workflows.mdskills/lark-slides/references/lark-slides-replace-pages.mdskills/lark-slides/references/lark-slides-update-slide.md
💤 Files with no reviewable changes (1)
- skills/lark-slides/references/lark-slides-replace-pages.md
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@4e7d1ffe0923f2f6632ab94e43583e894498438a🧩 Skill updatenpx skills add larksuite/cli#codex/slides-update-skill-restore -y -g |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2227 +/- ##
=======================================
Coverage 76.34% 76.35%
=======================================
Files 991 991
Lines 106019 106037 +18
=======================================
+ Hits 80940 80960 +20
+ Misses 18944 18942 -2
Partials 6135 6135 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
be62cfd to
86be67c
Compare
|
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/slides/slides_update_slide.go`:
- Around line 202-206: Update updateSlideInvalidParamHint in
shortcuts/slides/slides_update_slide.go:202-206 to remove the claim that
block_id is never missing and instruct callers to verify --presentation and
--slide-id for missing-page errors. Update the corresponding failed-reason
expectation in shortcuts/slides/slides_update_slide_test.go:191-195 and
invalid-parameter expectation at
shortcuts/slides/slides_update_slide_test.go:508-512 to match the revised hint.
In `@skills/lark-slides/references/lark-slides-update-slide.md`:
- Around line 93-101: Extend the “提交前的版式检查” guidance in the +update-slide
workflow to document post-write validation after a successful full-page rewrite:
read back the full XML, verify the page count and key elements, and execute the
validation checklist defined in the loaded lark-slides/SKILL.md. Keep the
existing preflight lint and warning screenshot guidance 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: 8371dc56-1225-46c1-b59b-c9d3d6deecd2
📒 Files selected for processing (7)
shortcuts/slides/slides_update_slide.goshortcuts/slides/slides_update_slide_test.goskills/lark-slides/SKILL.mdskills/lark-slides/references/lark-slides-edit-workflows.mdskills/lark-slides/references/lark-slides-replace-pages.mdskills/lark-slides/references/lark-slides-update-slide.mdtests/cli_e2e/slides/coverage.md
💤 Files with no reviewable changes (1)
- skills/lark-slides/references/lark-slides-replace-pages.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/lark-slides/references/lark-slides-edit-workflows.md
86be67c to
2e42e80
Compare
|
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
🤖 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/lark-slides-update-slide.md`:
- Around line 35-39: Update the documentation for the read-modify-write flow
around --revision-id and --tid to state that --revision-id only selects the base
snapshot and does not prevent overwriting later edits. Require a server-side
--tid lock check or an equivalent re-read/rebase precondition when writes must
fail after the page changes, and clarify that sending only these query
parameters is insufficient without that validation.
🪄 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: c023208e-0f0d-42ba-8e70-d65ef265b1e2
📒 Files selected for processing (7)
shortcuts/slides/slides_update_slide.goshortcuts/slides/slides_update_slide_test.goskills/lark-slides/SKILL.mdskills/lark-slides/references/lark-slides-edit-workflows.mdskills/lark-slides/references/lark-slides-replace-pages.mdskills/lark-slides/references/lark-slides-update-slide.mdtests/cli_e2e/slides/coverage.md
💤 Files with no reviewable changes (1)
- skills/lark-slides/references/lark-slides-replace-pages.md
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/cli_e2e/slides/coverage.md
- shortcuts/slides/slides_update_slide.go
- shortcuts/slides/slides_update_slide_test.go
- skills/lark-slides/references/lark-slides-edit-workflows.md
2e42e80 to
a5b2276
Compare
|
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. |
a5b2276 to
2e63b16
Compare
|
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/lark-slides/references/lark-slides-update-slide.md`:
- Line 85: Align the `<slide/>` guidance with `detect_blank_slide()` and the
`summary.release_ready` error gate: either document a narrowly scoped exception
for the `blank_slide` error or update the allowed-case wording and lint behavior
so an empty self-closing slide has consistent treatment. Ensure the table and
lint requirements no longer contradict each other.
In `@skills/lark-slides/SKILL.md`:
- Line 111: Update the validation requirement in the slide-editing guidance to
apply after every slides +update-slide call, not only after creation or major
rewrites. Require the same full XML readback, page-count and key-element checks,
and xml_text_overlap_lint.py validation for each update-slide operation.
🪄 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: aeba9c52-3f02-4dfd-9e99-bd469e07cce3
📒 Files selected for processing (8)
shortcuts/slides/slides_update_slide.goshortcuts/slides/slides_update_slide_test.goskills/lark-slides/SKILL.mdskills/lark-slides/references/lark-slides-edit-workflows.mdskills/lark-slides/references/lark-slides-replace-pages.mdskills/lark-slides/references/lark-slides-update-slide.mdtests/cli_e2e/slides/coverage.mdtests/cli_e2e/slides/slides_update_slide_workflow_test.go
💤 Files with no reviewable changes (1)
- skills/lark-slides/references/lark-slides-replace-pages.md
🚧 Files skipped from review as they are similar to previous changes (2)
- shortcuts/slides/slides_update_slide_test.go
- skills/lark-slides/references/lark-slides-edit-workflows.md
825ac87 to
a7b9a95
Compare
a7b9a95 to
4e7d1ff
Compare
fangshuyu-768
left a comment
There was a problem hiding this comment.
Re-reviewed at 4e7d1ff. All actionable findings are resolved, the required live update-slide E2E ran successfully, and all checks pass.
… changes Two follow-ups to #2227's restoration of the +update-slide route. 1. Generalize the whole-page validation gate. The validation CRITICAL in SKILL.md and the migrated workflow/validation-xml.md are command-agnostic quality gates keyed on the "whole-page write-back" action, not on any single shortcut. #2227 hardcoded +update-slide into them, reintroducing the special-casing that #2143 avoided. Rephrase back to the behavior ("整页写回") so the gate stays bound to the action rather than the currently-routed command. The lint CRITICAL keeps its explicit command list unchanged — it has enumerated the whole-page XML entry points since the replace-pages era, so it is not the special-case introduced by #2227. 2. Revert two unrelated example edits carried by #2227: - SKILL.md changed only the /wiki/ example domain to xxx.feishu.cn while the /slides/ row kept example.larkoffice.com, leaving two domains in one table. Restore the consistent domain and the <wiki_url> placeholder. - The +replace-slide batch --parts JSON was collapsed onto one line. Restore the multi-line form for readability. Documentation only; no behavior change.
Generalize the whole-page validation gate so it is keyed on the action, not on a single shortcut name. The validation CRITICAL in SKILL.md and the migrated workflow/validation-xml.md are command-agnostic quality gates: they apply to the "whole-page write-back" action regardless of which shortcut performs it. #2227 hardcoded +update-slide into them, reintroducing the special-casing that #2143 avoided. Rephrase back to the behavior ("整页写回") so the gate stays bound to the action rather than the currently-routed command. The lint CRITICAL keeps its explicit command list unchanged — it has enumerated the whole-page XML entry points since the replace-pages era, so it is not a special-case introduced by #2227. The /wiki/ example URL and the +replace-slide single-line --parts JSON introduced by #2227 are left as-is: the quality-gate harness executes skill examples under --dry-run, so the placeholder <wiki_url> fails the wiki token length check and a multi-line single-quoted --parts is not parseable. Both current forms are required to pass the gate. Documentation only; no behavior change.
Normalize deterministic +replace-slide part aliases (replace→block_replace, target_id→block_id, and payload field folding), and reject semantically different actions up front. Point whole-page actions at +update-slide now that it is GA: - page_replace / slide_replace recovery guidance and the reference error table now direct callers to `slides +update-slide` (in-place whole-page rewrite) instead of the deprecated +replace-pages. - drop the +replace-pages reference doc, completing the #2227 deprecation (the command stays; its deprecation signal is carried by --help and the output JSON `deprecated` field). Generalize whole-page validation gating to the behavior rather than a command name (SKILL.md, workflow/validation-xml.md): "整页写回后" instead of "每次通过 +update-slide 整页写回后". Folds in PR #2255.
Normalize deterministic +replace-slide part aliases (replace→block_replace, target_id→block_id, and payload field folding), and reject semantically different actions up front. Point whole-page actions at +update-slide now that it is GA: - page_replace / slide_replace recovery guidance and the reference error table now direct callers to `slides +update-slide` (in-place whole-page rewrite) instead of the deprecated +replace-pages. - drop the +replace-pages reference doc, completing the #2227 deprecation (the command stays; its deprecation signal is carried by --help and the output JSON `deprecated` field). Generalize whole-page validation gating to the behavior rather than a command name (SKILL.md, workflow/validation-xml.md): "整页写回后" instead of "每次通过 +update-slide 整页写回后". Folds in PR #2255.
Normalize deterministic +replace-slide part aliases (replace→block_replace, target_id→block_id, and payload field folding), and reject semantically different actions up front. Point whole-page actions at +update-slide now that it is GA: - page_replace / slide_replace recovery guidance and the reference error table now direct callers to `slides +update-slide` (in-place whole-page rewrite) instead of the deprecated +replace-pages. - drop the +replace-pages reference doc, completing the #2227 deprecation (the command stays; its deprecation signal is carried by --help and the output JSON `deprecated` field). Generalize whole-page validation gating to the behavior rather than a command name (SKILL.md, workflow/validation-xml.md): "整页写回后" instead of "每次通过 +update-slide 整页写回后". Folds in PR #2255.
Normalize deterministic +replace-slide part aliases (replace→block_replace, target_id→block_id, and payload field folding), and reject semantically different actions up front. Point whole-page actions at +update-slide now that it is GA: - page_replace / slide_replace recovery guidance and the reference error table now direct callers to `slides +update-slide` (in-place whole-page rewrite) instead of the deprecated +replace-pages. - drop the +replace-pages reference doc, completing the #2227 deprecation (the command stays; its deprecation signal is carried by --help and the output JSON `deprecated` field). Generalize whole-page validation gating to the behavior rather than a command name (SKILL.md, workflow/validation-xml.md): "整页写回后" instead of "每次通过 +update-slide 整页写回后". Folds in PR #2255.
Normalize deterministic +replace-slide part aliases (replace→block_replace, target_id→block_id, and payload field folding), and reject semantically different actions up front. Point whole-page actions at +update-slide now that it is GA: - page_replace / slide_replace recovery guidance and the reference error table now direct callers to `slides +update-slide` (in-place whole-page rewrite) instead of the deprecated +replace-pages. - drop the +replace-pages reference doc, completing the #2227 deprecation (the command stays; its deprecation signal is carried by --help and the output JSON `deprecated` field). Generalize whole-page validation gating to the behavior rather than a command name (SKILL.md, workflow/validation-xml.md): "整页写回后" instead of "每次通过 +update-slide 整页写回后". Folds in PR #2255.
Normalize deterministic +replace-slide part aliases (replace→block_replace, target_id→block_id, and payload field folding), and reject semantically different actions up front. Point whole-page actions at +update-slide now that it is GA: - page_replace / slide_replace recovery guidance and the reference error table now direct callers to `slides +update-slide` (in-place whole-page rewrite) instead of the deprecated +replace-pages. - drop the +replace-pages reference doc, completing the #2227 deprecation (the command stays; its deprecation signal is carried by --help and the output JSON `deprecated` field). Generalize whole-page validation gating to the behavior rather than a command name (SKILL.md, workflow/validation-xml.md): "整页写回后" instead of "每次通过 +update-slide 整页写回后". Folds in PR #2255.
Summary
Restore the
lark-slidesskill's whole-page editing route from the deprecated+replace-pagesworkflow to+update-slidenow that the in-place update path is available. The guidance now makes page and element ID preservation, omitted-element deletion, and the safe read-modify-write workflow explicit for AI agents.Changes
slides +update-slide, while retaining+replace-slidefor targeted block edits.lark-slides-replace-pages.mdrollout guidance with a completelark-slides-update-slide.mdreference covering flags, aliases, overwrite semantics, validation, and common errors.Test Plan
make unit-testgit diff --check+update-slidebehavior and flags against the existing shortcut implementation and tests.Related Issues
Summary by CodeRabbit