Skip to content

feat(slides): normalize replace-slide part aliases - #2225

Merged
tianyouskrrr merged 1 commit into
mainfrom
feat/slides-replace-parts-normalization
Aug 10, 2026
Merged

feat(slides): normalize replace-slide part aliases#2225
tianyouskrrr merged 1 commit into
mainfrom
feat/slides-replace-parts-normalization

Conversation

@tianyouskrrr

@tianyouskrrr tianyouskrrr commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Normalize slides +replace-slide --parts compatibility aliases so agent-authored payloads can use deterministic action and field names while still producing the canonical Slides API request shape. Ambiguous or conflicting inputs stay as typed validation errors, and — now that the Slide Engine backend is GA — all whole-page recovery guidance points at slides +update-slide (in-place whole-page rewrite) instead of the deprecated +replace-pages. Also folds in the whole-page validation-gating generalization from #2255.

Changes

  • Accept compatible replace / insert action aliases and field aliases such as target_id, content, and element, then report the canonical conversions in structured dry-run output.
  • Reject ambiguous actions, conflicting alias values, unknown fields, and malformed JSON with actionable typed validation hints.
  • 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.
  • Complete the fix(slides): restore update-slide skill guidance #2227 deprecation by dropping the +replace-pages reference doc (references/cli/lark-slides-replace-pages.md). The command stays registered; 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 docs(slides): generalize whole-page validation guidance #2255.
  • Fix a stale comment in slides_shared.go (+replace-pages is deprecated but still registered, not "retired").
  • Update the lark-slides skill docs and slides e2e coverage notes for the normalized +replace-slide contract.
  • Add focused unit, dry-run e2e, and opt-in live workflow coverage for alias replace/insert persistence and cleanup.

Test Plan

  • go test -race -gcflags="all=-N -l" -count=1 ./shortcuts/slides
  • go test -count=1 ./tests/cli_e2e/slides -run 'TestSlidesReplaceSlide.*DryRunE2E'
  • Re-tested with the freshly built binary: slides +replace-slide with page_replace / slide_replace now exits 2 and steers to slides +update-slide (validated against the F23 comparison report).
  • gofmt -l / go vet / go build ./... clean.
  • make unit-test (fails in unrelated shortcuts/im: TestAllIMShortcutsUseAffordanceExamples/+messages-search; the touched shortcuts/slides package passed)
  • Manual local verification confirms the lark-cli slides +replace-slide flow works as expected

Related Issues

Summary by CodeRabbit

  • New Features
    • Added compatibility support for replace and insert actions, target aliases, and supported payload field aliases.
    • Dry-run and execution results now report applied input normalizations.
    • Added clearer guidance when invalid JSON is provided.
  • Bug Fixes
    • Conflicting aliases are rejected, while equivalent duplicate aliases remain supported.
    • Unsafe whole-page replacement actions are now rejected.
  • Documentation
    • Updated shortcut and CLI documentation with supported aliases, normalization behavior, and error handling.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The +replace-slide shortcut now normalizes supported action, target, and payload aliases. It reports each normalization, rejects conflicting or unsupported inputs, adds whole-page action guidance, and expands unit, CLI, workflow, and documentation coverage.

Changes

Replace-slide normalization

Layer / File(s) Summary
Normalize and validate replace-slide parts
shortcuts/slides/slides_replace_slide.go
Supported aliases convert to canonical block operations. Normalization records appear in dry-run and execution results. Conflicts, invalid JSON, unsupported actions, and unsafe whole-page actions return validation errors.
Cover parser normalization and validation
shortcuts/slides/slides_replace_slide_test.go
Tests cover alias conversion, payload variants, conflicts, duplicate equivalent values, unsupported actions, invalid JSON guidance, and deterministic unknown-field errors.
Document and exercise compatibility workflows
tests/cli_e2e/slides/*, skills/lark-slides/*
CLI tests and documentation cover dry-run normalization, persisted replacement and insertion, output records, ordering, ID preservation, control-block survival, and cleanup.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • larksuite/cli#516: Introduced the +replace-slide shortcut that this change extends.
  • larksuite/cli#2143: Shares slide replacement validation and whole-page replacement handling.
  • larksuite/cli#2174: Established strict --parts validation that this change extends with aliases.

Suggested reviewers: liangshuo-1

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely identifies the main change: normalization of replace-slide part aliases.
Description check ✅ Passed The description includes all template sections and clearly documents scope, changes, tests, known unrelated failures, and related issues.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/slides-replace-parts-normalization

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@tianyouskrrr
tianyouskrrr requested a review from ethan-zhx August 7, 2026 03:10
@tianyouskrrr tianyouskrrr self-assigned this Aug 7, 2026
@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label Aug 7, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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_replace_slide_test.go`:
- Around line 360-364: Update shortcuts/slides/slides_replace_slide_test.go
lines 360-364 and 1118-1140 so each validation error-path test uses
errs.ProblemOf to assert the validation category and invalid-argument subtype,
then errors.As to inspect *errs.ValidationError and verify Param is "--parts".
Update lines 1142-1157 similarly and additionally assert the wrapped cause is
preserved as *json.SyntaxError; retain the existing message checks where useful.
- Around line 978-1027: Update the XML-preservation assertions in
shortcuts/slides/slides_replace_slide_test.go:978-1027 within
TestReplaceSlideNormalizesCompatibleParts to verify wire replacement and
insertion values exactly match the input XML; update
shortcuts/slides/slides_replace_slide_test.go:1029-1063 to assert every alias
produces the exact canonical payload value; and update
shortcuts/slides/slides_replace_slide_test.go:1102-1116 to assert the folded
canonical Replacement equals "<shape/>".

In `@skills/lark-slides/references/lark-slides-replace-slide.md`:
- Around line 89-93: Update the successful response example and field table in
the replace-slide documentation to include the normalizations field, documenting
its path and the array item shape {part_index, kind, from, to}. Ensure the
documentation covers both normal success output and dry-run responses
consistently.

In `@tests/cli_e2e/slides/slides_replace_slide_dryrun_test.go`:
- Around line 52-56: Update the normalization assertions in
tests/cli_e2e/slides/slides_replace_slide_dryrun_test.go:52-56 to verify
part_index, kind, from, and to for each of all five entries, not just the count
and first entry. Apply the same complete success-response metadata assertions in
tests/cli_e2e/slides/slides_replace_slide_workflow_test.go:125-126 for the live
alias workflow, using the existing normalizations result and expected alias
conversion values.
🪄 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: f7a39626-e16f-402c-9765-1b096b2d123a

📥 Commits

Reviewing files that changed from the base of the PR and between 5919e86 and 92d0fcb.

📒 Files selected for processing (8)
  • shortcuts/slides/slides_replace_slide.go
  • shortcuts/slides/slides_replace_slide_test.go
  • skills/lark-slides/SKILL.md
  • skills/lark-slides/references/lark-slides-replace-slide.md
  • tests/cli_e2e/slides/coverage.md
  • tests/cli_e2e/slides/slides_history_dryrun_test.go
  • tests/cli_e2e/slides/slides_replace_slide_dryrun_test.go
  • tests/cli_e2e/slides/slides_replace_slide_workflow_test.go

Comment thread shortcuts/slides/slides_replace_slide_test.go
Comment thread shortcuts/slides/slides_replace_slide_test.go
Comment thread skills/lark-slides/references/lark-slides-replace-slide.md Outdated
Comment thread tests/cli_e2e/slides/slides_replace_slide_dryrun_test.go Outdated
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@5b874b0a85504337d909253dc8596e6f67d5b06f

🧩 Skill update

npx skills add larksuite/cli#feat/slides-replace-parts-normalization -y -g

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.91667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.38%. Comparing base (2016120) to head (5b874b0).

Files with missing lines Patch % Lines
shortcuts/slides/slides_replace_slide.go 97.91% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2225      +/-   ##
==========================================
+ Coverage   76.36%   76.38%   +0.01%     
==========================================
  Files        1011     1011              
  Lines      111269   111352      +83     
==========================================
+ Hits        84970    85051      +81     
- Misses      19815    19816       +1     
- Partials     6484     6485       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-replace-parts-normalization branch from 92d0fcb to 2312cc1 Compare August 7, 2026 03:23
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

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.

@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-replace-parts-normalization branch from 2312cc1 to 773138f Compare August 7, 2026 08:42
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

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.

@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-replace-parts-normalization branch from 773138f to cc155bf Compare August 7, 2026 09:06
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

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.

@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-replace-parts-normalization branch from 3df22cf to 99eb42f Compare August 10, 2026 07:11
Comment thread shortcuts/slides/slides_replace_slide.go
ethan-zhx
ethan-zhx previously approved these changes Aug 10, 2026
@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-replace-parts-normalization branch from 99eb42f to 1bf712d Compare August 10, 2026 08:21
@github-actions github-actions Bot added size/L Large or sensitive change across domains or core paths and removed size/M Single-domain feat or fix with limited business impact labels Aug 10, 2026
@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-replace-parts-normalization branch from 1bf712d to 719420f Compare August 10, 2026 08:46
Comment thread skills/lark-slides/SKILL.md Outdated
@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-replace-parts-normalization branch 2 times, most recently from 30b66d5 to a673dc5 Compare August 10, 2026 09:56
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.
@tianyouskrrr
tianyouskrrr force-pushed the feat/slides-replace-parts-normalization branch from a673dc5 to 5b874b0 Compare August 10, 2026 10:56
@github-actions github-actions Bot added size/M Single-domain feat or fix with limited business impact and removed size/L Large or sensitive change across domains or core paths labels Aug 10, 2026
Comment thread skills/lark-slides/references/cli/lark-slides-replace-slide.md
@tianyouskrrr
tianyouskrrr merged commit 7cfbea6 into main Aug 10, 2026
31 checks passed
@tianyouskrrr
tianyouskrrr deleted the feat/slides-replace-parts-normalization branch August 10, 2026 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants