Skip to content

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

Open
tianyouskrrr wants to merge 1 commit into
mainfrom
feat/slides-replace-parts-normalization
Open

feat(slides): normalize replace-slide part aliases#2225
tianyouskrrr wants to merge 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. The change keeps ambiguous or conflicting inputs as validation errors, and improves recovery guidance for malformed JSON and whole-page replacement attempts.

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, malformed JSON, and whole-page <slide> replacements with actionable typed validation hints.
  • 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'
  • 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

  • None

Summary by CodeRabbit

  • New Features
    • +replace-slide accepts compatible action names and field aliases, converting them to standard formats automatically.
    • Dry runs and successful executions report applied normalizations.
  • Bug Fixes
    • Conflicting aliases and unsupported whole-page replacement actions are rejected with clearer errors.
    • Invalid JSON errors include file or standard-input guidance.
  • Documentation
    • Updated help and reference documentation with supported aliases, conversions, and error examples.
  • Tests
    • Expanded coverage for normalization, conflicts, dry runs, and live slide replacement workflows.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 67d8def4-d148-49cc-a1e5-2d13cbf7dcd5

📥 Commits

Reviewing files that changed from the base of the PR and between 46e2186 and cc155bf.

📒 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
🚧 Files skipped from review as they are similar to previous changes (8)
  • skills/lark-slides/SKILL.md
  • tests/cli_e2e/slides/slides_history_dryrun_test.go
  • tests/cli_e2e/slides/coverage.md
  • tests/cli_e2e/slides/slides_replace_slide_workflow_test.go
  • skills/lark-slides/references/lark-slides-replace-slide.md
  • shortcuts/slides/slides_replace_slide.go
  • shortcuts/slides/slides_replace_slide_test.go
  • tests/cli_e2e/slides/slides_replace_slide_dryrun_test.go

📝 Walkthrough

Walkthrough

The +replace-slide shortcut normalizes supported action, target, and XML payload aliases. It reports normalization records, rejects conflicts and unsupported actions, adds invalid-JSON guidance, and expands unit, dry-run, live workflow, and documentation coverage.

Changes

Replace-slide normalization

Layer / File(s) Summary
Normalize and validate replace-slide parts
shortcuts/slides/slides_replace_slide.go
The parser converts supported aliases to canonical fields, reports normalization records, detects conflicts, and validates unsupported actions and invalid JSON input.
Cover parser normalization and validation
shortcuts/slides/slides_replace_slide_test.go
Tests cover alias mappings, conflicts, duplicate equivalent values, unsupported actions, invalid JSON guidance, and updated unknown-field behavior.
Document and exercise compatibility workflows
skills/lark-slides/SKILL.md, skills/lark-slides/references/lark-slides-replace-slide.md, tests/cli_e2e/slides/*
Documentation describes compatibility rules and errors. Dry-run and live workflow tests verify canonical payloads, normalization records, persistence, ordering, ID preservation, and control-block survival.

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

Sequence Diagram(s)

sequenceDiagram
  participant ReplaceSlideCLI
  participant NormalizationParser
  participant SlidesAPI
  ReplaceSlideCLI->>NormalizationParser: provide aliased --parts JSON
  NormalizationParser->>NormalizationParser: normalize aliases and validate conflicts
  NormalizationParser-->>ReplaceSlideCLI: canonical parts and normalization records
  ReplaceSlideCLI->>SlidesAPI: submit canonical replacement and insertion parts
  SlidesAPI-->>ReplaceSlideCLI: return execution result and normalization records
Loading

Possibly related PRs

  • larksuite/cli#516: Introduced the +replace-slide shortcut extended by this change.
  • larksuite/cli#2174: Also modifies replace-slide --parts validation and normalization.
  • larksuite/cli#2143: Modifies the related slide replacement implementation and whole-slide replacement flow.

Suggested reviewers: ethan-zhx

🚥 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
Title check ✅ Passed The title clearly and concisely describes the main change: normalizing aliases for the replace-slide feature.
Description check ✅ Passed The description includes all required sections and clearly documents the changes, tests, known unrelated failures, and issue status.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 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
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@cc155bffdb7a9f36d897da0245747cb8455ade98

🧩 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.35%. Comparing base (bc0ba22) to head (cc155bf).
⚠️ Report is 2 commits behind head on main.

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.33%   76.35%   +0.02%     
==========================================
  Files         993      993              
  Lines      106633   106774     +141     
==========================================
+ Hits        81400    81531     +131     
- Misses      19032    19038       +6     
- Partials     6201     6205       +4     

☔ 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.

Accept deterministic action and field aliases while rejecting ambiguous semantics and conflicting values. Add actionable recovery guidance for malformed JSON and whole-page actions, plus dry-run and live workflow coverage.
@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.

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.

1 participant